fix(bin): harden supervision and worktree lifecycle safety - #3511
fix(bin): harden supervision and worktree lifecycle safety#3511Kallas95 wants to merge 44 commits into
Conversation
A teardown rerun after a post-return failure (e.g. a refused focus-unsafe pane close) re-executed every worktree-scoped step, so a slot the pool had meanwhile reissued to a newer task was reset under the live tenant, killing its session (observed 2026-08-08). Ownership is now decided from two provable signals before any worktree step: a durable state/<id>.worktree-returned marker touched immediately after this task's own successful return, and the current fleet bindings - another task's meta recording the same worktree path proves reissue even when the marker is lost. When either shows the worktree is no longer this task's, the safety inspection, run-abort, worktree process reap, branch delete, hook removal, and the return itself are skipped with one line, while tasktmp reaping, the pane close retry, and durable-record cleanup continue unchanged. The marker is removed with the rest of the volatile state. Tests: an end-to-end replay of the incident (return ok, close refused, slot reissued, rerun converges without re-returning), a plain first-run return, a rerun with the worktree still bound and un-returned, and the lost-marker reissue caught by fleet bindings alone. All four fail against the unpatched script, so they pin the guard rather than passing vacuously. Verification: bin/fm-lint.sh clean under pinned ShellCheck 0.11.0; bin/fm-doc-audience-check.sh ok; tests/fm-teardown.test.sh 62/62 and tests/fm-teardown-endpoint-safety.test.sh 7/7 pass with one pre-existing, unrelated local-environment failure (herdr-preflight-missing-adapter) disabled: on this host's bash, sourcing a deliberately removed backends/herdr.sh from fm-backend.sh kills the shell before the graceful refusal. That failure reproduces identically at origin/main and is unrelated to this change.
…tenant-safety guard
A declared paused: was ignored by supervision while the agent was still alive on a healthy idle pane, so a legitimate long external wait raised repeated possible-wedge stale alerts. pause_state_class only returned the bounded pause cadence for an exited agent; a live agent fell through to the none (surface) path. Now a declared pause absorbs on the long cadence whether the agent is alive or has exited, while a live agent at a captain-held (non-pause) gate still surfaces once. Endpoint-death detection keeps priority: a paused agent that subsequently dies still re-surfaces on the bounded cadence for a recheck, and an agent that resumes working returns to normal wedge cadence.
Local adoption of alexlopespereira's upstream PR 2373 (herdrdev/herdr#2669, dup of kunchenguid#2387), pending its upstream merge. On herdr 0.8.0 a text-format 'pane read --source recent' against an idle alt-screen agent triggers the wheel-event history harvest and the pane visibly scrolls for seconds on every poll. Fetch --format ansi and strip locally via fm_composer_strip_ansi so the harvest is skipped entirely; pin --format ansi in the capture test so a regression cannot silently reintroduce the scroll. This commit disappears cleanly when the upstream fix lands.
…session The Stop-owned watcher auto-arm proved its right to act with harness ancestry alone: state/.lock had to name one of the hook process's own contiguous harness ancestors. Claude Code serves hook and tool commands from a shared per-user worker pool (claude bg-spare -> claude bg-pty-host -> claude daemon run), and a pool whose owning session has since exited survives with its top process reparented to init. A hook served by such a pool has a contiguous claude ancestry that never reaches the live session, so the check failed for the session's own hook and the script exited 0 silently. Watcher continuity then depended entirely on the synchronous guard, which blocked every turn end instead. Add a second, ancestry-independent membership proof in the lib that owns this decision, and accept either. The new proof is a conjunction: the delivered payload names a session id, the delivering session exported that same session id, the session pid it exported is exactly the pid this home's lock records, and that pid is still a live Claude process. Pid equality is stricter than ancestry membership, so a session that does not hold the lock still cannot claim the home, and separate firstmate homes on one machine stay independent. Nothing is removed: the ancestry proof and every other gate are unchanged. Add FM_AUTOARM_TRACE, an opt-in stderr diagnostic that names the gate ending a run, so an inert hook can be diagnosed under real hook conditions without changing any decision. Tests cover a hook that cannot reach its own session claiming its home, a foreign session refused, an unconfirmed or malformed identity refused, a missing or malformed recorded owner refused, and two homes on one machine staying independent. The opt-in live guard additionally fails, naming the harness and version, if Claude stops delivering the identity the proof depends on or stops matching the recorded owner.
Local adoption of the pending upstream PR 2419 (long pause cadence for a live declared-pause agent), merged with our local main so the diff carries only the cadence fix. The teardown worktree guard and the herdr ansi-scroll fix deployed on local main are preserved. Origin stays traceable to the upstream PR, which remains open and unmerged.
The session lock identifies its owner by process ancestry, but ancestry is not a stable session identity. Claude Code serves one session's hooks and tool calls from more than one worker pool, and a pool whose top process has been reparented to init yields a contiguous harness run that never reaches the session's own lineage. A session that acquired the lock through a pool that could reach it is then refused its own home for the rest of its life: bin/fm-session-start.sh prints "another live firstmate session holds the lock (pid N); operate read-only until resolved" and skips every mutating step. Record the acquiring session's identity beside the lock, and accept it as a second proof of ownership in addition to the ancestry proof, which is left unchanged. A lock carrying no binding keeps exactly its previous behavior, as does any harness that exposes no session identity. The added proof is strictly narrower than it first appears: the recorded session must match, the binding must name exactly the pid the lock records, that pid must still be a live harness, and the claimed session process must be corroborated by the current process's own ancestry. That last conjunct is load-bearing rather than defensive - a session identity travels down every child of a tool call, so without it any process launched from the owner's environment could claim the home, which the existing concurrency guard in tests/fm-session-start.test.sh caught. tests/fm-session-lock-identity.test.sh pins both directions and is red without this change. tests/fm-sessionstart-hook-live-e2e.test.sh gains the vendor half, because whether a harness supplies a usable session identity at all can only be proven against the real harness.
Reconcile the session-identity lock binding with the Stop hook's own identity proof already on this line. Both ownership capabilities survive and neither is a fallback for the other. The Stop auto-arm keeps ancestry plus the identity Claude delivers with the hook payload, which proves the delivering session directly through a strict lock-pid equality. The session-start gates keep ancestry plus the durable binding recorded beside the lock, which is the only proof available to a caller that carries no payload. The shared ancestry-membership helper introduced on the branch stays the single owner of "is this pid inside the current harness ancestry", so the file holds exactly one such walk. The Stop hook proof needs pid equality rather than membership by design and correctly does not use it. The two disjunctions now state their scopes explicitly so the one-owner rule still reads unambiguously, and the lock-ownership docs describe all three proofs and which gate asks for which.
A task worktree is always freshened from origin's default-branch tip, never from the local default branch. On a project whose local default branch carries commits origin does not have - a fork that cannot merge upstream, or local adoptions of unmerged upstream work - a branch cut from that fresh base never contained them, so landing it naively erased them. Keeping origin's tip as the base is deliberate: a branch destined for an upstream PR must never carry local-only commits. The missing piece was the guard at the local landing, so that is where the refusal now lives. - fm-merge-local.sh refuses a landing that would drop commits the local default branch has and the task branch does not, counts and names them, and points at the reconciliation that keeps them (git merge <default> inside the branch). It replaces the previous ancestry refusal, whose "rebase" advice named neither what would disappear nor a reconciliation that preserves it. - --drop-local-commits is the explicit escape hatch for a deliberate drop: never the default, never silent, printing every dropped commit and recording recoverable full SHAs in state/<id>.local-merge-drop before the branch moves. - fm-spawn.sh warns at spawn when the local default branch is ahead of origin, without changing the base, so upstream PRs stay free of local-only commits. Colocated regressions cover the clean landing, the trap, the reconciliation, both escape-hatch paths, and the spawn warning's advisory-only behavior.
…tool Pi writes the session file before it paints the transcript, so run_followup_case captured the pane the moment that file settled and could read the screen while it was still on its start-up state. The count of CAPTAIN_ANSWER_<label> was then zero and the assertion reported a duplicate captain answer that never existed. On the adjacent case this failed 9 times in 24 runs, 4 of 12 under bash 3.2.57 and 5 of 12 under bash 5.3.15: the same rate on both, so it was never interpreter dependent. Every captured pane held zero occurrences, and re-capturing 50ms later always showed exactly one. The case now waits for the follow-up answer, which Pi renders after the captain answer, before counting. A genuine duplicate still fails because the count is taken after that anchor and is never satisfied by it. 24 consecutive runs now pass on both interpreters. The three E2E cases also reported "pi or tmux not found" whichever tool was missing. tmux commonly lives outside a trimmed PATH while pi does not, so a run that skipped all three read as a clean nine-check pass. That is how the export defect already fixed in kunchenguid#2461 was misread as a bash 5 regression: the control run removed /opt/homebrew/bin, which removed tmux, which skipped the very case that was failing. The skip now names the tool that is absent.
… into fm/fm-adoption-lot-0816
…' into fm/fm-adoption-lot-0816
Reintegrate the 17 upstream commits with our 26 local commits. Merge preserves both lines; no rebase, no history rewrite.
# Conflicts: # tests/fm-watch-triage.test.sh
…mont-0826 # Conflicts: # AGENTS.md # bin/fm-claude-stop-autoarm.sh # bin/fm-merge-local.sh # docs/verification/supervision.md # docs/watcher-continuity.md
# Conflicts: # AGENTS.md # bin/fm-claude-stop-autoarm.sh # bin/fm-spawn.sh # docs/watcher-continuity.md # tests/fm-spawn-pool-base-freshen.test.sh
An away-mode episode ended with the epoch ledger on its own terminal entry, and from the captain's return every turn boundary blocked while the Stop-owned auto-arm claimed nothing: 172 consecutive blocked stops in five hours, the model re-arming the watcher by hand 175 times, and the documented one-time attended fail-open never opening. Two independent defects, each reproduced in a disposable lab and pinned by a counterfactual that changes one condition at a time. The hook can be permanently inert on a home that is genuinely its own. bin/fm-lock.sh records the OUTERMOST pid of the contiguous harness run that acquired the lock, while the Stop payload proof requires that pid to BE the pid the harness exports for the event. For one session those can be two different processes, and ancestry cannot reach the recorded owner either, so both proofs fail and the hook exits before it can claim anything - the measured "live session N owns this home and neither identity proof applies". The lock's own durable binding already records which session acquired it, so a hook may now prove membership through it, corroborated by the payload naming the same session the environment does. A foreign session still carries its own id and stays out, and an unbound lock keeps the previous behavior exactly. The guard could not bound that at all. Its block budget is keyed on the auto-arm's epoch identity, which is a real bound while the auto-arm runs and no bound once it stops writing: the epoch never changes and the count froze at 1. The one attended fail-open additionally required a verified failure episode, evidence only an auto-arm that runs can produce. The guard now also counts the blocked stops themselves and opens that same one alarm when a run of them passes FM_CLAUDE_TURNEND_STALL_BUDGET (default 8), naming the automatic path that never ran. A stop it lets through ends the run, away mode never opens it, and it deliberately does not write the failure-alarm marker, which would suppress the exit-2 continuations of an auto-arm that comes back. Not relaxed: the first blocked stop still refuses, and the guard keeps blocking after its one alarm. Other primary harnesses are unaffected by inspection: every line changed here runs after the --claude gate, so codex, grok, opencode, pi, and cursor (which calls back with --cursor) block and return before reaching it. Cursor's own adapter already bounds unproductive nags rather than epochs. Also fixes a pre-existing failure of the opt-in live guard, which counted wake drains as Stop-owned cycles after the tracked SessionStart hook began running a digest of its own.
…third ownership proof
Pi ships with no web tool and no MCP, so a Pi scout could not establish an external fact and had to escalate it. That mattered most exactly when it hurt: when the Claude window is tight, scouts fall back to Pi. bin/fm-ollama-websearch.sh holds the Ollama key and answers queries; bin/fm-pi-websearch-ext.ts registers a web_search tool that does nothing but call it. The key never becomes readable through the proxy: not in output, not in a child's argv or environment, never written to disk, and only ever sent to ollama.com. It reaches curl through a configuration on stdin, which lives in a pipe rather than in an argument vector or a file. The proxy is not a privilege boundary - it runs as the same user as its caller - and its header says so. Exposure is limited to Pi scouts. An implementation worker's answers are in its repository, and Claude workers already have web search. Key presence is the only switch; a home without one launches scouts exactly as before. Searches spend the same metered quota this home already reports, itemized there as "web search" (measured, see docs/verification/ollama-websearch.md), so the tool caps a scout at 3 searches per turn and 20 per session. The session cap is the one that bounds a loop: Pi starts a new turn after every tool call. Results are reshaped rather than relayed. Three raw results measured 101 KB, which would spend a scout's context on one call; each result's content is truncated and marked truncated so a cut-off page is never quoted as whole. Tests: a portable contract test drives the real script with real curl against a local endpoint and asserts the key's absence from every surface an agent can inspect while confirming the one channel that carries it; a spawn test pins the scout-only rule; an opt-in live test proves the installed Pi actually registers and executes the tool.
The previous line cited a 2-result measurement while describing the 3-result default, so the number did not support the claim it was attached to. Replace it with the same query at true defaults, with the commands that produced it.
…stall progression
…urrent checkout work
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Reviews (2): Last reviewed commit: "no-mistakes(ci): Fixed Greptile’s valid ..." | Re-trigger Greptile |
…-swap now removes a newly created landing rescue ref while preserving any rescue from an earlier completed drop. Updated the behavioral regression test to verify cleanup and audit recording. Verified with `bash tests/fm-merge-local.test.sh`, `bin/fm-lint.sh`, `bin/fm-test-run.sh --check-coverage`, and `git diff --check`. The CI and Require no-mistakes runs were `action_required` with zero jobs, indicating external workflow approval/attestation rather than a code failure
|
Speaking as Kun's firstmate: first-time fork CI approved after a full-thread + full-diff + security review (workflow-zero: no Inspected HEAD Attestation: MATCH. Body Contract-class: new-default. Dominant always-on expansions vs main's specified defaults:
VISION (per rule):
Security: clean (no workflow/action pins, no credentials, fail-closed identity proofs, conservative session_id parse). Not otherwise ready for a captain decision yet: fork CI/NM just approved and still queued on this HEAD. Do not merge (new-default). Do not Firstmate-flag while CI is pending. Overlap note only: open |
Intent
Audit and reconcile this locally diverged Firstmate line with Kun Chen's current upstream setup without losing still-needed local safety behavior. Merge current origin/main normally on fm/srv-firstmate-upstream-integration, preserve the proved teardown rerun tenant safety, Claude session identity and away-return supervision, local-only landing rescue refs, bounded declared-pause cadence, and synchronized Pi Calm coverage, while accepting upstream's current architecture. Retire only Firstmate's obsolete Ollama-backed scout web_search extension/proxy and its tests/docs because it conflicts with the globally pinned pi-web-access package. Do not remove Ollama model access: separately selectable Ollama Cloud and local inference are staged and tested outside the tracked tree, with OpenAI Codex remaining the default fleet route. Stage Kun's rose-pine-moon, terminal-title, settings, and global Calm resources with a narrow duplicate guard, but do not activate the real Pi authored configuration before separate captain approval; do not touch auth, trust, sessions, cache, or credentials. Preserve xAI authentication and optional xai/grok-4.5 and xai/grok-4.6 visibility without making either default or buying/routing anything. Update only safe stable consumed tools with rollback copies; no-mistakes is now stable 1.60.2 using Pi openai-codex/gpt-5.6-sol, while Herdr remains deferred because this task itself is Herdr-backed and must not drive Herdr lifecycle. Validate, push, and open a green PR, but do not merge.
What Changed
Risk Assessment
✅ Low: The safety-sensitive changes are narrowly coordinated, preserve the stated invariants, and no material source-verifiable defect remains reachable.
Testing
Inspected the integration diff, exercised targeted teardown/reissue safety, Claude identity and bounded supervision recovery, local landing rescue/concurrency behavior, Pi Calm, pause cadence, pool refresh, and deferred Herdr behavior; all runnable checks passed. Opt-in live harness checks skipped without their environment flags, while a reviewer-visible CLI transcript directly demonstrated failed marker publication preserving progression and the next Stop emitting the bounded attended alarm. No visual artifact was applicable because the tested surface is CLI/hook output.
Evidence: Claude Stop marker-failure and bounded attended-alarm transcript
Source: Claude Stop marker-failure and bounded attended-alarm transcript
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
.agents/skills/afk/SKILL.md- branch carries 34 commit(s) that exist on your local main branch but were never pushed to origin/main; rebasing would bundle this unrelated work (198 file(s)) into the PR:Push main to origin, or rebase your branch onto origin/main, before gating.
🔧 **Review** - 2 issues found → auto-fixed (6) ✅
bin/fm-teardown.sh:2852- The required “preserve the proved teardown rerun tenant safety” invariant still has a reachable failure path. Aftertreehouse returnsucceeds, marker publication is best-effort; iftouchfails, a new spawn can acquire/reset the slot before publishing its.metarecord, and a concurrent teardown rerun sees neither marker nor competing binding, concludes it still owns the worktree, and reaps/resets the new tenant. A durable fix requires coordinating the return/reissue boundary or adding recoverable transition state; that remedy extends the protocol and needs authorization.bin/fm-turnend-guard.sh:81- The new default stall budget is 8, butfail_open_episodeonly opens whenSTALLED == STALL_BUDGET + 1. Thus the alarm is attempted on the ninth blocked Stop, after the documented Claude hard override at eight consecutive blocks may already bypass the hook. The default path can therefore still end without the promised attended supervision alarm. Choose a threshold that fires before the hard limit; this changes deliberate turn-boundary behavior and needs user confirmation.🔧 Fix: Coordinate teardown reissue and preempt Claude stall override
3 errors still open:
bin/fm-teardown.sh:2643- The new transition lock is acquired after teardown already holds this task's meta lock (line 300), opposite fresh spawn's order: spawn holds.task-set.lockbefore waiting for the task meta lock (fm-spawn.sh:1038,2068). A concurrent fresh spawn using an existing ID can therefore hold the task-set lock waiting on teardown's meta lock while teardown waits indefinitely here. Establish one lock order at the earliest shared lifecycle boundary, or refuse an existing-ID spawn before it waits on the meta lock.bin/fm-turnend-guard.sh:85- The required alarm point must be “strictly before Claude hard-bypasses after eight consecutive blocks,” but the changed validation accepts any positiveFM_CLAUDE_TURNEND_STALL_BUDGET. WithFM_CLAUDE_TURNEND_STALL_BUDGET=8, eight Stops are blocked and Claude may bypass beforefail_open_episodecan alarm on attempt nine. Authorize clamping/rejecting values above 7 so the invariant holds on every supported configuration path.bin/fm-lock.sh:128- The durable session binding is published only after writing a newly acquired lock. If a live session already owns.lockwhen this version is deployed—or initial best-effort publication failed—the unchangedold == mefast path exits at line 67 without retrying publication. A later reparented hook whose exported PID differs from the outermost lock PID then fails all three proofs and remains inert. Backfill a missing/invalid binding when the current ancestry proves the existing lock is ours, while preserving an already-valid binding.🔧 Fix: Fix lifecycle lock ordering, stall clamps, and binding upgrades
2 errors still open:
bin/fm-lock.sh:73- The new fast-path backfill runs outside.lock.acquire. If the old harness exits afterfm_session_lock_owned_by_selfsucceeds, a new session can acquire the claim lock, replace.lock, and publish its binding before this call reachesbackfill_session_binding; the old call then removes that new binding and may publish the obsolete PID. Reuse the existing claim-lock boundary, then reread and reprove ownership before backfilling.bin/fm-turnend-guard.sh:517- When the stall threshold is reached butterminal_fail_openobserves a concurrently opened auto-arm claim, it returns 2 and this branch allows the Stop without clearingSTALLED. The persisted value remains 8; if recovery later disappears, subsequent blocks advance it to 9+, and the equality check at line 357 can never raise the attended alarm again. Clear the stalled counter before every recovery-owned exit-0 path, including terminal status 2.🔧 Fix: Serialize binding backfills and reset recovered stall progression
2 errors still open:
bin/fm-turnend-guard.sh:517- The successful stalled fail-open exits without clearingSTALLED. After seven blocked Stops, attempt eight alarms withstalled=8; if supervision remains unavailable, the next Stop increments it to 9, the equality check can never alarm again, and the guard blocks until Claude's hard override. This contradicts the documented invariant that every allowed Stop ends the series. Clear the stall counter on this successful exit before emitting/returning the alarm, and cover the post-alarm next-episode sequence.bin/fm-merge-local.sh:162- The destructive landing pinsbefore, but later moves the default branch with an unconditionalreset --hard. A concurrent commit or ref update after rescue-ref publication is therefore discarded without being reachable from that rescue ref, violating the promised local-only landing safety. Make branch movement conditional on the default ref still equalingbefore, while synchronizing the checked-out worktree without overwriting a newer tip.🔧 Fix: Reset stall episodes and preserve concurrent local commits
3 issues (2 errors, 1 warning) still open:
bin/fm-turnend-guard.sh:504- Afterterminal_fail_openhas committed the attended decision and, for failure episodes, created the one-shot alarm marker,budget_clear_stalltries to reacquire the budget lock. A concurrent guard can hold that lock, causing this invocation to exit 2 before emitting the alarm; failure episodes can never retry because the marker already exists, while stalled episodes advance past the equality threshold. Clear the counter whileterminal_fail_openstill holds the budget lock, before committing or returning the successful decision.bin/fm-merge-local.sh:165- Both concurrency paths synchronize the checkout withread-tree --reset -u, which unconditionally overwrites working-tree and index changes made after the initial cleanliness check. For example, after another process advancesmainand begins its next uncommitted edit, the CAS-failure branch preserves the new ref but silently deletes that edit. Use a synchronization operation that refuses local changes and does not overwrite a concurrent checkout owner.bin/fm-merge-local.sh:155- The durable record writesdropped=<sha>and the script printsDROPPINGbefore the compare-and-swap at line 163. Ifmainadvances concurrently, the landing is refused and nothing is dropped, but the audit record permanently claims otherwise. Record the operation as pending and append an explicit completion only after the branch move succeeds, or otherwise distinguish failed attempts from completed drops.🔧 Fix: Commit stall resets atomically and preserve concurrent checkout work
1 error still open:
bin/fm-turnend-guard.sh:433- The stall counter is cleared before the failure-alarm marker is committed. If marker creation fails—for example because FAILURE_ALARM is a dangling symlink—this invocation returns to block_stop with stalled=0. Every subsequent verified-failure attempt increments to 1, clears back to 0, fails marker creation, and blocks, so the stall alarm never becomes reachable before Claude's hard override. Preserve or restore the prior stalled value whenever marker publication fails, while still committing the successful reset under the held budget lock.🔧 Fix: Preserve stall progression when alarm publication fails
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
tests/fm-turnend-guard.test.shtests/fm-claude-stop-autoarm.test.shtests/fm-session-lock-identity.test.shtests/fm-teardown.test.shtests/fm-merge-local.test.shtests/fm-calm-pi-extension.test.shtests/fm-watch-triage.test.shtests/fm-spawn-pool-base-freshen.test.shtests/fm-backend-herdr.test.shtests/fm-sessionstart-hook-live-e2e.test.shandtests/fm-claude-stop-autoarm-live-e2e.test.shwere attempted and skipped by their explicit opt-in guardsHermetic end-user guard scenario invokingfm-turnend-guard.sh --claudetwice with forced alarm-marker publication failure, capturing exit behavior, alarm output, and persisted stall stategit status --shortconfirmed testing left the worktree clean✅ **Document** - passed
✅ No issues found.
🔧 **Lint** - 1 issue found → auto-fixed ✅
🔧 Fix: Use explicit race fixture paths across subshells
✅ Re-checked - no issues remain.
✅ **Push** - passed
✅ No issues found.