Skip to content

fix: protect shared worktrees during task teardown - #25

Merged
TastyTom13 merged 3 commits into
mainfrom
fm/fm-teardown-shared-worktree-guard
Sep 8, 2026
Merged

TastyTom13 merged 3 commits into
mainfrom
fm/fm-teardown-shared-worktree-guard

Conversation

@TastyTom13

@TastyTom13 TastyTom13 commented Sep 8, 2026 •

Copy link
Copy Markdown
Owner

Intent

Add a preflight guard to bin/fm-teardown.sh so it refuses to return or reset a worktree that another live task still references, and add a --release-shared-record flag for the stale-record case.

Why: a treehouse pool path outlives the record that named it. Measured 2026-09-08 02:05, state/fm-scout-compliance-reaudit.meta still recorded worktree=~/.treehouse/scout-43588a/3/scout after the pool had handed that same path to fm-scout-ci-caching, whose own meta carried the identical worktree= line and a live window. Tearing down the stale record ran the pool return on that path, reset it to the default branch, and killed the live agent.

Required behaviour the captain asked for, all deliberate:

  1. Before any pool return or directory removal, scan every other state/.meta in the same FM_HOME for an identical worktree= value; if another task record has one, refuse with a clear message naming the sibling task id and the path, exit non-zero, and change nothing, printing the safe alternative (close the stale record's backlog item and remove only its own state files).
  2. Add --release-shared-record which tears down ONLY the calling task's own records (endpoint/window, meta, status presentation, steering inbox, check and PR poll artifacts, busy state, per-task temp root, backlog close) without touching the shared worktree.
  3. Keep every existing refusal untouched (dirty worktree, unlanded work, scout report, public follow-up, --force semantics).
  4. Extend the existing tests/fm-teardown.test.sh with the shared-worktree cases: refusal without the flag, records-only teardown with it.
  5. Document the flag in the script header and in docs; AGENTS.md section 7 gets at most one sentence.

Deliberate design decisions made while implementing, so they are not mistakes:

  • --force deliberately does NOT bypass the new shared-worktree refusal: --force authorizes discarding this task's own work, never another live task's copy. --release-shared-record is the intended escape hatch and is mutually exclusive with --force.
  • --release-shared-record is accepted ONLY while another record still names this task's worktree, and is refused for kind=secondmate (a secondmate home is retired whole), so the flag cannot be misused as a general skip-the-worktree teardown.
  • The worktree comparison is a literal string compare of the worktree= values, because fm-spawn records one resolved path per task, so an identical string is the collision and a differing string is a different slot. No path normalization was added on purpose.
  • The preflight is placed right after the task kind is read and before the remote-secondmate path, so it precedes every destructive step including the no-mistakes run abort, browser bridge sweep, process reap, branch delete and pool return.
  • Under --release-shared-record the endpoint kill, backlog close and record removal still run (that is the point of the flag); only worktree-directed work is skipped.
  • Repo conventions followed per the firstmate-coding-guidelines skill: one sentence per line in Markdown, plain dashes, script header owns the exact mechanics, AGENTS.md kept to one sentence, tests colocated in the existing tests/fm-teardown.test.sh rather than a new runner and asserting behaviour through the executable with a treehouse mock that logs invocations.

Already verified locally before this run: bin/fm-test-run.sh tests/fm-teardown.test.sh (total=1 failed=0), bin/fm-test-run.sh --changed (total=41 failed=0), bin/fm-lint.sh clean, bin/fm-doc-audience-check.sh ok.

Delivery: this is a PR on the firstmate repo; the firstmate CI requires the no-mistakes attestation, so this run must push and attach it (PR 25 already exists for this branch).

What Changed

  • Added a shared-worktree preflight that refuses teardown before destructive actions, including when --force is used.
  • Added --release-shared-record to remove only the stale task's records and backlog item while preserving the shared worktree.
  • Added shared-worktree teardown tests and documented the new behavior.

Risk Assessment

✅ Low: The change is bounded to teardown protection, records-only cleanup, documentation, and behavioral tests, with no source-verifiable correctness issues found.

Testing

The teardown test family passed after rerunning with a sufficient timeout; shared-worktree behavior was exercised end-to-end through the executable, including refusal output, preservation of the worktree, and records-only cleanup.

Evidence: Shared-worktree behavior transcript
ok - teardown refuses a worktree another task record still names, and changes nothing
ok - --force authorizes discarding this task's work, never another task's shared worktree
ok - --release-shared-record retires only the stale record and leaves the shared worktree intact
ok - --release-shared-record refuses outside the shared-worktree case it exists for

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • bin/fm-test-run.sh tests/fm-teardown.test.sh
  • Shared-worktree refusal, --force refusal, records-only release, and unshared release refusal cases
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

TastyTom13 and others added 2 commits September 8, 2026 03:58
A pool path outlives the record that named it. On 2026-09-08 a stale task
record still carried worktree=<path> after the pool had handed that exact
path to a live task; tearing the stale record down ran the pool return on
that path, reset it to the default branch, and killed the live agent.

Add a metadata-only preflight that runs before any pool return, branch
delete, worktree reset, process reap, bridge sweep, or run abort: if any
other task record in the same FM_HOME carries an identical worktree=
value, teardown refuses, names the other task and the path, changes
nothing, and prints the records-only alternative. --force does not bypass
it, because --force authorizes discarding this task's own work, not
another task's.

Add --release-shared-record for exactly that stale-record case: it
retires only the calling task's own records (endpoint, meta, status
presentation, steering inbox, check and PR poll artifacts, busy state,
per-task temp root, backlog close) and touches the shared worktree in no
way at all. It is accepted only while another record still names the
worktree, never for kind=secondmate, and is mutually exclusive with
--force.

Every existing refusal (dirty worktree, unlanded work, scout report,
public follow-up, --force semantics) is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RwpKERxmGp8xMZ9A3tqZwt
@TastyTom13 TastyTom13 changed the title fix(teardown): refuse to return a worktree another task still records fix: protect shared worktrees during task teardown Sep 8, 2026
…o use --release-shared-record when the new shared-worktree guard refuses forced teardown. Syntax and diff checks pass
@TastyTom13
TastyTom13 merged commit 43552e5 into main Sep 8, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant