From cf840b5d25832b2449bb4c382acccdeef73da35c Mon Sep 17 00:00:00 2001 From: MLA82 <212341412+MLA82@users.noreply.github.com> Date: Fri, 18 Sep 2026 15:08:45 +0200 Subject: [PATCH 1/3] fix(bin): prove Orca worktree ownership before reaping, not after An Orca scout teardown, or any Orca teardown run with --force, skipped the early Orca path-match check and only re-verified it after reap_task_worktree_processes had already run against the recorded worktree path, so a stale or reassigned Orca worktree id could have its processes signalled before ownership was ever proven. Fold the Orca proof into require_owned_task_worktree_slot, the same early, single-owner ownership determination the treehouse pool-slot claim already uses, so it runs once, well before Fix 1/Fix 2, for every kind and --force alike, instead of being re-checked ad hoc at each later call site. --- bin/fm-teardown.sh | 42 +++++--- tests/fm-teardown-endpoint-safety.test.sh | 118 ++++++++++++++++++++++ 2 files changed, 144 insertions(+), 16 deletions(-) diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index ec792392b98..e4d1fab6ba4 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -988,7 +988,6 @@ if [ -z "$BUSY_GEN" ]; then BUSY_GEN=$(cat "$STATE/$ID.busy-gen" 2>/dev/null || true) fi ORCA_WORKTREE_ID=$(fm_meta_get "$META" orca_worktree_id) -ORCA_PATH_MATCH_VERIFIED=0 CLEANUP_RECOVERY=$TEARDOWN_CLEANUP_RECOVERY KIND=$TEARDOWN_META_KIND @@ -2273,14 +2272,25 @@ require_owned_worktree_slot_record() { # return 1 } -# The one ownership determination for this task's recorded slot. Every later -# step that would read or touch $WT consults teardown_owns_worktree, so a -# reassigned slot is skipped consistently rather than by each step's own guess. +# The one ownership determination for this task's recorded slot, treehouse +# pool or Orca alike. Every later step that would read or touch $WT consults +# teardown_owns_worktree, so a reassigned slot is skipped consistently rather +# than by each step's own guess, and Fix 1/Fix 2 below can never run ahead of +# either backend's own ownership proof - this runs long before them, not +# beside them at each individual destructive call site. TEARDOWN_SLOT_REASSIGNED=0 TEARDOWN_SLOT_REASSIGNED_TO= TEARDOWN_SLOT_REASSIGNED_HOME= require_owned_task_worktree_slot() { local slot rc=0 + # Orca is not a pool slot; it owns its own worktree and proves that through + # require_orca_worktree_path_match_if_present instead of the treehouse claim + # below. That function itself refuses on any unresolved proof (missing CLI, + # mismatched path), never falling through to "nothing to check". + if [ "$KIND" != secondmate ] && [ "$BACKEND" = orca ]; then + require_orca_worktree_path_match_if_present "$ORCA_WORKTREE_ID" "$WT" || return 1 + return 0 + fi slot=$(teardown_live_slot_path) || return 0 require_owned_worktree_slot_record "$ID" "$slot" || rc=$? case "$rc" in @@ -3264,14 +3274,16 @@ if [ -n "$X_REQUEST" ]; then echo "warning: task $ID still carries an unreconciled Relay request link ($X_REQUEST) on its task record." >&2 fi +# require_owned_task_worktree_slot already proved this task's own recorded +# worktree id resolves to $WT (or tolerated its absence) before any refusal +# above ran. A ship task additionally requires the worktree to still be +# inspectable, since the landed/dirty-work checks above need it. if [ "$BACKEND" = orca ] && [ "$KIND" != scout ] && [ "$KIND" != secondmate ] && [ "$FORCE" != "--force" ]; then if ! inspectable_git_worktree "$WT"; then echo "REFUSED: Orca ship task $ID has no inspectable git worktree at ${WT:-}." >&2 echo "Cannot verify dirty or unlanded work; restore the worktree path or get explicit OK to discard, then --force." >&2 exit 1 fi - require_orca_worktree_path_match "$ORCA_WORKTREE_ID" "$WT" || exit 1 - ORCA_PATH_MATCH_VERIFIED=1 fi if teardown_owns_worktree && [ -d "$WT" ] && [ "$FORCE" != "--force" ]; then @@ -3383,12 +3395,13 @@ else fi # Every landed/discard-work refusal above has now passed (or --force skipped -# them). Fix 1 and Fix 2 (see script header) run here, unconditionally on -# --force, and before ANY destructive step below - a still-parked run or a -# leaked process can own live work in this exact worktree. Not for -# kind=secondmate: a secondmate home's own runtime lifecycle is owned by the -# dedicated process-event and firstmate-home removal machinery further below, -# not by task-worktree cleanup. +# them), and require_owned_task_worktree_slot has already proved this task +# still owns $WT - treehouse pool claim or Orca path match alike. Fix 1 and +# Fix 2 (see script header) run here, unconditionally on --force, and before +# ANY destructive step below - a still-parked run or a leaked process can own +# live work in this exact worktree. Not for kind=secondmate: a secondmate +# home's own runtime lifecycle is owned by the dedicated process-event and +# firstmate-home removal machinery further below, not by task-worktree cleanup. if [ "$KIND" != secondmate ] && teardown_owns_worktree; then conclude_task_no_mistakes_run "$WT" reap_task_worktree_processes worktree "$WT" "$TASK_TMP" @@ -3401,11 +3414,8 @@ fi "$SCRIPT_DIR/fm-remote-job-reap-orphans.sh" >&2 || true # Best-effort: drop the local task branch so the shared repo does not accumulate refs. +# require_owned_task_worktree_slot already verified the Orca path match above. if [ "$BACKEND" = orca ] && [ "$KIND" != secondmate ]; then - if [ "$ORCA_PATH_MATCH_VERIFIED" != 1 ]; then - require_orca_worktree_path_match_if_present "$ORCA_WORKTREE_ID" "$WT" || exit 1 - ORCA_PATH_MATCH_VERIFIED=1 - fi if [ -d "$WT" ]; then branch=$(git -C "$WT" rev-parse --abbrev-ref HEAD 2>/dev/null || echo HEAD) if [ "$branch" != "HEAD" ]; then diff --git a/tests/fm-teardown-endpoint-safety.test.sh b/tests/fm-teardown-endpoint-safety.test.sh index 4002cf4df3e..bd66a3fa16f 100755 --- a/tests/fm-teardown-endpoint-safety.test.sh +++ b/tests/fm-teardown-endpoint-safety.test.sh @@ -53,6 +53,27 @@ claim_pool_slot() { # [home] printf 'task=%s\nhome=%s\n' "$id" "$home" > "$dir/pool/1/.fm-slot-owner" } +# A fake `orca` CLI whose `worktree show` answers with whatever path +# FM_TEST_ORCA_WORKTREE_PATH names at run time, so one case can prove Orca +# itself resolves the recorded worktree id to a DIFFERENT, still-real path - +# the stale/reassigned-worktree shape - without a real Orca runtime. +install_fake_orca() { # + local dir=$1 + cat > "$dir/fakebin/orca" <<'SH' +#!/usr/bin/env bash +printf 'orca' >> "${FM_RUNTIME_LOG:?}" +printf ' <%s>' "$@" >> "${FM_RUNTIME_LOG:?}" +printf '\n' >> "${FM_RUNTIME_LOG:?}" +if [ "$1" = worktree ] && [ "$2" = show ]; then + printf '{"ok":true,"result":{"worktree":{"path":"%s"}}}\n' "${FM_TEST_ORCA_WORKTREE_PATH:?}" + exit 0 +fi +printf '{"ok":true,"result":{}}\n' +exit 0 +SH + chmod +x "$dir/fakebin/orca" +} + run_case() { # local dir=$1 id=$2 FM_HOME="$dir/home" FM_ROOT_OVERRIDE="$ROOT" \ @@ -1324,6 +1345,101 @@ test_orca_close_failure_refuses_even_under_force() { pass "fm-teardown: an Orca close its missing CLI never attempted refuses even under --force, keeping the record naming the terminal" } +# Orca proves worktree ownership through require_orca_worktree_path_match, +# never through the treehouse pool-slot claim (Orca worktrees are not pool +# slots, so that claim silently has nothing to check for them). A stale +# recorded worktree - Orca reassigned the id to a different real path - must +# refuse before Fix 1/Fix 2 (no-mistakes conclude, process reap) or the hook +# removal below ever touch the recorded path, for every kind and --force +# alike; a scout task and a forced ship task are exactly the two shapes that +# used to skip the early Orca check and only re-verify it after the reap. +assert_orca_stale_worktree_refuses_before_touching_it() { # + local dir=$1 id=$2 worker=$3 description=$4 + [ -s "$dir/stderr" ] || fail "$description: teardown produced no refusal output" + assert_grep "not inspected worktree" "$dir/stderr" \ + "$description: the refusal should name the Orca worktree path mismatch" + kill -0 "$worker" 2>/dev/null \ + || fail "$description: teardown reaped a live process before proving the stale Orca worktree was still this task's" + assert_present "$dir/worktree/.claude/settings.local.json" \ + "$description: teardown removed the Claude hook file before proving Orca worktree ownership" + assert_no_grep "reaping leaked" "$dir/stderr" \ + "$description: teardown reaped worktree processes before the Orca path-match proof ran" + assert_no_grep "teardown $id complete" "$dir/stdout" \ + "$description: teardown reported a completed cleanup despite the stale worktree" +} + +test_orca_scout_stale_worktree_refuses_before_reaping() { + local dir id=orca-scout-stale worker rc orca_free + dir=$(make_case orca-scout-stale) + install_fake_orca "$dir" + mkdir -p "$dir/worktree/.claude" "$dir/elsewhere-worktree" + printf '{}' > "$dir/worktree/.claude/settings.local.json" + orca_free=$(fm_test_base_path_sans "$PATH" orca) + + fm_write_meta "$dir/home/state/$id.meta" \ + "window=fm-$id" "endpoint_task_id=$id" "terminal=term-1" \ + "worktree=$dir/worktree" "project=$dir/project" \ + "backend=orca" "orca_worktree_id=worktree-1::/orca/worktree-1" "kind=scout" + + # Staged in this shell, not a command substitution: see the reassigned pool + # slot fixture above for why this must not be a $(...) subshell child. + ( cd "$dir/worktree" && exec sleep 30 ) & + worker=$! + + set +e + env -u TMUX -u TMUX_PANE \ + FM_HOME="$dir/home" FM_ROOT_OVERRIDE="$ROOT" FM_RUNTIME_LOG="$dir/runtime.log" \ + FM_TEST_ORCA_WORKTREE_PATH="$dir/elsewhere-worktree" \ + PATH="$dir/fakebin:$orca_free" "$TEARDOWN" "$id" \ + > "$dir/stdout" 2> "$dir/stderr" + rc=$? + set -e + + [ "$rc" -ne 0 ] \ + || fail "an Orca scout task whose recorded worktree no longer matches Orca's own record completed cleanup" + assert_orca_stale_worktree_refuses_before_touching_it "$dir" "$id" "$worker" \ + "orca scout task with a stale recorded worktree" + + kill "$worker" 2>/dev/null || true + wait "$worker" 2>/dev/null || true + pass "fm-teardown: an Orca scout task's stale recorded worktree refuses before any process is reaped or hook removed" +} + +test_orca_forced_ship_stale_worktree_refuses_before_reaping() { + local dir id=orca-ship-stale-forced worker rc orca_free + dir=$(make_case orca-ship-stale-forced) + install_fake_orca "$dir" + mkdir -p "$dir/worktree/.claude" "$dir/elsewhere-worktree" + printf '{}' > "$dir/worktree/.claude/settings.local.json" + orca_free=$(fm_test_base_path_sans "$PATH" orca) + + fm_write_meta "$dir/home/state/$id.meta" \ + "window=fm-$id" "endpoint_task_id=$id" "terminal=term-2" \ + "worktree=$dir/worktree" "project=$dir/project" \ + "backend=orca" "orca_worktree_id=worktree-2::/orca/worktree-2" "kind=ship" "mode=no-mistakes" + + ( cd "$dir/worktree" && exec sleep 30 ) & + worker=$! + + set +e + env -u TMUX -u TMUX_PANE \ + FM_HOME="$dir/home" FM_ROOT_OVERRIDE="$ROOT" FM_RUNTIME_LOG="$dir/runtime.log" \ + FM_TEST_ORCA_WORKTREE_PATH="$dir/elsewhere-worktree" \ + PATH="$dir/fakebin:$orca_free" "$TEARDOWN" "$id" --force \ + > "$dir/stdout" 2> "$dir/stderr" + rc=$? + set -e + + [ "$rc" -ne 0 ] \ + || fail "a forced Orca ship teardown continued past a stale recorded worktree that no longer matches Orca's own record" + assert_orca_stale_worktree_refuses_before_touching_it "$dir" "$id" "$worker" \ + "forced orca ship task with a stale recorded worktree" + + kill "$worker" 2>/dev/null || true + wait "$worker" 2>/dev/null || true + pass "fm-teardown: a forced Orca ship teardown's stale recorded worktree still refuses before any process is reaped or hook removed" +} + test_already_gone_endpoint_still_completes_without_a_refusal() { local dir socket session='already gone' id=gone-task [ -n "$REAL_TMUX" ] || { echo "skip - tmux not installed"; return 0; } @@ -1380,6 +1496,8 @@ test_forced_teardown_continues_past_a_close_it_could_not_make test_unreadable_close_read_refuses_while_a_definitive_absence_completes test_forced_secondmate_child_close_failure_still_refuses test_orca_close_failure_refuses_even_under_force +test_orca_scout_stale_worktree_refuses_before_reaping +test_orca_forced_ship_stale_worktree_refuses_before_reaping test_already_gone_endpoint_still_completes_without_a_refusal test_bare_relative_origin_shares_project_lock_with_clone test_reused_pool_slot_refuses_before_touching_the_other_task From 7539d41ef8923fffeee73406955d09c3f79623b5 Mon Sep 17 00:00:00 2001 From: MLA82 <212341412+MLA82@users.noreply.github.com> Date: Fri, 18 Sep 2026 15:20:32 +0200 Subject: [PATCH 2/3] no-mistakes(review): correct Orca guard comment, drop redundant test PATH sanitization --- bin/fm-teardown.sh | 5 +++-- tests/fm-teardown-endpoint-safety.test.sh | 10 ++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index e4d1fab6ba4..4759970fb8c 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -2285,8 +2285,9 @@ require_owned_task_worktree_slot() { local slot rc=0 # Orca is not a pool slot; it owns its own worktree and proves that through # require_orca_worktree_path_match_if_present instead of the treehouse claim - # below. That function itself refuses on any unresolved proof (missing CLI, - # mismatched path), never falling through to "nothing to check". + # below. Whenever $WT still exists, that function refuses on any unresolved + # proof (missing CLI, mismatched path); when $WT is absent there is nothing + # left to protect, so it returns without calling Orca at all. if [ "$KIND" != secondmate ] && [ "$BACKEND" = orca ]; then require_orca_worktree_path_match_if_present "$ORCA_WORKTREE_ID" "$WT" || return 1 return 0 diff --git a/tests/fm-teardown-endpoint-safety.test.sh b/tests/fm-teardown-endpoint-safety.test.sh index bd66a3fa16f..0dba89a6e42 100755 --- a/tests/fm-teardown-endpoint-safety.test.sh +++ b/tests/fm-teardown-endpoint-safety.test.sh @@ -1369,12 +1369,11 @@ assert_orca_stale_worktree_refuses_before_touching_it() { # "$dir/worktree/.claude/settings.local.json" - orca_free=$(fm_test_base_path_sans "$PATH" orca) fm_write_meta "$dir/home/state/$id.meta" \ "window=fm-$id" "endpoint_task_id=$id" "terminal=term-1" \ @@ -1390,7 +1389,7 @@ test_orca_scout_stale_worktree_refuses_before_reaping() { env -u TMUX -u TMUX_PANE \ FM_HOME="$dir/home" FM_ROOT_OVERRIDE="$ROOT" FM_RUNTIME_LOG="$dir/runtime.log" \ FM_TEST_ORCA_WORKTREE_PATH="$dir/elsewhere-worktree" \ - PATH="$dir/fakebin:$orca_free" "$TEARDOWN" "$id" \ + PATH="$dir/fakebin:$PATH" "$TEARDOWN" "$id" \ > "$dir/stdout" 2> "$dir/stderr" rc=$? set -e @@ -1406,12 +1405,11 @@ test_orca_scout_stale_worktree_refuses_before_reaping() { } test_orca_forced_ship_stale_worktree_refuses_before_reaping() { - local dir id=orca-ship-stale-forced worker rc orca_free + local dir id=orca-ship-stale-forced worker rc dir=$(make_case orca-ship-stale-forced) install_fake_orca "$dir" mkdir -p "$dir/worktree/.claude" "$dir/elsewhere-worktree" printf '{}' > "$dir/worktree/.claude/settings.local.json" - orca_free=$(fm_test_base_path_sans "$PATH" orca) fm_write_meta "$dir/home/state/$id.meta" \ "window=fm-$id" "endpoint_task_id=$id" "terminal=term-2" \ @@ -1425,7 +1423,7 @@ test_orca_forced_ship_stale_worktree_refuses_before_reaping() { env -u TMUX -u TMUX_PANE \ FM_HOME="$dir/home" FM_ROOT_OVERRIDE="$ROOT" FM_RUNTIME_LOG="$dir/runtime.log" \ FM_TEST_ORCA_WORKTREE_PATH="$dir/elsewhere-worktree" \ - PATH="$dir/fakebin:$orca_free" "$TEARDOWN" "$id" --force \ + PATH="$dir/fakebin:$PATH" "$TEARDOWN" "$id" --force \ > "$dir/stdout" 2> "$dir/stderr" rc=$? set -e From e2eeed0b3008bc1a9135bbdabd39dbaa9e42d7c5 Mon Sep 17 00:00:00 2001 From: MLA82 <212341412+MLA82@users.noreply.github.com> Date: Fri, 18 Sep 2026 15:36:27 +0200 Subject: [PATCH 3/3] no-mistakes(document): document Orca path-match proof running before all destructive steps --- bin/fm-teardown.sh | 7 +++++-- docs/orca-backend.md | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index 4759970fb8c..28c8e95b13c 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -131,8 +131,11 @@ # These refusals are not relaxed by --force: --force authorizes discarding THIS # task's unlanded work, never another task's live work. Nothing of this task's # own is removed by a refusal; reconcile whichever record is wrong and re-run. -# Orca is not a pool slot and proves its path through -# require_orca_worktree_path_match instead. +# Orca is not a pool slot: it proves its recorded worktree through +# require_orca_worktree_path_match instead, at the same single ownership +# determination (require_owned_task_worktree_slot) and therefore before every +# destructive step, --force included. An Orca worktree path that is already +# gone has nothing left to protect and needs no proof. # Orca tasks use the same safety checks, then close the recorded terminal and # remove the recorded worktree through `orca worktree rm`; teardown never guesses # an Orca target from ambient CLI state. diff --git a/docs/orca-backend.md b/docs/orca-backend.md index 000782a3536..7f795977a0c 100644 --- a/docs/orca-backend.md +++ b/docs/orca-backend.md @@ -60,8 +60,10 @@ Grok alone retains its isolated rendered-tail fallback. Cleanup keeps all shared Firstmate safety checks. A scout still requires its report and completed decision inventory. A ship still refuses dirty or unlanded work. -Before release, cleanup resolves the recorded Orca worktree id and verifies its path matches the recorded worktree path. +Cleanup resolves the recorded Orca worktree id and verifies its path matches the recorded worktree path once, before any destructive step - before the task's no-mistakes run is concluded, before leaked processes under the worktree are reaped, and before the branch delete, terminal close, and worktree release. +`--force` does not lift that proof: it authorizes discarding this task's own unlanded work, never acting on a worktree that may not be this task's. A missing, unreadable, or mismatched identity preserves metadata and stops rather than deleting anything. +A recorded worktree path that no longer exists has nothing left to protect, so cleanup proceeds without asking Orca. After those checks, Firstmate closes the exact terminal and releases the exact worktree with Orca's worktree command. It never raw-deletes an Orca worktree. A close the CLI never attempted, because `orca` is not on the path, stops cleanup with the metadata intact even under `--force`: removing those records would leave nothing on disk naming a terminal that may still be live.