Skip to content

fix(bin): verify recorded worktree ownership before teardown - #1177

Open
twaldin wants to merge 8 commits into
kunchenguid:mainfrom
twaldin:fm/teardown-worktree-verify-t1
Open

fix(bin): verify recorded worktree ownership before teardown#1177
twaldin wants to merge 8 commits into
kunchenguid:mainfrom
twaldin:fm/teardown-worktree-verify-t1

Conversation

@twaldin

@twaldin twaldin commented Jul 28, 2026

Copy link
Copy Markdown

Intent

Make bin/fm-teardown.sh fail closed before destructive cleanup when the recorded worktree cannot be proven to belong to the task. Teardown must verify the recorded path is an inspectable git worktree owned by resume_branch= when present, otherwise fm/, and it must loudly refuse wrong-owner, missing, or empty paths while preserving task metadata. Wrong-owner refusals must name the recorded path, found branch, and expected branch, and must not reset, detach, return, or kill the live owner. A genuinely absent path must be distinguishable from a foreign owner and must not silently authorize deleting the task's only durable record; --force remains the explicit operator override after discard approval. Keep existing dirty, landed-work, local-only, PR, content-in-default, stale-lock, and lock-blocked checks intact. Scouts still skip landed-work inspection because their report is the work product, but they must still prove worktree ownership before a pooled worktree is returned so a recycled scout record cannot destroy another live lane. Secondmate teardown remains carved out because it validates and removes a seeded firstmate home rather than returning a task worktree. Detached HEAD should be accepted only when its commit exactly equals the expected branch tip, so legitimate shed-lane teardown works without normalizing operators toward --force, while detached foreign tips still refuse.

What Changed

  • bin/fm-teardown.sh now proves the recorded worktree belongs to the task before any non-force cleanup: the path must be an inspectable git worktree sitting on resume_branch= (falling back to fm/<id>), or detached at exactly that branch's tip. Missing, empty, uninspectable, or foreign-branch paths refuse loudly — naming the recorded path, found branch, and expected branch — and preserve state/<id>.meta instead of resetting, returning, or killing a live lane. A detached HEAD with no expected branch stays undecidable by identity and falls through to the existing dirty/unpushed/unmerged/landed-work checks; a verified-ownership memo lets the post-lock recheck run after HEAD is detached and the task branch deleted. --force and the secondmate carve-out are unchanged.
  • Scout lanes lost their blanket worktree exemption and now take the same ownership and unlanded-work checks as ship lanes (an already-absent scout path is the one carve-out, since there is no lane to return and data/<id>/report.md is untouched). Because a scout may not push or open a PR, unlanded-work refusals print scout-specific remedies — report, restore the lane, or promote via bin/fm-promote.sh — instead of push/merge advice. bin/fm-spawn.sh records resume_branch=fm/<id> for every non-secondmate task, and bin/fm-brief.sh makes git checkout -b fm/<id> the scout's first action plus a new rule 8 telling it to hand the lane back at its starting commit.
  • Aligned the surrounding contracts with the new behavior: AGENTS.md rules 3 and the meta-key inventory, the scout-promotion section, bin/fm-promote.sh and bin/fm-crew-state.sh comments, docs/architecture.md, docs/orca-backend.md, docs/scripts.md, and the Orca skill. tests/fm-teardown.test.sh grew 20 ownership cases (recycled foreign branch, absent/empty path, resume_branch preference, detached-at-tip vs. wrong-tip, --force override, secondmate carve-out, scout foreign lane and scratch refusals, lock-blocked stale-lock recovery) and tests/fm-gotmp.test.sh was updated for the absent-path refusal.

Risk Assessment

✅ Low: The ownership state machine is now internally consistent and fail-closed, the round-4 fixture breakage is fixed at the fixture level with a documented rationale rather than by weakening the guard, scout lanes have a test-pinned clean exit plus scout-appropriate refusal remedies, and every source-verifiable acceptance criterion is met except the scout landed-work skip that four captain rulings deliberately superseded.

Verification Reality

This change is fully validated locally: no-mistakes review, tests, documentation, and lint all completed and passed. It is not CI-verified because this repository has no CI configured for this PR, so no checks exist to be green or red. This is not a failure and not a gap in the work; it is just a verification channel that does not exist here.

Key Regression Coverage

  • test_recycled_worktree_foreign_branch_refuses_without_touching_owner - a recycled path now owned by a different live task must refuse, and the test asserts the live owner tree is unchanged afterward so a refusal that still reset or killed something fails rather than passes.
  • test_absent_recorded_worktree_refuses_without_deleting_meta - a genuinely absent path must not report safe and must not authorize deleting the only durable record.
  • test_task_branch_ownership_happy_path_allows - the ordinary case still tears down cleanly.

Testing

Ran the targeted teardown suite plus the two collaterally-touched suites (gotmp, brief) — all green — then drove the real fm-teardown.sh end-to-end over a genuine git worktree pool for six operator scenarios and captured the CLI transcript: a recycled record pointing at another live lane refuses naming recorded path, found branch, and expected branch, with the live owner's HEAD, branch, and uncommitted edit unchanged and zero treehouse-return/tmux-kill calls; an absent path gets its own distinguishable refusal and keeps the meta; --force still clears it after approval; detached HEAD at the expected tip tears down without --force; a recycled scout lane refuses while its report survives; and a scout holding its own scratch gets report/restore/fm-promote.sh remedies with no "push the branch" or fm-merge-local.sh advice. A second manual run of the real fm-spawn.sh shows the resume_branch=fm/<id> key teardown reads is actually written. No UI surface is involved — this is a CLI safety gate — so evidence is operator-visible terminal transcripts. Everything passed and the working tree is clean.

Evidence: End-to-end teardown ownership CLI transcript (6 operator scenarios)

