fix(teardown): remove the scout worktree-safety exemption - #1160
Conversation
validate_worktree_teardown_safety() returned 0 before any check ran for KIND=secondmate|scout. On 2026-07-26 a teardown sweep across 104 worktrees hit 11 that held uncommitted work and destroyed 46 files. The files were untracked and unstaged, so they were never in git's object store and are unrecoverable. Drop the scout half. A scout's worktree is a real git worktree, so every check applies to it, and the exemption's stated rationale (scouts leave scratch behind) is already covered by the dirty filter's .claude/ and .fm-*-turnend exclusions. The premise that scouts are read-only was false in practice: a scout lane authored ADR 0058 and a guard script. Keep the secondmate half. It is load-bearing, not redundant: a secondmate home is a plain directory with a .fm-secondmate-home marker rather than a git worktree, so the git-based checks fail on it and fail-safe to REFUSED. Removing it makes every secondmate teardown impossible. Also drop the same exemption from the post-lock-cleanup recheck, which is what catches work written between the first safety check and the treehouse return that resets the worktree. secondmate never reaches that branch. --force remains the escape hatch in both places. The exemption survived because it was never tested: fm-teardown.test.sh had 1,405 lines and zero occurrences of scout or secondmate. Against the old code the new test reproduces the incident exactly, exit 0 with empty stderr. A companion case pins the original rationale so scratch-only scouts still tear down clean. There is deliberately no secondmate twin: a secondmate refuses earlier on the seeded-home check, so such a case would pass with or without the exemption and would only look like coverage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V5bvMEeW6SqECT3kg4Utry
![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: 8fbbe9087c
ℹ️ 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".
| # --force remains the explicit escape hatch. | ||
| case "$KIND" in | ||
| secondmate|scout) return 0 ;; | ||
| secondmate) return 0 ;; |
There was a problem hiding this comment.
Captain, when a completed scout has clean local scratch commits but no remote branch or merged PR, removing scout from this return sends it through the unpushed/landed checks and refuses cleanup even after its report and decision gate are complete. Scout worktrees are explicitly disposable after those gates, and promotion is expected to leave scratch commits behind, so run the dirty-state check for scouts but return before applying ship-style commit landing checks.
AGENTS.md reference: AGENTS.md:L324-L328
Useful? React with 👍 / 👎.
1 participant
What
validate_worktree_teardown_safety()returns0before any check runs whenKINDissecondmateorscout:This removes the
scouthalf and keepssecondmate.Why
A teardown sweep across 104 worktrees hit 11 that held uncommitted work and destroyed 46 files. They were untracked and unstaged, so they had never entered git's object store and were unrecoverable. Branches survived, so committed history was fine; the uncommitted deltas were not.
The safety function itself is well built (checks dirty, unpushed, unmerged; refuses on any). It simply never ran.
Why
scoutgoesA scout's worktree is a real git worktree, so every check below the exemption applies to it. The stated rationale, that scouts leave scratch behind, is already covered by the dirty filter's
.claude/and.fm-*-turnendexclusions.The premise that scouts are read-only is false in practice: a scout lane in our fleet authored an ADR and a guard script, both lost.
Why
secondmatestaysThis half is load-bearing rather than redundant, and worth stating explicitly so it is not "cleaned up" later.
A secondmate home is a plain directory carrying a
.fm-secondmate-homemarker, not a git worktree. Every check in the function shells out togit -C "$WT", which fails on a non-repo and correctly fail-safes toREFUSED. Removing this exemption makes every secondmate teardown impossible.tests/fm-secondmate-safety.test.shcatches it immediately ("teardown failed for empty secondmate home").There is no uncommitted git state there for these checks to protect.
Also
The same exemption guarded
post_lock_cleanup_check, the recheck that catches work written between the first safety check and thetreehouse returnthat resets the worktree. Leaving it would have kept the hole open on the lock path.secondmatenever reaches that branch, since theelifabove it already excludes it, so no exemption is needed there.--forceremains the explicit escape hatch in both places.Tests
The exemption survived because it was never tested:
tests/fm-teardown.test.shwas 1,405 lines with zero occurrences ofscoutorsecondmate.test_scout_dirty_worktree_refusesreproduces the incident against the old code exactly: exit 0, empty stderr, worktree torn down with an uncommitted edit present.test_scout_scratch_only_worktree_tears_downpins the exemption's original rationale, so a scout holding only agent scratch still tears down clean.Reaching the dirty check as a scout means satisfying two earlier scout gates first (
data/<id>/report.md, anddecisions_reviewed=1). Those are about the work product, not the worktree, which is why they are not a substitute for this check.There is deliberately no
secondmatetwin: a secondmate refuses earlier on the seeded-home check, so such a case would pass with or without the exemption and would only look like coverage.tests/fm-teardown.test.sh: 34/34, up from 32.Full suite on this branch shows no new failures. Note that
tests/fm-secondmate-safety.test.shcurrently fails on macOS atorigin/mainfor an unrelated reason:bin/fm-brief.shhas not parsed under bash 3.2 since #945, which I am happy to send as a separate PR.