feat(scripts): targeted orphaned Docker volume/image prune (SMI-5750)#1968
Merged
Conversation
wrsmith108
force-pushed
the
fix/smi-5750-targeted-volume-prune
branch
from
July 20, 2026 03:58
d510293 to
17353e8
Compare
E2E Test ResultsE2E Test Results - July 20, 2026Summary
Test Results
Generated by skillsmith E2E test suite |
Points at the reviewed SMI-5750 plan (docs/internal fix/smi-5750-targeted-volume-prune @ c64da64) — SPARC research + Codex plan-review complete, ready for implementation. Refs SMI-5750 Co-Authored-By: claude-flow <ruv@ruv.net> Co-Authored-By: Claude <noreply@anthropic.com>
A Docker Desktop crash (SMI-5616) took down every concurrent dev session
when the host disk filled to 1.6GB free -- Docker had accumulated ~74GB of
orphaned volumes/images because the existing aggressive reclaim in
remove-worktree.sh is deliberately manual-only (SMI-5145), and this
environment runs continuous concurrent worktree sessions with no idle
window to run it in.
Adds scripts/prune-orphaned-docker-volumes.sh: a targeted prune whose
safety predicate is worktree-existence (git worktree list --porcelain,
unioned with a .worktrees/* scan), not container-running-state -- safe at
any concurrency level, so it runs unconditionally on every
remove-worktree.sh invocation (new Step 3.5, opt out via
--no-orphan-prune or SKILLSMITH_ORPHAN_PRUNE_DISABLE=1) instead of
requiring a manual step that never gets exercised.
An app.skillsmith.owned label (docker-compose.yml's node_modules volume +
dev build, and scripts/_lib.sh's per-worktree native-seed-<module>
volumes) gates automatic deletion -- unlabeled candidates are report-only
("UNCONFIRMED ownership") unless --include-unlabeled is passed
explicitly, so the pre-label orphan backlog and any other repo's
similarly-shaped resources are never silently deleted.
Plan reviewed by Codex (gpt-5.6-sol via NEEDLE, 3 blockers found and
fixed before implementation) and adversarially re-reviewed post-build,
which found and fixed two real implementation bugs: a sanitize()
newline-handling bug that silently broke multi-worktree protection on
BSD sed, and a scope gap where the native-seed-<module> volume
convention (SMI-5650) -- the numerically dominant orphan class on the
incident machine -- was entirely invisible to the original
_node_modules-only implementation. Full detail in
docs/internal/implementation/smi-5750-targeted-volume-prune.md's Review
Summary.
19 new/updated shim-based tests (no real Docker daemon required, split
into a sibling .helpers.ts fixture file per the 500-line file-length
standard); verified live against this machine's actual orphaned-volume
backlog via --dry-run with zero false positives against currently-live
worktrees.
Refs SMI-5750
Co-Authored-By: claude-flow <ruv@ruv.net>
Co-Authored-By: Claude <noreply@anthropic.com>
Post-commit governance review of the SMI-5750 diff (docs/internal code_review/2026-07-19-smi-5750-targeted-volume-prune-review.md) found prune-orphaned-docker-volumes.sh's sanitize() and remove-worktree.sh's project_name derivation were the same tr/sed pipeline duplicated verbatim across two files instead of one shared implementation -- the sibling-implementation drift risk the governance checklist's Duplicate Security-Gate audit calls out (same principle applies to any safety-relevant predicate duplicated in two places, not only auth gates). Extracts scripts/_lib.sh's new sanitize_project_name(), called by both sites; no behavior change (verified via full test re-run and a live --dry-run cross-check against this machine's real orphan backlog with zero false positives against currently-live worktrees). Refs SMI-5750 Co-Authored-By: claude-flow <ruv@ruv.net> Co-Authored-By: Claude <noreply@anthropic.com>
wrsmith108
force-pushed
the
fix/smi-5750-targeted-volume-prune
branch
from
July 20, 2026 16:33
17353e8 to
e08f7b7
Compare
E2E Test ResultsE2E Test Results - July 20, 2026Summary
Test Results
Generated by skillsmith E2E test suite |
wrsmith108
added a commit
that referenced
this pull request
Jul 21, 2026
…udge (#2001) * fix(scripts): SMI-5773 rebase-worktree.sh post-rebase git-crypt re-smudge `git checkout HEAD -- <paths>` in restore_filters() skips stat-clean files entirely (Git's checkout_entry_ca()/ie_match_stat() short-circuit). Files a rebase rewrites under a disabled (smudge=cat) git-crypt filter land on disk as ciphertext but get recorded stat-clean, so the existing post-rebase re-checkout silently no-ops on exactly the files needing re-smudge -- twice observed during SMI-5750's landing (PR #1968). Splits restore_filters() into restore_filter_config() (trap-safe, config-only -- what the EXIT trap now calls at all 4 sites) and force_resmudge() (destructive rm-then-checkout, success-path only, never reachable from the trap). Adds scan_ciphertext() as a hard post-condition: scans every file under the encrypted-path globs for the \x00GITCRYPT magic header or a missing tracked file, returns status (never exits internally), and is sequenced by step_restore_filters()/main() to run after step_pop_stash AND step_verify_branch so a detected residue never strands the user's stash or skips the branch-switch safety check (SMI-2536) -- exits 4 with remediation instructions if residue survives. Root-caused via SPARC research (Fable) verified against live Git source, then adversarially plan-reviewed and code-reviewed by Codex/ Sol via NEEDLE (two passes: pre-implementation plan review found an internal contradiction in the original draft about EXIT-trap safety; post-implementation code review found the check_resmudge_scan_result step-ordering bug fixed here, and a genuine pre-existing bug in the unrelated Step 7/9 git-crypt-filter-disable + git-stash-push interaction, filed separately as SMI-5781 rather than fixed inline since it requires its own ADR-109 SPARC+plan-review). Regression/detector tests use a deterministic mtime-backdate + git update-index --refresh technique rather than relying on incidental test-run timing, after discovering mid-implementation that git's checkout_entry_ca() only takes the stat-clean shortcut when the cached mtime is non-racy (CE_MATCH_RACY_IS_DIRTY is not set) -- explaining why SMI-5750's original bug was intermittent. scripts/rebase-worktree.sh split into itself plus two sourced siblings (_rebase-git-crypt.sh, _rebase-submodule.sh) and scripts/tests/rebase-worktree.test.ts split into the original SMI-3102 suite plus a new rebase-worktree.git-crypt-resmudge.test.ts for the SMI-5773 cases (shared rebase-worktree.helpers.ts), per CLAUDE.md's 500-line file-length convention. Refs SMI-5773, SMI-5750 Refs SMI-5781 (filed as a follow-up, not in scope here) Co-Authored-By: claude-flow <ruv@ruv.net> Co-Authored-By: Claude <noreply@anthropic.com> * chore(docs): bump docs/internal pointer for SMI-5773 review + plan Refs skillsmith-docs#498 (SMI-5773 implementation plan + governance code review docs, not yet merged). Co-Authored-By: claude-flow <ruv@ruv.net> Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Ryan Smith <wrsmith108@users.noreply.github.com> Co-authored-by: claude-flow <ruv@ruv.net> Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Business Summary
What shipped:
remove-worktree.shnow automatically reclaims orphaned Docker volumes and images left behind by crashed or hand-deleted worktrees on every run, instead of relying on a manual step that this environment's continuous concurrent-worktree workflow never actually exercised — the root cause of a Docker Desktop crash (SMI-5616) that took the host disk down to 1.6GB free. A--no-orphan-pruneflag opts out per-invocation if needed. Both the basenode_modulesvolume and the 5 per-worktree native-module volumes now carry a positive ownership marker so newly-created volumes are unambiguously reclaimable; pre-existing unlabeled resources are only ever reported, never auto-deleted, unless an operator explicitly confirms with--include-unlabeled.Quality bar: Plan reviewed pre-implementation by an independent model (Codex via NEEDLE) — 3 blockers found and fixed before any code was written. The implementation was then adversarially re-reviewed twice more after building it, which caught and fixed two additional real bugs: one that would have silently defeated the whole safety check on any machine with more than one worktree, and one where the numerically most common kind of orphaned volume on this machine wasn't being cleaned up at all. A follow-up governance pass caught and fixed a maintainability issue (the same sanitization logic duplicated across two scripts) and split an oversized test file to stay under the repo's file-length standard. 39 new/updated automated tests plus a pre-existing 54-test suite all pass; also live dry-run-verified against this machine's real backlog of ~240 leftover volumes/images with zero false positives against any currently active worktree.
Found along the way: the disk-space-threshold trigger for
create-worktree.shrecommended by the plan was intentionally scoped out of this PR and filed separately as SMI-5757 (blocked on this PR landing first).Net result: Fully shipped. One follow-up (SMI-5757) tracked, not blocking.
Technical detail
New:
scripts/prune-orphaned-docker-volumes.sh— targeted prune whose safety predicate is worktree-existence (git worktree list --porcelain, unioned with a.worktrees/*scan), not container-running-state, so it's safe to run unconditionally at any concurrency level. Recognizes both the<project>_node_modulesand<project>_native-seed-<module>(SMI-5650) volume conventions, plus<project>-devimages.--dry-runand--include-unlabeledflags;SKILLSMITH_ORPHAN_PRUNE_DISABLE=1opt-out (registered indocs/internal/process/guards-and-opt-outs.md).Changed:
scripts/remove-worktree.sh— new Step 3.5 invoking the prune script unconditionally after all existing steps (so it can't shift any docker-call position the existing shim-based tests assert on positionally);--no-orphan-pruneflag.scripts/_lib.sh—enumerate_native_module_volumes()now emits theapp.skillsmith.ownedlabel; new sharedsanitize_project_name()(de-dup fix, see governance report below).docker-compose.yml—app.skillsmith.ownedlabel on thenode_modulesvolume + devbuild.labels..claude/development/docker-guide.md— one-line pointer to the new routine-reclaim path.Tests:
scripts/tests/prune-orphaned-docker-volumes.test.ts(19 cases) + sibling.helpers.tsfixture file (split for the 500-line file-length gate), 2 new cases inscripts/tests/remove-worktree.test.ts, updated assertion inscripts/tests/worktree-override-root-mount-smi5650.test.tsfor the new label.Plan + reviews:
docs/internal/implementation/smi-5750-targeted-volume-prune.md(full Review Summary with both external reviews' findings). Governance report:docs/internal/code_review/2026-07-19-smi-5750-targeted-volume-prune-review.md.CI: docs/internal submodule pointer bumped alongside code (4 commits total: feature, pointer bump, governance de-dup fix, pointer bump). Full pre-push suite green (security tests, npm audit, secrets scan, format, full package test suite).