SCENARIO 1 - a recycled task record points at ANOTHER live crewmate's lane live lane before teardown: HEAD commit : 3d47076 HEAD ref : fm/task-b2 dirty files : M feature.txt $ bin/fm-teardown.sh task-x1 REFUSED: recorded worktree path does not belong to task task-x1. recorded path: .../recycled-lane/lane found branch: fm/task-b2 expected branch: fm/task-x1 Correct .../state/task-x1.meta to point at the task's real worktree, or get the captain's explicit OK to discard, then --force. exit status: 1 live lane AFTER the refusal: HEAD commit : 3d47076 HEAD ref : fm/task-b2 dirty files : M feature.txt task metadata preserved : yes treehouse return calls : 0 tmux kill calls : 0 SCENARIO 2 - the recorded path is simply gone (absent, not foreign) $ bin/fm-teardown.sh task-x1 REFUSED: task task-x1 has no inspectable git worktree at recorded path .../absent-path/vanished-lane. expected branch: fm/task-x1 Cannot inspect landed work or verify that the recorded worktree belongs to this task; preserving .../state/task-x1.meta. Restore the task worktree path, correct the metadata pointer, or get the captain's explicit OK to discard, then --force. exit status: 1 task metadata preserved : yes SCENARIO 3 - captain approved the discard, operator overrides with --force $ bin/fm-teardown.sh task-x1 --force teardown task-x1 complete (window fm-task-x1, worktree .../vanished-lane) exit status: 0 SCENARIO 4 - detached HEAD exactly at the task branch tip lane HEAD ref: HEAD (detached at 19a677c) $ bin/fm-teardown.sh task-x1 teardown task-x1 complete exit status: 0 SCENARIO 5 - scout lane recycled to another owner $ bin/fm-teardown.sh task-x1 (kind=scout) REFUSED: recorded worktree path does not belong to task task-x1. found branch: fm/task-c3 / expected branch: fm/task-x1 live lane still on : fm/task-c3 treehouse return calls : 0 SCENARIO 6 - scout lane holding its own scratch $ bin/fm-teardown.sh task-x1 (kind=scout, lane on fm/task-x1 with unlanded scratch) REFUSED: worktree .../lane has work not on any remote and not landed. The report is this scout's deliverable, not its worktree: ... restore the lane to the commit its branch started at ... If this code should ship instead of being discarded, promote the task with bin/fm-promote.sh ... exit status: 1


=============================================================
SCENARIO 1 - a recycled task record points at ANOTHER live crewmate's lane
=============================================================
state/task-x1.meta (stale record):
  window=fm-task-x1
  worktree=/var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-teardown-demo.B5c1kX/recycled-lane/lane
  project=/var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-teardown-demo.B5c1kX/recycled-lane/project
  kind=ship
  mode=no-mistakes
live lane before teardown:
  HEAD commit : 3d47076
  HEAD ref    : fm/task-b2
  dirty files :  M feature.txt 

$ bin/fm-teardown.sh task-x1
REFUSED: recorded worktree path does not belong to task task-x1.
recorded path: /var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-teardown-demo.B5c1kX/recycled-lane/lane
found branch: fm/task-b2
expected branch: fm/task-x1
Correct /var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-teardown-demo.B5c1kX/recycled-lane/state/task-x1.meta to point at the task's real worktree, or get the captain's explicit OK to discard, then --force.
exit status: 1

live lane AFTER the refusal:
  HEAD commit : 3d47076
  HEAD ref    : fm/task-b2
  dirty files :  M feature.txt 
task metadata preserved : yes
treehouse return calls  : 0
tmux kill calls         : 0

=============================================================
SCENARIO 2 - the recorded path is simply gone (absent, not foreign)
=============================================================

$ bin/fm-teardown.sh task-x1
REFUSED: task task-x1 has no inspectable git worktree at recorded path /var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-teardown-demo.B5c1kX/absent-path/vanished-lane.
expected branch: fm/task-x1
Cannot inspect landed work or verify that the recorded worktree belongs to this task; preserving /var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-teardown-demo.B5c1kX/absent-path/state/task-x1.meta.
Restore the task worktree path, correct the metadata pointer, or get the captain's explicit OK to discard, then --force.
exit status: 1
task metadata preserved : yes

=============================================================
SCENARIO 3 - captain approved the discard, operator overrides with --force
=============================================================

$ bin/fm-teardown.sh task-x1 --force   (same absent-path record)
/var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-teardown-demo.B5c1kX/absent-path/project: already current
teardown task-x1 complete (window fm-task-x1, worktree /var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-teardown-demo.B5c1kX/absent-path/vanished-lane)
Backlog: task-x1 just finished. Run tasks-axi done task-x1 --pr PR_URL, then run tasks-axi ready for dependency-cleared candidates, check date gates, and dispatch only work whose blockers are gone and date is due.
exit status: 0
task metadata removed   : yes

=============================================================
SCENARIO 4 - legitimate shed lane: detached HEAD exactly at the task branch tip
=============================================================
lane HEAD ref: HEAD (detached at 19a677c)

$ bin/fm-teardown.sh task-x1
/var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-teardown-demo.B5c1kX/detached-at-tip/project: already current
teardown task-x1 complete (window fm-task-x1, worktree /var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-teardown-demo.B5c1kX/detached-at-tip/lane)
Backlog: task-x1 just finished. Run tasks-axi done task-x1 --pr PR_URL, then run tasks-axi ready for dependency-cleared candidates, check date gates, and dispatch only work whose blockers are gone and date is due.
exit status: 0
task metadata removed   : yes

=============================================================
SCENARIO 5 - scout lane recycled to another owner: report exists, lane is not ours
=============================================================
scout report on disk: data/task-x1/report.md

$ bin/fm-teardown.sh task-x1   (kind=scout)
REFUSED: recorded worktree path does not belong to task task-x1.
recorded path: /var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-teardown-demo.B5c1kX/scout-recycled/lane
found branch: fm/task-c3
expected branch: fm/task-x1
Correct /var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-teardown-demo.B5c1kX/scout-recycled/state/task-x1.meta to point at the task's real worktree, or get the captain's explicit OK to discard, then --force.
exit status: 1
live lane still on      : fm/task-c3
treehouse return calls  : 0
task metadata preserved : yes

