fix(bin): make one session-lock verdict govern arming and fleet mutation - #18
Open
BohnBawerick wants to merge 24 commits into
Open
fix(bin): make one session-lock verdict govern arming and fleet mutation#18BohnBawerick wants to merge 24 commits into
BohnBawerick wants to merge 24 commits into
Conversation
…sk record Reads a project's registered "+hardened" annotation and carries it to the worker's instructions and the task's durable record, so the quality loop that bin/fm-quality.sh will drive has a posture and a fixed base commit to work from. That script is not part of this change; it is referenced by name only. - bin/fm-project-mode.sh: --quality prints one word, standard or hardened. The two-word stdout its three callers parse is untouched, so it gets its own output path. The bracket grammar is now position-tolerant: a "+"-prefixed token is a flag and never a mode, so "[+hardened local-only]" resolves the mode behind it instead of reading the flag as an unknown mode. Unrecognized flags are still ignored rather than refused. - bin/fm-brief.sh: --quality standard|hardened, defaulting to standard and refused on scout, dreamer, and secondmate scaffolds. A hardened brief records the sibling "Quality contract: quality=hardened" line and one short quality gate section; a standard brief records neither and stays byte-identical to the pre-quality scaffold. - bin/fm-spawn.sh: the brief's quality line must agree with --quality, the same check the delivery line already gets, in both directions. quality= and base_sha= land in the task record; the base commit is captured once at spawn and read back on relaunch, never recaptured, because the loop commits each round and a later capture would narrow the gate while still reporting success. - AGENTS.md: one sentence placing quality resolution at intake. Tests execute the real interfaces. The load-bearing ones prove a project without "+hardened" and a brief scaffolded without --quality behave exactly as before: the two-word stdout is pinned across every annotation form, the two scaffolds are compared byte for byte, and the task record's key set is pinned so only quality= and base_sha= are additive.
…ture and scripts inventory
The stage 0a pilot showed the receipt cannot express real findings and that a missing head_sha makes a drifted base report not-applicable and exit 0. This revises unpublished schema v1 in place: require head_sha, duration_ms, engine, threshold, and a stable finding id; replace survivors[] with per-phase findings[]; and make verify one envelope with phases[]. bounds.budget_minutes is the missing wall-clock bound.
A short herdr recent tail can drop Claude's opening rule and classify an idle composer unknown, so native-hosted away-mode never injected. Classify a glyph immediately under a closing rule as empty, read the visible viewport for composer capture, and let herdr native idle deliver when the composer is still unknown. Max-defer retries that path before alarming. The native-hosted daemon still injects into the captain pane. A dead shell has no idle agent registration and still defers.
… proven idle composers
…erdr composer read
A Claude Code background continuation runs in its own process tree, so the harness-ancestry walk answered a different question in a hook than in a tool shell. One session could hold the lock by the auto-arm's reckoning and not hold it by every mutating path's, which left supervision off while wake drains, gate answers, merges, teardowns and a promotion all proceeded. - bin/fm-session-lock-lib.sh becomes the single owner of the ownership verdict and its refusal. Identity resolves in three tiers: the vendor-declared CLAUDE_PID, then the conversation id recorded in state/.lock.session, then the ancestry walk for harnesses that declare neither. - fm_require_session_lock gates the eight fleet-mutation entry points before argument validation, so the read-only rule is enforced where mutation happens. It refuses only on a live foreign owner plus a caller that is itself in a harness session, leaving ssh, detached and CI callers alone. - bin/fm-lock.sh and bin/fm-session-start.sh state ownership in words; the digest carries an explicit HELM: line. - bin/fm-turnend-guard.sh tells a correct decline from an auto-arm failure, reports it once per holder, and then stops blocking. - tests/fm-session-lock-ownership.test.sh pins all three parts with real competing processes; tests/fm-session-identity-live-e2e.test.sh proves the two vendor-declared values against real Claude Code.
…tity, and turn-end notices
BohnBawerick
force-pushed
the
fm/fm-session-lock-split-guard
branch
from
August 22, 2026 09:23
3442531 to
563b759
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Fix the split between which firstmate session holds the helm and which one actually does the work, so a Claude Code background continuation can never again mutate fleet state while supervision stays off.
Observed in the captain's home: the session lock was held by the foreground Claude Code session, while the session doing all the work was a background continuation of that same conversation, running in a separate process tree. It resolved its own harness pid differently, so fm_session_lock_owned_by_self correctly returned false against the recorded lock. This is Claude Code's own background-session architecture meeting a lock contract written for one process per home. It is not operator error, and closing the foreground session is not the fix.
Three parts, all of which must be fixed.
Part 1 - the lock contract was enforced in exactly one place, and it was the wrong place. The Claude Stop auto-arm refused to arm, so supervision stayed off and the turn-end guard fired every turn, while every mutating path proceeded without complaint: wake drain and acknowledgement, two validation gate answers, a merge into local main, a push to the landing remote, two teardowns, a scout promotion, and ship instructions. AGENTS.md section 3 says a lock-refused session must stay read-only and must not do any of those. The two candidate shapes were (a) teach the lock contract about a forked background continuation of the lock-holding conversation so it inherits the helm legitimately, and (b) apply the auto-arm's ownership predicate to the mutating paths so a non-owning session actually refuses. Relaxing the auto-arm was explicitly ruled out. Both (a) and (b) were needed and both were done: (a) alone would leave the read-only rule to agent memory on every other split, and (b) alone would stall the captain's home whenever Claude Code runs a turn in a background continuation.
Part 2 - the digest told the non-owning session it owned the home: session start printed "lock acquired: harness pid ", which reads as confirmation of ownership when it is in fact a warning that a different process holds the lock. A session must be able to tell "I hold this" from "someone else holds this" without comparing pids by hand.
Part 3 - the turn-end guard could not escape this state, so it blocked every turn forever. The one attended fail-open was reachable only after the auto-arm recorded an exhausted failure, but a clean stand-down exits 0 at the ownership gate before any failure epoch is written, so state/.turnend-claude-blocks stayed pinned instead of progressing toward the block budget. The guard must distinguish "the auto-arm tried and failed" from "the auto-arm correctly declined because this session does not own the home", and treat the second as a stable condition to report once rather than a recovery to keep waiting for.
Requirement: reproduce before fixing. Each of the three parts had to be reproduced first - a non-owner running a mutating fleet path without refusal, the auto-arm declining silently with no failure epoch, and the turn-end guard blocking repeatedly without its block counter advancing - and each reproduction turned into a regression test that fails before the change and passes after.
Acceptance criteria:
Constraints:
What Changed
bin/fm-session-lock-lib.shbecomes the single owner of session identity and refusal: it resolves the holder fromCLAUDE_PID, then the conversation id recorded instate/.lock.session, then the existing harness-ancestry walk, and exposesfm_require_session_lock. A background continuation of the lock-holding conversation now inherits the helm, whilebin/fm-spawn.shstrips both declared values from every worker launch so a spawned worker cannot borrow its parent's identity.bin/fm-control.sh,fm-merge-local.sh,fm-pr-merge.sh,fm-promote.sh,fm-send.sh,fm-spawn.sh,fm-teardown.sh, andfm-wake-drain.shcall that gate and refuse with a message naming the holder;bin/fm-claude-stop-autoarm.shandbin/fm-lock.shread the same verdict.bin/fm-lock.shandbin/fm-session-start.shstate ownership in words instead of a bare pid: every acquisition line says whether THIS session or ANOTHER session holds the lock, and the digest prints aHELM:line covering held, unresolved, and foreign-owner cases.bin/fm-turnend-guard.shtells a declined auto-arm apart from a failed one. When another live session holds the home it reports the decline once per (session, lock owner) pair viastate/.turnend-unowned-notice.<session>, retires records whose writer identity no longer resolves to a live process, and then stops blocking without spending the block budget.tests/fm-session-lock-ownership.test.sh(824 lines, run in thewatcher-wake-lockfamily) plus the opt-intests/fm-session-identity-live-e2e.test.shlive guard;AGENTS.md,docs/turnend-guard.md,docs/watcher-continuity.md,docs/scripts.md,docs/sessionstart-nudge.md, and a dated 2026-08-22 record indocs/verification/runtime-backends.mdare updated to the new contract.Risk Assessment
Testing
Beyond the targeted suites (session-lock ownership, session start, turn-end guard, Stop auto-arm, ancestry, spawn dispatch - all passing), I built a three-session lab that reproduces the reported split with real live processes and drove the actual fleet scripts through it, capturing a CLI transcript that shows all three acceptance parts: a background continuation of the lock-holding conversation keeping the helm while an unrelated session is refused by every mutating entry point, the lock and session-start digest naming ownership in words, and the turn-end guard reporting the decline once then standing down without spending the auto-arm block budget. While testing I found the new regression suite leaked four orphaned
sleepprocesses per run (the runner reported FM_TEST_LEAK and reaped them); I fixed the fixture to reap its own children via the repo's existingfm_test_reap_pidhelper, which removed the leak and halved the suite runtime. No product code was changed and the worktree carries only that test-file edit.Evidence: End-to-end CLI transcript: three sessions, one home, all three parts
Source: End-to-end CLI transcript: three sessions, one home, all three parts
Evidence: Driver script that produced the transcript
Source: Driver script that produced the transcript
Evidence: Fixture process leak, before and after the reap fix
Source: Fixture process leak, before and after the reap fix
Evidence: Helm verdict as the reading agent sees it in the session-start digest
Evidence: Turn-end guard: reports once, then stands down
Pipeline
Updates from git push no-mistakes
... (15 earlier update rounds omitted to keep the PR body within GitHub's 65536-char limit; full history is in the run log.)
Part 2's goal is that the session-start output makes ownership unmistakable, so a contradiction here is worse than the bare pid it replaced. Gate the positive HELM line on LOCK_RC being 0 as well as on fm_session_lock_owned_by_self, or emit a third wording for "ownership resolves but acquisition failed".
bin/fm-wake-drain.sh:49- docs/watcher-continuity.md states that every fleet-mutation entry point "calls fm_require_session_lock before argument validation", but in bin/fm-wake-drain.sh the argument case statement runs at lines 30-42 and the gate only at line 49.Concrete effect: a non-owning session running
bin/fm-wake-drain.sh --ack-through abc --recovery-generation ggets "wake drain: invalid acknowledgement sequence" and exit 2, never the lock refusal that names the holder and says what to do instead - the same failure mode the gate-placement rule exists to prevent, and the one tests/fm-session-lock-ownership.test.sh:158-160 calls out. That test passes only because it invokes every script with no arguments, and no arguments is a valid case for fm-wake-drain.sh so it falls through to the gate.Move the source and fm_require_session_lock call above the case statement (STATE is already available from fm-wake-lib.sh, sourced at line 12), or correct the doc sentence.
🔧 Fix: harden session-lock helm inheritance, spawn identity, and turn-end notices
4 issues (1 error, 2 warnings, 1 info) still open:
bin/fm-spawn.sh:2939- The new universalLAUNCH="env -u CLAUDE_PID -u CLAUDE_CODE_SESSION_ID $LAUNCH"prefix rewrites the literal launch line every backend sends to a worker pane, and five existing assertions on that exact line were not updated. They fail as written.The prefix is inserted at bin/fm-spawn.sh:2939, BEFORE the existing
env -u CURSOR_AGENT -u CURSOR_INVOKED_ASprefix at 2942 and before theCLAUDE_CONFIG_DIR=prefix at 2953, and the final string is sent verbatim byspawn_send_literal "$T" "$LAUNCH"(bin/fm-spawn.sh:3019) - which is exactly what every one of these tests captures.Concrete breakages:
env -u CURSOR_AGENT -u CURSOR_INVOKED_AS CLAUDE_CODE_ENABLE_PROMPT_SUGGESTION=false claude ...; actual nowenv -u CURSOR_AGENT -u CURSOR_INVOKED_AS env -u CLAUDE_PID -u CLAUDE_CODE_SESSION_ID CLAUDE_CODE_ENABLE_PROMPT_SUGGESTION=false claude ....assert_containsonCLAUDE_CONFIG_DIR='/opt/test/claude-work' env -u CURSOR_AGENT -u CURSOR_INVOKED_AS CLAUDE_CODE_ENABLE_PROMPT_SUGGESTION=false claude. The new prefix is inserted inside that substring, so it no longer matches.[ "$launch" = "custom-agent --flag" ], whose failure message is literally "raw launch command changed". The raw unverified-adapter escape hatch (bin/fm-spawn.sh:1220) now also gets the prefix, so it does change.env -u CURSOR_AGENT -u CURSOR_INVOKED_AS '<bin>/kimi' --model 'kimi-code/k3' --auto.env -u CURSOR_AGENT -u CURSOR_INVOKED_AS '<fallback>' --auto.(assert_contains-based harness tests - agy, muse, orca, secondmate - still match, because their needles sit after the seam.)
The prefix itself is mechanically sound for every template (
envaccepts the trailing NAME=VALUE assignments thatclaude,opencode,museand thepiwrapper carry), so the fix is to update the five expectations. Folding the two-uflags into the EXISTINGenv -u CURSOR_AGENT -u CURSOR_INVOKED_ASlist at bin/fm-spawn.sh:2942 - what the review instruction actually asked for - would keep 1, 2, 4 and 5 passing unchanged and leave only the raw escape hatch (3) to decide; note that list does not covercursor,agy, or the raw form, so those still need their own clearing.bin/fm-claude-stop-autoarm.sh:114- The round-1 fix makes bin/fm-lock.sh rewrite a dead recorded pid, and bin/fm-session-lock-lib.sh:94-99 plus docs/watcher-continuity.md:33 now state the invariant "inheriting the helm still leaves a LIVE pid in state/.lock". That invariant only holds while fm-lock.sh is actually invoked, and there is a reachable state where it never is again.bin/fm-claude-stop-autoarm.sh:114 sets RECOVER_SESSION_LOCK=1 only inside
if ! fm_session_lock_owned_by_self "$STATE". For a background continuation that owns the home through tier 2 (the conversation id), ownership DOES resolve, so the delegation at bin/fm-claude-stop-autoarm.sh:137 is skipped no matter how dead the recorded pid is.Concrete sequence:
Before this change there was no tier 2, so B would have failed fm_session_lock_owned_by_self, RECOVER_SESSION_LOCK would have been 1, and the auto-arm would have repaired the pid on the first Stop. Tier 2 introduces the gap.
Earliest shared boundary: the auto-arm's identity block already owns "a dead recorded owner is reclaimed through fm-lock.sh". Decide RECOVER_SESSION_LOCK from the recorded pid's LIVENESS rather than from ownership - read state/.lock first, and set RECOVER_SESSION_LOCK=1 whenever it is numeric and fails fm_harness_pid_alive, keeping the existing
live owner + not this session -> exit 0refusal. The delegation at line 137 already re-verifies ownership afterwards, so nothing else moves.bin/fm-test-run.sh:1195- tests/fm-session-lock-ownership.test.sh is mapped to familywatcher-wake-lock(bin/fm-test-run.sh:386), but the two source files whose new invariants it pins do not select that family, so the regressions added this round will not run when someone changes the code they guard. The map's own header says it "over-selects rather than under-selects" (bin/fm-test-run.sh:1076).bin/fm-spawn.sh matches the case at bin/fm-test-run.sh:1195 and emits only
backend-dispatchandpure-contract-unit. So test_a_spawned_worker_does_not_inherit_the_spawning_sessions_helm - the regression written specifically for the bin/fm-spawn.sh:2939 boundary - is never selected by a change to bin/fm-spawn.sh.bin/fm-lock.sh matches
bin/fm-lock*at bin/fm-test-run.sh:1157 and emits onlysession-bootstrap. So test_an_inherited_helm_records_its_own_live_pid - the regression for the bin/fm-lock.sh:100-107 fast-path fix - is never selected by a change to bin/fm-lock.sh.The explicit cases win over the generic
bin/*reference-scan fallback at bin/fm-test-run.sh:1254, so nothing else recovers the selection. bin/fm-session-lock-lib.sh was correctly given the extra families at bin/fm-test-run.sh:1146-1152; the same treatment is missing for the two files that round's fixes actually landed in.Add
watcher-wake-lockto the bin/fm-spawn.sh and bin/fm-lock* entries.docs/scripts.md:40- docs/scripts.md:40 still reads "Shared session-lock harness identity (ancestry walk and holder liveness) for fm-lock.sh and the Claude Stop auto-arm".That is the pre-change contract on both halves. The lib now resolves identity in three tiers (CLAUDE_PID, then the conversation id, then the ancestry walk) and owns fm_require_session_lock, the fleet-mutation gate that bin/fm-wake-drain.sh, bin/fm-send.sh, bin/fm-spawn.sh, bin/fm-teardown.sh, bin/fm-promote.sh, bin/fm-merge-local.sh, bin/fm-pr-merge.sh, and bin/fm-control.sh all call. The intent requires that no doc be left describing the old behaviour; AGENTS.md, docs/turnend-guard.md, docs/watcher-continuity.md, and docs/verification/runtime-backends.md were all updated, this index row was not.
🔧 Fix: reclaim dead lock pids and fix launch-line expectations
4 issues (1 error, 3 warnings) still open:
bin/fm-lock.sh:44- Thestatusbranch was rewritten from one live-holder line into two ownership-worded lines, and two existing assertions on the old string were not updated. They fail as written.Base (672c6d0) printed a single line for any live holder:
lock: held by live harness pid $oldHEAD replaces it with
lock: held by THIS session (harness pid $old)(bin/fm-lock.sh:42) orlock: held by ANOTHER live session (harness pid $old)(bin/fm-lock.sh:44). Neither contains the old substring.Reproduced against the real script with the same fake
psthose tests install (a fixture home whose state/.lock holds a live pid the harness predicate accepts, no declared identity):actual:
lock: held by ANOTHER live session (harness pid 63439)Both of these are exercised with no CLAUDE_PID / CLAUDE_CODE_SESSION_ID (tests/lib.sh:37-45 unsets them), so tier 1 and tier 2 are skipped and the tier-3 ancestry walk resolves fm-lock.sh's OWN pid rather than the recorded test-shell pid - which is why the ANOTHER branch is taken:
assert_contains "$out" "lock: held by live harness pid"("fm-lock did not recognize grok as a live holder").No other consumer greps the old string (checked bin/, tests/, docs/, .claude, .agents), so updating these two expectations to the new wording is the whole fix. Both cases only ever mean "a live harness process is recognized as the holder", so the ANOTHER-session line satisfies their intent unchanged.
bin/fm-turnend-guard.sh:208- The per-session notice slot is keyed on the stop payload'ssession_id, which two of this repo's own turn-end callers never send. For those harnesses the slug is the constantunknown, so one slot is shared by every session in the home - across process lifetimes - and the "report once" becomes "report once, ever".SESSION_ID is read at bin/fm-turnend-guard.sh:179 as
.session_id // "unknown", and the slug at line 208 is derived from it. But:.pi/extensions/fm-primary-turnend-guard.ts:169writes the guard's stdin as the literal{"stop_hook_active":false}- no session_id at all.bin/fm-turnend-guard-grok.sh:63reads.sessionIdfor its own use and then pipes the UNCHANGED payload to the guard at line 71, which looks for.session_id.Both therefore land on
unknown.Concrete sequence, Pi primary, home whose lock is held by another live session with supervision off:
owner=<pid>to state/.turnend-unowned-notice.unknown and exits 2. Correct.That is the opposite of the acceptance criterion the decline exists for: the report is owed once PER SESSION, and Q is a different session that has read nothing. It also silently disables the only signal Q gets that its turns are ending blind.
Earliest shared boundary: the slug is the only place that assumes session_id exists. Fall back to this session's own resolved harness identity when the payload carries none -
fm_session_lock_self_pidis already sourced here (bin/fm-turnend-guard.sh:122) and returns a value that is stable within a session and distinct across sessions for exactly the harnesses that publish no session_id. tests/fm-session-lock-ownership.test.sh:541 covers two peers only through the Claude payload, which always carries session_id, so it does not reach this.bin/fm-test-run.sh:1183- Four more scripts gainedfm_require_session_lockin this change, and tests/fm-session-lock-ownership.test.sh (familywatcher-wake-lock, bin/fm-test-run.sh:386) is the only suite that drives that gate through them - but none of the four selects that family. The map's own header says it "over-selects rather than under-selects" (bin/fm-test-run.sh:1076), and the previous round already applied exactly this correction to bin/fm-spawn.sh and bin/fm-lock*.pr-forge.pure-contract-unit.All four are named in the refusal loop at tests/fm-session-lock-ownership.test.sh:176-177, so their gate placement (before argument validation) and their refusal wording are pinned there and nowhere else. Because these are EXPLICIT cases, the generic
bin/*reference-scan fallback at bin/fm-test-run.sh:1266 never runs for them, so nothing recovers the selection. (bin/fm-control.sh has no explicit case and does reach that fallback, which finds the suite by name, so it is already covered; bin/fm-wake-drain.sh matchesbin/fm-wake*at line 1122 and already emits watcher-wake-lock.)Add
watcher-wake-lockto the bin/fm-test-run.sh:1183 entry and give bin/fm-promote.sh its own entry emittingpure-contract-unitpluswatcher-wake-lock.bin/fm-session-lock-lib.sh:248- The new INVARIANT block states that the recorded pid always names a live process while a session holds the home, and that bin/fm-claude-stop-autoarm.sh "reclaims through bin/fm-lock.sh whenever it finds one". The auto-arm does not do that unconditionally, and the exception is deliberate and pinned by an existing test, so the stated invariant is not the one the code keeps.bin/fm-claude-stop-autoarm.sh sets RECOVER_SESSION_LOCK=1 at line 125, but the delegation to fm-lock.sh is deferred to line 137, AFTER the AFK gate (line 129) and the supervision-need gate (line 135). Both exit 0 first. tests/fm-claude-stop-autoarm.test.sh:264 (test_stale_lock_recovery_preserves_afk_and_need_gates) asserts exactly that a dead recorded pid is NOT reclaimed in an away home or an idle home, so this ordering is intended, not an oversight.
Concrete reachable state, away mode:
The risk is that this text is written as a load-bearing invariant "relied on by every predicate below", so later code will assume a dead recorded pid is unreachable. Correct the claim to name the two gates the reclaim sits behind (away homes and idle homes stay inert by design, and a stale lock there is treated as a free home by fm_session_lock_held_by_other), and mirror it in docs/watcher-continuity.md:33-34, which carries the same unqualified sentence. Moving the reclaim ahead of those gates would instead contradict the byte-for-byte-inert contract that test pins, so it is not the fix.
🔧 Fix: key turn-end notices per session and refresh stale expectations
4 issues (2 warnings, 2 infos) still open:
bin/fm-session-start.sh:647- The HELM else-branch asserts that ANOTHER session holds the fleet lock whenever the acquisition failed and ownership does not resolve to this session. It never checks that a competing holder exists, so it states a false cause for every non-competing acquisition failure.Concrete reachable state, exactly the fixture tests/fm-session-start.test.sh:905 already builds (
chmod 0500 "$home/state"):The same holds for the other non-competing failures: state/.lock being a symlink (bin/fm-lock.sh:120-124), an unreadable lock (bin/fm-lock.sh:125-128), and "cannot identify this session harness process" (bin/fm-lock.sh:49) in a home with no lock at all.
This contradicts the invariant the suite already pins one line away: tests/fm-session-start.test.sh:925 asserts
assert_not_contains "$out" "ANOTHER LIVE FIRSTMATE SESSION HOLDS THE FLEET LOCK"with the failure message "lock publication failure falsely claimed a live lock holder". The new line makes the same false claim in different words, so that assertion does not catch it. It also contradicts AGENTS.md:154, which says "another active session is only one possible cause", and it undercuts Part 2 of the intent, whose whole point is that this section states ownership correctly rather than leaving the reader to interpret a pid.Earliest boundary: the same three-way branch. Only claim a competing holder when one is demonstrable - gate the else on
fm_session_lock_held_by_other "$STATE"(or on a live fm_session_lock_pid), and emit a third wording such as "HELM: fleet-lock ownership could NOT be resolved - this session is READ-ONLY" for the remaining cases. Then extend the fixture at tests/fm-session-start.test.sh:905 to assert the new wording, since it already reaches this state.bin/fm-turnend-guard.sh:179- SESSION_ID reads only.session_id, but grok's stop payload spells it.sessionId, so grok always lands on the literal "unknown" and the new fallback at line 216 keys the notice on a resolved harness pid instead of the stable per-session id the payload already carries.Evidence in this repo: bin/fm-turnend-guard-grok.sh:53-56 pipes the UNCHANGED payload to this guard on the native path, and its own legacy path at line 63 reads that payload's
.sessionIdas a required non-empty string. This guard already knows grok is camelCase - lines 143-150 accept bothstopHookActiveandstop_hook_active- so handling only the snake-case session id is an oversight, not a decision. The new comment at line 208 ("Pi and grok send a stop payload with no session_id") is therefore wrong for grok; only Pi (.pi/extensions/fm-primary-turnend-guard.ts:169, which sends the literal{"stop_hook_active":false}) genuinely sends none.Why it matters rather than being cosmetic: the pid fallback re-reports whenever the resolved pid changes for one logical session. That is reachable on grok's own compatibility path - bin/fm-turnend-guard-grok.sh:87-90 re-enters the session with
grok --resume, a distinct grok process, whose Stop runs this guard again and resolves a different ancestry pid, so the same session is told the decline a second time. The pid key is also vulnerable to pid reuse in the other direction (see the separate finding on line 216), which silently suppresses the report instead.Fix at the same boundary, one jq expression:
.session_id // .sessionId // "unknown", mirroring the camelCase handling already present at lines 145-148, and correct the line 208 comment to name Pi alone. The pid fallback stays for Pi. tests/fm-session-lock-ownership.test.sh:565 drives only a payload with no id at all, so it does not distinguish the two.bin/fm-turnend-guard.sh:216- The fallback slug isself-<pid>from fm_session_lock_self_pid, and nothing ever removes state/.turnend-unowned-notice.* (grep across bin/, tests/, docs/ finds only the writer at line 219 and the AGENTS.md:127 inventory row).Two consequences, both low-probability but both silent:
have == wantat line 225 and exits 0 without ever printing the decline, so it is never told supervision is off and it is read-only - the exact signal the decline path exists to deliver.Not proposing a new mechanism for it: the cheapest containment is to also clear these slots wherever the recorded holder changes (the notice is already invalidated by content in that case), or to note the leak explicitly in the AGENTS.md:127 inventory row so it is a known accepted cost rather than an oversight.
bin/fm-lock.sh:70- report_acquired decides its wording purely fromme != $1, then states the reason as "same conversation as this session". But fm_session_lock_owned_by_self (bin/fm-session-lock-lib.sh:276-301) can also return 0 through tier 3 - membership in the contiguous harness ancestry - with the recorded pid being an inner pid of that run andme(fm_harness_ancestry_pid, the outermost) being a different one. The line then names a conversation match that never happened.Reachable for a Claude session that publishes no CLAUDE_CODE_SESSION_ID: state/.lock holds pid P written by an earlier call whose walk reported P as outermost; a later call from a different depth in the same nested worker chain reports Q as outermost while P is still a member of the ancestry list, so tier 3 grants ownership,
me=Q != P=$1, and the else branch fires. The ownership verdict itself ("THIS session holds the fleet lock") stays correct; only the parenthetical explanation is wrong.Cheap fix at the same boundary: have the caller pass which tier matched, or only use the conversation wording when fm_harness_session_id and fm_session_lock_recorded_id actually agree, falling back to a neutral "(recorded harness pid $1, this session's harness ancestry)" otherwise.
🔧 Fix: state real helm cause and retire stale turn-end notices
3 issues (1 warning, 2 infos) still open:
bin/fm-lock.sh:109- The fast path publishes THIS session's conversation id before it decides which tier granted ownership, so a tier-3 (ancestry) grant both overwrites another session's recorded conversation and then reports itself as a tier-2 conversation match.Order of operations at bin/fm-lock.sh:105-111: ownership is decided (108), then publish_session_id (109) writes $CLAUDE_CODE_SESSION_ID into state/.lock.session, then report_acquired (110) reads that same file back at bin/fm-lock.sh:74 to choose its wording. The comparison at line 75 is therefore always self-consistent whenever this session publishes any id, so the "same conversation as this session" branch fires for every acquisition where
me != recorded, including the ancestry-only ones the round-4 fix added the third branch for.Reproduced against the real script (fixture: outer fake-claude -> inner fake-claude -> fm-lock.sh; state/.lock = inner pid, state/.lock.session = conv-Y, caller CLAUDE_CODE_SESSION_ID=conv-X, no CLAUDE_PID):
lock acquired: THIS session holds the fleet lock (recorded harness pid 74663, same conversation as this session)
lock=74663
sidecar=conv-X
Tier 2 could not have matched (conv-X != conv-Y); tier 3 matched because the recorded pid is an inner member of the caller's harness ancestry, and
me(fm_harness_ancestry_pid) is the outer pid. The line names a conversation match that did not happen.The sidecar rewrite is the part that changes state rather than just wording. bin/fm-session-lock-lib.sh's own header documents the case that makes this reachable outside a lab: "the walk from its ordinary tool shells can climb past it into an unrelated harness further up the real tree". Concretely - session A (harness pid 100, conversation Y) holds the home and recorded conv-Y; session B's tool shell has pid 100 above it in the real tree and publishes conversation X. B's fm-lock.sh takes tier 3, leaves state/.lock naming A's pid 100, and rewrites state/.lock.session to conv-X. A's own background continuation - the exact case tier 2 exists to protect - now fails tier 1 (its pid is not 100) and fails tier 2 (Y != X), so it is refused a home A still holds, and fm_require_session_lock turns that into a hard refusal at every mutating entry point.
Earliest boundary, same function: decide the tier BEFORE publishing. Capture whether tier 1 or tier 2 matched (compare fm_harness_session_id against fm_session_lock_recorded_id while the file still holds the prior value), pass that verdict into report_acquired, and do not publish this session's id over a recorded id that tier 2 did not match - a tier-3 grant inherits an existing owner's record and has no authority to rename the conversation on it. tests/fm-session-lock-ownership.test.sh drives tier 1, tier 2, and a no-id tier-3 case, but never tier 3 with a conflicting published id, so nothing catches either half.
bin/fm-session-start.sh:645- The second HELM branch prints "ownership resolves to THIS session, but the fleet lock was NOT acquired - this session is READ-ONLY", but fm_require_session_lock lets that same session mutate freely: fm_session_lock_held_by_other returns 1 as soon as fm_session_lock_owned_by_self returns 0 (bin/fm-session-lock-lib.sh:321), which is precisely the condition this branch is reached under.Reachable state, the one tests/fm-session-start.test.sh:1487 already builds: state/.lock is a symlink whose target holds this session's declared pid. fm-lock.sh refuses at bin/fm-lock.sh:130, LOCK_RC=1, the read-only banner prints - and bin/fm-send.sh, bin/fm-spawn.sh, bin/fm-teardown.sh and the rest all proceed.
Noting it rather than proposing a change: Part 1 of the intent is that the read-only rule stops being a banner the session may not have read, and here it still is one. The gate is deliberately scoped to competing sessions, and closing this would need a different signal (acquisition success, not ownership), which is a design call rather than a defect in what was written. Worth an explicit decision that the acquisition-failure-without-a-competitor case stays advisory.
tests/fm-session-lock-ownership.test.sh:563- The comment reads "Pi and grok end a turn with a payload that carries no session_id at all", but the same fix round changed bin/fm-turnend-guard.sh:180 to read.session_id // .sessionId // "unknown"precisely because grok DOES carry the id under the camelCase spelling (bin/fm-turnend-guard-grok.sh:63 reads.sessionIdfrom that same payload as a required non-empty string). bin/fm-turnend-guard.sh:209 states the corrected version - "Pi sends a stop payload with no session id at all" - so the two comments in one change now disagree.The test itself is correct and drives a payload with no id at all; only its rationale is wrong. It matters because it is the comment a later maintainer reads when deciding whether the
.sessionIdfallback is still needed, and dropping it would silently push every grok session back onto the shared pid-keyed slot.🔧 Fix: decide lock tier before publishing the conversation record
2 infos still open:
bin/fm-turnend-guard.sh:214- The four notice globals (UNOWNED_NOTICE_HOLDER, UNOWNED_NOTICE_IDENTITY, UNOWNED_NOTICE_KEYED_BY_PID, UNOWNED_NOTICE_SLUG/UNOWNED_NOTICE) are computed at top level on lines 214-224, but grep shows they are read only by notice_record, notice_want_key, notice_have_key, prune_retired_notices and decline_stop - every one of which runs only after the ownership test on line 305 returns true.So on the ordinary blocking path (supervision unhealthy in a home this session DOES hold - the common repair case), the guard pays for a full extra fm_session_lock_self_pid ancestry walk (up to 32
pscalls for Pi, OpenCode, grok and any other harness that publishes no CLAUDE_PID) plus an fm_pid_identity read that runsodover /proc/<pid>/cmdline, and then throws all of it away.No wrong result: the values are correct wherever they are used, and fm_session_lock_held_by_other already walks the ancestry itself, so this is redundant work rather than new work. Purely a locality and cost cleanup: move lines 214-224 to the top of decline_stop, where the functions that read them are already defined below and nothing else can observe them.
ℹ️
bin/fm-turnend-guard.sh:296- Two lines of the new decline banner read as broken English because the printf format strings are single-quoted and an apostrophe would close the quote:line 296: "holds this home fleet lock" (should be "this home's fleet lock")
line 297: "so supervision is that session job" (should be "that session's job")
The existing block_stop banner just below (lines 329-342) keeps the same single-quoted house style but stays grammatical, so this is new text rather than an established convention. It matters a little more than usual here because this banner is the single message an agent reads when it is being told it may not touch the home, and Part 2/3 of the intent is specifically about that message being unambiguous.
Fix is mechanical: use double quotes for those two format strings (there is nothing in them that would expand), or reword to avoid the possessive.
🔧 **Test** - 1 issue found → auto-fixed ✅
tests/fm-session-lock-ownership.test.sh:618- tests/fm-session-lock-ownership.test.sh::test_two_non_owning_sessions_each_report_once_and_both_stand_down failed deterministically (peer-one blocked again on turn 2, exit 2 instead of 0). Cause was the fixture, not the product: guard_turn_as launches a fresh detached harness process per turn, so each peer's recorded notice holder was already dead when the other peer's next turn ran prune_retired_notices, and the two peers swept each other's records. A real session's recorded holder (CLAUDE_PID, or the outermost harness ancestry pid) outlives every turn it ends, which is why the neighbouring no-session-id and notice-sweep tests already use long-lived processes. Auto-fixed by adding start_peer_session/peer_turn helpers so each peer is one long-lived harness process driven turn by turn, and asserting the peers stay silent on later turns as well. Product code was not changed.bash tests/fm-session-lock-ownership.test.sh(16 tests; failed ontest_two_non_owning_sessions_each_report_once_and_both_stand_down, then passed after the fixture fix, confirmed over two consecutive runs)bash tests/only-part1|part2|part2b|part3.test.shagainst a base-commit (672c6d0) checkout ofbin/- each part of the new suite fails before the changebin/fm-test-run.sh tests/fm-turnend-guard.test.sh tests/fm-claude-stop-autoarm.test.sh tests/fm-session-lock-ancestry.test.sh tests/fm-session-start.test.sh tests/fm-control.test.sh tests/fm-merge-local.test.sh tests/fm-pr-merge.test.sh tests/fm-teardown.test.sh tests/fm-wake-drain-unread-status.test.sh tests/fm-spawn-dispatch-profile.test.sh tests/fm-sessionstart-nudge.test.sh tests/fm-startup-network.test.sh tests/fm-grok-harness.test.sh tests/fm-kimi-harness.test.sh(FM_TEST_SUMMARY total=14 failed=0)FM_SESSION_IDENTITY_LIVE=1 bash tests/fm-session-identity-live-e2e.test.sh(opt-in live guard, real Claude Code 2.1.239)bin/fm-test-run.sh --check-coverage(FM_TEST_COVERAGE ok total=161)Manual end-to-end transcript: a fixture home held by a live foreign session, with a second live session runningbin/fm-lock.sh,bin/fm-lock.sh status,bin/fm-session-start.sh, andbin/fm-spawn.sh/fm-send.sh/fm-merge-local.sh/fm-pr-merge.sh/fm-teardown.sh/fm-promote.sh/fm-control.sh/fm-wake-drain.sh, plus six consecutive turn-ends throughbin/fm-turnend-guard.sh --claude, captured against both the base and target commitsManual end-to-end transcript: a background continuation carrying the lock-holdingCLAUDE_CODE_SESSION_IDversus an unrelated conversation id, both runningbin/fm-lock.sh statusandbin/fm-wake-drain.sh, captured against both commits🔧 Fix: model two-peer turn-end sessions as long-lived processes
✅ Re-checked - no issues remain.
bin/fm-test-run.sh tests/fm-session-lock-ownership.test.sh- 16 assertions, all pass, no FM_TEST_LEAK after the fixture reap fixbin/fm-test-run.sh tests/fm-session-start.test.sh- session-start digest HELM verdict, including the resolved-but-unacquired contradiction guardbin/fm-test-run.sh tests/fm-turnend-guard.test.shbin/fm-test-run.sh tests/fm-claude-stop-autoarm.test.shbin/fm-test-run.sh tests/fm-session-lock-ancestry.test.shbin/fm-test-run.sh tests/fm-spawn-dispatch-profile.test.shManual end-to-end lab: three live fake harness sessions in one fleet home (holder A, background continuation B with the same CLAUDE_CODE_SESSION_ID, stranger C), driving the realbin/fm-lock.sh,bin/fm-lock.sh status,bin/fm-wake-drain.sh,bin/fm-send.sh,bin/fm-spawn.sh,bin/fm-teardown.sh,bin/fm-promote.sh,bin/fm-merge-local.sh,bin/fm-pr-merge.sh,bin/fm-control.sh,bin/fm-turnend-guard.sh --claude(4 turns) andbin/fm-session-start.sh- transcript capturedProcess-leak diagnosis: ran the suite in its own process group and captured the surviving group members at script exit (4 orphanedsleep 600), then re-ran after the fix to confirm the group is emptyReviewed the already-recorded opt-in live guard evidenceFM_SESSION_IDENTITY_LIVE=1 tests/fm-session-identity-live-e2e.test.shagainst real Claude Code 2.1.239 and its dated record indocs/verification/runtime-backends.md; not re-run this round because no product code changeddocs/turnend-guard.md:144- Judgment call: the new### Not this sessionsubsection in docs/turnend-guard.md was inserted between the Claude blocking paragraphs and the passive-adapter paragraphs, which put the OpenCode/Pi/Grok/Cursor harness prose under that subsection and left "expose passive callbacks for this purpose" with the wrong antecedent. I moved the whole subsection to the end of "## Harness integrations" (just before "## Compatibility limits") instead of rewriting the surrounding prose. Its "the bounded progression above" reference still resolves upward. If the author intended it to sit next to the Claude progression, the alternative is to add a heading before the passive-adapter paragraphs instead.🔧 **Lint** - 1 issue found → auto-fixed ✅
🔧 Fix: silence SC2016 false positive in session-identity live test
✅ Re-checked - no issues remain.
✅ **Push** - passed
✅ No issues found.