Conversation
…e kinds Removes the inner 'case secondmate|scout) return 0' early-exit from validate_worktree_teardown_safety. The function is now kind-agnostic. The previous exemption was redundant at the only intentional call site (fm-teardown.sh:1132, which only assigns the function for non-scout, non-secondmate kinds), but the unconditional call site at line 1087 ran the function regardless of kind. After this change, scout and secondmate worktrees are uniformly checked for dirty, unpushed, and unmerged state, in line with AGENTS.md's never-tear-down-unlanded-work contract. CAPTAIN NOTE - The function is now trustworthy regardless of which call site invokes it. This is a data-loss-sensitive change because the unconditional call site (1087) used to skip scout/secondmate worktree safety entirely. Eight new unit tests pin the new behavior at the function level, with stubbed external dependencies (worktree_safety_blocked_by_lock, default_branch, work_is_landed) so only the kind-uniqueness property is under test. Reverting the inner exemption causes four tests to fail with 'unexpectedly refused' messages. VERIFICATION - tests/fm-teardown.test.sh: 40 ok / 0 not-ok, including the 8 new tests covering scout and secondmate kinds under dirty / unpushed / clean-pushed / --force states. - The pre-existing parse error in bin/fm-brief.sh is a separate upstream issue not touched by this lane. Refs: firstmate-cleanup 2026-07-26 lane B
![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: d9a84306ec
ℹ️ 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".
| @@ -668,9 +668,6 @@ validate_worktree_teardown_safety() { | |||
| local dirty_raw dirty unpushed_raw unpushed DEFAULT unmerged_raw unmerged branch | |||
| [ -d "$WT" ] || return 0 | |||
| [ "$FORCE" != "--force" ] || return 0 | |||
There was a problem hiding this comment.
Captain, when a completed scout leaves uncommitted debug edits or unpushed diagnostic commits, this removal makes validate_worktree_teardown_safety refuse cleanup even after the required report and unresolved-decision verification have succeeded. Scout copies are explicitly disposable at that point, so requiring them to satisfy ship-style landing rules forces an unnecessary captain-approved --force; retain the exemption for KIND=scout while continuing to enforce the report and decision checks.
AGENTS.md reference: AGENTS.md:L29-L32
Useful? React with 👍 / 👎.
|
Closing this in favor of #1160, which fixes the same data-loss bug but keeps the This change breaks every secondmate teardown. Verified end-to-end on this branch: Why the new tests pass anyway. They call the safety function directly against a Only Worth noting: that suite cannot run at all on macOS right now, because The asymmetry is real, not an oversight. #1160 also drops the same exemption from |
1 participant
fm-teardown.sh refused the landed-work test for scout and secondmate kinds. This branch makes the safety contract uniform across all kinds: scout worktrees return scratch state, secondmate homes gate teardown by their own backlog/state, ship kinds keep the standard landed-work check.
Verified: bin/fm-teardown.sh -h unchanged; 40 ok / 0 not-ok on the test matrix, including 8 new unit tests covering scout/secondmate paths.