=============================================================
SCENARIO 6 - scout lane holding its own scratch: refused with scout-shaped remedies
=============================================================

$ bin/fm-teardown.sh task-x1   (kind=scout, lane on fm/task-x1 with unlanded scratch)
REFUSED: worktree /var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-teardown-demo.B5c1kX/scout-scratch/lane has work not on any remote and not landed.
unpushed commits:
9a1bc62 scout scratch
The report is this scout's deliverable, not its worktree: put the findings in /var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-teardown-demo.B5c1kX/scout-scratch/data/task-x1/report.md, then restore the lane to the commit its branch started at so nothing of yours is left in it.
If this code should ship instead of being discarded, promote the task with bin/fm-promote.sh rather than tearing it down. Otherwise get the captain's explicit OK to discard, then --force.
exit status: 1
scout report preserved  : yes
task metadata preserved : yes
Evidence: Real fm-spawn.sh records the resume_branch ownership key

$ bin/fm-spawn.sh demo-task-9 .../project spawned demo-task-9 harness=codex kind=ship mode=no-mistakes yolo=off window=firstmate:fm-demo-task-9 worktree=.../wt $ cat state/demo-task-9.meta window=firstmate:fm-demo-task-9 worktree=.../wt project=.../project harness=codex kind=ship mode=no-mistakes yolo=off tasktmp=/tmp/fm-demo-task-9 resume_branch=fm/demo-task-9 model=default effort=default ownership key teardown reads: resume_branch=fm/demo-task-9


$ bin/fm-spawn.sh demo-task-9 /var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-spawn-owner-demo.GtArWw/project
warn: no registry at /var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-spawn-owner-demo.GtArWw/home/data/projects.md; defaulting project to no-mistakes off
spawned demo-task-9 harness=codex kind=ship mode=no-mistakes yolo=off window=firstmate:fm-demo-task-9 worktree=/var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-spawn-owner-demo.GtArWw/wt

$ cat state/demo-task-9.meta
  window=firstmate:fm-demo-task-9
  worktree=/var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T//fm-spawn-owner-demo.GtArWw/wt
  project=/var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T/fm-spawn-owner-demo.GtArWw/project
  harness=codex
  kind=ship
  mode=no-mistakes
  yolo=off
  tasktmp=/tmp/fm-demo-task-9
  resume_branch=fm/demo-task-9
  model=default
  effort=default

ownership key teardown reads: resume_branch=fm/demo-task-9
Evidence: Manual teardown ownership demo driver
#!/usr/bin/env bash
# Manual end-to-end demo of bin/fm-teardown.sh recorded-worktree ownership verification.
# Builds a real firstmate-shaped sandbox (bare origin + project clone + pooled
# worktrees) and drives the real bin/fm-teardown.sh as an operator would.
set -u

ROOT=${ROOT:?set ROOT to the firstmate checkout}
SANDBOX=$(mktemp -d "${TMPDIR:-/tmp}/fm-teardown-demo.XXXXXX")
export GIT_AUTHOR_NAME=demo GIT_AUTHOR_EMAIL=demo@example.invalid
export GIT_COMMITTER_NAME=demo GIT_COMMITTER_EMAIL=demo@example.invalid

banner() { printf '\n=============================================================\n%s\n=============================================================\n' "$*"; }
step()   { printf '\n$ %s\n' "$*"; }

new_case() {
  # new_case <name>: bare origin, project clone, one pooled worktree at $case/lane
  local name=$1 case_dir
  case_dir="$SANDBOX/$name"
  mkdir -p "$case_dir/state" "$case_dir/config" "$case_dir/data" "$case_dir/fakebin"
  # PATH shims: prove teardown never reaches the destructive lifecycle steps.
  : > "$case_dir/treehouse.log"; : > "$case_dir/tmux.log"
  cat > "$case_dir/fakebin/treehouse" <<SH
#!/usr/bin/env bash
printf 'treehouse %s\n' "\$*" >> "$case_dir/treehouse.log"
exit 0
SH
  cat > "$case_dir/fakebin/tmux" <<SH
#!/usr/bin/env bash
printf 'tmux %s\n' "\$*" >> "$case_dir/tmux.log"
exit 0
SH
  cat > "$case_dir/fakebin/gh-axi" <<'SH'
#!/usr/bin/env bash
case "${1:-} ${2:-}" in
  "pr list") printf '%s\n' "count: 0 (showing first 0)" "pull_requests[]: []"; exit 0 ;;
  "pr view") echo "error: pull request not found" >&2; exit 1 ;;
esac
exit 0
SH
  cp "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh"
  chmod +x "$case_dir/fakebin/"*
  git init -q --bare "$case_dir/origin.git"
  git -C "$case_dir/origin.git" symbolic-ref HEAD refs/heads/main
  git clone -q "$case_dir/origin.git" "$case_dir/_seed" 2>/dev/null
  git -C "$case_dir/_seed" commit -q --allow-empty -m "origin baseline"
  git -C "$case_dir/_seed" push -q origin main
  rm -rf "$case_dir/_seed"
  git clone -q "$case_dir/origin.git" "$case_dir/project"
  git -C "$case_dir/project" remote set-head origin main 2>/dev/null || true
  git -C "$case_dir/project" worktree add -q -b fm/task-x1 "$case_dir/lane" main
  touch "$case_dir/state/.last-watcher-beat"
  printf '%s\n' "$case_dir"
}

teardown() {
  local case_dir=$1; shift
  # Same escape hatch tests/lib.sh exports: this demo runs from a no-mistakes gate
  # worktree, which bin/fm-gate-refuse-lib.sh refuses by design.
  FM_GATE_REFUSE_BYPASS=1 \
  FM_ROOT_OVERRIDE="$ROOT" \
  FM_STATE_OVERRIDE="$case_dir/state" \
  FM_DATA_OVERRIDE="$case_dir/data" \
  FM_CONFIG_OVERRIDE="$case_dir/config" \
  PATH="$case_dir/fakebin:$PATH" \
    "$ROOT/bin/fm-teardown.sh" task-x1 "$@"
}

