Fork-internal verification: prove Orca worktree ownership before destructive teardown (upstream PR 4838)

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.
@@ -988,7 +991,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 +2275,26 @@ require_owned_worktree_slot_record() {  # <task-id> <worktree>
   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. 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
+  fi
   slot=$(teardown_live_slot_path) || return 0
   require_owned_worktree_slot_record "$ID" "$slot" || rc=$?
   case "$rc" in
@@ -3264,14 +3278,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:-<missing>}." >&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 +3399,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 +3418,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

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.

tests/fm-teardown-endpoint-safety.test.sh

@@ -53,6 +53,27 @@ claim_pool_slot() {  # <case> <task-id> [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() {  # <case>
+  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() {  # <case> <id>
   local dir=$1 id=$2
   FM_HOME="$dir/home" FM_ROOT_OVERRIDE="$ROOT" \
@@ -1324,6 +1345,99 @@ 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() {  # <case> <id> <worker-pid> <description>
+  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
+  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"
+
+  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:$PATH" "$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
+  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"
+
+  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:$PATH" "$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 +1494,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