fix(#4415): treat an absent worktree as removed, not as a branch mismatch

[behruznassre]

Fix PR

Linked Issue

Fixes #4415

What was broken

worktree cleanup-wave blocked an entry whose worktree directory no longer existed, even though the branch was intact and mergeable. Claude Code removes a subagent's worktree the moment the subagent finishes with a clean tree, and a gsd-executor that committed everything — SUMMARY.md included, under commit_docs: true — is exactly that case, so by the time the orchestrator reached wave cleanup the directory was routinely gone.

Two shapes, depending on when the harness removed it:

• Before the check: git -C <path> rev-parse --abbrev-ref HEAD failed with cannot change to '<path>' → blocked as branch_mismatch, nothing merged, branch left dangling.
• After the merge: git worktree remove <path> --force failed with "is not a working tree" → worktree_remove_failed, branch undeleted, and a manual git worktree prune + git branch -D + rm -rf every wave.

What this fix does

Disambiguates the absent worktree from a genuine mismatch at the point of failure. A successful in-worktree read still decides identity exactly as before; only a failed read consults the filesystem.

There are two such reads, and they are not the same path:

• The branch read fails and the directory is absent. Identity has no checkout to come from, so it falls back to refs/heads/<branch> read from repoRoot; a missing ref still blocks. The SUMMARY rescue and dirty check are then skipped.
• The branch read succeeded and the later status read fails with the directory now absent — the harness removed it while the repoRoot-side checks ran. Identity was already established from the checkout, and the rescue has already run; only the dirty decision is skipped.

Teardown prunes the stale .git/worktrees admin entry instead of removing a path that is not there. For an entry accepted as absent it prunes only — it never issues worktree remove --force, because such an entry was merged without the rescue and dirty checks, and force-removing a checkout recreated at that path would delete contents that never passed either one.

The presence probe resolves worktree_path against repoRoot, the way git resolves it — manifest paths are taken verbatim and may be relative, and a bare existsSync would resolve against the process working directory instead.

The base and deletion gates, and the advisory scope check, already ran against repoRoot and are untouched. (Scope emits warnings; it has never been a gate.) Skipping the rescue is not a claim that the worktree was clean — this code cannot tell who removed it, and a forced rm -rf of a dirty worktree would already have destroyed an uncommitted SUMMARY before cleanup ran. The narrow claim is only that a missing source cannot be read. A rescue that genuinely fails still blocks.

Root cause

executeWorktreeWaveCleanupPlan never checked entry.worktree_path for existence. git's filesystem failure and a real branch disagreement arrived at the same if, so they produced the same verdict. Long-standing — present in the function's earliest indexed version and unchanged through the #2852 per-entry-isolation refactor.

Testing

How I verified the fix

Ran the issue's own reproduction verbatim against a build of this branch:

"status": "merged_removed", "reason": "ok"

with the merge commit present in git log, worktree-agent-t1 deleted, and no prunable entry left in git worktree list.

The same reproduction against a build whose worktree-safety.cts is at the merge-base returns blocked / branch_mismatch, no merge, the branch still present, and wt1 ... prunable in git worktree list — the reported bug, confirmed both directions.

Regression red-check: with src/worktree-safety.cts restored to the merge-base and rebuilt, 5 of the original 8 rows fail. The 3 that stay green are the safety-preservation rows (a present worktree on the wrong branch, a present worktree whose read fails, and a missing branch ref) — they pin that this change did not move them. The two rows added after review — mid-entry removal, and relative-path resolution — likewise go red against the pre-review commit and green after it.

Race coverage is improved, not made atomic, and I want to be precise about that: a disappearance immediately after the teardown probe can still report worktree_remove_failed with the merge already applied. No finite number of presence checks closes every window; what changed is that the windows this bug actually hits are closed, the destructive one is closed by construction rather than by timing (an absent-accepted entry never force-removes), and the residual is visible in the result rather than silent.

• npm run lint:ci — exit 0, run cold
• Full suite — see the PR comment below for totals

Regression test added?

• Yes — added a test that would have caught this bug
• No — explain why:

tests/worktree-safety.test.cjs gains a #4415 regression block covering all four acceptance criteria, including the two negative rows that must keep blocking.

Three existing rows in that file — does not delete a branch when worktree removal fails, #2852: a worktree_remove_failed on entry 1 does not abort entry 2, and #2852: worktree_dirty (status query failed) on entry 1 does not abort entry 2 — now inject existsSync: () => true. Each stubs a git failure against a worktree that is genuinely present; that premise used to be implicit in a fake path that never existed on disk, and now has to be stated because an absent path takes a different route. No assertion in any of the three changed.

Five rows were added after the review round specifically because every original row held presence constant, so none could reach a window where the directory changes state mid-entry — which is the actual shape of the bug.

Platforms tested

• macOS
• Windows (including backslash path handling)
• Linux
• N/A (not platform-specific)

Runtimes tested

• Claude Code
• Gemini CLI
• OpenCode
• Other: ___
• N/A (not runtime-specific)

---

Checklist

• Issue linked above with Fixes #NNN — PR will be auto-closed if missing
• Linked issue has the confirmed-bug label
• Fix is scoped to the reported bug — no unrelated changes included
• Regression test added (or explained why not)
• All existing tests pass (npm test)
• .changeset/ fragment added if this is a user-facing fix (npm run changeset -- --type Fixed --pr <NNN> --body "...") — or no-changelog label applied
• No unnecessary dependencies added

Breaking changes

None. Every entry whose worktree directory is present takes exactly the path it took before, including all existing block reasons. The only behavior change is for an entry whose directory is absent, which previously could not succeed at all.

[behruznassre]

Round 3 — pre-push full-PR review (Codex), plus the Windows shard fix

Disclosing what changed and why, since the last push.

1. conformance test (windows-latest, shard 3/3) — fixed (420b397f7)

The row the presence probe resolves a relative worktree_path against repoRoot was red on Windows while the code under test was correct:

+ actual   'D:\repo\main\.claude\worktrees\agent-a1'
- expected '\repo\main\.claude\worktrees\agent-a1'

The expectation was built with path.join while production uses path.resolve, and on win32 resolve prepends the current drive to a drive-less absolute path where join does not. The expectation now uses path.resolve, matching how git resolves -C <path> against cwd: plan.repoRoot.

2. Self-found in review, and it is a real defect this PR introduced (526aae075)

A full-PR Codex pass against the current merge-base flagged the presence probe as a permissive fallback, and it is right.

fs.existsSync returns false for a genuinely missing path and for one it cannot traverse (EACCES on a parent, an unreachable mount). Verified locally: with a parent at mode 000, existsSync returns false while statSync throws EACCES.

That mattered because "absent" is exactly what lets an entry skip the SUMMARY rescue and the dirty check. An unreadable-but-present worktree was read as absent, so cleanup would merge over uncommitted work the dirty check exists to refuse — and it contradicted this code's own comment that a present checkout whose git read fails stays blocked. Pre-PR, a failed git read blocked unconditionally, so this is a regression the PR introduced, not a pre-existing gap.

The default probe is now statSync, which reports why it failed. Only ENOENT is absence; EACCES/EIO read as present and block as before. An injected probe stays authoritative, so tests state presence directly (no hidden dependency on the real filesystem) and may throw to state a path is unreadable.

Two rows added:

• an unreadable worktree still blocks as branch_mismatch, with no merge and no teardown
• a confirmed-ENOENT probe still takes the absent path

Mutation-verified: reverting the discrimination to the permissive return false turns the unreadable row red while the ENOENT row stays green. The asymmetry is the point — it shows the test discriminates rather than merely over-blocking. The mutation was confirmed to reach the compiled artifact the test loads (gsd-core/bin/lib/worktree-safety.cjs).

3. Two claims narrowed rather than left standing

• The row named for a checkout that "reappeared" never modeled reappearance — on that path production probes presence once, at identification, so a stub flipping on a later call is never asked. Renamed to the unconditional no-force-remove contract it does prove; assertions unchanged.
• The comment crediting the two notEqual rows with removing circularity was too strong. Computing the expectation the way production computes it gives no independent oracle for win32 or git semantics; it catches drift between the two, and the equality already rejects the repoRoot-anchoring mutation. Comment narrowed to that.
• The changeset now states that only a confirmed absence takes the new path.

Verification

• npm run lint:ci — exit 0
• npm test — 24/24 chunks, 37,249 tests, 0 failures, 25 skipped (per-chunk summaries aggregated; the exit code is not treated as evidence)
• tests/worktree-safety.test.cjs — 418/418; the #4415 block 16/16

Not verified by me: the win32 behavior itself. macOS has no current drive, so path.win32.resolve cannot reproduce the D: prefix. That fix rests on production and the expectation now calling the same path.resolve with the same arguments; the Windows shard in CI is the actual test.

[trek-e]

Summary

Bug-fix PR for #4415 (confirmed-bug). Makes worktree cleanup-wave treat an absent worktree directory as "removed" rather than reporting branch_mismatch / worktree_remove_failed.

The bug is real, the diagnosis of where the two failure sites are is correct, and the write-up is one of the most careful in this batch. The fix rests on a premise about git that does not hold, and the consequence is a bypassed identity gate.

Classification & gate compliance

• RULESET.CONTRIB.CLASSIFY.fix=requires confirmed/confirmed-bug before implementation — satisfied; #4415 carries confirmed-bug.
• Changeset present (.changeset/serene-dogs-wake.md). Scope is tight — three files, one concern.
• CI green at 526aae07.

Findings

Blocker 1 — "the branch ref is the only identity evidence" is false

src/worktree-safety.cts:1118-1127 proceeds from the premise that once the directory is gone, identity has no checkout to come from, so it must fall back to refs/heads/<branch> read from repoRoot.

Git does not lose that binding. I measured it rather than reasoning about it — create a worktree, rm -rf the directory, then ask git:

$ git worktree list --porcelain          # after rm -rf
worktree /tmp/.../wt
HEAD b00db6674ff328e8aa0391d12e808a6dd420155d
branch refs/heads/feat-x
prunable gitdir file points to non-existent location

The path → branch mapping survives intact, and git adds a prunable line that states positively why the entry is stale. That is strictly better evidence than an existsSync probe: it gives you identity and a git-authored reason, instead of inferring "the harness cleaned up" from an absence that has several other possible causes.

So the fallback is not forced — it is chosen, and it discards the authoritative source. git worktree list --porcelain should be the identity source here, with prunable as the signal that distinguishes "removed" from every other reason a path might not resolve.

Blocker 2 — the absent path leaves allowed_bases as the only remaining gate

src/worktree-safety.cts:1137-1146 — once the path is accepted as absent, the manifest-supplied allowed_bases is the only surviving gate on what gets merged. The merge-base check that would otherwise refuse a foreign branch no longer runs.

This was reproduced: a foreign sibling branch that the merge-base build blocked merges successfully through the absent path. That is a real gate bypass, not a theoretical one, and it is the direct consequence of Blocker 1 — with the porcelain output, identity is still verifiable and the gate need not be skipped at all.

Blocker 3 — the #3677 swap control is bypassed, and untested in this variant

tests/gsd-quick-batch-merge-integration.test.cjs:305-410 documents the branch_mismatch swap control introduced for #3677. That control is bypassed when worktree_path is absent, and no test covers the absent variant.

This is why the suite stays green on the two Blockers above: the control is exercised only on the present-path shape. Please add the absent-path variant of that swap test — it is the test that distinguishes this fix from a gate removal.

Major — an injected existsSync silently replaces the real probe, inverting the fail-safe

src/worktree-safety.cts:1012 and :1059-1062 — deps.existsSync replaces the statSync probe with no fallback to the real filesystem behavior. Measured consequence: a parent directory returning EACCES yields merged_removed instead of blocked.

That is absence-as-grant in the unsafe direction — a permissions failure is not evidence the harness cleaned up. statSync's error code distinguishes ENOENT from EACCES; existsSync collapses both to false. Keep the distinction, and treat anything that is not ENOENT as a block.

On the SUMMARY-rescue argument

The PR argues that skipping the rescue loses nothing, because a forced rm -rf of a dirty worktree would already have destroyed an uncommitted SUMMARY before cleanup ran. I tested that claim rather than accepting it, and it holds for the harness-removal case the PR is aimed at. I want to note it explicitly as accepted, so it does not get re-litigated: the rescue skip is not among my findings.

What changes the picture is Blocker 4's EACCES path, where the directory (and any uncommitted SUMMARY) is still present and readable-by-someone. Once the probe distinguishes ENOENT from other errno values, that case blocks and the rescue runs, and this concern disappears with it.

What passes

• The two failure sites are correctly identified and genuinely distinct — the branch read failing versus the later status read failing, with identity already established in the second. That analysis is right and should survive the rework.
• Path resolution is correct: resolving worktree_path against repoRoot rather than the process cwd matches how git resolves manifest paths, and handles the relative form properly.
• Prune-only teardown is the right instinct — not force-removing a path that may have been recreated is correct, and the reasoning given for it is sound.

Memtrace Evidence

• get_impact(target=executeWorktreeWaveCleanupPlan, direction=upstream, depth=2, repo_id=gsd-core): risk LOW, total_affected=1 (cmdWorktreeCleanupWave), total_affected_is_lower_bound=false. Recording the qualifier that matters: the low symbol count understates consequence here, because the function's output is a merge authorization decision. The blast radius of a wrong answer is measured in branches merged, not in callers — which the call graph does not model.
• get_symbol_context(symbol=executeWorktreeWaveCleanupPlan, file_path=src/worktree-safety.cts, repo_id=gsd-core): single caller cmdWorktreeCleanupWave, no cross-repo callers, confirming this is a one-entry-point command surface rather than a shared seam — which is why the fix can be corrected in place without a wider ripple.
• recall_decision("worktree cleanup wave branch mismatch identity verification swap control"): returned on-topic records, all pointing at src/worktree-safety.cts:21 ("silently dropped cleanup-wave manifest"), confirming this manifest path has a recorded history of failures in the silent-acceptance direction. No decision prohibits this change, so per the tool's semantics that is unproven, not unconstrained — but the recorded pattern is the same failure mode as Blocker 2 and is worth reading before the rework.
• find_code_review_issues(diff, repo_root, repo_id=gsd-core, review_mode=online, max_candidates=40): 0 findings, _graph_state=ready. With find_cross_module_issues 0 (_graph_state=ready — genuine clean verdict, not a staleness note), find_yaml_rule_matches 0, find_ast_review_issues skipped (no .py). All four findings are fail-safe-direction and gate-coverage questions, which none of these detectors model — Blocker 1 came from measuring git's actual behavior against the code's stated premise.

Verdict

Changes requested. The bug is real and your analysis of the two failure sites is correct — I would keep that part as-is. The rework is narrower than it may sound: source identity from git worktree list --porcelain (which retains path → branch and adds prunable) instead of inferring from absence, which lets the merge-base gate keep running rather than being skipped; and make the probe distinguish ENOENT from EACCES so a permissions error blocks. Then add the absent-path variant of the #3677 swap test, which is what would have caught this.

[behruznassre]

Round 4 — rework answering the review

Thank you for the measurement in Blocker 1; it disproved the premise this fix rested on, and re-measuring it myself is what made the rest of the rework obvious. Two further defects fell out of that, both in my own first cut.

Blockers 1 & 2 — identity now comes from the registration

I reproduced your rm -rf result before changing anything: git keeps worktree <path> + branch refs/heads/<branch> and adds prunable. So the ref fallback was chosen, not forced, and it weakened identity from "the checkout registered at this path is on this branch" to "a branch by this name exists" — which is the gate bypass you reproduced.

Identity is now sourced from git worktree list --porcelain (the existing parseWorktreeEntries, extended with the prunable line). The existsSync probe and both dep locals are deleted.

One correction, with evidence. The finding says the merge-base check "no longer runs". It does run, unconditionally, at src/worktree-safety.cts:1138-1145, blocking on base_mismatch. What the absent path actually lost was the path→branch binding, leaving allowed_bases as the only manifest-independent check — which is why your foreign-sibling repro merged. The substance stands; I mention it only so the fix is aimed at the right gate, and restoring identity is what closes it.

Blocker 3 — the absent-path swap test

Added: #3677 swap control: an absent path registered to a DIFFERENT branch blocks. Mutation-verified — dropping the branch conjunct from the predicate turns only that row red.

Major — and a correction to my own first attempt at it

My first rework claimed an EACCES parent keeps its gitdir file, so git would decline to mark it prunable and the entry would block. That is false, and I measured it. With a parent directory at mode 000:

worktree .../wt-c
branch refs/heads/feat-c
prunable gitdir file points to non-existent location

Git cannot traverse the parent, so it reports the gitdir file as missing. prunable therefore does not separate removed from unreadable, and relying on it reintroduced this exact finding by another route.

Each source now answers only the question it can answer — which is what your review asked for on both counts, and I had collapsed them into one signal:

• identity → porcelain registration (existsSync could never give this)
• removal → statSync's errno; only ENOENT is removal, EACCES/EIO blocks

A wave-level defect my rework introduced

Reading the porcelain per entry is unsafe, because git worktree prune is repository-wide. Measured: two removed worktrees, one prune, and neither registration survives. So entry A's teardown erased the identity evidence of every entry after it — a wave would merge its first harness-removed worktree and block the rest as branch_mismatch. That is worse than the bug this PR fixes, since a wave of parallel executors is the normal case.

The identity read is now a snapshot, captured lazily on the first entry that needs identity and reused for the wave: pre-prune by construction (every teardown that prunes consults the predicate first), and off the happy path entirely. Covered by #4415 P1: entry 1's repository-wide prune must not strand entry 2, whose porcelain empties after prune exactly as git's does; removing the snapshot turns only that row red.

The comment calling repository-wide prune "Harmless" was wrong under the new identity rule and now says so.

Accepted as you asked

The SUMMARY-rescue-skip argument is recorded as accepted and I have not re-litigated it. Your note that it holds for the harness-removal case, and that the EACCES path is what changed the picture, is what the errno split now handles.

Also added

A bare prunable marker row (git emits it both bare and with a reason) and a fail-safe row for a worktree list that cannot be read — both gaps flagged in pre-push review.

Verification

• npm run lint:ci — exit 0
• npm test — 24/24 chunks, 37,164 tests, 0 failures, 28 skipped (per-chunk summaries aggregated; the exit code is not treated as evidence)
• tests/worktree-safety.test.cjs — 422/422; the #4415 block 20/20
• Three mutations, each killing exactly the intended rows, verified against the compiled artifact the tests load: drop the branch conjunct → only the swap row; drop the ENOENT confirmation → the present/unreadable rows; drop the snapshot → only the P1 row

One fixture that still stated presence through the removed existsSync seam was passing for the wrong reason (blocking on unknown registration rather than its stated locked-removal premise) and now states both axes.

Not verified by me: win32 behavior — macOS has no current drive, so the D: prefix cannot be reproduced locally; CI's Windows shard is the real check.

[trek-e]

Summary

Fixes worktree cleanup-wave misclassifying an absent worktree directory (routinely removed by the harness the instant a clean subagent finishes) as a genuine branch mismatch or removal failure, blocking cleanup that should have proceeded. Disambiguates "the directory is gone" from "a real mismatch" at the point of failure, sourcing identity from git's own worktree list --porcelain registration (which survives rm -rf) rather than a live in-directory read.

Classification & gate compliance

Linked issue #4415 (OPEN, confirmed-bug) → Bug-Fix Track. RULESET.CONTRIB.CLASSIFY.fix — satisfied.

Root-cause verdict

Confirmed accurate against the actual pre-image diff, not just plausible-sounding: both documented failure shapes (pre-merge branch_mismatch from a failed in-directory git read; post-merge worktree_remove_failed from git's own "is not a working tree" text) were genuinely indistinguishable from real mismatches in the old code. The fix targets the actual root cause — it doesn't retry, ignore errors, or blanket-accept absence; it sources identity from git's own porcelain registration, which is a real improvement in rigor over a bare existsSync fallback the code's own comments say an earlier draft used and abandoned. Not a paper-over.

Blast radius & risk tier

executeWorktreeWaveCleanupPlan has exactly one production caller (cmdWorktreeCleanupWave, the CLI entry point) confirmed by get_symbol_context. get_impact's raw CRITICAL/199-affected reading is the known direction-inflation artifact on a low-fan-in function (contradicted by the narrower, more credible preflight_check signal: risk: LOW, 1 dependent symbol) — the real blast radius is narrow. However, get_timeline shows this exact function has been incrementally patched 15 times since 2026-07-30, most recently across multiple rounds within this PR itself ("Codex review round 1-4" per its own commit history) — a genuine complexity hotspot (cyclomatic complexity 43, "critical" band). That history is exactly the kind of surface where a subtle interaction between edge cases survives several review rounds, which is what happened here (see Findings).

Memtrace Evidence

• get_impact(executeWorktreeWaveCleanupPlan): raw CRITICAL/199 — attributed to graph direction-inflation noise, not a real signal (see Blast radius above).
• get_symbol_context(executeWorktreeWaveCleanupPlan): 1 real caller (cmdWorktreeCleanupWave), 7 callees — narrow, single-entry-point shape as expected for a CLI-only module.
• recall_decision("worktree cleanup wave absent directory branch mismatch safety"): no directly relevant recorded decision or prior convention on this exact pattern — this PR isn't contradicting a recorded decision.
• find_code_review_issues (online mode, max_candidates 50, full diff): 0 issues — expected, since the finding below is a cross-statement control-flow/timing issue, not a pattern AST/YAML/cross-module detectors are built to catch.
• preflight_check: cyclomatic complexity 43 (critical band), 1 dependent symbol, generated checklist notes "~43 branch paths — that's the test count for full branch coverage," corroborating the hotspot characterization above.

Findings

Major

• src/worktree-safety.cts (the worktreeAbsent teardown branch of executeWorktreeWaveCleanupPlan) — a TOCTOU gap between presence classification and the destructive teardown action. Once an entry is classified worktreeAbsent from the first failed check, the code re-verifies the symmetric PRESENT→ABSENT race (a worktree vanishing mid-processing) but never re-confirms the ABSENT→PRESENT direction before the final git worktree prune + unconditional git branch -D. The code's own inline safety argument — that a reappeared checkout's branch delete would merely report branch_delete_failed, visibly and non-destructively — only holds if git worktree prune itself fails to clear the admin entry for a genuinely-still-present worktree. If the same filesystem-visibility gap that produced the initial false absence also fools prune's own staleness check (plausible: it's the identical race, one call later), prune succeeds, and the subsequent branch -D then succeeds too — force-deleting the branch of a live, unreviewed, un-rescued worktree. This is worse than the pre-fix behavior (which only ever blocked, never destroyed state).

  This is not speculative: the PR's own new test suite documents the gap directly. The test named "an entry accepted as ABSENT tears down by prune, never by force-remove" carries this comment verbatim: "It does NOT model the reappearance transition itself: on this path production probes presence once, at identification, so a stub that flips on a later call would never be asked." The authors know presence is checked exactly once and rely on "no force-remove" as the sole safety net — but that net's actual sufficiency against a reappeared worktree is asserted only in a comment, never verified by a test.

  Suggested fix: re-run the local confirmedGone() check (cheap statSync, no subprocess) immediately before the worktree prune call in the worktreeAbsent teardown branch, and treat a since-reappeared directory as a real failure (e.g. worktree_remove_failed) rather than proceeding to branch -D. This closes the window to essentially zero without adding a new git subprocess call.

Minor

• The new #4415 test block (758 lines) is 100% mock-based (stubbed execGit/statSync) — neither documented failure shape nor the race-condition rows are reproduced against a real git binary/filesystem, even though this same test file already contains real-git integration helpers (makeWorktree, addWorktree, git()) used by other describe blocks in the file. The identity mechanism rests on factual claims about real git's porcelain output ("git never loses the path→branch binding after rm -rf") that the comments say were measured against real git, but nothing in this PR's own suite proves it executably.
• Pre-existing, not introduced by this PR: resolveAgainstRepoRoot (new) consistently resolves entry.worktree_path against plan.repoRoot, but the unchanged rescueSummaryArtifacts→defaultFindSummaryFiles resolves the same value via path.join against process.cwd() instead. Latent risk of a silently-skipped SUMMARY rescue if a caller ever supplies a relative worktree_path with repoRoot !== process.cwd() (not reachable via the CLI's typical same-cwd invocation today). Worth a follow-up issue.

Verdict

Changes requested. The root-cause fix and the "prune-only, never force-remove" teardown discipline are both correctly implemented and well-tested for the scenarios the suite covers. The one Major finding is a real, if narrow, safety gap in a module whose whole job is preventing destructive mistakes — the fix is small (a second cheap local check) and worth closing before this lands.

[behruznassre]

Review round 2 — Major closed, and the Minor's claim is now proved executably

The Major is right, and the asymmetry you identified is the part that makes it worth fixing rather than arguing: the bug this PR set out to repair only ever blocked, while the teardown path could destroy state. Those are not the same severity, and a module whose job is preventing destructive mistakes does not get to rely on an argument that holds "unless the same race bites one call later".

Major — absence is re-confirmed before teardown

confirmedGone() now runs immediately before the worktree prune in the worktreeAbsent branch, exactly as suggested — no new subprocess, just the statSync already in hand. A directory that has reappeared blocks as worktree_remove_failed with a reason naming the reappearance, and never reaches prune or branch -D.

You were also right that the gap was known and unverified — the existing row says so in its own comment ("it does NOT model the reappearance transition itself"). It is modelled now: a stat that answers gone at identification and present at teardown, which is the race.

Mutation-verified, and the result makes your point better than I can: removing the re-confirmation turns only the new row red, while an entry accepted as ABSENT tears down by prune, never by force-remove stays green. The old row was structurally incapable of catching this, which is precisely why the comment was doing the work a test should have been doing.

Minor — the all-mock suite

Fair, and it was the weakest part of the PR: the factual claim the whole identity mechanism rests on was asserted in comments and measured out of band, but nothing in the suite proved it. Two real-git rows now do, using this file's existing initRepo/addWorktree helpers:

• real git keeps the path -> branch binding after rm -rf, and says prunable — deletes the checkout and asserts git still lists the worktree, still reports branch refs/heads/<branch>, and adds prunable.
• real git also reports prunable for an UNREADABLE worktree, so prunable is not absence — a parent at mode 000, asserting git reports prunable for a checkout that is still there. This is the one that explains why removal is confirmed by errno and not by prunable; it skips as root, where mode bits do not deny traversal.

Minor 2 — rescueSummaryArtifacts vs process.cwd()

Agreed on your framing: pre-existing, and not reachable through the CLI's same-cwd invocation today. I have left it alone rather than widen this PR, and will open a follow-up issue for it.

Verification

• npm run lint:ci — exit 0
• npm test — 27/27 chunks, 37,739 tests, 0 failures, 28 skipped, against the true merge-base
• tests/worktree-safety.test.cjs — 425/425

One note on that base, since it nearly cost me a wrong call: my first run of this suite reported two failures — differential attribution over the real tree and quick-batch: /gsd:quick command + workflow stay byte-identical. Both were phantoms from a stale hardcoded GSD_EMITTED_BASE in my own runner after this branch merged next; both pass against the real merge-base, and the runner now computes it instead of pinning it. Flagging it because a stale pin produces failures that look exactly like regressions in files the branch never touched.

[behruznassre]

Follow-up for your Minor 2 filed as #4758 — the rescueSummaryArtifacts / process.cwd() resolution mismatch, kept out of this PR as you framed it.

I verified the claim against source before filing rather than restating it: defaultFindSummaryFiles (src/worktree-safety.cts:780-781) uses a bare path.join on the manifest value, so a relative worktree_path resolves against process.cwd(), while the identity path uses path.resolve(plan.repoRoot, …). gsd-tools --cwd makes the divergence representable.

One thing I recorded there that softens it: if it is reached, the rescue finds nothing, reports no failure, and the uncommitted SUMMARY then trips the dirty check — so the entry BLOCKS rather than merging over uncommitted work. A confusing block, not data loss. I found no shipped caller that reaches it, and said so in the issue rather than implying a live defect.

[trek-e]

Summary

PR #4612 fixes #4415: worktree cleanup-wave previously conflated "the executor's worktree directory is absent because the harness already removed it cleanly" with "a genuine branch mismatch," blocking the former case and leaving branches dangling. The fix sources identity from git worktree list --porcelain (which keeps the path → branch binding after rm -rf) and removal from statSync's errno (only ENOENT counts as gone; EACCES/EIO still block). This is the correct architecture for the problem and matches all 4 of the issue's acceptance criteria. The PR went through 4 review rounds (visible in PR comments) that caught and fixed real regressions in earlier cuts (ref-fallback weakening identity, prunable mistaken for a removal test, repo-wide prune erasing wave-mate registrations, a reappearance race at teardown). The implementation in the current head commit is sound on all of those points.

Two things still need attention before merge: an unused/dead field (prunable) added with no consumer, and a genuine (if narrow) observability gap the self-adversarial pass surfaced — see Findings.

Classification & gate compliance

• Author: behruznassre (not trek-e/trekkie/Tom Boucher). Confirmed independent author.
• Linked issue #4415: OPEN, labeled bug + confirmed-bug. Gate satisfied.
• mergeable: MERGEABLE. mergeStateStatus: BEHIND (branch trails next; per this org's own convention this is a CI condition, not a reviewer blocker).
• CI rollup: all checks green, including the full OS×Node conformance matrix (Windows/macOS/Linux shards) and mutation testing. No pending fork-CI approval (action_required query returned 0 runs for this head sha).
• Staleness: last PR comment 2026-09-15T04:52 (same day as this review). Not stale.
• Scope: diff is exactly 3 files — src/worktree-safety.cts (+329/-44), tests/worktree-safety.test.cjs (+881 new), .changeset/serene-dogs-wake.md. Matches the PR's "no unrelated changes" claim. Changeset format is compliant with CONTRIBUTING.md conventions (type: Fixed, pr: 4612, symptom-led body, no file paths).
• CONTEXT.md's executeWorktreeWaveCleanupPlan predicate ("per-entry gauntlet: branch → base → deletions → scope → SUMMARY-rescue → clean-worktree → merge → remove") is unchanged by this PR and remains accurate — the gauntlet order is preserved, only per-step failure handling changed. Docs Required CI passed. Not a finding.

Root-cause verdict

Confirmed correct. executeWorktreeWaveCleanupPlan (src/worktree-safety.cts, function starts ~line 1048 in this branch) never checked entry.worktree_path for existence before the in-worktree git calls, so git's filesystem failure (cannot change to '<path>') and a real branch mismatch produced the identical branch_mismatch verdict. The fix disambiguates at each of the three failure points (branch read, status read, worktree remove) using the same absentAndIdentified() predicate, which requires BOTH a matching git-porcelain registration AND a confirmed ENOENT. A present-but-unreadable worktree (mode 000 parent) and a path git has no record of both correctly continue to block.

Blast radius

• Single call site: cmdWorktreeCleanupWave (same file) — non-exported, CLI-only entry point. mcp__memtrace__analyze_relationships find_callers on executeWorktreeWaveCleanupPlan (repo_id gsd-core, depth 2) returned exactly this one caller.
• mcp__memtrace__get_impact (direction: both) reported risk: LOW, total_affected: 0, affected_files: [] — but the response carried graph_converging: true with relink_running: true, repos_pending: 32→36 (growing during this session) and total_affected_is_lower_bound: true, so this is a low-confidence/stale-graph answer, not a confirmed zero blast radius. Structurally, the change is well-isolated (one function, one caller, no exported surface change) regardless.

Co-change completeness

get_cochange_context failed outright (memdb: ... backend readiness probe went unanswered) — could not verify historical co-change partners. Manually verified: only src/worktree-safety.cts + its own test file changed; no CLI/docs consumer of the new WorktreeEntry.prunable field or deps.statSync exists elsewhere in the diff or (per grep) in the repo.

File-level overlap with PR #4766 (also open, fix(#4721), "worktree cleanup-wave merge under 10s timeout kills long-running pre-merge-commit hooks"): both PRs modify src/worktree-safety.cts and tests/worktree-safety.test.cjs, and #4766's hunks (base lines ~1160-1220, ~645-663) land inside/adjacent to the same executeWorktreeWaveCleanupPlan merge-and-teardown block this PR restructures. Whichever merges second will need a real rebase, not just conflict resolution — #4766 adds a timeout/dirty-index-recovery step immediately after the same git merge call this PR now branches around (worktreeAbsent true/false paths). Noting per instructions, not resolving.

Memtrace Evidence

• get_impact(target: executeWorktreeWaveCleanupPlan, repo_id: gsd-core, direction: both) → risk: LOW, total_affected: 0 (low-confidence: graph_converging, 32-36 repos_pending mid-relink).
• analyze_relationships(query_type: find_callers, target: executeWorktreeWaveCleanupPlan) → single caller cmdWorktreeCleanupWave, confirming narrow blast radius.
• get_symbol_context(executeWorktreeWaveCleanupPlan, repo_id: gsd-core) — exceeded the tool's own output cap (123k chars); substituted with the narrower analyze_relationships call above.
• recall_decision(query: "worktree cleanup-wave absent worktree vs branch mismatch") → no prior recorded decision on this exact question; top hits were unrelated (t.after() cleanup convention, Codex-worktree model pinning). No historical constraint this PR violates.
• find_code_review_issues(diff, repo_root, repo_id: gsd-core, review_mode: online) on the small placeholder call returned cleanly (0 issues, _graph_state: ready); the same call against the real diff timed out at 1800s twice (backend under load — relink_running, repos_pending growing from 32 to 36 over the session). Disclosed rather than silently skipped.
• find_yaml_rule_matches on the changed confirmedGone/statSync region → 0 issues (multi-language rule pack, ran cleanly).
• find_ast_review_issues → 0 issues, but this detector is Python-only per its own description; not meaningful evidence for this .cts change.
• Memtrace backend was in a degraded state for this entire review (get_repository_stats timed out at 1800s before recovering; list_indexed_repositories also timed out; get_cochange_context failed with an unreachable-endpoint error). All findings below are grounded in direct source/diff reading, not solely graph queries, given this degradation.

Self-adversarial pass

The brief's question: could treating "absent" as "removed" mask a genuine failure mode — a worktree force-deleted by something else mid-operation, losing uncommitted work — rather than a legitimate harness cleanup?

The code's own acceptance guard (absentAndIdentified) cannot distinguish "the harness removed a clean worktree" from "an operator or external process rm -rf'd a dirty worktree" — both produce an identical signature (git still registers the path → branch binding, statSync reports ENOENT). The PR author acknowledges this explicitly in-code ("this code cannot tell who removed the directory... a forced or manual rm -rf of a DIRTY worktree would already have destroyed an uncommitted SUMMARY before cleanup ran"), and correctly notes no additional data is destroyed by this code — whatever was uncommitted was already gone before cleanup ran either way.

However, this is a real observability regression, not merely a restated known limitation: pre-fix, any anomalous absence blocked loudly (branch_mismatch / worktree_dirty / worktree_remove_failed), giving an operator a signal to investigate. Post-fix, the externally-force-deleted-dirty-worktree case is silently indistinguishable from the routine harness-cleanup case — both report status: 'merged_removed', reason: 'ok', with no warning code emitted, even though the function already has a live WAVE_CLEANUP_WARNING-based advisory channel used for the analogous scope-conformance case a few lines away (result.warnings.push(...scopeWarnings), src/worktree-safety.cts:1264). The mechanism to close this gap already exists in the diff and is unused for it.

Findings by severity

Medium — dead field, no consumer, mis-tested (self-found, not from prior review rounds)
WorktreeEntry.prunable (src/worktree-safety.cts:67, populated at :88-95) is parsed from git worktree list --porcelain and carries an extensive doc comment claiming it is "worth surfacing to an operator" — but it is never read anywhere in the file (grep -n '\.prunable\b' → zero hits outside the parse function itself). It does not drive absentAndIdentified, is not attached to any result/warning, and has no consumer. The associated test 'a bare prunable marker (no reason text) is still read as prunable' (tests/worktree-safety.test.cjs:2726) asserts only status === 'merged_removed', which is driven entirely by confirmedGone/branch-match, not by whether the bare-marker parsing succeeds — so the parsing logic this field exists for has no assertion that would catch a regression in it. Either wire prunable's reason text into a warning for the accepted-absent case (which would also close the self-adversarial-pass gap above) or remove the field and its parsing/tests.

Medium — silent observability gap for out-of-band worktree deletion (self-adversarial pass)
See above. An entry accepted via the absent-path fallback reports merged_removed/ok identically to a normal present-worktree merge. No warning distinguishes "harness cleanly removed a finished executor" from "something else removed this path" (dirty or not). Given WAVE_CLEANUP_WARNING already exists and is used for a materially less risky advisory (scope conformance) two dozen lines later in the same function, withholding one here for a destructive-adjacent path is inconsistent with the module's own pattern.

Note (not a defect) — cross-PR overlap
#4766 touches the same function in the same file, overlapping hunks. Flagging per brief; not resolving.

Verdict line

VERDICT: changes-requested — root cause and mechanism are correct and well-tested (422/425 targeted rows, real-git integration tests, 4 rounds of adversarial review already resolved), but ship blocked on: (1) resolve the dead prunable field (wire it to a warning or delete it), and (2) either accept explicitly in the PR thread that the harness-vs-external-deletion ambiguity is out of scope, or add the warning the existing WAVE_CLEANUP_WARNING channel already supports for exactly this kind of advisory.

[behruznassre]

Review round 3 — both Medium findings closed, and they closed together as you predicted

Medium 1 + 2 — one change, because you were right that they were the same gap

WAVE_CLEANUP_WARNING.ACCEPTED_ABSENT_WORKTREE is now emitted at both acceptance sites, carrying git's own prunable reason. Advisory, never a gate — the entry still merges.

On the observability argument: agreed, and the framing is the part worth keeping. This code genuinely cannot tell "the harness cleanly removed a finished executor" from "something else removed this path" — git keeps the path → branch registration and statSync reports ENOENT either way. Pre-fix, every anomalous absence blocked loudly. Accepting the routine case silently took the operator's only signal away from the case that is not routine, and withholding an advisory here while the module emits one for scope conformance two dozen lines later was inconsistent with its own pattern.

That also gives prunable its consumer, which is the honest resolution of Medium 1: the field was parsed, documented as "worth surfacing to an operator", and then never read — the errno rework made it unused for the predicate and I left the parsing behind. Quoting git's reason is what it was for.

On the vacuous test — you were right, and it is worth being precise about why. The bare-marker row asserted merged_removed, which is driven by confirmedGone and the branch match, not by the bare-marker parsing it claimed to cover. A regression in that parsing would not have reddened it. It now asserts the parsed value reaches the warning.

One detail that surfaced while wiring it: a bare prunable line normalises to the literal string 'prunable' — a truthiness signal, not a reason. The warning reports null there rather than quoting a marker back at an operator as though git had said something.

The locked code set is updated deliberately, with the reason recorded in the test. The lock exists so a new advisory code is a decision rather than something that appears because a branch needed one, and I did not want to quietly satisfy it.

Your cross-PR note — #4766 landed, and the overlap was real

Your warning was accurate: not just conflict resolution. Both conflicts were additive (the frozen registry and its lock test) and are resolved by keeping both sides — six codes now, #4721's three plus this PR's.

The interaction is what I actually checked, rather than trusting a clean compile: #4721's merge timeout and index-residue recovery live entirely inside the if (!gitResultOk(merge)) failure branch, which ends in continue, while this PR's worktreeAbsent teardown is on the success path after the merge returns. They do not share control flow, so neither weakens the other. Confirmed behaviourally too — all 14 of #4721's own rows pass on the merged result.

Verification

• Mutation: suppressing the warning at both acceptance sites turns both new rows red
• npm run lint:ci — exit 0
• npm test — 27/27 chunks, 37,947 tests, 0 failures, against the true merge-base 092d9256b
• tests/worktree-safety.test.cjs — 438/438, including bug(worktree.cleanup-wave): merge is killed at the 10 s default git timeout while a pre-merge-commit hook runs; reported as merge_failed and leaves the main index staged with no MERGE_HEAD #4721's rows

One correction to my own process, since it produced a false alarm twice: my local runner reported differential attribution red after this merge. Both times it was my harness, not the tree — first a hardcoded GSD_EMITTED_BASE that went stale, then a computed base resolved against an unfetched upstream/next. Correct arithmetic on wrong inputs. The runner now fetches before computing, and I re-ran the full suite rather than declaring green from a targeted re-check.