lane_state() {
  local lane=$1
  printf '  HEAD commit : %s\n' "$(git -C "$lane" rev-parse --short HEAD)"
  printf '  HEAD ref    : %s\n' "$(git -C "$lane" rev-parse --abbrev-ref HEAD)"
  printf '  dirty files : %s\n' "$(git -C "$lane" status --porcelain | tr '\n' ' ')"
}


banner "SCENARIO 1 - a recycled task record points at ANOTHER live crewmate's lane"
C=$(new_case recycled-lane)
# The lane got recycled to task-b2, who is mid-flight with uncommitted work.
git -C "$C/lane" checkout -q -b fm/task-b2
printf 'work in progress\n' > "$C/lane/feature.txt"
git -C "$C/lane" add feature.txt
git -C "$C/lane" commit -q -m "task-b2: in-flight commit"
printf 'uncommitted edit\n' >> "$C/lane/feature.txt"
# ... but a stale task-x1 record still points at that same path.
cat > "$C/state/task-x1.meta" <<EOF
window=fm-task-x1
worktree=$C/lane
project=$C/project
kind=ship
mode=no-mistakes
EOF
echo "state/task-x1.meta (stale record):"; sed 's/^/  /' "$C/state/task-x1.meta"
echo "live lane before teardown:"; lane_state "$C/lane"

step "bin/fm-teardown.sh task-x1"
teardown "$C" 2>&1; echo "exit status: $?"

echo
echo "live lane AFTER the refusal:"; lane_state "$C/lane"
printf 'task metadata preserved : %s\n' "$([ -f "$C/state/task-x1.meta" ] && echo yes || echo NO)"
printf 'treehouse return calls  : %s\n' "$(wc -l < "$C/treehouse.log" | tr -d ' ')"
printf 'tmux kill calls         : %s\n' "$(wc -l < "$C/tmux.log" | tr -d ' ')"


banner "SCENARIO 2 - the recorded path is simply gone (absent, not foreign)"
C=$(new_case absent-path)
cat > "$C/state/task-x1.meta" <<EOF
window=fm-task-x1
worktree=$C/vanished-lane
project=$C/project
kind=ship
mode=no-mistakes
EOF
step "bin/fm-teardown.sh task-x1"
teardown "$C" 2>&1; echo "exit status: $?"
printf 'task metadata preserved : %s\n' "$([ -f "$C/state/task-x1.meta" ] && echo yes || echo NO)"


banner "SCENARIO 3 - captain approved the discard, operator overrides with --force"
step "bin/fm-teardown.sh task-x1 --force   (same absent-path record)"
teardown "$C" --force 2>&1; echo "exit status: $?"
printf 'task metadata removed   : %s\n' "$([ -f "$C/state/task-x1.meta" ] && echo NO || echo yes)"


banner "SCENARIO 4 - legitimate shed lane: detached HEAD exactly at the task branch tip"
C=$(new_case detached-at-tip)
printf 'landed work\n' > "$C/lane/done.txt"
git -C "$C/lane" add done.txt
git -C "$C/lane" commit -q -m "task-x1: work"
git -C "$C/lane" push -q origin fm/task-x1
git -C "$C/project" fetch -q origin
git -C "$C/lane" checkout --detach -q
cat > "$C/state/task-x1.meta" <<EOF
window=fm-task-x1
worktree=$C/lane
project=$C/project
kind=ship
mode=no-mistakes
EOF
printf 'lane HEAD ref: %s (detached at %s)\n' \
  "$(git -C "$C/lane" rev-parse --abbrev-ref HEAD)" "$(git -C "$C/lane" rev-parse --short HEAD)"
step "bin/fm-teardown.sh task-x1"
teardown "$C" 2>&1; echo "exit status: $?"
printf 'task metadata removed   : %s\n' "$([ -f "$C/state/task-x1.meta" ] && echo NO || echo yes)"


banner "SCENARIO 5 - scout lane recycled to another owner: report exists, lane is not ours"
C=$(new_case scout-recycled)
git -C "$C/lane" checkout -q -b fm/task-c3
printf 'other lane work\n' > "$C/lane/other.txt"
git -C "$C/lane" add other.txt
git -C "$C/lane" commit -q -m "task-c3: in-flight commit"
cat > "$C/fakebin/tasks-axi" <<'SH'
#!/usr/bin/env bash
[ "${1:-}" = --version ] && { printf '0.1.1\n'; exit 0; }
case "${1:-} ${2:-}" in
  "update --help") printf '%s\n' 'usage: tasks-axi update <id> [flags]' '  --body-file <path>' '  --archive-body'; exit 0 ;;
  "mv --help") printf '%s\n' 'usage: tasks-axi mv <id> [<id>...] --to <path-or-dir>'; exit 0 ;;
  "hold --help") printf '%s\n' 'usage: tasks-axi hold <id> --reason <reason> --kind captain'; exit 0 ;;
esac
exit 0
SH
chmod +x "$C/fakebin/tasks-axi"
mkdir -p "$C/data/task-x1"
printf '# Scout report\n' > "$C/data/task-x1/report.md"
cat > "$C/state/task-x1.meta" <<EOF
window=fm-task-x1
worktree=$C/lane
project=$C/project
kind=scout
mode=no-mistakes
decisions_reviewed=1
decision_keys=
EOF
echo "scout report on disk: data/task-x1/report.md"
step "bin/fm-teardown.sh task-x1   (kind=scout)"
teardown "$C" 2>&1; echo "exit status: $?"
printf 'live lane still on      : %s\n' "$(git -C "$C/lane" rev-parse --abbrev-ref HEAD)"
printf 'treehouse return calls  : %s\n' "$(wc -l < "$C/treehouse.log" | tr -d ' ')"
printf 'task metadata preserved : %s\n' "$([ -f "$C/state/task-x1.meta" ] && echo yes || echo NO)"


