Stale git-worktree scanner (fn-5) - #460
Open
acebytes wants to merge 160 commits into
Open
Conversation
…l gitdir resolver Phase 1 of the stale-worktree epic — the plumbing every later fn-5 task consumes. No scanner, no gates, no deletion. - GitCommandRunner (Sendable, argv-only /usr/bin/env git, never a shell): cleaner-cloned fixed PATH + injected HOME, fully substitutable environment, bounded waitForExit(within:), per-INVOCATION timeout (10s scan default; delete-time callers pass their own budget). - Concurrent stdout+stderr drains started BEFORE the wait (64KiB pipe deadlock), non-blocking poll(2) reads instead of readabilityHandler EOF (SR-12080), close and read serialized on one lock so a close can never land mid-read. - FULL termination protocol on expiry: terminate -> bounded grace -> SIGKILL if still running -> close pipe handles -> boundedly join BOTH drains -> .timeout, proven against a SIGTERM-ignoring stub. - D17 safety profiles classified by COMMAND, never by call phase: read-only commands carry GIT_OPTIONAL_LOCKS=0 + -c core.fsmonitor=false wherever they run; worktree remove/prune carry the fsmonitor neutralization only; unrecognized commands fall back to the mutation profile. The invocation record exposes profile/argv/environment. - INSTANCE-scoped availability cache under an NSLock with probe-once-under-lock; env exit 127, launch failure, and a failed probe all resolve to .gitUnavailable, never a silent zero. - GitWorktreeInventory: pure --porcelain -z parser (NUL attributes, double-NUL records, no line splitting anywhere), position-derived isMain, forward-compatible unknown attributes, fail-closed on undecodable bytes or a record with no worktree attribute. - GitWorktreeGitdirResolver: .git-file pointer -> worktrees/<id> admin dir with the bidirectional back-link ENFORCED, commondir resolved (never path-stripped), and the two cross-validation branches (bare / non-bare) against the porcelain first record. Split of authority pinned in the type: inventory owns parentRepoWorkingDir, resolver owns parentGitDir, and parentAdminContainer is the ONE <parentGitDir>/worktrees derivation. - GitWorktreeAdminMapper: the SHARED oracle->admin mapping fn-5.4 and fn-5.5 both consume. Admin-ENTRY traversal gates (real no-follow directory, canonical containment, same device, non-symlink regular gitdir) run over every container entry BEFORE any back-link read; locked prunable records are excluded without suppression; verdict is .complete(set) or .incomplete(reason naming the entry). Documented empirical finding (git 2.50.1, hermetic fixture): git derives the porcelain main record by stripping a /.git suffix from the common git dir, so `git init --separate-git-dir=<external>` reports <external> as the first record. Cross-validation then finds no <external>/.git and the membership fails CLOSED — the safe direction — with a hand-built fixture proving first-record authority when the git dir's parent is not the working tree. Validation: swift build clean; swift test 854 tests, 1 pre-existing skip, 0 failures (baseline 803 + 51 new). Task: fn-5-stale-git-worktree-scanner.1 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…t parsing (review r1) An unterminated final field or a record missing its closing NUL meant the listing was not read faithfully — half a path is exactly the wrong deletion target the -z grammar exists to prevent (D8). The parser now fails CLOSED for the whole stream rather than flushing the partial record. - parse() requires every field NUL-terminated and every record closed by its empty field; anything else returns nil. - Two new fixtures: an unterminated final field, and a record whose fields are all terminated but whose own closing NUL is gone. Validation: swift test 856 tests, 1 pre-existing skip, 0 failures. Task: fn-5-stale-git-worktree-scanner.1 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…iew r2) Enumeration failure was collapsing into `.complete([])` whenever nothing was prunable, so a permission denial, an I/O error, or a container that is a file or symlink all read as "nothing to prune". A repo-wide `worktree prune` still traverses such a container, so that broke the complete-or-incomplete contract. - The container is now probed no-follow FIRST: `.absent` is benign only when no prunable record remains (absent + prunable records => `.incomplete`); a non-directory, an lstat failure, and an unenumerable directory each yield `.incomplete` with a reason naming the container. - Four new cells: absent-with-prunable, a FILE container, a chmod-000 container (registered for teardown restore per the house rule), and the retained benign absent-with-nothing-prunable case. Validation: swift test 859 tests, 1 pre-existing skip, 0 failures. Task: fn-5-stale-git-worktree-scanner.1 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…mbly
Phase 2 of the stale-worktree epic — the candidate assessment. Pure
decision logic over fn-5.1's inventory types plus read-only git through
fn-5.1's runner. No walking, no items, no deletion (D15: non-candidates
are never emitted, so this task produces assessments, not items).
- WorktreeStalenessAssessor.assess(entry:parentRepoWorkingDir:) runs ALL
FOUR gates every time — evidence completeness is a requirement, not an
optimization target — and is conjunctive: isCandidate iff all four pass.
- G1 not-main/not-bare from porcelain POSITION and the bare attribute; no
git call. G4 not-locked from the porcelain lock attribute; a locked
worktree is never a candidate (this epic has no lock handling at all).
- G2 clean: `status --porcelain --ignore-submodules=none`, EXPORTED as the
standalone GitWorktreeCleanCheck (arguments/verdict/run) because fn-5.4's
revalidator re-runs exactly this check before the filesystem fallback —
one implementation, one argv, two call sites. Only an EMPTY success is
clean; the dirty entry count is display-only and can never flip the
byte-level gate decision.
- G3 merged: LOCAL ancestry only, never network. The D6 ladder runs in the
PARENT repo (origin/HEAD -> refs/heads/main -> refs/heads/master -> fail
closed) and only its provably benign class — a NONZERO EXIT, i.e. the ref
is simply unset/absent, exit 128 on 2.50.1 — continues the ladder;
timeout and gitUnavailable stop it immediately with the real cause named
rather than relabeled "unresolvable". Continuing on a nonzero exit cannot
fail open: a pass needs a successful rev-parse AND a successful
--is-ancestor. `merge-base --is-ancestor HEAD <default>` then passes on
exit 0 only; exit 1 is git's ANSWER (hedged wording) and exit 128 is
"could not answer" (distinct wording) — never conflated, neither passes.
- Every gate is fail-CLOSED over the runner's FULL outcome enum: command
failure, timeout, git unavailability, and unreadable output each fail the
affected gate with the cause NAMED. A successful symbolic-ref whose
output is not a ref fails closed instead of falling through as "unset".
- Canonical FOUR-CLAUSE evidence (round 10) for candidates AND
non-candidates, in G1..G4 order; the D5 hedge lives INSIDE the G3 clause
("HEAD not an ancestor of <default> (squash/rebase merges not detected)")
so evidence never asserts non-merger as fact. Candidates additionally
carry the date slot — the date OR the explicit "last commit unavailable"
marker, never a silently absent field — and the field-verified branch-ref
sentence. `show -s --format=%ct HEAD` is display-only: every failure
class yields the marker, never a failed assessment.
- Prunable records are refused with a distinct NON-GATE reason before any
git runs (fn-5.5's orphaned-admin tier owns them) — four gate answers
about a directory that no longer exists would be a lie.
- Read-only commands ride fn-5.1's D17 read-only profile automatically
(classification is by COMMAND); the assessor never bypasses the runner,
and the tests assert profile + GIT_OPTIONAL_LOCKS=0 + -c
core.fsmonitor=false on every recorded invocation of a REAL runner.
Tests (32): real hermetic fixture matrix — merged+clean candidate,
modified, untracked, unmerged, locked, detached at merged (candidate) and
at unmerged (not), main and bare refused; a dirty-submodule fixture whose
committed `submodule.sub.ignore = all` makes the bare default report the
tree CLEAN while --ignore-submodules=none catches it (the premise is
proven in the test, not assumed); the ladder on real git (origin/HEAD via
a clone, local main after an exit-128 origin/HEAD with the -C parent
target asserted, local master, and neither -> closed); injected failure
classes on every gate command including --is-ancestor exit 128; the pinned
candidate and pinned MIXED-FAILURE shapes asserted VERBATIM as literals
against a pinned committer date in an injected UTC zone.
Validation: swift build clean; swift test 891 tests, 1 pre-existing skip,
0 failures (baseline 859 + 32 new).
Task: fn-5-stale-git-worktree-scanner.2
Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…oting the next ref (review r1)
The ladder previously treated EVERY nonzero exit as "this rung is missing"
and continued. That absorbs a real failure class into a benign one — the
exact shape fn-5.1's review caught twice. Concrete fail-open: `origin/HEAD`
unset, `rev-parse refs/heads/main` fatal (unreadable/corrupt), and
`refs/heads/master` resolving — the worktree would then be judged against
master and could pass G3 while unmerged into the repository's actual
default branch.
- `symbolic-ref` gains `-q`, which is what SPLITS the two classes: verified
on git 2.50.1, `-q` exits 1 for unset / deleted / non-symbolic
origin/HEAD (the common fetched-repo shape) and 128 when git could not
look at all (not a repository, unreadable ref file). Without `-q` the
ordinary unset case also dies with 128 and is indistinguishable from a
broken repo.
- Exit 1 — `refMissingExitCode`, shared by both ladder commands
(`rev-parse --verify --quiet` answers 1 for an absent or git-ignorable
broken ref) — is now the ONLY nonzero exit that continues the ladder.
Every other nonzero exit fails G3 CLOSED with the RUNG and the exit
named ("refs/heads/main lookup failed (git exit 128: …)"), joining the
timeout and gitUnavailable classes that already stopped it.
Tests: the new fatal-rung case covers all three rungs — fatal origin/HEAD
(no rev-parse follows), fatal main WHILE master resolves (the reviewer's
promotion scenario: not a candidate, master rung never runs), and fatal
master after an absent main (no ancestry check) — plus the real-git ladder
test now asserts `-q` on argv and the MISS exit rather than a fatal.
Validation: swift build clean; swift test 892 tests, 1 pre-existing skip,
0 failures (baseline 859 + 33 new).
Task: fn-5-stale-git-worktree-scanner.2
Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…g (review r2) Exit 1 alone was still too coarse. Git answers 1 for a ref that EXISTS and cannot be read — `warning: ignoring broken ref refs/heads/main`, verified on git 2.50.1 against a corrupted loose ref — which is a different fact from "there is no such ref". The ladder would fall through to `master` and a worktree unmerged into the repository's actual default branch could pass G3. - `isRefMissing(exitCode:stderr:)` now gates the continuation: exit 1 AND a whitespace-only stderr. Verified discriminator — an absent ref (`rev-parse --verify --quiet`) and an unset/deleted/non-symbolic origin/HEAD (`symbolic-ref -q`) are both SILENT, while every diagnostic answer says something. A talkative exit 1 now fails G3 closed with the rung and git's own message named. Strictness costs at most a missed reclaim; the other direction costs human work. Tests: the failed-rung matrix gains the talkative-exit-1 cells for main (while master resolves — the promotion scenario) and for origin/HEAD, plus a REAL-git proof — a corrupted refs/heads/main beside a healthy master makes the linked worktree a non-candidate with "refs/heads/main lookup failed (git exit 1: …)". Validation: swift build clean; swift test 893 tests, 1 pre-existing skip, 0 failures (baseline 859 + 34 new). Task: fn-5-stale-git-worktree-scanner.2 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
… ignore (review r3) Second repository-CONFIGURATION hole in the same family as the submodule one: `status.showUntrackedFiles = no` makes `git status --porcelain` return EMPTY for a worktree holding only untracked files (verified on git 2.50.1). G2 would have read that as clean, all four gates would pass, and untracked work — the work that exists nowhere else — could be deleted. - `statusArguments` gains `--untracked-files=normal`. A gate that authorises deletion must not let the repository decide what "clean" means; both flags now exist for exactly that reason and both holes are fixture-proven by asserting the bare-default command reports the tree clean. - `normal`, not `all`: for a gate that only asks "is there ANY output", listing every file inside an untracked directory instead of the directory itself changes nothing but cost — and this scanner's subject matter is worktrees carrying multi-GB untracked build trees, where the extra walk could exhaust the scan budget and fail the gate closed on an ordinary tree. Tests: a repo configured with status.showUntrackedFiles=no plus untracked work — the configured default is asserted EMPTY, the assessment is not a candidate, and fn-5.4's exported clean-check surface reports dirty through the same argv; the pinned-argv assertions (exported arguments + the real recorded invocation) now cover both flags. Validation: swift build clean; swift test 894 tests, 1 pre-existing skip, 0 failures (baseline 859 + 35 new). Task: fn-5-stale-git-worktree-scanner.2 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…e sites, fail-closed placeholder
The epic's only cross-cutting core change. ONE parameterized ReclaimAction
case whose payload is a plan of structured paths — never argv: the cleaner
builds the git argv from plan fields plus registry constants, so command
argv stays trusted registry code (the fn-2.3 argv-provenance rule).
`.commands` cannot express either mode at all — checks (f)/(g) require a
registered CacheCategory whose cleanCommands equal the argv, and fn-5 has
none, so one such item would malform the whole outcome.
Compile-through census (the epic's early proof point): adding the case with
every site unpatched errored at EIGHT production sites, not the seven the
epic listed — fn-4.8 gave `structuralRefusal` a SECOND exhaustive switch
(`revalidatableAction`) after the census was taken. Epic spec corrected.
1. CacheCleaner:373 zero-root-record — composite JOINS the refusal, both
modes (its binding rule demands a measured record anyway)
2. CacheCleaner:410 zero-byte skip — composite EXCLUDED: a prune-only item
frees ~0 bytes and must still run; this skip precedes dispatch
3. CacheCleaner:423 dispatch — FAIL-CLOSED PLACEHOLDER (fn-5.4 replaces):
per-item error, never a silent no-op; the missing-runner cause is worded
apart from the unwired-arm cause so a wiring regression cannot hide
4. CacheCleaner:504 structuralRefusal — delegates to the shared rule set
5. SpaceScanner:148 wireString — FROZEN `git_worktree_reclaim`, kind only
6. SpaceScanner:1327 validator structure arm — same rules + origin binding
and the converse-ownership refusal (validator-only facts)
7. SpaceScanner:1485 nested argv-coherence switch — explicit unreachable
arm, mirroring the `.removeItem: break` precedent
8. CacheCleaner:494 revalidatableAction (THE MISSED SITE) — false: this
build has no composite performer, so a marked item is refused rather
than reclaimed without the seam it structurally demands
One rule set, two enforcers (the missingRevalidatorRefusal precedent):
`GitWorktreeReclaimPlan.violation(for:plan:)` is called by the cleaner's
chokepoint AND the runtime validator, so they can never disagree. It rejects
a raw `..` component in ANY plan path FIRST and standardizes only afterwards
for component-prefix containment — standardization erases `..`, and the
verbatim spelling is what reaches git. Both modes bind the mutation scope to
the item's own admitted container (admin container strictly inside;
`-C` target descendant-or-equal, so a dev root that IS a repository stays
legal), and deletable states must carry the `.removeItem`-mirrored
measured-record/display binding.
Also: the runner-injection seam (trailing defaulted `gitRunner`, zero
call-site churn, nil = fail-closed) and `ScanIssue.Kind.toolUnavailable`
(wire `tool_unavailable`, nil url) with both of its exhaustive-switch arms.
Suite 917 tests (894 baseline + 23), 1 pre-existing skip, 0 failures.
Task: fn-5-stale-git-worktree-scanner.3
Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…d fallback, gated prune - WorktreeReclaimPerformer: stale-mode git removal (never --force) with the four-class runner routing (exit 0 done / nonzero → re-check / timeout → abort "may be partially removed — rescan" / gitUnavailable → refuse), the guarded rm fallback behind fn-5.2's clean re-check, and the round-8 GATED post-fallback prune (runs only when the recomputed prunable set is exactly the just-deleted worktree's admin entry, else the D11 warning) - prune-only mode: subset check against the disclosure, admit-then-measure per recomputed dir with mount-boundary fail-closure before any claim, the round-8 SECOND oracle check immediately before the subprocess, --expire=now on every execution prune, verified-removal acceptance, four-class routing - PathGuard.validateSubprocessTraversalDirectory (D13): real-directory leaf, full canonicalization, canonical containment (strict, or-equal for the parent alone), same-device fail-closed — re-run before every git invocation per the audit table - CacheCleaner: real dispatch arm replacing fn-5.3's placeholder, injectable 300s delete-time budget, site 8 revalidatableAction flipped TRUE (the performer routes composite items through the seam in both modes) - D11 warning channel end-to-end: CleanupReport.Entry.warning, the pure rowAnnotations(for:) presentation helper, CleanupReportSheet rendering, and the CLI confirmed-clean row's two-source warning merge - GitWorktreeOracle: one spelling of the oracle listing argv, shared with fn-5.5 Task: fn-5-stale-git-worktree-scanner.4 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
… (review r1) The runtime factory deliberately leaves `gitRunner` at its fail-closed nil default: fn-5.3 designed the seam, fn-5.4 built the performer behind it, and fn-5.6 threads the SHARED runner beside the scanner registration (a second instance here would fork fn-5.1's deliberately per-instance availability cache). Nothing regresses meanwhile — no registered scanner emits a composite item until fn-5.5 — and the refusal is per item, never a silent no-op. - makeCleaner: document the omission, its owner, and why it is unreachable - testTheRunnerSeamIsTrailingAndDefaulted: name itself as the cell fn-5.6 must update when it threads the runner Task: fn-5-stale-git-worktree-scanner.4 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…cleaners (review r2) The performer was unreachable through the normal composition: every GUI and CLI clean builds its cleaner from `SpaceScannerRuntime.makeCleaner`, which left `gitRunner` at its fail-closed nil default, so a composite item would have refused before execution. - SpaceScannerRuntime holds the SHARED runner (trailing, defaulted nil so every existing composition compiles) and hands it to every cleaner it makes - production() builds it ONCE — one instance per runtime, because fn-5.1 made the availability cache instance-scoped and detection and execution must agree about whether git exists; fn-5.5's scanner takes THIS instance - tests: a runner-less runtime still fails closed, a wired runtime reaches the performer's own gate, and production() provably carries the runner Task: fn-5-stale-git-worktree-scanner.4 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
A bare parent's git directory is not `<wd>/.git`, and a linked worktree of a bare main is not itself `bare`, so a `<wd>/.git/worktrees` reconstruction would mis-path the mutation scope with no gate to catch it. Both modes now execute against a real bare clone through the RESOLVER-CARRIED `<parentGitDir>/worktrees`: stale removal (tree gone, admin entry gone, branch ref surviving, `-C` pointed at the bare repository) and prune-only (the admin entry actually removed, `--expire=now` present), with the absence of `<bare.git>/.git` asserted so the forbidden reconstruction is provably impossible. Task: fn-5-stale-git-worktree-scanner.4 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…tier, emission The per-item `SpaceScanner` (slug `git_worktrees`) that turns fn-5.1's plumbing, fn-5.2's gates and fn-5.3's composite action into items. Discovery is a `ProjectTreeWalker` consumer over `DevRootsStore.effectiveRoots`: `.git` is SEEN and classified by lstat (directory = main checkout, regular file = linked worktree) but never descended, hidden parents are traversed, and no name-based skip list is inherited — the three ways the 23 GB field case stayed invisible. - ONE porcelain listing per repository: a pre-fetch grouping on the canonical COMMON GIT DIRECTORY (resolver `commondir`, or an lstat-proven `.git` dir) collapses every checkout and every root spelling of one repo, while the authoritative repo key stays the canonical FIRST-RECORD path, cross-validated by fn-5.1's own `crossValidate` before anything is derived from it - THREE distinct id keys (round 8/F3): repo dedupe on the first record, stale `stableID` on the WORKTREE path, prune `stableID` on the ADMIN CONTAINER — the multi-item repo fixture proves all-unique ids and a non-malformed outcome - D15: assessed non-candidates are OMITTED from items entirely; their assessment rides a `GitWorktreeAssessmentLog` through an injected observer (default: os.Logger) instead of a lying `.denied` row - D14: ONE repo-level prune item per repository via fn-5.1's SHARED mapper, `.measured` ALWAYS with itemCount = the disclosed count (an `.empty` prune item could never run — the cleaner's zero-byte skip precedes dispatch); ANY unmapped record, gated-out admin entry, mount boundary or sizing denial SUPPRESSES the item with a visible issue, while locked entries stay undisclosed without suppressing - D13: an item is emitted only when the worktree, the parent (or-equal) and the RESOLVER-CARRIED `<parentGitDir>/worktrees` share ONE declared root; outside worktrees and parent-outside repos are `.containerRefused` issues, never items. Containment is decided canonically and the plan paths are then RE-SPELLED under the declared root, so the validator's lexical checks hold for alias-declared roots instead of malforming the outcome - TWO TCC gates, both reusing fn-4's `isProtectedRoot`: the walker's root gate and the round-7 SECONDARY gate over the git directory, the `-C` target and each worktree — silent deferral on `.automatic`, assessed when user-initiated - `gitUnavailable` withdraws every item and publishes the frozen `.toolUnavailable` issue (nil url, "git unavailable" prefix); empty dev roots stay a benign clean-empty outcome with no issue and no subprocess 32 new tests; every outcome round-trips the 8-family validator. Suite: 995 tests, 1 pre-existing skip, 0 failures. Task: fn-5-stale-git-worktree-scanner.5 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…condary TCC gate (review r1) The secondary gate ran too late for linked worktrees. `repositoryGroups` resolves each worktree's `.git` pointer — probing the admin directory and reading its `gitdir`/`commondir` — BEFORE `process` can classify the resulting git directory, so an automatic scan could read protected git admin files for a worktree in an unprotected dev root whose parent repository lives in `~/Documents`. No git subprocess is involved, so the existing "no git argv touches the protected path" assertions could never have caught it. The path is not knowable before the pointer is read, so the gate cannot precede the resolution; it is applied INSIDE it. On `.automatic` the per-scan resolver runs on a `DeferringIdentityProvider` that reports every deferred path as ABSENT, and the resolver's own fail-closed structure (probe, then read) stops before anything under a protected ancestor is opened, enumerated or read — the worktree simply attributes nowhere, silently, like every other policy skip. `canonicalize` still runs on deferred paths, exactly as fn-4's pinned protected-root classification already does before skipping a protected root; the wrapper does not move that line. - the resolver is now per-scan and threaded through `process`/`handle`, so the forged-pointer edge (a `.git` file naming an arbitrary protected path) is covered by the same gate - new test: the protected git directory sits OUTSIDE the dev root so only the resolver can reach it — an automatic scan probes NOTHING under it, a user-initiated scan does (the deferral is policy, not a capability), both spellings of the path checked so the alias cannot make it vacuous Suite: 996 tests, 1 pre-existing skip, 0 failures. Task: fn-5-stale-git-worktree-scanner.5 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
… (review r2) Cross-validation inspects the porcelain first record (`<firstRecord>/.git`), and the stage-2 gate ran after it. For a repository whose common git directory is unprotected but whose first-record working tree sits under a protected ancestor — the split-repository shape the epic already relies on for the first-record authority proof — an automatic scan reached cross-validation with the deferring provider, saw the protected path reported absent, and published a VISIBLE `.unreadable` membership refusal for what is a silent policy deferral. The gate now runs immediately after the listing is parsed, before anything inspects the first record. New regression test (proven to fail without the reorder): unprotected git directory and worktree, protected first record — automatic yields no items, NO issue and no git argv touching the parent, while a user-initiated scan queries the ladder against it and emits the item. Suite: 997 tests, 1 pre-existing skip, 0 failures. Task: fn-5-stale-git-worktree-scanner.5 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…ared runner - production(devRoots:) resolves the dev roots ONCE and hands them to both dev-root scanners; the git runner is built once and given to the scanner AND the runtime, so detection and execution share fn-5.1's instance-scoped availability cache - production(gitRunner:) substitutes that one instance for hermetic tests (GIT_CONFIG_GLOBAL/SYSTEM pinned) — the default still builds the real one - trash-honesty disclosure (F7): gitWorktreeTrashDisclosures names selected composite items, stale removal (unlinked permanently, fallback named) apart from repo prune (admin data, branch refs survive); the sheet renders them - GitWorktreeEndToEndTests: registration + admission union + one-runner proof, three-worktree scan/select/clean/report with the dirty tree assessed and omitted, CLI --confirm gate, tool_unavailable wire shape, and the git boundary grep gates over the production source tree - registry-list assertions updated for the fourth registered scanner Task: fn-5-stale-git-worktree-scanner.6 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…ule, CATEGORIES section - PROTOCOL: git_worktree_reclaim on the action row (plan paths never exposed), tool_unavailable added to the kind list and to the COMPOSABLE path rule's non-filesystem set, the confirmed-clean warning key's second delete-time source (D11), and the Subprocess Timeout exception — NO client-side timeout for composite-capable confirmed cleans, the four-clause caller-decidable trigger, the CLI's own budgets as the only bounds, and an honest outer-kill caveat (orphaned git child, partial tree, next-scan recovery). No formula. - CATEGORIES: a Stale Git Worktrees section (gates, tiers, selection posture, deletion sequence, trash honesty, timeouts, out-of-scope) - CLI-REFERENCE/API-REFERENCE/ARCHITECTURE/README: the slug, the conformer, the production() signature, the fn-5 source files - CHANGELOG: the scanner, the git-mediated removal, tool_unavailable, and the RELEASE-BLOCKING cacheout-mcp gate with owner and a two-part source-scoped verification whose passing states are stated - TCC usage strings in all three build paths now name worktree discovery — git_worktrees walks the same protected roots build_artifacts does - doc-accuracy tests: action wire strings from the ENUM, the extended kind taxonomy + path rule, the D18 rule/trigger/no-formula, and the gate's shape Task: fn-5-stale-git-worktree-scanner.6 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…the sync rule The checked-in project.pbxproj is a fourth, GENERATED copy of the TCC usage strings and it ships through the Xcode build path — it had drifted silently. Regenerated from project.yml, and added to the sync test's source list so a stale checked-in project fails the suite instead of shipping the wrong prompt. Task: fn-5-stale-git-worktree-scanner.6 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
… builder The generic row builder needed no per-scanner branch, and the plan payload never reaches the wire — the documented base key set, asserted over the item the production scan actually produced. Task: fn-5-stale-git-worktree-scanner.6 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…eline and required change The gate now records what was actually observed in the sibling checkout (63edbfc: AppEngine._run wraps every CLI call in a 120s wait_for at engine.py:475) and what closing it requires, so a release engineer can verify the gate is unmet without reading the other repository. Its status line is pinned by the doc-accuracy test. Task: fn-5-stale-git-worktree-scanner.6 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…eview r2) Review r2's remedy for the still-open cacheout-mcp timeout gate: merging with it open is deliberate — the scanner reaches users only at release — so the deferral is now stated AND enforced instead of trusted to memory. - scripts/bundle.sh gains check_release_gates, run FIRST in the --notarize/--release arm (before build, sign, DMG and notarization): it aborts while CHANGELOG [Unreleased] still carries a RELEASE-BLOCKING gate reading NOT SATISFIED, and aborts on an unreadable CHANGELOG. Shipped sections are history and never block. Verified in all three states. - CHANGELOG records the deferral explicitly and names the enforcement point - doc-accuracy test pins the check's existence, its position before every expensive step, its markers, and its fail-closed arm - self-review fixes: productionScannerIDs had gone stale (the rebuild guard silently stopped covering git_worktrees); the prune-argv gate now asserts non-vacuity instead of over-claiming "exactly one builder"; the outer-kill caveat assertion no longer depends on prose line wrapping; the grep gate's multi-line-string limit is stated; CATEGORIES states the prune tier's provably-complete-or-no-item rule Task: fn-5-stale-git-worktree-scanner.6 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…unambiguous (review r3)
r3 found two real holes in the release gate:
- `--direct` produces a signed, distributable DMG and skipped the gate
entirely — the shorter command bypassed the protection. Both distribution
arms now run check_release_gates FIRST, before build/sign/package/notarize.
The unsigned no-flag testing build ships nothing and stays ungated, stated
in the script and the CHANGELOG.
- the documented close ("replace OPEN with the hash") did not match what the
script keyed on, so following it left the release blocked forever. There is
now ONE status line and ONE edit: `Status: **NOT SATISFIED**` becomes
`Status: **SATISFIED at <commit-hash>**`. The gate keys on that LINE, so the
paragraph explaining the mechanism no longer blocks every build.
Tests: string-presence coverage replaced by EXECUTING the extracted shell
function over fixture CHANGELOGs — open aborts, missing aborts, a gate in a
shipped section never blocks, and applying the CHANGELOG's own documented
close instruction verbatim really does open the release path.
Task: fn-5-stale-git-worktree-scanner.6
Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…failing open (review r4) r4: searching only for the open marker FAILED OPEN on every way the record can break — deleting the status line, renaming [Unreleased], or writing `Status: **SATISFIED**` with no commit all read as "closed" and shipped. The checker now verifies the recorded FORM: each `**RELEASE-BLOCKING` marker in [Unreleased] must pair with exactly one status line, and a status line has exactly two admissible spellings — `**NOT SATISFIED**` (blocks) or `**SATISFIED at <7-40 hex>**` (passes). Every other state is named and refused: missing section, missing/extra/orphan status, hash-less or non-hex "satisfied". Only a provably-satisfied gate lets a distribution build run. The executable test now drives all ten classes through the real shell function; the CHANGELOG states the two admissible spellings so the instruction and the checker cannot drift. Task: fn-5-stale-git-worktree-scanner.6 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…(review r5) r5's counterexample passed the count check while leaving a gate unverified: two markers, two valid statuses — both statuses under gate A, none under gate B. Counting is not pairing. The [Unreleased] section is now parsed statefully: a marker requires the previous gate to have been resolved, a status requires an active gate (so a second one is an orphan), and an unresolved gate at section end is refused. Spelling validation of each paired status is unchanged. The executable fixtures gain the multi-gate cases: the reviewer's both-statuses-under-one-gate layout, first-gate-unstated, last-gate-unstated, one-of-two-still-open (all blocked), and two gates both satisfied (passes). Task: fn-5-stale-git-worktree-scanner.6 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…pelling (review r6) r6: the parser recognized only well-formed `Status: **` lines, so a typo'd open status (`Status: *NOT SATISFIED*`) was skipped as prose and the valid status beneath it closed the gate — the very ambiguity the checker claims to refuse. Structure now runs over EVERY `Status:` line, well-formed or not; spelling is validated afterwards. A typo above a valid status is therefore an orphan (two statuses, one gate) and a typo alone is refused by name. Fixtures cover both. Task: fn-5-stale-git-worktree-scanner.6 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…ng every gate (review r7) r7 reproduced it on both distribution arms: when bash cannot create the heredoc's temporary file it prints an error, SKIPS the `while read` body entirely, and the function falls through to "Every release-blocking gate ... is satisfied". A machine with signing credentials would have gone on building a distribution artifact with an unverified open gate — the gate passing because the disk was full is exactly the fail-open it exists to prevent. Validation is now counted through pipes and arithmetic instead of iterated: open statuses and well-formed satisfied statuses are counted, an open count above zero blocks, and a satisfied count that does not equal the gate count blocks. A grep that cannot run yields zero, which fails the equality — every failure mode lands on "blocked". Coverage: the whole seventeen-class matrix now runs TWICE, once with TMPDIR pointing nowhere writable, and the verdicts must match; plus a structural guard that the gate function contains no heredoc at all. Task: fn-5-stale-git-worktree-scanner.6 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
r8: the satisfied regex was prefix-anchored, so `**SATISFIED at 63edbfc**x` counted as well-formed and let the gate pass. After the closing token the line must now either STOP or continue with WHITESPACE — the documented edit leaves a spaced sentence tail, which stays legal, while anything ATTACHED (`**x`, `***`) means the token is something else and the status cannot be read as satisfied. Fixtures added for both attached-character shapes; the whole matrix still runs twice (normal and unwritable TMPDIR). Task: fn-5-stale-git-worktree-scanner.6 Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
The consumer adopted D18. `cacheout-mcp` commit `a854996` on branch `fn-1.3-memory-stats-mcp-tool` (PR #1) replaces the blanket `asyncio.wait_for(proc.communicate(), timeout=120)` in `AppEngine._run` with a per-invocation budget derived by a pure policy function, `resolve_cli_timeout(args, item_actions) -> Optional[float]`, returning None — wait indefinitely — on all four of PROTOCOL.md's caller-decidable triggers: a `git_worktrees` target token, a `git_worktrees:` address prefix, a preflight scan row whose action is `git_worktree_reclaim`, and a scanner-ambiguous target. Every other command keeps its budget. Verified before flipping the status line, not asserted: - gate (1) `"git_worktrees"|git_worktrees:` over `src tests` — 10 matches (non-zero required: the rule is implemented AND tested). - gate (2) `proc\.communicate\(\), timeout=120` over `src` — 0 matches (zero required: the blanket bound is derived away, not branched past). - consumer suite re-run here: 241 passed / 6 failed, up from 211 / 6, the same 6 pre-existing app-mode failures by name. - `check_release_gates` extracted from the shipped `scripts/bundle.sh` and executed against this CHANGELOG: "✅ Every release-blocking gate in CHANGELOG.md [Unreleased] is satisfied". Also de-pins two contract tests from the gate's OPEN state. Both encoded a transient condition as a permanent invariant, so closing the gate — the thing the gate exists to allow — turned them red: - `testTheReleaseGateOpensAndClosesExactlyAsDocumented` used the LIVE CHANGELOG as its "open" fixture. It now normalizes the real document to the open state (rewriting whichever admissible status line it carries) and drives the transition from there, so every fixture stays anchored to the shipped changelog's structure while none depends on today's status. A `live` case is added asserting the CHANGELOG AS SHIPPED passes the gate — the property that actually matters at release time, and one the old open-pinned form could never express. - `testRecordedTimeoutGateIsBlockingNamedAndVerifiable` asserted the literal `NOT SATISFIED` substring. It now requires a status line in either admissible spelling, which is the real invariant: an absent or unparseable status is the unverifiable case the release script refuses. Suite: 1014 passed, 1 pre-existing skip, 0 failures. Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
…ommit `a854996` satisfied the gate's two greps but carried a FAIL-OPEN in the very rule the gate exists to enforce, found by Codex review on cacheout-mcp PR #1. `_remember_scanner_actions` REPLACED the scanner-action cache on every scan, and that cache was an input to the timeout decision. A scan that merely omitted an item — concurrency, a transient scanner error, a `malformed_outcome` row excluding a whole scanner's outcome — dropped its `git_worktree_reclaim` entry, so a later confirmed clean of that still-valid address fell back to the 120 s budget and could be SIGKILLed mid-`git worktree remove`. That is precisely the partial-repository-state outcome D18 forbids. `0b50b62` removes the possibility rather than patching the retention policy: `resolve_cli_timeout(args)` is now a pure function of the invocation's own argv plus the static category registry. A confirmed clean keeps the finite budget ONLY when EVERY target is a bare, KNOWN category slug; any `<scanner>:<item-id>` address, any bare scanner slug and any unrecognized token get NO timeout. That is a strict superset of PROTOCOL's four caller-decidable triggers, so it cannot grant a timeout the contract forbids, and every unknown case resolves to no-timeout — the safe direction the contract itself prescribes. The consumer now holds no per-scan state at all, so there is no cache left to lose. The gate names `0b50b62` rather than the branch tip because that is the commit where the rule became sound; the three commits after it are schema-4 contract fixes unrelated to D18. Worth recording for the next gate author: the two greps still passed against the fail-open. A grep-shaped gate can verify that a rule is PRESENT and TESTED; it cannot verify that the rule is airtight. The hash in this line is what makes that reviewable by a human, which is why it must name a commit someone actually read. Re-verified in the cacheout-mcp checkout at 0b50b62's branch: grep (1) 14 hits (non-zero), grep (2) 0 hits; suite 258 passed / 6 failed, the same 6 pre-existing failures by name (baseline 241/6). `check_release_gates` extracted from the shipped `scripts/bundle.sh` and run against this CHANGELOG: satisfied. DocumentedContractTests: 17 passed. Claude-Session: https://claude.ai/code/session_01P6LVX7RkPM8NW7dHx16ewU
12 conflicted files / 27 hunks, plus the seam changes the textual merge could not see. Resolutions: - SpaceScanner.production(): both sides register a scanner. Kept both, with each side's parameters in their original relative order so any existing labelled call still type-checks. - CacheCleaner pre-delete seam: main turned the optional refusal tuple into a non-optional `PreDeleteOutcome` carrying the inspected OBJECT BINDING; fn-5 had made the same function `nonisolated` so the composite performer could call it synchronously. Took main's richer contract and re-applied `nonisolated`, then added a refusal-only adapter for the performer. `git_worktrees` registers no revalidator, so the adapter can only ever see `.unestablished` — it matches that arm explicitly and trips `assertionFailure` if an established binding ever arrives, rather than silently dropping the identity proof. - ScanIssue.Kind: fn-5's `.toolUnavailable` does not collide with fn-6's four new kinds; the hand-maintained taxonomy list is now the full 13-case union in declaration order. - Docs/tests: union of both sides' scanner lists, with "to follow" clauses deleted where both scanners now exist. Seam changes the auto-merge hid (compile-forced): - `removeItemConcurrently` gained `expecting:`/`provider:`/`containedIn:` (fn-6 DepthSafeRemoval). The performer's `removeTree` seam now carries the `AdmittedParent`, captured from a descriptor BEFORE the TOCTOU rechecks — main's ordering verbatim, so the binding covers the rechecks and not merely the queue hop. - The Trash handler now answers where the item landed; the composite arm discards it. That arm is still the raw mover with no container binding, unlike every other item's disposal — left as-is and flagged in a comment for its own round. Sources build clean; the one remaining warning is byte-identical to main.
…urement (PR #460 codex r16, A-P4) A-P4 was offered as "recorded, your call, not asserted as defects". Two of the three are recorded WITH a cell, because a note nobody ran is the thing this branch keeps catching; one is a message correction. (a) `.strandedInTrash` COVERS A CONTAINER THAT IS NO LONGER A FOLDER, AND ONLY `.destinationNotTheAdmittedContainer` TELLS THE USER THE FOLDER CHANGED. `rollBack`'s destination `catch` splits exactly two ways — `.notTheAdmittedContainer` and EVERYTHING ELSE — and a container replaced by a regular file cannot be opened as a directory at all, so it never reaches the identity comparison. MEASURED by the new cell `testADestinationContainerTurnedIntoAFileIsReportedAsAStranding`: four Trash arms × two container spellings = EIGHT rows, `.strandedInTrash` on every one, the object left at the landing the refusal names. Sanity mutation (expect `.destinationNotTheAdmittedContainer`): 8/8 red. RECORDED RATHER THAN SPLIT, and the cell says why: the item is safe and its path is named, which is the whole of what the cause promises; a seventh cause needs its own user-facing entry and its own evidence, which is not a change to make on the strength of a note. The cell exists so the claim is a measurement, and so the row moves visibly if anyone does split it. (b) `.putBack`'s "Nothing was moved to the Trash" WAS A NET-EFFECT CLAIM, and the net effect is not what happened: an object WAS moved to the Trash and then retrieved. What the arm proves is narrower, and is now what it says — "it was moved back out of the Trash and identified at this path". The whole-message fence gains the row (forbidden: the old clause; required: "PUT BACK" and "identified at this path"). The lowercase "nothing was freed" clause is kept verbatim: two cells assert it. (c) THE `.absent` + `absenceProves: true` + `.noDirectoryTree` → `nil` BRANCH IS NO LONGER ON A PRODUCTION PATH, and now says so. Production reaches `absenceProves: true` only through `proveStandingUnderAdmittedContainer`, which `dispose(_:expecting:…)` calls for `.directory` and `.unestablished` — never for `.noDirectoryTree`, which r13 routed to `disposeBoundLeaf`, throwing `.posix(ENOENT)` one call earlier. MEASURED at 073371c: replacing that `return nil` with `.posix(ENOENT)` and running the FULL suite reddens exactly TWO cells and nothing else — 1558 executed / 2 skipped / 2 failures, exit 1, 205 s. Both call `TrashDisposal.proveStanding` DIRECTLY; neither reaches production. (The note this round was handed said ONE test-only cell; measurement says two, and the comment names both.) The branch is kept: the asymmetry it encodes is the contract `absenceProves` exists to express, and a deleted branch is a contract nobody can re-check. TESTS swift test --filter 'TrashDisposalHopProofTests|CacheCleanerTests|OrphanedCachesScannerTests' 0 failures.
…er arm, and the reason is git's (PR #460 codex r16, G2) G2, from the push gate. The reachability claim on `testEveryDestructivePathDisposesUnderASymlinkedContainer` — "No shipped scanner reaches it today … so it was latent, not shipping" — is CORRECT, and its stated reasons cover only the two `.directory` producers (they emit DIRECT children of their admitted roots) and `ContainerSnapshot` (it refuses both arms when the symlink IS the admitted root). Neither reason covers the GIT-WORKTREE path, which does not reach its target by listing an admitted root at all: `GitWorktreeInventory` runs `git worktree list --porcelain -z` and takes the path from the REGISTRY. The reason that path is out is git's own: `git worktree add` RESOLVES the symlink before it writes the registry, so the spelling the scanner reads back is always the real one. MEASURED at r16 on this machine (git 2.50.1, Apple Git-155), THREE fresh repositories, each worktree created at `<base>/link<N>/wt<N>` where `link<N>` is a symlink to `<base>/real/holder<N>`: both `.git/worktrees/wt<N>/gitdir` and `git worktree list --porcelain` came back naming `<base>/real/holder<N>/wt<N>` — the RESOLVED spelling — in 3 of 3 cells. Recorded at the site where the reachability claim lives, and recorded as a property of `git` rather than fenced by a cell of ours, since a cell here would pin somebody else's behaviour. TESTS swift test --filter 'TrashDisposalHopProofTests|SourceAnchorIntegrityTests|DocumentedContractTests' 61 executed, 0 failures.
…the four missing ordinals confirmed (PR #460 codex r16, G1) G1, from the push gate. FOUR STRAND ORDINALS EXISTED NOWHERE ON THE BRANCH. Every commit body in `f88ef10..HEAD` and every source, test and CHANGELOG file was grepped: only 3, 4, 7, 8, 9, 10, 11 and 12 are ever STATED. 1, 2, 5 and 6 were being RECONSTRUCTED in the draft PR body out of commit subjects. CONFIRMED HERE BY CHRONOLOGY rather than reconstructed — the stated ordinals bracket the gaps and the commit timestamps are strictly ordered, so nothing else can occupy those slots: 1 8fa8ad3 r3 01:49 trapping integer subscript after a count assertion 2 60a1696 r4 06:35 SIGPIPE on a bare client socket, no SO_NOSIGPIPE 3 9d63e0a r5 08:06 (STATED "THIRD") 4 193b043 r6 10:17 (STATED "FOURTH") 5 88c9a1c r7 11:57 force-unwrap of a PRODUCTION-DECIDED optional 6 619aac4 r7 12:01 withoutActuallyEscaping over a genuinely escaping closure 7 e0f27ef r9 (STATED "SEVENTH") 8 101753b r10 (STATED "EIGHTH") 9 065b173 r11 (STATED "NINTH") 10 72dd0e9 r12 (STATED "TENTH") 11 7afd585 r13 (STATED "ELEVENTH") 12 828d8c3 r14 (STATED "TWELFTH") NOTE THE ORDER OF 5 AND 6: the brief reconstructed them as withoutActuallyEscaping then force-unwrap. The commit timestamps are the other way round — 88c9a1c 11:57:07, 619aac4 12:01:13, four minutes apart on the same day — so 5 is the force-unwrap and 6 is `withoutActuallyEscaping`. Both are independently measured strands (493 of 1471 cells never ran; a trap that appeared only under the FULL suite). The list is recorded in `StrandFenceTests`' header, WITH each mechanism's own measured strand, so the PR body cites one site instead of rebuilding it. AND THE TWO SCALES ARE NAMED. This file numbers FENCE POSITIONS; the commit messages number STRAND MECHANISMS; fence position 5 is the EIGHTH mechanism. The mapping is now a table (positions 1-6 ↔ mechanisms 1, 3, 4, 7, 8, 9), with the four that have no fence position accounted for: 2 is fenced by `TestSocketClient`/`SuiteIntegritySocketTests`, 5 and 6 by the force-unwrap rule and by a type change, and 10-12 are PRODUCTION bounds with no source shape to scan. Also: the section heading said "The four positions" while listing SIX — stale since r10 added the fifth. TESTS swift test --filter 'StrandFenceTests|SourceAnchorIntegrityTests|DocumentedContractTests' 39 executed, 0 failures.
…codex r16) `swift test` AT COMMIT bce1cd9 reported 1559 executed / 2 skipped / 0 failures, exit 0, 180 s wall. Five cells added this round — `…WithoutFullDiskAccessEveryUndoStrandsTheItemInTheRealTrash` (A-P3), `…NoTrashFailureMessageAssertsAnythingItsOwnProofDidNotEstablish` and its two fixture cells (A-P1/A-P2), and `…ADestinationContainerTurnedIntoAFileIsReportedAsAStranding` (A-P4a). The three sites that carry the running endpoint all still ended at 1554/afaea66, which is the r16 FIRST fixer's head: Sources/Cacheout/Cleaner/WorktreeReclaimPerformer.swift Tests/CacheoutTests/WorktreeReclaimPerformerTests.swift (class header) Tests/CacheoutTests/StrandFenceTests.swift (the running list) Every figure names the commit it was taken at, which is the only spelling that cannot rot. Also one wording fix in the A-P3 CHANGELOG entry: "the entry above did not say so" read as if it meant the PREVIOUS bullet, when it corrects two sentences inside its OWN. It now says "neither sentence above said so". TESTS swift test --filter 'StrandFenceTests|SourceAnchorIntegrityTests|DocumentedContractTests|WorktreeReclaimPerformerTests|TrashDisposalHopProofTests' 157 executed, 0 failures.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6afc9fc25
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+1329
to
+1335
| try await removeTree( | ||
| directory, admittedParent, LastInstantProof { | ||
| if let revived = revivedCheckoutRefusal(for: directory) { | ||
| throw LastInstantRefusal( | ||
| tag: "prune-checkout-revived", detail: revived | ||
| ) | ||
| } |
There was a problem hiding this comment.
If another process replaces a prunable admin directory at the same pathname after the final oracle check (for example, pruning the old entry and leaving a newly created worktree orphaned under the reused name) but before this removal's queue-hop proof runs, revivedCheckoutRefusal accepts the replacement whenever its checkout is absent or its backlink is unreadable. The injected removeTree then calls DepthSafeRemoval with expecting: nil, so it recursively deletes the replacement even though none of the oracle, admission, or measurement checks examined that object; this can discard the new entry's HEAD/reflog and make a detached commit unreachable. Capture each final-set directory's identity and verify it in the last-instant closure or pass an equivalent leaf binding into the removal.
Useful? React with 👍 / 👎.
Comment on lines
+1331
to
+1335
| if let revived = revivedCheckoutRefusal(for: directory) { | ||
| throw LastInstantRefusal( | ||
| tag: "prune-checkout-revived", detail: revived | ||
| ) | ||
| } |
There was a problem hiding this comment.
If a user locks an orphaned worktree after the final oracle listing but before this last-instant closure, the direct removal still deletes its admin directory because the only far-side check is revivedCheckoutRefusal, which does not inspect <admin>/locked. I checked Git 2.43 (git worktree lock -h documents git worktree lock [--reason <string>] <worktree>) and confirmed that locking a missing checkout succeeds and changes its listing from prunable to locked; therefore this race defeats the user's explicit preservation lock and can remove the locked entry's HEAD/reflog. Although the previously reviewed fallback path was removed, the fresh evidence is that the new direct prune path recreates the bypass; re-probe the lock file and refuse inside this closure.
Useful? React with 👍 / 👎.
Comment on lines
+943
to
+947
| try await removeTree( | ||
| worktreePath, admittedParent, | ||
| LastInstantProof { | ||
| if case .refuse(let tag, let detail) = | ||
| reproveFromTheFilesystem( |
There was a problem hiding this comment.
If the checkout directory is renamed aside and another directory containing a compatible .git file is renamed onto the same path after the final status but before this far-side proof, every check here still passes: the replacement resolves to the unchanged admin entry, the admin inode and HEAD witness match, and no lock exists. The production removeTree closure then invokes DepthSafeRemoval with expecting: nil (and the Trash path binds only the replacement it finds after the swap), so arbitrary ignored or untracked files in the replacement are deleted even though the clean check inspected the original tree. Carry the scanned checkout root's identity and verify that identity inside the far-side removal/Trash proof.
Useful? React with 👍 / 👎.
Comment on lines
+1313
to
+1315
| for directory in directories { | ||
| if let revived = revivedCheckoutRefusal(for: directory) { | ||
| return ("prune-checkout-revived", revived) |
There was a problem hiding this comment.
For a prune item containing multiple admin directories, this loop removes entries sequentially, but a revival refusal on any entry returns immediately with text asserting that “nothing was pruned.” If the first entry was already removed and the second is repaired before its turn, the cleaner emits only a failed item with zero credited bytes and no indication that the repository was partially cleaned; the analogous catch path below correctly warns about partial cleanup. Track completed removals and report the partial result, or ensure every possible pre-removal refusal is evaluated before the first deletion.
Useful? React with 👍 / 👎.
Comment on lines
+1126
to
+1129
| let report = measure( | ||
| directory, .deletionTarget, await registry.knownIdentities | ||
| ) | ||
| // (5) MOUNT DOCTRINE (epic round 9): the removal is a |
There was a problem hiding this comment.
If an admin directory develops an enumeration, permission, or I/O denial between the oracle recompute and this delete-time measurement, report.denials is ignored: only mount boundaries are checked before the claims are registered and the directory is recursively removed. The scan-time prune tier suppresses the entire item for the same condition because the directory cannot be safely characterized, but the more current delete-time reading can therefore proceed on a partial measurement and either delete unmeasured metadata or fail after partially emptying the directory. Refuse the whole prune before registration whenever this report contains any denial.
Useful? React with 👍 / 👎.
…— witness the admin entries instead (PR #460 codex r17, W1) r16 refused every porcelain-listed worktree the WALK never reached, and the guard's own comment said "It CLEARS: the next scan re-walks." For the depth cause that was false. `discoveryWitnesses` was populated only from walk discoveries and the walk is bounded by `ProjectTreeWalker.defaultMaxDepth` = 8 — a `static let` no production call site overrides (`SpaceScanner.swift:2182` passes no maxDepth) and no user setting reaches. Every future scan re-walked to the same depth and refused again: a fail-closed refusal on a DETERMINISTIC limit, whose named remedy could never fire. MEASURED at e6afc9f, one repository, worktrees identical but for depth, real git, production defaults: depth 8 offered, depth 9 refused, byte-identical across three consecutive scans (`testAWorktreeBelowTheWalksDepthBudgetIsOffered\ OnEveryScan`, RED at e6afc9f with `["wt-edge"]` against `["wt-deep","wt-edge"]`). THE DESIGN ANSWER: the witness never needed the walk. What it must be is an identity of the admin entry taken BEFORE the read that produced the record's evidence — the listing — and the admin entries are lstat-able straight out of `<gitDirectory>/worktrees`. A new pass at (a2) witnesses every linked worktree of the repository, whatever depth its checkout sits at, immediately before the listing. (e2) prefers the walk's capture where there is one (it is earlier, and covers the rest of the walk too) and falls back to (a2)'s. CAN A RETRY DIFFER? Yes, for every cause that still reaches the refusal: what is left is a failed lstat/read on BOTH sides. Those are I/O outcomes, not a bound. The message and the guard's comment now say that instead of promising a re-walk. Cells, both RED at e6afc9f: - the depth cell above, three consecutive scans, asserting the property (offered) rather than the refusal — permanence, not the refusal, was the defect. - `testAWalkUnreachedWorktreeReplacedTheInstantTheListingReturnedIsRefused`: the fallback is a WITNESS, not a waiver. Two walk-unreached checkouts, one replaced the instant the listing returns; the replaced one is refused and the UNTOUCHED SIBLING IS STILL OFFERED. That control is the discriminator: an earlier draft asserting only "an issue mentioning wt-deep exists" PASSED at e6afc9f, because the strand message also contains the word "replaced". Measured, and it is why the cell is shaped this way. MUTATIONS (whole GitWorktreeScannerTests, 44 cells): - (a2) returns [:] → 4 red, incl. both new cells - drop the `?? containerWitnesses` join → 3 red, incl. both new cells - take (a2) AFTER the listing → 5 red: the replacement cell, NOT the depth cell. That asymmetry is what proves (a2) is a pre-listing capture. Two existing cells were re-aimed rather than left drifting: - `testATransientLstatFailureAtTheWitnessLoopIsNotReportedAsAReplacement` asserts the admin entry's identity call ORDER; (a2) inserts a call, so it went red and is now aimed at the third call of five. It did exactly the job it was written for. - `testOracleToAdminMappingFlowsThroughTheOneSharedComponent` banned the literal string `contentsOfDirectory` in the scanner source. That is phrasing-fencing in both directions — it passes a hand-rolled readdir and it fails a container read that derives no removal target — so it is replaced by the proposition it protects: the offered admin-directory set must equal the shared mapper's answer, recomputed independently in the cell from a fresh listing. Non-vacuous by construction (two orphans, one live worktree). Disclosed, not just committed: CHANGELOG gains the walk-unreached paragraph and CATEGORIES.md's claim that those worktrees are refused is removed rather than re-worded. CHANGELOG's "no tree depth can strand an item" is re-scoped to the descriptor bound it was written about — the project tree walk's own per-root budget is a different limit and this fix does not make that sentence true product-wide.
…it — r16's capture ran after the whole walk (PR #460 codex r17, W2) r16 moved the capture to `repositoryGroups` and closed the listing window. That part holds and is not re-derived here (see the negatives recorded in r16). But it stopped one loop short: `repositoryGroups` runs after `walker.walk(...)` has RETURNED, so a replacement hooked on an earlier discovery's lstat — after the walk, before the first `git worktree list` — was described by every read the scan then made. The three-way re-proof agreed with itself and the row was offered SILENTLY, armed with the replacement's inode. MEASURED at a0cd51d, real git, production defaults, `.userInitiated`: `testAWorktreeReplacedAfterTheWalkObservedItAndBeforeTheListingIsRefused` RED with `["wt-a","wt-z"]` against `["wt-a"]`, `issues == []`, armed inode 150284626 while the checkout the walk had observed was 150284603. r16's disclosed residual was wrong in BOTH its boundary and its growth claim — the same indictment r16 wrote against r15. The window ended at that checkout's OWN grouping capture, not at the walk; and `repositoryGroups` paid one `adminDirectory` read plus one `identity` lstat for EVERY linked discovery, so the window for the Nth discovery contained N-1 of those plus the whole walk. THE BOUNDARY: the capture is now taken inside the walk consumer, at the instant the `.git` entry is observed for that directory — `Self.consume(_:into:resolver:provider:)`, which now resolves `<wt>/.git` and lstats the admin entry there and carries both on the discovery record. `repositoryGroups` only groups; it re-reads nothing. The discovery record and its witness describe the same object, so the residual is exactly [the parent's `readdir` returned this `.git` entry -> this lstat] — constant per checkout, independent of tree size and worktree count. No extra I/O: the same pointer read and the same lstat, in the same numbers, moved earlier. THE FIXTURE'S WINDOW is deterministic without assuming any ordering inside a directory: two declared roots, the subject under the first, the trigger under the second, and roots are walked in caller order. The cell asserts that ZERO git commands had run when the replacement fired, so the instant is inside the walk and not merely before the listing. The untouched sibling `wt-a` must still be offered — that control is what makes the cell unsatisfiable by any refusal aimed at the population rather than the object. FIRST DRAFT OF THE FIXTURE WAS VACUOUS AND IS RECORDED AS SUCH IN THE CELL: re-adding on `-b brand-new` leaves the replacement UNMERGED, so `wt-z` was omitted as a non-candidate and the assertion passed for free. The replacement is re-added on the same branch, so it is an ordinary candidate and the only thing that can refuse it is the witness. MUTATIONS (whole GitWorktreeScannerTests, 45 cells): - take the witness in `repositoryGroups` again (r16 behaviour) → the W2 cell RED, B-P1 and B-P2 GREEN. - `consume` records no witness at all → the W2 cell RED, B-P1 and B-P2 GREEN. (the ordering canary `testATransientLstatFailure…` reddens under both, which is what it is for.) NEGATIVE RESULT, recorded so the next round does not re-derive it: B-P1 and B-P2 stay GREEN under both mutations because r17's (a2) admin-container pass also captures before the listing, so it now subsumes their windows. The walk-instant capture is load-bearing for exactly one window — [the walk observed this `.git` -> the container pass] — and the new cell is the only thing measuring it. Do not read B-P1/B-P2's greenness as coverage of this. Disclosed rather than repeated: CHANGELOG and CATEGORIES.md both already CLAIMED the walk instant while the code took it after the walk. Both now record that as the third correction to that entry, name what it cost, and state the residual that is actually left instead of one that grew.
…describes the anchor (PR #460 codex r17) W1 added a second pre-listing capture and W2 moved the first one; three comments still described a single capture taken in `repositoryGroups`. The (e2) block, its `Keyed by` note and `handle`'s `discoveryWitness` parameter doc now each name both sources, which of the two is preferred and why, and the one key all three maps share. No behaviour change.
…codex r17) Three sites carry the running total, each naming the commit its figure was taken at. r17's first fixer added three cells — the depth-budget cell, the walk-unreached replacement cell and the walk-instant replacement cell — so all three now end at 1562: `swift test` AT COMMIT ac5215f reported 1562 executed / 2 skipped / 0 failures, exit 0, 177 s (full, unfiltered, under a 900 s wall timeout, output never piped).
…ed — the run says it is (PR #460 codex r17, M3) r16 added a MUTATION note claiming that deleting `rollBack`'s trash-open guard "kills the run rather than reddening — the descriptor is -1 and `probeChild` is asked about it". It cited no run. MEASURED at cd58104, that exact line deleted and rebuilt, `swift test --filter TrashDisposalHopProofTests`: the run does NOT die. 36 executed, 5 failures, exit 1, no crash, 6 s wall. close(-1) is an EBADF no-op and fstatat(-1, ...) answers EBADF -> a mismatch, so the deletion downgrades .strandedInTrash to .lastSeenInTrash on all four arms of this cell (printed per arm under the mutation) and three cells redden: ...AnUnopenableLandingStillCatchesAnObjectThatIsNotOurs, ...TheDirectoryVerdictArmStillCatchesAStrangerUnderADeniedLanding, and the A-P3 cell itself. The guard IS evidenced. The note is retired and replaced by what was run. Trash hygiene: the mutation runs' landings were removed by name and their absence re-checked with lstat; the Trash was never emptied.
…r16 published it as a fact about the user (PR #460 codex r17, M2) r16's A-P3 measured ~/.Trash, found it EPERM without Full Disk Access, and published "the default user never gets a put-back" — .putBack, .putBackTookAnotherObject and .destinationNotTheAdmittedContainer ALL unreachable. rollBack opens landed.deletingLastPathComponent(), and that is ~/.Trash only for HOME-VOLUME items. FileManager.trashItem gives every other mounted volume its own /Volumes/<vol>/.Trashes/<uid>, which TCC does not gate at all — so off the home volume the undo runs to completion for a process with no Full Disk Access whatsoever. A-P3's own cell cannot see it: every landing it makes is in the home Trash by construction. MEASURED at be445a0, ~/.Trash answering -1 EPERM in the same process, no trashHandler injected (the shipped FileManager.trashItem inside the production main-actor hop), on a temporary APFS disk image attached under the test's own directory, all four verdict arms, 8/8 runs: other volume, ordinary swap .putBack other volume, container swapped .destinationNotTheAdmittedContainer New cell testANonHomeVolumeUndoPutsTheItemBackWithoutFullDiskAccess: skips if the machine HAS Full Disk Access (the two rows coincide then), asserts ~/.Trash closed and <volume>/.Trashes/<uid> open side by side, and detaches + deletes the image in teardown. Nothing goes near the user's Trash. Process waits use waitForExit(within:), not waitUntilExit. Claim corrected at all three sites: rollBack's population list (now split by volume first and permission second), the A-P3 cell's own doc, and the CHANGELOG entry, which said it of every item.
…r16 retired (PR #460 codex r17, M4) .putBack was the only one of the six Trash refusals to say "nothing was freed". The other five say "nothing was REPORTED freed" — a fact about the report this code writes. "Nothing was freed" is a claim about the DISK, and this arm establishes nothing about the disk: on the fixture that produces the cause the inspected object really is gone from where it stood, the mover removed it and trashed a replacement, and what the undo brought back is the replacement. The after-proof never looks at bytes. r16 revised this exact message to retire its OTHER net-effect claim ("Nothing was moved to the Trash") and left this one, and the whole-message guard's forbidden list did not cover it — because that list is phrases somebody already thought of. M1 replaces the list with a structural fence; this commit is the claim itself. Two cells asserted the old wording and now assert the new one (CacheCleanerTests, OrphanedCachesScannerTests — both on .putBack). swift test --filter 'TrashDisposalHopProofTests|CacheCleanerTests|OrphanedCachesScannerTests': 254 executed, 0 failures, exit 0, 26 s.
…ix — assert the proposition (PR #460 codex r17, M1) r16 widened r15's opening-clause guard to the whole message by adding a list of FORBIDDEN PHRASES. Measured at e6afc9f: restoring either retired tail reddens it, and saying the SAME FALSE THING IN NEW WORDS passes — "Check your Trash for the item." on .lastSeenInTrash was GREEN 36/36. A blocklist of strings IS phrasing-fencing. The messages are no longer string literals. TrashDisposal.Failure gains: * Established — the closed vocabulary of propositions a refusal may carry, including the TWO no arm establishes (theTargetWasReplaced, nothingWasFreedOnDisk) kept nameable so the fence can assert it; * Claim — one clause and the single proposition it asserts; * established(for:) — what each cause's own code path proved, derived at the raise sites, through a default-less switch; * claims(path:cause:) — the message, clause by clause. errorDescription is the JOIN of those clauses and nothing else, so there is no untagged text to hide a sentence in. Verified byte-identical to the previous messages on all six causes. The fence now asserts, per clause: (2) the proposition must be one the cause established; (3) naming a place at all commits the clause to a whereabouts proposition, and doing so unhedged to a positive one; (4) any net-effect word must belong to the report clause and say "reported freed"; (5) no sentence break, so a new sentence is necessarily a new tagged clause; (6) the two retired propositions are established by nobody and their contract is unsatisfiable. MUTATION, measured at 36cf469, 8 mutations of claims(path:cause:), each rebuilt, swift test --filter TrashDisposalHopProofTests (37 cells). None of these wordings existed in this repo before this round: " Check your Trash for the item." tagged whereabouts-unknown RED (2) the same tagged theTrashHoldsWhatItTook RED (1) the same GRAFTED onto the whereabouts clause RED (1) " The object the Trash took remains where the Trash put it." RED (2) "; nothing was reported freed" -> "; the bytes ... reclaimed" RED (8) " Empty your Trash when you are done" as a REMEDY RED (3) "; your disk now has that space back" as a REMEDY GREEN "at this path" -> "at this path within the last second" GREEN Every RED row is this cell. The two GREEN rows are the residual and are recorded on the cell: a net-effect claim made with no word in the net-effect lexicon, and a false clause grafted INTO an existing clause that adds no place and no net-effect word. Also recorded: the "Move to Trash" control-name normalisation, and that nothing here can check established(for:) against the code it claims to derive from. Full suite at this tree: 1563 executed / 2 skipped / 0 failures, exit 0, 183 s.
…codex r17) Three sites carry the running total, each naming the commit its figure was taken at. r17's SECOND fixer added one cell — the non-home-volume undo on a temporary APFS disk image (M2) — so all three now end at 1563: `swift test` AT COMMIT b73b639 reported 1563 executed / 2 skipped / 0 failures, exit 0, 178 s (full, unfiltered, under a 900 s wall timeout, output never piped).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ca11298b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+186
to
+188
| override func realPath(of path: String) -> String? { wrapped.realPath(of: path) } | ||
|
|
||
| override func canonicalize(_ url: URL) -> URL { wrapped.canonicalize(url) } |
There was a problem hiding this comment.
During an automatic scan, when an unprotected linked worktree points to an admin directory under ~/Documents, Desktop, or Downloads, GitWorktreeGitdirResolver.adminDirectory calls canonicalize(pointer) before its first gated probeKind. This override delegates that operation directly to the wrapped provider, causing realpath to traverse the protected path before the deferral can return .absent; moreover, the deferral predicate itself uses the canonicalizing isProtectedRoot. Thus a background scan can access or prompt for a protected repository despite the secondary TCC gate. Classify direct protected paths without dereferencing them first, and ensure canonicalization occurs only after that gate permits access.
Useful? React with 👍 / 👎.
…ex r18, C1) Every gate on the stale-worktree path is about something OTHER than the tree it destroys. R1b and the last-instant re-proof read the ADMIN DIRECTORY — its inode, its `locked` file, its HEAD witness. `admittedParent` binds the FOLDER THAT HOLDS the checkout. The clean check and the D2 ignored witness are about a PATH. Nothing bound the checkout itself: the permanent arm reaches `DepthSafeRemoval.remove(at:expecting: nil …)`, and the Trash arm's `disposeBoundLeaf` takes its binding INSIDE the disposal — after the window — so both of its readings describe the replacement and the after-proof confirms the replacement landed. The attack therefore does not have to defeat a gate. A directory renamed onto the checkout path after the clean check, whose `.git` file names the SAME admin entry, answers every proposition truthfully. MEASURED at 9ca1129 on both arms, two real `rename(2)`s staged on the last `status`: the stranger's `secret.env` destroyed (Trash arm: left in the Trash), `errors == []`, and a SUCCESS entry of 45056 bytes. ONE MECHANISM, because C2 needs the same one. `BoundObject` is `TrashDisposal.boundLeaf` under the container this file already proves with — the same function `disposeBoundLeaf` binds with, so the readings cannot disagree — captured beside `admittedParent`, before the two re-admissions, the traversal guard, the clean check, the ignored witness and both hops. It is re-read inside `reproveFromTheFilesystem`, which both arms already run on the far side of their own hop, as step (4) and LAST: every gate above states a more specific fact about the same event when it can see it. DISCLOSED: the delete-time measurement and `reestablishStaleGates` run BEFORE the capture, so a swap in that earlier window is refused by R1b but the bytes reported were measured on the object that was swapped out. Also here, because the C1 cells are what exposed it: the near-side re-proof refusal was logged TWICE (`logRefusal` beside `failure(…, tag:)`) — the defect r7 fixed at the sibling `catch` arm and left standing at this one. Nothing read the log at this site, so nothing caught it. MUTATION: delete step (4) and exactly `testThePermanentArmRefusesACheckoutSwappedAfterTheCleanCheck` and `testTheTrashArmRefusesACheckoutSwappedAfterTheCleanCheck` go red (86 executed in WorktreeReclaimPerformerTests, those two and no others).
#460 codex r18, C2) Same root cause as C1, other mode. The oracle, both recomputes, the per-directory admission, the measurement and the mount gate are all about PATHNAMES, and `removeAdminDirectories` reached `DepthSafeRemoval.remove(at:expecting: nil …)` — so past the admitted parent it destroyed whatever answered to the name. The only far-side check was `revivedCheckoutRefusal`, which ACCEPTS a replacement whose checkout is absent or whose back-link is unreadable: exactly the field shape where another process prunes the old entry and the name is reused by a newly created worktree whose checkout has not been written yet. Its HEAD and reflog were removed, and a detached commit made unreachable, with `errors == []` and a 24576-byte success entry (measured at 9ca1129). `removeAdminDirectories` now takes `[BoundObject]` — the SAME binding C1 introduced, so the two modes cannot drift — and the caller captures it at the point its last check answered: the prune-only mode one line after its final oracle recompute and final re-admission, the gated post-removal prune right after the gate that proves the set is exactly this worktree's own admin entry. The container the binding was read under is the value the removal is then proved against, so the two cannot be about different folders. The traversal guard and R0's subprocess are inside what the binding covers. MUTATION: delete the far-side arm and exactly `testTheDirectPruneRefusesAnAdminDirectoryReplacedInsideItsOwnHop` goes red. NEGATIVE, recorded in `replacementRefusal`: making its unreadable arm permissive (`return nil`) leaves the FULL suite green at 1567 executed / 2 skipped (the inherited `testDescriptorPeakDuringAReAnchorClimb` flake aside) — subsumed by the removal's own refusals, kept anyway and disclosed.
…x r18, C3) `git worktree lock` on an entry whose checkout is MISSING succeeds and moves that entry from `prunable` to `locked` in `worktree list --porcelain` (verified on git 2.43). That is why the mapper excludes locked entries — git's own prune skips them — and why an entry locked between the two oracle checks already survives (`testAnEntryLockedBetweenTheTwoOracleChecksIsNotRemoved`). A lock landing after the LAST oracle call had nothing to catch it. The only far-side check on this path was `revivedCheckoutRefusal`, which reads the back-link and never looks at `<admin>/locked`, so the removal deleted the entry, the user's explicit preservation mark and the HEAD/reflog with it. NOTE THE HISTORY: an earlier round removed the fallback prune path that had exactly this bypass; the DIRECT prune path added since then re-created it. Same sibling-path pattern, one path over. ONE READING, TWO WORDINGS. The `<admin>/locked` probe is now `readLock`, shared by the stale arm's last-instant re-proof (G4, which has always had it) and the prune arm's — so the two can never disagree about what "locked" means, only about the remedy, which genuinely differs. It runs on both sides of the hop, like the revival and binding checks beside it. Also here, now that all three arms are closed and the claim is exactly true: `DepthSafeRemoval.remove`'s "BOTH CALL SITES STATE THEIR `nil`" paragraph ended "if a third call site appears … this paragraph stops being true and must change with it". A third appeared at r7 — this performer's `removeTree` seam — and the paragraph did not change with it. The enumeration is re-taken: all three `expecting: nil` sites, why each is sound, and the residual site 3 still carries (its proof runs immediately before the call rather than inside it — the same residual LOCKED and HEAD-MOVED have carried since r7, for the same reason). MUTATION: delete the far-side lock arm and exactly `testTheDirectPruneRefusesAnAdminDirectoryLockedInsideItsOwnHop` goes red. NEGATIVE, recorded in the code: deleting the NEAR-side lock and binding calls (keeping the far-side copies) leaves the full suite green at 1567 executed / 2 skipped — subsumed, kept, disclosed.
…at names it (PR #460 codex r18, C4) A worktree added with `--detach` and committed into has its tip named by exactly one thing: the `HEAD` file inside its admin directory. Delete the checkout and git calls the record `prunable`; the prune tier then mapped every unlocked prunable record and offered the removal of that admin directory as a `.safe` item whose evidence says "branch refs and repository objects are untouched". MEASURED end to end on git 2.50.1 (Apple Git-155), today, outside the suite: `git fsck --unreachable --no-reflogs` SILENT before the admin directory was removed; `unreachable commit a3289a4` plus its tree and blob immediately after; `git gc --prune=now` then deleting the object (`git cat-file -t` → "could not get object info"). That is the user's own commit, and the claim was user-facing. REFUSE RATHER THAN PRESERVE, deliberately. The alternative — writing a ref at the commit before pruning — would make a scanner WRITE INTO the user's repository; every git command this app issues is read-only and the last two mutating argvs were retired at r5/r6. So the arm proves the commit survives or offers nothing. TWO READ-ONLY QUERIES, one shared implementation (`GitOrphanedHeadPreservation`, beside the shared mapper for the same reason: detection and execution must agree). `rev-parse --verify --quiet <oid>^{commit}` first — a commit already gone cannot be orphaned, and refusing on it would be a refusal no user action could clear. Then `rev-list --single-worktree --max-count=1 --count <oid> --not --all`. `--single-worktree` IS THE CHECK. By default `--all` pretends every working tree's HEAD is a ref — INCLUDING the doomed record's own, since a prunable worktree stays registered until it is pruned. MEASURED on the same fixture: without the flag the query prints `0` (reachable) for the very commit `git fsck` calls unreachable one command later; with it, `1`. NOT A PERMANENT STRAND. The condition is a fact about the repository, not a bound of this process: `git branch`/`git tag`/a merge/a push makes the next scan read `0` and offer the item. Both halves are asserted in one cell. DELETE TIME TOO, in `recomputePrunableSet` — the one place all three callers (both prune-mode recomputes and stale mode's gated post-removal prune) already pay for the re-listing, so no path can miss it and a branch deleted after the scan cannot turn a true disclosure false. `rev-list` joins `GitSafetyProfile.readOnlyCommands`: it is a pure traversal of the object graph, and leaving it off would have handed the fallback `.mutation` profile to a command that cannot mutate. MUTATIONS, all RED, measured at this commit: - drop `--single-worktree` → the scanner cell AND the argv cell go red (the item ships). - neuter the scanner's `.refuse` arm → the item ships, no issue published. - `guard unreachable == 0, false` (always refuse, same message) → the clearability half goes red. - delete the delete-time `prove` call → the performer cell goes red, entry emitted, admin directory gone. Suites at this commit: 271 executed / 0 failures across GitWorktree*/WorktreeReclaim*/GitCommandRunner*/WorktreeStaleness*. SCOPE, STATED: the record's HEAD commit, not the admin directory's reflog. Reflog-only commits are already invisible to `fsck --no-reflogs`, git's own `worktree prune` discards them identically, and `gc.reflogExpireUnreachable` expires them anyway.
… was pruned" (PR #460 codex r18, C5) `removeAdminDirectories` removes a prune item's admin directories one at a time, and every refusal it can raise ends in "nothing was pruned" (revival, lock) or "nothing was removed" (replacement). For a set of ONE — the field shape, and the only shape the gated post-removal prune can have — that was always true. For a set of several it was a claim about the FIRST entry applied to the whole item: entry 1 destroyed, entry 2 revived or locked in the meantime, and the user is handed a failed item, zero credited bytes, and a sentence saying their repository was untouched. The failed-removal `catch` arm one screen below has warned about partial cleanup since it was written. TWO CHANGES, because neither alone is enough. 1. EVERY PRE-REMOVAL REFUSAL IS EVALUATED OVER THE WHOLE SET BEFORE THE FIRST DELETION. The refusals that are already visible now cost no sibling, and "nothing was pruned" is TRUE rather than merely rewritten. It cannot see them all — a checkout repaired after this pass but before its own turn still refuses mid-loop — which is why (2) exists. 2. THE LOOP TRACKS WHAT IT HAS DESTROYED, and every refusal past the first completed removal goes through `disclosePartialPrune`. It SUBSTITUTES the false clause rather than appending a correction beside it — a corrected lie is still a lie in the same sentence — and names exactly which directories are gone. A message carrying neither phrase gets the clause APPENDED, so a future rewording cannot silently drop the disclosure. The `catch` arm's "may be PARTIALLY cleaned" now carries the exact list too. The accounting is unchanged and deliberately so: a partial refusal still reports no row and credits no bytes, which under-reports what was freed rather than over-reporting it. MUTATIONS, both RED at this commit: - delete the pre-flight loop — `testAnEntryAlreadyLockedWhenTheRemovalStarts…` goes red three ways (the sibling is spent, and the message flips to the PARTIAL wording). - `disclosePartialPrune` returning its input — the extended `testACheckoutRevivedAfterTheFinalCheckIsNotSwept` goes red on all three new assertions. 211 executed / 0 failures across GitWorktree*/WorktreeReclaim* at this commit. MEASURED WHILE WRITING THE CELL, and it shapes the fix: a lock that predates the delete-time oracle never reaches this function at all — the mapper drops locked records — so the window the pre-flight owns is [last oracle answer → first removal], and the cell plants the lock inside R0's `rev-parse`, the last git call before it.
…sed (PR #460 codex r18, C6) The delete-time loop measured each recomputed admin directory and then checked the report for MOUNT BOUNDARIES only. `report.denials` — the sizer's record of every enumeration, permission, metadata or unaddressable-path failure — was dropped on the floor, and the claims were registered and the recursive removal run over a directory whose contents nobody could account for. THE ASYMMETRY IS THE ARGUMENT. The SCAN-TIME prune tier suppresses the ENTIRE item for exactly this condition ("the disclosure would be unverifiable"), so the older reading was strict and the fresher one — taken over the RECOMPUTED set, moments before the destruction — was permissive. A denial that developed between them (a chmod, a failing disk, an unreadable subtree) is precisely the thing the delete-time re-measure exists to catch. Refused BEFORE registration, beside the mount gate, for the same reason: this loop only measures, and (7) registers the claims once every directory has passed both gates. MUTATION: `if let denial = report.denials.first, false` — RED. And the mutant shows the harm rather than just a wording change: the removal is ATTEMPTED, `removefile` destroys what it can reach before failing on the part it cannot, and the cell's post-restore assertions on the payload and the `gitdir` back-link are what catch it. The cell uses a REAL denial — `chmod 000` on a subdirectory of the admin directory, restored in a teardown block — not the sizer seam, so it also proves the production sizer reports one there. 311 executed / 0 failures across GitWorktree*/WorktreeReclaim*/CacheCleaner* at this commit.
…'s own verifier found unevidenced (PR #460 codex r18, F) `guard let discoveryWitness` in `handle` is the only thing between a checkout with NO identity from before the listing and the three-way re-proof re-anchoring on the POST-listing capture — the exact state r16 was written to end, where every read describes a replacement and they all agree. r17 re-scoped the arm (adding (a2)'s pre-listing container pass as a second source) and left it with no cell anywhere. MEASURED by r17's verifier: the mutation `let discoveryWitness = discoveryWitness ?? assessmentWitness` plus `if false` on the refusal body left the FULL SUITE GREEN at 1564 executed / 2 skipped / 0 failures, and `identified before this scan`, `identity older than` and `pre-listing read` each returned ZERO hits over `Tests/`. THE STATE THE ARM IS FOR, staged: neither pre-listing capture can identify the admin entry while both post-listing reads can — a denial spanning the two reads before the listing that lifts after it. `BlindOnNthIdentityCallProvider` grew an `onCalls:` initializer (the single-call one now delegates to it) so calls 1 and 2 — the walk's discovery capture and (a2)'s container pass — are blinded together, and the cell asserts the five-call ordering exactly as its B-P4 sibling does, so a re-ordering of the captures re-aims the cell instead of letting it drift onto a different arm. MUTATION, the brief's own, now RED: the row is offered for `wt-a` and NO issue is published at all. 47 executed / 0 failures in GitWorktreeScannerTests at this commit. The cell also asserts the arm's wording is NOT the replacement wording — the distinction r16's B-P4 drew for the sibling arm, and the reason both exist separately.
…codex r18) Three sites carry the running total, each naming the commit its figure was taken at. r18's SECOND fixer added eleven cells — six routing the detached-HEAD preservation proof (C4), two running it end to end through the scanner and the performer, two on the partial-prune disclosure and its pre-removal pass (C5), one on the unmeasurable recomputed admin directory (C6) and one on the pre-listing-identity refusal that had no cell at all (F) — so all three now end at 1578: `swift test` AT COMMIT 2f4c2fe reported 1578 executed / 2 skipped / 0 failures, exit 0, 180 s (full, unfiltered, under a 900 s wall timeout, output never piped, total line printed). The 1567 figures in this file's subsumed-arm notes are r18's FIRST fixer's own measurements at its own commits and are left alone; this fixer did not re-run them.
…odex r18, C8) Both termination steps targeted `process.processIdentifier` alone. Killing the parent of a timed-out `git` — a `git` that had spawned a helper while inspecting a submodule, say — leaves that helper ORPHANED and RUNNING: still holding the inherited pipe write end the runner exists to survive, still traversing repositories, after the runner has already answered `.timeout`. THE OLD CELL COULD NOT SEE IT. `…SigtermIgnoringStubIsEscalatedToSigkill…` launches `sleep` from its fixture and then asserts on the SHELL's pid only, so a surviving grandchild passes it. MEASURED, on this machine (Darwin 25.5, 2026-08-23), with a 15-line Foundation program: `Process` already places every child in a NEW process group whose id IS the child's pid — parent `pid 47862 pgrp 47770`, child `pid 47866 pgid 47866`. So the fix is to signal the GROUP, and the `group == pid` test that guards it is the safety: if a future Foundation left the child in OUR group, `-group` would signal the whole app. When that test fails the runner signals the pid alone, which is exactly what it did before and never worse. AND THE ESCALATION IS DECIDED ON THE GROUP, NOT THE PARENT. A parent that exits inside the grace window says nothing about a descendant that ignored the same SIGTERM, so after the grace the group is probed (`kill(-group, 0)`) and SIGKILLed if anything answers. EVIDENCE — `testATimedOutCommandsDescendantIsKilledWithIt`. The stub blocks in the `wait` BUILTIN, so it spawns exactly one extra process and every pid is accounted for; the descendant is `( trap '' TERM; exec sleep 300 )`, a single process whose SIG_IGN disposition survives the `exec`, so it dies of neither a pid-aimed signal nor a group SIGTERM. Two ARMS, mutated independently, target rebuilt each time: signal(_:to:group:) aimed at the pid alone RED 8/8 treeStillThere pinned to false RED 8/8 unmutated GREEN 4/4, and the cell leaves no stray process in either state (it kills what it recorded in a `defer`, checked with `pgrep` after every sweep). DISCLOSED RESIDUAL, in the code: `group` is the child's pid, so once the kernel reaps that child the pid may be reissued and a new group leader with the same id would receive the SIGKILL. macOS pids are issued sequentially and wrap near 99999. The pre-r18 code had the same class of window on its bare `kill(pid, SIGKILL)`; what is new is that the post-exit probe can fire after the parent has already been reaped. Suite: GitCommandRunnerTests 25 executed, 0 failures.
…S (PR #460 codex r18, C7) Third finding on the same four statements — r14 bounded `close()`, r15 bounded the capture — and it is NOT a boundedness bug. `closeAndCapture()` is bounded either way, because `close()` announces itself through `stateLock`. What was wrong is that BOTH `join(within:)` results were dropped on the floor. THE WHOLE PATH, ENUMERATED ONCE rather than bounding a fourth call: * A drain reaches EOF only when EVERY write end is closed. git has already exited on this path, so a join that times out means something git SPAWNED still holds the inherited write end — this runner's founding scenario, one path over from the one it was written for. * The close that follows then DROPS whatever is still unread; that is `closeAndCapture()`'s own disclosed cost, and it was sound only under the assumption the join had finished. * The short bytes went back as `.success(stdout:)`, which is a promise of COMPLETE output — the porcelain parsers downstream COUNT entries, so a silently truncated `worktree list` is a smaller worktree set: a wrong answer wearing a right one's clothes. This file's header already says nothing collapses a failure into an empty success; a truncation is the same class. An unfinished drain is now answered `.timeout` — the invocation could not be COMPLETED within its bounds, which is what that case means to every one of its eleven call sites (they re-scan or refuse; none strands, because a retry can differ). The tree feeding the pipe is terminated first, through C8's group protocol, or the orphan goes on running exactly as C8 describes. `.timeout`'s doc now names both ways in. WHAT IT COSTS, STATED IN THE CODE: a git command that really did exit 0 with complete output, whose descendant merely outlives it holding the pipe, is now refused rather than believed. The runner cannot tell that case from a truncated one, and refusing is the retryable direction. EVIDENCE — `testAnUnfinishedDrainOnANormalExitIsNotReportedAsSuccess`. The stub starts `( exec sleep 300 ) &`, which holds the inherited stdout and never closes it, then exits 0 itself, so `waitForExit` succeeds and the drain can never see EOF. Two ARMS, mutated independently, target rebuilt: drop `guard stdoutJoined, stderrJoined` RED 8/8 (2 failures) drop `terminate(process, group:)` RED 8/8 (1 failure) unmutated GREEN 8/8. NEGATIVE RESULT, RECORDED IN THE CELL because it chose the fixture: with a WRITING holder (`while :; do echo drip; sleep 0.02; done`) the second mutation is GREEN 8/8 — the writer dies of SIGPIPE the instant the drain closes the read end, so a writing fixture cannot evidence the termination at all. The holder is silent on purpose. Suite: GitCommandRunnerTests 26 executed, 0 failures.
…there are THREE (PR #460 codex r18, E3) `established(for: .strandedInTrash)` — the table the whole message fence rests on — said "Reached from two places, and BOTH have an object identified at the landing". `rollBack` raises it from THREE: 1. the Trash open failing, 2. `openAdmittedContainer` throwing anything but `.notTheAdmittedContainer`, 3. `renameatx_np` failing with a non-`ENOENT` errno — `EEXIST`/`ENOTEMPTY` from `RENAME_EXCL`, `EACCES`, `EROFS`, `EXDEV`. The third is the ternary's else branch and carries no `return` of its own, which is how it was missed. The CONCLUSION survives — every one of those errnos leaves the source at the landing — so no user-facing clause was false because of it. This is a derivation that was wrong about its own code in the commit that introduced it, in the entry written to be exhaustive, and the fence's own residual ("nothing here can check a derivation against the code") bit on day one. WHAT THE OMISSION DID COST, AND IT IS USER-FACING: on `EEXIST` something now stands at the target, so "Move it back from there" collides with it. The clause now says so — "and if something already stands at <path>, move that aside first, because the automatic put-back refuses to overwrite" — which is hedged and true on all three arms, not just the one that motivated it. EVIDENCE — `testTheStrandedDerivationEnumeratesEveryRaiseSiteInRollBack`, a ROT GATE rather than a proof, and labelled as one. It counts the raise sites the derivation claims to have enumerated and names all three, so a REPLACEMENT that keeps the count cannot slip past either. Deterministic source-text check: ternary else branch folded into `.lastSeenInTrash` (a site removed) RED (2 failures) a fourth `return .strandedInTrash(landed.path)` added RED (1 failure) Suites: TrashDisposalHopProofTests 38 executed 0 failures; with OrphanedCachesScannerTests + CacheCleanerTests, 255 executed 0 failures.
…r left it (PR #460 codex r18, E2) `.destinationUnknown`'s message ended "Check the Trash, and use permanent delete …", and the derivation table POSITIVELY ENDORSED it: `established(for:)` listed `.theTrashHoldsWhatItTook` for that cause, so the fence r17 built to catch false placement claims blessed this one instead. WHY THE DERIVATION WAS UNSOUND. Its justification was "the mover is a TRASH disposal and it returned without throwing, so whatever it took went to the Trash" — a claim about the SEAM'S CONTRACT, and the ONLY entry in the table derived from an assumption about an injectable seam rather than from something this code READ. "Whatever it took" begs the question when it took NOTHING. It is the identical inference an earlier round falsified for `.lastSeenInTrash`, one cause over. MEASURED, all four Trash arms: a Mover that proves, moves nothing and returns a nil landing raises `.destinationUnknown`, and `lstat` before and after gives the SAME `st_ino`. The item is on disk where it always was and the message tells the user to go and look in the Trash. The claim is DROPPED, not re-derived. `.theTrashHoldsWhatItTook` joins `.nothingWasFreedOnDisk` and `.theTargetWasReplaced` as vocabulary NO arm establishes, with an unsatisfiable contract, and the fence now asserts that no cause claims it. The clause it licensed is replaced by the one thing this arm does establish: "Where the item is now was NOT established". REACHABILITY, STATED RATHER THAN GLOSSED: the counterexample uses an injected Mover. Through the shipped composition `.destinationUnknown` needs `FileManager.trashItem` to SUCCEED with a nil `resultingItemURL`, and NOTHING IN THIS TREE MEASURES WHETHER THAT HAPPENS. That is the argument for dropping the claim rather than measuring Foundation: the drop is the right answer under both outcomes, and the code says so where the case is defined. EVIDENCE — `testTheUnknownDestinationMessageDoesNotSendTheUserToTheTrashForAnItemStillOnDisk`, four arms, premise asserted on both sides (same inode, landing empty), plus the derivation asserted off the type. MUTATION — restore the `" Check the Trash,"` clause AND its `established` row: RED 8/8, 9 failures per run. Suites: TrashDisposalHopProofTests 39 executed 0 failures; with OrphanedCaches/CacheCleaner/WorktreeReclaim/BuildArtifacts, 482 executed, 1 skipped, 0 failures.
…he fact set (PR #460 codex r18, E) META-PATTERN, NINTH FAILURE. r17 replaced a list of banned phrases with a `Claim` carrying a tag and its own FREE TEXT, plus a fence that inspected that text — `namesAPlace`, `namesANetEffect`, `isHedged`, per-proposition `all`/`any` markers — and asserted "a NEW false sentence fails without anyone having predicted its wording". MEASURED at 9ca1129: 8 of 8 new false wordings PASSED, five of them saying the same false thing r16 spent a round retiring, one passing the FULL suite, while r17's own recorded RED row still reddened — so the harness was sound and the fence was not. Six mechanisms, each a CATEGORY: (a) `isHedged` was a whole-clause boolean; (b) a whereabouts+positive tag licensed ARBITRARY extra placement claims; (c) the markers were a phrase list inverted, i.e. a PASSWORD; (d) the remedy had no markers at all; (e) the sentence-break rule tested `". "` only; (f) the net-effect lexicon was six words. And the fence's own disclosed residual was wrong: it said a false graft passes only if it adds no place, yet five of the eight DID add a place. NOT A TENTH BLOCKLIST. A fence that inspects free text will always be one, because "is this clause entailed by what the proof established?" is a semantic judgement and every approximation of it in strings is a password. THE FREE TEXT IS GONE. `claims(path:cause:)` chooses no words: it walks `messageOrder`, keeps the facts `established(for:)` says the cause proved, and renders each through `sentence(for:path:landed:remedy:)` — ONE wording per proposition, shared by every cause. The remedy comes from a closed `Remedy` enum (two members) rather than being free text bounded by two word lists. A clause asserting a proposition its cause did not establish is UNREPRESENTABLE: there is no call site at which one could be written. (b) and (c) die with the per-cause wording; (a), (d), (e), (f) die with the text inspection. THE FENCE IS NOW ALL STRUCTURE: messageOrder is a permutation of the vocabulary; each message is exactly the established set in order, each fact once, nothing dropped by a nil rendering; one opening, first, never elsewhere; report then remedy last; errorDescription is the path plus the join and nothing else; a proposition renders IDENTICALLY under every cause; the remedy is from the closed set; the three propositions no arm establishes render nil. AND ONE NEW PROPERTY OF THE VOCABULARY, `placesTheItem(_:)`, decided once for each of the fourteen members. It lets a fixture cell that has MEASURED where the item is fence the DERIVATION TABLE — the one residual `established(for:)` has always carried and which E3 just found already broken. Both "the item never left" cells now assert that their cause establishes no placing proposition. MEASURED, each mutation alone, target rebuilt, filter TrashDisposalHopProofTests (39 cells). Type-level checks with no timing, so one run each: claims appends a free-text clause ("Check your Trash for the item.") RED (30) messageOrder drops .theItemsWhereaboutsAreNotEstablished RED (12) messageOrder puts the remedy before the report RED (12) sentence() returns nil for an ESTABLISHED fact (silent drop) RED (13) an opening rendered upper case (the join's grammar) RED (14) established(.lastSeenInTrash) gains .theItemIsAtTheLanding RED (4) established(.destinationUnknown) gains .theItemIsBackAtTheTarget RED (4) a false clause GRAFTED INTO an existing wording GREEN THE RESIDUAL, MEASURED RATHER THAN ASSERTED AWAY — that last row. Someone can still word one of the fourteen sentences to say more than the proposition it stands for; no test here can catch it. What changed is the SIZE and SHAPE of the judgement: fourteen wordings in one `default`-less switch, reviewed once and shared by every cause, instead of one wording per (cause × clause) written afresh at six call sites — which is where all nine of this branch's false sentences were written. A smaller surface, not a proof, and the code and the CHANGELOG both say so. THE CHANGELOG ENTRY THAT MADE THE FALSE CLAIM IS CORRECTED IN PLACE rather than quietly dropped: it ended "so a false sentence nobody has thought of yet fails as well", and the new entry says that eight were written and all eight passed. Four assertions elsewhere in the trash suite were updated to the canonical wording of the proposition they were already asserting — two because a proposition now has ONE wording where two causes used to phrase it differently, which is the point of the change. `CacheCleanerTests` and `OrphanedCachesScannerTests` needed no edits. Full suite AT THIS TREE: 1582 executed / 2 skipped / 0 failures, exit 0, 176 s (unfiltered, under a 900 s wall timeout, output never piped, total line printed).
…codex r18) Three sites carry the running total, each naming the commit its figure was taken at. r18's THIRD fixer added FOUR cells — the descendant a timed-out git used to orphan (C8), the normal exit whose drain never finished (C7), the rot gate on the stranding derivation's raise sites (E3) and the unknown-destination message that sent the user to the Trash (E2) — so all three now end at 1582: `swift test` AT COMMIT c5888a8 reported 1582 executed / 2 skipped / 0 failures, exit 0, 176 s (full, unfiltered, under a 900 s wall timeout, output never piped, total line printed). E's restructuring of the refusal messages added no cell; it replaced the fence in place, and that fence is the same one cell it was.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0546753880
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Main | ||
| case "${1:-}" in | ||
| --direct) | ||
| # A signed, distributable DMG — the same shipping artifact the | ||
| # release arm produces, minus notarization. Gated identically. | ||
| check_release_gates |
There was a problem hiding this comment.
This check only runs in bundle.sh, but the repository's documented distribution command is bash scripts/build-dmg.sh (docs/v1/BUILD-AND-DISTRIBUTION.md:76-89); that script independently creates a distributable DMG and prints signing, notarization, and gh release upload instructions (scripts/build-dmg.sh:105-147) without invoking this gate. Consequently a release can follow the documented path and ship while a RELEASE-BLOCKING status is still open, defeating the stated enforcement guarantee; move the gate into a shared helper and call it from every distribution-producing script.
Useful? React with 👍 / 👎.
Comment on lines
+3250
to
+3254
| switch await GitOrphanedHeadPreservation.prove( | ||
| prunableRecords: GitWorktreeAdminMapper.removalTargets(in: inventory.entries), | ||
| repositoryAt: plan.parentRepoWorkingDir, | ||
| run: { [runner, gitTimeout] arguments in | ||
| await runner.run(arguments, timeout: gitTimeout).outcome |
There was a problem hiding this comment.
When a detached orphan's commit is reachable from exactly one branch or tag, this query can return .nothingOrphaned, after which another process can delete that final ref before removeAdminDirectories deletes the admin directory containing HEAD. The binding added later protects only the directory inode, and the intervening repository re-establishment does not repeat reachability, so the removal can still leave the commit unreachable. The fresh evidence in this fix is that the new preservation proof remains a snapshot inside recomputePrunableSet, rather than being held or re-proved at the destructive boundary; preserve the commit under a durable ref or hold an appropriate repository/ref lock through removal.
Useful? React with 👍 / 👎.
Comment on lines
+571
to
+575
| private static func ownProcessGroup(of process: Process) -> pid_t? { | ||
| let pid = process.processIdentifier | ||
| guard pid > 0 else { return nil } | ||
| let group = getpgid(pid) | ||
| guard group == pid else { return nil } |
There was a problem hiding this comment.
When a command spawns a helper and exits between process.run() and this getpgid call, the group leader is already gone, so getpgid returns ESRCH and the stored group becomes nil even though its descendant remains in that group. If the inherited pipe then causes the normal-exit drain join to time out, terminate(process, group: nil) signals only the already-dead parent PID and leaves the helper running. The fresh evidence in the process-tree fix is this post-launch group-discovery race, which the tests miss because their parent remains alive long enough to be sampled; establish the isolated group as part of process creation rather than discovering it afterward.
Useful? React with 👍 / 👎.
| resolver: GitWorktreeGitdirResolver, | ||
| provider: FileSystemIdentityProvider | ||
| ) -> Set<String> { | ||
| for entry in event.entries where entry.name == ".git" { |
There was a problem hiding this comment.
When a bare repository under a configured dev root has no surviving linked checkout, this discovery pass produces no repository group: a bare repository stores HEAD, objects, and worktrees directly at its root rather than under an entry named .git, while each missing checkout no longer has a .git file for this loop to observe. The prune tier is therefore never reached for exactly the all-checkouts-gone case it is meant to reclaim, even though later code explicitly supports bare parent repositories. Add a safely validated bare-repository discovery path so its orphaned admin directories can be listed.
Useful? React with 👍 / 👎.
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.
Adds the
git_worktreesper-item scanner: stalegit worktreecheckouts underyour configured dev roots, offered for removal with the branch and the
repository left intact.
New:
GitWorktreeScanner,GitWorktreeInventory,GitCommandRunner,WorktreeStalenessAssessor,WorktreeReclaimPerformer.The opening line of this description used to say the scanner was about "the
'Directory not empty' fallback that
git worktree remove --forceleavesbehind". That premise was retired at review round 5, along with the fallback
arm itself:
git worktree removeis no longer on the delete path at all.Eleven review rounds have changed enough of what a user sees that the sections
below are the description, not a footnote to the old one.
Who removes the tree — git is READ-ONLY on the delete path
Through r4 the default disposal was
git worktree remove, with a filesystemfallback for the classes git refused. There is now ONE arm, and Cacheout
performs both removals itself: the checkout, and then — separately and gated —
that worktree's own admin entry under
.git/worktrees/. Every git invocationthat remains is a read: the porcelain record, the ancestry ladder, the
cleanliness check, the prune recompute. The property is stronger than a test
assertion: there is no mutating
worktreeargv BUILDER left in production. Theonly
worktreeargv the app can construct is["worktree", "list", "--porcelain", "-z"](
GitWorktreeInventory.listArguments), and nobranchargv is built anywhere— so
--forceandgit branch -dare not fenced out of a code path, they haveno code path.
GitCommandRunner.GitSafetyProfile.classifyadditionallyclassifies any unrecognised argv as
.mutation, the conservative direction.On top of that, a private test helper
assertNoForbiddenArgvinspects therecorded invocations after the fact; it is called from 15 sites, ALL in
WorktreeReclaimPerformerTests— the ten cells inGitWorktreeEndToEndTestsdo not call it, and an earlier revision of this description said "every cell
that runs the real removal".
Why: the spawn is not the destructive act. git starts up, reads the registry,
runs its own
statusover the whole tree and only then unlinks — and itre-reads no identity, no lock and no HEAD in between. MEASURED with the harness
checked in at
scripts/measure/worktree-removal-window.{c,sh}(git 2.50.1,macOS 15, APFS), SPAWN → first destruction:
git worktree removemedian14.87 ms on a one-file worktree and 156.8 ms on a 2001-file one — it
grows with the tree — against 0.417 ms and 62.5 ms for a direct
removefileon the same two shapes.Two caveats the source carries and this comparison must carry with it. The
removefilerow is a PROXY: a small C harness callingremovefiledirectly,"which is not this code". And it is "constant" only in the sense the header
states — the 2001-file column is the same file being watched, not the same
position in the walk, so most of the difference between the two git numbers
is git's own pre-unlink work (~94 ms of registry read plus a
statuswalk thatgrows with the tree). The shipped path's own instrumented figures, with their
load conditions, are in the two sections below.
What is lost is stated in the source rather than smoothed over: git's
--force-less dirty refusal (replaced by the samestatus --porcelainrun asour own last gate, extended so it refuses strictly more), its submodule
refusal, and the atomicity of removal-plus-registry-cleanup — a failure between
our two steps leaves admin data behind, which is a WARNING on a successful row
and an item the next scan offers.
Move to Trash now applies to the checkout
The GUI ships
moveToTrash = trueand the worktree arm ignored it, so theapp's most common worktree removal was unconditionally unrecoverable. It now
goes through
TrashDisposal.dispose(_:containedIn:provider:via:)— the sameno-leaf-verdict overload the item and contents arms use — which proves the
container from a descriptor, binds the leaf under it on both sides of the move,
and PUTS BACK anything it cannot prove: no entry, no bytes.
~/.Trashis TCC-protected and that container open is DENIED to every processwithout Full Disk Access — the GUI's normal state — so until round 10 the proof
could never be taken: every Trash disposal was reported as a refusal with zero
entries and zero bytes, and the user was told a checkout sitting recoverable in
the Trash "could not be put back". Rounds 10 and 11 fixed it: the identification
falls back to a path probe for the permission class ONLY (EPERM/EACCES), while
every other failure of that open — ELOOP, a swapped parent, a parent that became
a file — still refuses, so a symlinked landing container is never resolved
through. A trash failure is
an error and never falls through to a permanent delete.
The CLI is unaffected: both of its clean call sites pass
moveToTrash: false.Delete-time re-establishment, and a last-instant re-proof
The scan's four gates (the registry record, the worktree's own identity, the
lock, the ancestry answer) are re-established at DELETE time, with the
cleanliness check last. After them — closer to the destruction than any
subprocess can be — a pure-filesystem re-proof re-answers the three
propositions the filesystem itself can answer: WHICH checkout stands here,
whether it is LOCKED, and whether HEAD MOVED. It costs a few
lstats andspawns nothing.
It is not decoration. MEASURED at the r3 ordering, staging each event in the
window that ordering left open: a same-path
worktree remove+worktree adddestroyed a brand-new checkout and a
secret.envhidden by a committed.gitignoreand reported success witherrors == []; aworktree lockacquired after the gates reached the effect of
remove -f -fwithout the flag;a commit made on a detached HEAD was destroyed and left reachable from no ref.
All three are cells now, and all three go red if the re-proof is removed.
The proof crosses the disposal's hop — in both arms
Each disposal seam hops before it destroys anything: the Trash arm to the MAIN
ACTOR (
FileManager.trashItemrequires it), the permanent arm toDispatchQueue.global. Every proof used to be taken on the NEAR side of thathop, so the interval between the last proof and the destruction was not a
syscall — it was a QUEUE DEPTH. Both seams now run the proof on the far side,
immediately before the destruction, and destroy nothing if it throws.
MEASURED through the production composition:
n=5 and n=3, at identical measured queue delays in each row. The Trash half
of this affects EVERY Trash disposal in the app, not only worktrees — it is a
change to
TrashDisposalandCacheCleaner, and its three far-side proofshave their own cells in
TrashDisposalHopProofTests.Cleanliness: what it sees, and what it does not
The last gate is
git status --porcelain --ignore-submodules=none --untracked-files=normal --ignored=traditional.--ignoredis the addition:a path your
.gitignorehides is invisible to plain--porcelain, and thisremoval would have destroyed it without a word. An ignored path that appears
while the delete-time GATES run now refuses the removal by name: the ignored
list is read once as a witness before them and again by the last
git status,and anything new aborts. That second reading is the last GIT COMMAND, not the
last thing before the destruction —
--ignorednarrows the gates' own window,and does NOT narrow the one described below.
The dirtiness rule was widened to match:
!!lines no longer count as dirt.An ignored build tree is this scanner's SUBJECT, not a reason to refuse; every
other line, including an unrecognised one, still is.
Cleanliness is the one proposition that does NOT cross the disposal hop, and
that is disclosed rather than implied: it costs a subprocess, and a
fork/execinside a disposal seam — on the main actor, for the Trash arm —would be a worse trade than the window it closes. MEASURED under load, last git
answer → destruction: 241.156 ms (permanent, saturated pool) and
185.864 ms (Trash, 120 ms main-thread items), against 0.269 ms / 0.674 ms
on an idle queue. Work saved into the worktree inside that interval is
destroyed with the tree. A re-scan clears it — it is a fact about a concurrent
writer, not a fixed property of the item.
The registry removal is scoped to the disclosed set
There is no repository-wide
git worktree pruneanywhere on this path. After asuccessful removal the prunable set is recomputed, and the removal runs ONLY
when that set is EXACTLY the just-deleted worktree's own admin entry —
otherwise it is skipped and the row carries a warning. An unconditional prune
would sweep other pre-existing orphans the item never disclosed, and the
accounting would not notice: it iterates the REGISTERED directories, so an
extra victim contributes no bytes, no row and no warning (measured end to end).
Equivalence to git's own effect was measured on git 2.50.1: two identically
built repositories, one pruned by git and one whose single orphan admin
directory was removed directly, produced structurally identical
.gittrees,identical
worktree list --porcelain, identical branch lists, a cleangit fsckon both, and a subsequentworktree prune --expire=nowon thescoped repository was a silent no-op.
Reconciled with fn-6 (PR #459)
This branch was 29 ahead / 133 behind. The merge produced 12 conflicted files
across 27 hunks — but the two real defects were in files that merged with NO
conflict, so the audit diffed every both-sides file against the MERGE BASE
rather than trusting the marker set:
removeItemConcurrentlygainedexpecting:/provider:/containedIn:onmain; this branch's seam still called the old signature. It now carries the
AdmittedParentcaptured from a descriptor BEFORE the TOCTOU rechecks, sothe binding covers the rechecks, not just the queue hop.
seam, and
FailingProbeProvideroverrode only the path overload, so belowthe root it stopped intercepting and a denial cell passed with an EMPTY error
list.
ProbeRecordingProviderhad the same gap while backing a NEGATIVEassertion. Both now cover both seams.
PathGuard(the highest-risk auto-merge) is clean: disjoint regions, andrefusalTagis exhaustive with nodefault, so both new error cases get truetags. No CLI flags were added, so the trailing-valued-flag trap has no new
surface;
schema_versionstays 4.Verification
Every guard added on this branch is mutation-tested against a cell that fails
when it is gone; a guard that survives its own deletion is deleted or
evidenced, not shipped. Round 7 closed the last six that had not been (an
earlier revision of this description said five and then listed six):
testTheTrashArmRefusesACheckoutLockedInsideTheMoversHopand
testThePermanentArmRefusesACheckoutLockedInsideItsOwnHop, which stage agit worktree lockINSIDE the seam (it touches neither the checkout's inodenor its contents, so the disposal's own leaf binding still passes and only
the re-proof can refuse);
TrashDisposal's three far-side proofs —TrashDisposalHopProofTests, EIGHTcells over two
disposeoverloads: one per verdict arm (no leaf verdict,file, directory), each reddened by its own mutation and no other, plus
testAnUndisturbedHopStillDisposesOnEveryArmfor the far side of eachrefusal;
LastInstantRefusalcatch arm, which is invisible in the per-itemmessage (its
errorDescriptionIS the detail) and shows up only in therefusal LOG — where writing the assertion found that r6 had been logging
every seam-raised refusal twice.
The container binding on the Trash arm is evidenced by
testTheRemovalRefusesATrashDisposalItCannotProveTookTheWorktree(an earlierrevision of this description cited a
…FallbackRefuses…name that no longerexists, along with the fallback it was named for).
The suite also carries three checks against defects that are invisible to the
compiler and to every other cell:
StrandFenceTests— constructs that STRAND the run rather than failing acell:
try!, a literal-integer subscript, force-unwraps whose optional isdecided by PRODUCTION code, (round 8) a variable-index subscript whose index
is loop-bound to an integer, (round 9) a
Rangewhose two bounds come from twoINDEPENDENT searches over a document the tests do not control, (round 10)
Dictionary(uniqueKeysWithValues:), and (round 11) a hand-built gate that canpark the run forever — which does not kill the process but STOPS it, so it
prints no total line either, and is the one failure mode the "check the total
line printed" habit does not catch by itself. A trap kills the PROCESS, so every cell
sorting after it never runs and the total line never prints. Round 8 found
one live site of the last kind —
clauses[index]inWorktreeStalenessAssessorTests, sitting one line under its ownXCTAssertEqual(clauses.count, 4, …)— and measured it: with the productionevidence string shortened by one clause, the pre-fix shape died with
Fatal error: Index out of range, signal 5, and 25 cells never ran; thefixed shape fails the named cell and the run completes.
SourceAnchorIntegrityTests, the anchors — everyFile.swift:Nwrittenin a comment must name a real file, cite a real range, and still contain the
text pinned for it. Round 8 found seven anchors this PR had shifted by 12
lines (fourteen citing sites between them) and several older ones pointing at
blank lines or at a file that no longer exists; one had drifted onto
DIFFERENT prose that still read like a citation. Inserting 12 lines into
SpaceScanner.swiftreddens the cellnaming every affected anchor and every citing site.
SourceAnchorIntegrityTests, the cited cell names — a guard that cites atest by name is claiming that test is its evidence. Round 8 found three
citations naming cells that do not exist, two of them in round 7's own
headline guard.
Known residuals, recorded at measured scope
measured width are above.
and does not change when that branch commits, so a commit made inside the
window is not detected. It is also not LOST — the commit and the ref live in
the common git directory, which nothing here touches. The DETACHED case,
where the same commit would be unrecoverable, is covered and refused when it
cannot be. Repositories on the
reftablebackend get a strictly strongerguarantee, because
tables.listmoves on every ref write.is the product, not a defect. Two narrower limits ride with it:
--ignored=traditionalcollapses an ignored DIRECTORY to one line, and thewitness compares PATHS, so a change to an already-present ignored file is not
detected.
.containerRefused, whose fixed GUI labelreads "not a configured search root" while their own detail says the path IS
inside a configured dev root. None of main's kinds fits exactly; needs a
design call.
ProjectTreeWalkeremits.symlinkRootfor everynon-directory root — the case
.nonDirectoryRootwas added to split.One known flake, inherited from
mainOrphanedCachesScannerTests.testDescriptorPeakDuringAReAnchorClimbfailsroughly one run in five with "the census disagreed with the kernel". Its body
is BYTE-IDENTICAL between
origin/mainand the head of this branch, it istracked as its own task, and it is NOT this PR's: a run that fails only in that
cell is a re-run, not a regression.
Suite: 1500 executed / 2 skipped / 0 failures, exit 0 (
swift test, ATCOMMIT
a917447). Pinned to a commit rather than to "the head of this branch",which rots on every push — the in-tree totals pin theirs the same way.