banner "SCENARIO 6 - scout lane holding its own scratch: refused with scout-shaped remedies"
C=$(new_case scout-scratch)
printf 'scratch\n' > "$C/lane/scratch.txt"
git -C "$C/lane" add scratch.txt
git -C "$C/lane" commit -q -m "scout scratch"
cp "$SANDBOX/scout-recycled/fakebin/tasks-axi" "$C/fakebin/tasks-axi"
mkdir -p "$C/data/task-x1"
printf '# Scout report\n' > "$C/data/task-x1/report.md"
cat > "$C/state/task-x1.meta" <<EOF
window=fm-task-x1
worktree=$C/lane
project=$C/project
kind=scout
mode=no-mistakes
decisions_reviewed=1
decision_keys=
EOF
step "bin/fm-teardown.sh task-x1   (kind=scout, lane on fm/task-x1 with unlanded scratch)"
teardown "$C" 2>&1; echo "exit status: $?"
printf 'scout report preserved  : %s\n' "$([ -f "$C/data/task-x1/report.md" ] && echo yes || echo NO)"
printf 'task metadata preserved : %s\n' "$([ -f "$C/state/task-x1.meta" ] && echo yes || echo NO)"

rm -rf "$SANDBOX"
Evidence: Manual spawn resume_branch demo driver
#!/usr/bin/env bash
# Closes the loop the ownership check depends on: show the REAL bin/fm-spawn.sh
# writing the resume_branch= ownership key into state/<id>.meta for a task
# worktree, and NOT writing it for a secondmate (whose "worktree" is a seeded
# firstmate home, the documented carve-out).
set -u

ROOT=${ROOT:?set ROOT to the firstmate checkout}
# shellcheck source=/dev/null
. "$ROOT/tests/lib.sh"

SANDBOX=$(mktemp -d "${TMPDIR:-/tmp}/fm-spawn-owner-demo.XXXXXX")
SPAWN="$ROOT/bin/fm-spawn.sh"

fakebin=$(fm_fakebin "$SANDBOX/fake")
cat > "$fakebin/tmux" <<'SH'
#!/usr/bin/env bash
set -u
case "$*" in
  *"#{pane_current_path}"*) printf '%s\n' "${FM_FAKE_PANE_PATH:-}"; exit 0 ;;
esac
case "${1:-}" in
  display-message) printf 'firstmate\n'; exit 0 ;;
  list-windows) exit 0 ;;
  has-session|new-session|new-window|kill-window|send-keys) exit 0 ;;
esac
exit 0
SH
chmod +x "$fakebin/tmux"
fm_fake_exit0 "$fakebin" treehouse

HOME_DIR="$SANDBOX/home"
PROJ="$SANDBOX/project"
WT="$SANDBOX/wt"
mkdir -p "$HOME_DIR/data" "$HOME_DIR/projects" "$HOME_DIR/state" "$HOME_DIR/config"
printf 'codex\n' > "$HOME_DIR/config/crew-harness"
fm_git_worktree "$PROJ" "$WT" wt-owner-demo
ID=demo-task-9
mkdir -p "$HOME_DIR/data/$ID"
printf 'brief for %s\n' "$ID" > "$HOME_DIR/data/$ID/brief.md"
touch "$HOME_DIR/state/.last-watcher-beat"

printf '\n$ bin/fm-spawn.sh %s %s\n' "$ID" "$PROJ"
FM_ROOT_OVERRIDE='' FM_HOME="$HOME_DIR" \
  FM_STATE_OVERRIDE="$HOME_DIR/state" FM_DATA_OVERRIDE="$HOME_DIR/data" \
  FM_PROJECTS_OVERRIDE="$HOME_DIR/projects" FM_CONFIG_OVERRIDE="$HOME_DIR/config" \
  FM_SPAWN_NO_GUARD=1 TMUX="fake,1,0" FM_FAKE_PANE_PATH="$WT" \
  PATH="$fakebin:$PATH" \
  "$SPAWN" "$ID" "$PROJ" 2>&1

printf '\n$ cat state/%s.meta\n' "$ID"
sed 's/^/  /' "$HOME_DIR/state/$ID.meta"

printf '\nownership key teardown reads: %s\n' \
  "$(grep '^resume_branch=' "$HOME_DIR/state/$ID.meta" || echo '<<< MISSING >>>')"

rm -rf "$SANDBOX"

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 5 issues found → auto-fixed (4) ✅
  • 🚨 bin/fm-teardown.sh:733 - Ownership verification runs before the scout skip (:734), so a scout whose recorded worktree is already gone now refuses teardown even though nothing would be returned and its durable record (data/<id>/report.md) is untouched. The intent scopes the scout requirement to proving ownership "before a pooled worktree is returned"; here it also gates pure metadata/terminal cleanup. Two existing tests assert rc 0 for this exact shape and will now fail: tests/fm-backend-zellij.test.sh:812 ("fm-teardown should succeed for a zellij scout whose worktree is already gone") and tests/fm-backend-orca.test.sh:875 ("Orca teardown should release helpers even when the path is absent"). Consider gating the absent-path refusal on there actually being a worktree to return (i.e. let scouts with a missing path proceed, while still refusing a present-but-foreign path), or update those suites deliberately.
  • 🚨 bin/fm-teardown.sh:538 - A pool worktree that is still at its initial detached HEAD on the default branch — the documented starting state per bin/fm-brief.sh:351 ("at a detached HEAD on a clean default branch", with git checkout -b fm/$ID as the crewmate's first action) — has no fm/<id> ref, so expected_head is empty and teardown refuses with "found branch: HEAD / expected branch tip: <missing>". Any task whose crewmate died, was cancelled, or never branched now needs --force, which AGENTS.md rule 3 reserves for captain-authorized discard, and which the intent explicitly did not want operators normalized toward. This also breaks the real-spawn e2e teardowns tests/fm-backend-autodetect-smoke.test.sh:150 and tests/fm-backend-herdr-workspace-per-home-e2e.test.sh:207 (both spawn a trivial sh -c echo crew that never branches, then assert non-force teardown returns 0) whenever herdr/treehouse are installed. Suggest also accepting a detached HEAD whose commit is an existing remote-tracking/default-branch tip with no unpushed commits, since that provably holds no task work.
  • ⚠️ bin/fm-teardown.sh:169 - expected_worktree_branch() reads resume_branch= from meta, but nothing in the repo ever writes that key (only bin/fm-teardown.sh:171 and the new test reference it), so in practice the only accepted owner is fm/<id>. Meanwhile bin/fm-review-diff.sh:71-74 deliberately falls back to whatever branch HEAD points at when fm/<id> is absent, i.e. firstmate already supports crewmates working on a differently named branch. Such a task now refuses teardown with no supported way to record its real branch, leaving --force as the only path. Either add a producer for resume_branch (spawn/promote recording the actual branch) or reconcile with review-diff's looser convention.
  • ℹ️ bin/fm-teardown.sh:517 - The TEARDOWN_WORKTREE_OWNERSHIP_VERIFIED memo is silently load-bearing: validate_worktree_teardown_safety is reused as post_lock_cleanup_check (:1212), which runs after :1199-:1201 has already detached HEAD and deleted the task branch. Without the memo short-circuit that recheck would refuse with "does not belong to task" and abort a legitimate return. Add a comment at the memo (or at :1212) recording that dependency so a future refactor that resets or renames the flag doesn't silently break the stale-lock return path.
  • ℹ️ bin/fm-teardown.sh:729 - Open PR fix(teardown): remove the scout worktree-safety exemption #1160 ("fix(teardown): remove the scout worktree-safety exemption") edits the same case &#34;$KIND&#34; block and post_lock_cleanup_check condition from the same base, but resolves the scout question the opposite way: it subjects scouts to the full dirty/landed-work checks, whereas this branch keeps the landed-work skip and adds only an ownership proof. Whichever merges second will conflict textually and contradict the other's stated rationale; worth sequencing or reconciling explicitly.

🔧 Fix: scope teardown worktree ownership to real worktrees
6 issues (1 error, 3 warnings, 2 infos) still open:

  • 🚨 bin/fm-teardown.sh:781 - A scout whose worktree still exists is now required to prove ownership through the expected branch, but the scout contract never creates one: bin/fm-brief.sh:229-244 tells the scout it is "at a detached HEAD on a clean default branch" and to "install, run, edit, and make scratch commits freely; all of it is discarded at teardown", with no git checkout -b step anywhere. So a finished scout is detached with scratch commits and/or a dirty tree and no fm/<id> ref: expected_head is empty (:575) and worktree_holds_no_task_work fails on either the dirty check (:538) or the ancestor check (:542), so teardown refuses with "recorded worktree path does not belong to task" and the operator's only path is --force, which AGENTS.md rule 3 reserves for captain-authorized discard. This contradicts the intent's own scout rationale ("their worktree is declared scratch and the report at data/<task-id>/report.md is the work product") and the header text added at :39-42. No test catches it because every scout fixture (make_case's worktree add -b fm/task-x1, tests/fm-backend.test.sh:940 fm_git_worktree ... fm/$id) puts the scout on the branch a real scout never creates. Relatedly, bin/fm-spawn.sh:1450 records resume_branch=fm/$ID for scouts too, pinning an owner branch the scout contract never produces. Suggest proving a scout worktree's ownership by something the scout contract actually guarantees - e.g. worktree_registered_for_project "$PROJ" "$WT" plus no other live task meta in $STATE recording the same path - rather than by branch identity.
  • ⚠️ bin/fm-teardown.sh:537 - worktree_holds_no_task_work treats a failing git status --porcelain as "not the no-work state" and refuses hard. But a present index.lock is exactly why that command fails - the script models this elsewhere (worktree_safety_blocked_by_lock at :791, and tests/fm-teardown.test.sh:458 add_git_status_lock_failure mocks it) and recovers by waiting and clearing only a provably stale lock. On the detached + missing-expected-branch path the new proof bypasses that recovery entirely and prints "this detached worktree is not in the clean, no-work state..." (:599) for a check that never ran. The intent requires the existing stale-lock and lock-blocked checks stay intact; propagate the lock-blocked signal (TEARDOWN_WORKTREE_SAFETY_LOCK_BLOCKED) instead of collapsing a failed status into a wrong-owner refusal.
  • ⚠️ bin/fm-teardown.sh:591 - When the only reason the no-work proof fails is a dirty tree on a never-branched worktree - the likely case of this task's own crewmate editing before it branched - the refusal reads "recorded worktree path does not belong to task <id>" and offers only "Correct $META ... or get the captain's explicit OK to discard, then --force", i.e. it points at a discard for work that is probably ours and recoverable. tests/fm-teardown.test.sh:920 pins that wording. Name the actual cause (uncommitted changes present) and offer the non-destructive remedy (commit them on the expected branch) alongside the metadata-pointer explanation, the way the pre-existing dirty refusal at :809-811 does.
  • ⚠️ bin/fm-spawn.sh:1450 - AGENTS.md:91 (and its CLAUDE.md symlink) keeps an explicit inventory of the meta keys fm-spawn writes - "window=, worktree=, project=, harness=, model=, effort=, kind=, mode=, yolo=, tasktmp=" - and even enumerates optional later additions like x_platform=. The new resume_branch= key is written for every non-secondmate spawn but is not in that inventory, so the operator-facing contract no longer matches what spawn produces. Add it (noting the secondmate exclusion and that teardown falls back to fm/<id> for older metadata).
  • ℹ️ bin/fm-teardown.sh:582 - Intent conformance flag for the record: the stated criteria say "Detached HEAD should be accepted only when its commit exactly equals the expected branch tip, so legitimate shed-lane teardown works ... while detached foreign tips still refuse." The new branch at :582 accepts a detached HEAD that does not equal any expected branch tip - it accepts one that is merely an ancestor of the project's default branch whenever the expected branch does not exist. This was explicitly ordered in the round-1 captain ruling ("If the expected branch is absent, prove the worktree is pristine instead"), so the code is what was asked for, but the authoritative intent text and the script header still read as an exact-tip-only rule. Confirm the criteria should be amended to cover the no-work fallback so the two do not drift.
  • ℹ️ bin/fm-teardown.sh:535 - Residual gap worth recording, not a request to change the captain-approved design: the no-work proof is content-based only, so a recycled record pointing at a different lane that was just taken from the pool (clean, detached at the default branch, crewmate has not branched yet) passes ownership. Teardown then runs treehouse return --force, which kills that lane's processes including its agent. No committed or uncommitted work is lost, which is what the ruling reasoned about, but a live agent can still be killed by a stale record on this path.

🔧 Fix: decide worktree ownership only when provable
2 issues (1 error, 1 warning) still open:

  • 🚨 bin/fm-teardown.sh:555 - Intent criterion: "Scouts ... must still prove worktree ownership before a pooled worktree is returned so a recycled scout record cannot destroy another live lane." The new hunk expected_head=$(git -C &#34;$WT&#34; rev-parse --verify &#34;$expected^{commit}&#34; ...); [ -n &#34;$expected_head&#34; ] || return 0 (:554-555) returns an unproven pass whenever the expected branch does not exist - and for scouts it never exists: bin/fm-spawn.sh:1452 now writes resume_branch= only for kind=ship, and the scout brief (bin/fm-brief.sh:239-241) has no git checkout -b step, so expected falls back to fm/<id> which no scout creates. Net effect: no scout teardown proves ownership of anything. Concrete failure: a recycled scout meta pointing at another live lane that sits on its own branch (say fm/other-task) with a clean tree and everything pushed - the normal state right after /no-mistakes pushes and CI goes green - passes ownership (unproven), passes dirty, passes unpushed, and teardown runs treehouse return --force, which resets that lane and kills its agent. The previous commit 0bd607d refused this exact case (found=fm/other-task != expected reached the wrong-owner refusal). tests/fm-teardown.test.sh:766 test_scout_foreign_worktree_refuses_without_touching_owner still passes only because make_case leaves fm/task-x1 in existence, which real scouts never have, so that test asserts protection the shipped code does not provide for any real scout. The captain's round-2 ruling ordered scouts to fall through to the content checks, and the ruling's own condition ("still refusing present foreign worktrees when ownership can be proven foreign") is unsatisfiable for scouts under branch-identity proof - so either the intent criterion needs amending, or scout ownership needs a proof the scout contract actually guarantees (e.g. worktree_registered_for_project "$PROJ" "$WT" plus no other live task meta in $STATE recording the same path).
  • ⚠️ bin/fm-teardown.sh:769 - Intent criterion: "Scouts still skip landed-work inspection because their report is the work product." This commit deletes that skip - the [ &#34;$KIND&#34; != scout ] || return 0 line that followed require_recorded_worktree_ownership is gone, so scouts now fall straight into the dirty, unpushed, unmerged, and landed-work refusals. Per the captain's round-2 ruling that is deliberate, but three source contracts still describe the old behavior and now contradict the code: bin/fm-brief.sh:241 tells every scout "The worktree is your laboratory - install, run, edit, and make scratch commits freely; all of it is discarded at teardown", which is exactly the state that now refuses; AGENTS.md:32 says "A scout worktree is declared scratch and may be discarded only after its report exists and the shared unresolved-decision completion gate passes" while AGENTS.md:31 forbids --force "unless the captain explicitly authorized discarding that work", so the report gate no longer authorizes anything and every scout teardown becomes a --force escalation; and in local-only mode a scout with scratch commits now gets "Merge the branch into local $DEFAULT first (bin/fm-merge-local.sh after the captain approves)" (:806), guidance that cannot be followed because bin/fm-merge-local.sh:44 hardcodes BRANCH="fm/$ID", a branch no scout creates. Reconcile one way or the other: update the scout brief and AGENTS.md rule 3 to state that scout scratch must be committed or discarded with --force, or restore a scout allowance that the existing report + decision-hold gate already authorizes.

🔧 Fix: give scouts a task branch teardown can verify
4 issues (1 error, 2 warnings, 1 info) still open:

  • 🚨 bin/fm-teardown.sh:544 - tests/fm-gotmp.test.sh has three non-force teardowns of kind=ship metas whose worktree= points at a path that deliberately does not exist, and its make_fake_root comment states the dependency outright: "A nonexistent worktree path makes both if [ -d &#34;$WT&#34; ] guards skip, so teardown runs straight to the cleanup + state rm" (tests/fm-gotmp.test.sh:46-47). This branch removed the outer [ -d &#34;$WT&#34; ] guard (now if [ &#34;$FORCE&#34; != &#34;--force&#34; ] at :1204), so those runs reach require_recorded_worktree_ownership, fail inspectable_git_worktree, and exit 1 at :544. All three fail: tests/fm-gotmp.test.sh:146 ("teardown exited non-zero with a valid tasktmp"), :192 ("...when tasktmp= was absent"), :205 ("...when tasktmp dir was missing"), plus their follow-on [ ! -e &#34;$task_tmp&#34; ] assertions, since the refusal happens long before the TASK_TMP removal at :1330. That is also the real-world consequence the suite was pinning: a ship whose worktree is already gone now leaks its per-task temp root (/tmp/fm-<id>/, including gotmp build artifacts) and its state files until an operator --forces. Refusing an absent path for an ordinary task is what the intent requires, so the fixtures are what needs to move - give make_fake_root a real git worktree on fm/<id> (or tear down with --force) - but confirm the leak is acceptable, since the captain relaxed exactly this absent-path rule for scouts.
  • ⚠️ bin/fm-brief.sh:269 - New scout rule 8 tells the scout "So finish clean: put everything worth keeping in the report, then leave no uncommitted changes behind", but leaving no uncommitted changes does not make the lane teardown-clean. The same brief's Setup still invites "make scratch commits freely" (:241), and rule 1 forbids pushing (:249), so a scout that commits its scratch hits the unpushed path in validate_worktree_teardown_safety: pr_is_merged finds no PR and content_in_default sees the scratch content, so work_is_landed is false and teardown refuses with "Push the branch, land its PR, or get the captain's explicit OK to discard, then --force" (:818) - the first two remedies being things the scout is explicitly forbidden to do. A scout following this brief exactly still dead-ends at --force. There is a reachable clean exit the brief does not mention: reset the task branch back to the lane's starting commit before reporting done, which leaves dirty empty and unpushed empty. Either say that, or say plainly that a lane with scratch commits will need captain-approved --force so the scout records it in the report as rule 8's last sentence already anticipates.
  • ⚠️ bin/fm-teardown.sh:811 - Now that scouts run the full mode-aware checks (spawn's own comment at bin/fm-spawn.sh:1419-1423 says so), a scout in a local-only project with scratch commits reaches the local-only refusal and is told "Merge the branch into local $DEFAULT first (bin/fm-merge-local.sh after the captain approves)". For a scout lane that remedy is actively wrong: it would land laboratory scratch into the project's default branch, which AGENTS.md rule 1 treats as the one thing firstmate must never do outside approved merge paths. It is also newly actionable rather than inert, because scouts now create fm/<id>, so bin/fm-merge-local.sh:44's hardcoded BRANCH="fm/$ID" would resolve and the merge would actually run. Suppress the merge remedy for kind=scout and leave the push-to-remote / captain-approved --force options.
  • ℹ️ bin/fm-teardown.sh:772 - Intent conformance flag for the record, not a new defect: the authoritative criteria still say "Scouts still skip landed-work inspection because their report is the work product", and the code no longer does - the [ &#34;$KIND&#34; != scout ] || return 0 line that followed require_recorded_worktree_ownership is gone, so scouts fall into the dirty, unpushed, unmerged, and landed-work refusals. The captain's round-2 and round-3 rulings ordered this and this commit finished the job on the doc side (AGENTS.md:32, AGENTS.md:328, bin/fm-brief.sh, bin/fm-promote.sh, bin/fm-crew-state.sh, and the script headers all now describe the new behavior). The --intent text is the last artifact still describing the old rule; amend it so the recorded acceptance criteria and the shipped behavior stop disagreeing.

🔧 Fix: give scout lanes a followable teardown-clean exit
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • bash tests/fm-teardown.test.sh — all 52 cases pass, including the 20 new ownership cases (recycled foreign branch, absent path, empty path, resume_branch preference, detached-at-tip allow, detached-wrong-tip refuse, --force override, secondmate carve-out, scout foreign lane, scout scratch refusal, scout local-only remedies, scout reset-to-start allow, never-branched pristine/dirty/foreign, lock-blocked stale-lock recovery)
  • bash tests/fm-gotmp.test.sh — covers the --force adjustments the absent-path refusal forced on the tasktmp cleanup cases
  • bash tests/fm-brief.test.sh — covers the scout brief's new branch-first and hand-the-lane-back rules
  • Manual end-to-end: drove real bin/fm-teardown.sh over a real bare-origin + project-clone + pooled-worktree sandbox for 6 operator scenarios (recycled live lane, absent path, --force override, detached-at-expected-tip, recycled scout lane, scout holding scratch), asserting live-owner HEAD/branch/porcelain invariance and zero treehouse/tmux lifecycle calls via PATH shims — /var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T/no-mistakes-evidence/01KYKR0VNY105VJ4FCF70WQGB8/demo-teardown-ownership.sh
  • Manual end-to-end: drove real bin/fm-spawn.sh with a fake tmux backend and dumped the produced state/&lt;id&gt;.meta to prove resume_branch=fm/&lt;id&gt; is recorded — /var/folders/3b/9kjwhxp50j36ftjmfm3f1lm80000gn/T/no-mistakes-evidence/01KYKR0VNY105VJ4FCF70WQGB8/demo-spawn-records-owner-branch.sh
  • grep -rn &#39;fm-teardown.sh&#39; bin/ skills/ — confirmed no automated caller invokes teardown without --force, so the new fail-closed refusal cannot silently stall a scripted path
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

twaldin added 7 commits July 28, 2026 11:04
Non-force ordinary teardown now refuses when the recorded worktree is missing, empty, detached, or checked out to a branch other than resume_branch/fm/<id>. The explicit operator path for a genuinely gone worktree is --force after discard approval, so teardown no longer silently deletes the task's only durable record when landed-work inspection cannot run.
Rebasing onto the endpoint-cleanup hardening moved the first cleanup
authorization check ahead of everything else in fm-teardown: it is
metadata-only and refuses before fm-guard, any backend command, or any
mutation. The worktree-ownership cases here wrote meta by hand with the
pre-hardening window= shape, so they were refused as malformed tmux
endpoints before reaching the ownership check they exist to prove.

Give those metas the validated shape (session-qualified window= plus the
exact endpoint_task_id= binding) so each case reaches the check it names.
The empty-worktree case is the one whose owner changed: an empty
worktree= is now refused by the earlier endpoint validation, which is the
stricter of the two refusals and preserves task state the same way, so
the assertion follows it there.
@twaldin
twaldin force-pushed the fm/teardown-worktree-verify-t1 branch from 4158922 to a68ba15 Compare July 28, 2026 18:24
…kill ref

merge-base(HEAD, main) already carries the exact `-t "=$session:=$window"`
kill selectors, so build_old_bin's "old" tmux adapter was byte-identical to
the current one and the teardown conformance case could no longer observe the
permissive target it asserts. Resolve the newest first-parent revision whose
bin/backends/tmux.sh still uses a permissive kill-window target and overlay
that adapter in the old fixture, plus a guard test that the resolved ref stays
distinct from HEAD.

Cherry-picked from main (2459f77, kunchenguid#1209).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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