fix(bin): key turnend-guard block budget on live auto-arm signal, not raw generation - #3898
Closed
TomatoIceberg wants to merge 3 commits into
Closed
TomatoIceberg wants to merge 3 commits into
TomatoIceberg wants to merge 3 commits into
Conversation
bin/fm-turnend-guard.sh --claude de-duplicated its bounded block budget on
the raw generation number in state/.claude-autoarm-epoch. A superseded ledger
entry never changes that number again, so every later Stop firing looked like
a repeat observation of one event and the counter stopped advancing.
Reproduced two ways against the real guard:
- "arming" frozen behind a dead owner pid: count pinned at 1 across six
consecutive firings, the reported symptom.
- a stale terminal "failed" entry with its failure notice already consumed:
count pinned at 0, so the documented one loud attended fail-open could
never be reached however many turns went by.
The budget now keys its de-duplication on whether the ledger entry is a LIVE
signal for the Stop being decided - an open generation claim, a live legacy
claimant, or an outcome inside the freshness window - and accounts anything
else as no generation at all, exactly like an absent ledger. Behavior for a
live or fresh claim is unchanged. The legacy live-claim proof had three
copies; it is now stated once and shared.
Tests cover the orphaned arming claim advancing the budget, the stale failed
epoch reaching the bounded fail-open exactly once, the absent-auto-arm
baseline, and a live generation claim still being accounted only once. Both
stale-case tests fail against the unfixed guard.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ncyHHsUEn7CYSExbv73er
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; dead or identity-mismatched arming claims are rejected before freshness handling, and the changed budget path clears their frozen epoch instead of deduplicating against it. Reviews (2): Last reviewed commit: "no-mistakes: apply CI fixes" | Re-trigger Greptile |
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
|
Closed as superseded — this work already landed on main via #4221. — Kun's Firstmate |
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 a reproduced bug in bin/fm-turnend-guard.sh's --claude cooperation path with bin/fm-claude-stop-autoarm.sh.
REPORTED SYMPTOM: A primary Claude session that never acquires this home's fleet lock (another live session holds it) gets its Stop hook blocked identically, turn after turn, with the 'TURN WOULD END BLIND' banner - observed 5+ consecutive turns with no change. Per the script's own header comment this is supposed to be bounded: FM_CLAUDE_TURNEND_BLOCK_BUDGET (default 3) consecutive blocks, then allow one loud attended fail-open only for an already verified failure episode. That bound never engaged. Direct inspection during the reproduction showed state/.turnend-claude-blocks stayed at count=1 across at least 6 consecutive Stop-hook firings for the same session id, while state/.claude-autoarm-epoch held a two-day-old claim (outcome=arming) from an owner_pid independently confirmed dead via ps.
REQUIRED WORK: (1) Read bin/fm-turnend-guard.sh in full, especially the --claude mode block described in its header comment (steps 1-3), and bin/fm-claude-stop-autoarm.sh for how it writes state/.claude-autoarm-epoch. (2) Reproduce if practical, or reason precisely from the code. (3) Fix the guard so a stale/orphaned auto-arm claim (dead owner process) is correctly treated as absent rather than pending, so the block counter advances normally toward FM_CLAUDE_TURNEND_BLOCK_BUDGET and the documented one-loud-fail-open behavior actually fires instead of looping forever. (4) Add or update tests covering: a stale claim from a dead pid does not block progress indefinitely; a genuinely fresh/live claim still behaves as before (no regression to the documented cooperation contract); the block counter increments correctly across repeated invocations when the auto-arm is genuinely absent. (5) Do NOT change the documented behavior for a live, fresh auto-arm claim - this is deliberately a narrow fix for the stale/dead-owner case specifically. Commit 10b93b2 ('fix(bin): recover Claude auto-arm from hung claims', #3156) already landed on this branch and had to be read first to determine whether it covers this exact scenario or left a distinct gap.
WHAT I FOUND AND DECIDED. Commit 10b93b2 is NOT the gap: fm_autoarm_claim_open already correctly rejects a dead owner pid, so step 2 does fall through to step 3 as designed. The actual defect is one layer down, in step 3's budget accounting. budget_account_current_epoch de-duplicated the bounded block budget on the RAW generation number parsed from state/.claude-autoarm-epoch. A superseded ledger entry never changes that number again, so every later Stop firing looked like a repeat observation of one event and the counter stopped advancing.
I reproduced it two ways against the real guard, not just by reasoning: (a) 'arming' frozen behind a provably dead owner pid pinned count at 1 across six consecutive firings - the reported symptom exactly; (b) a stale terminal 'failed' entry whose failure notice was already consumed pinned count at 0, so the documented one loud attended fail-open could never be reached however many turns passed. Case (b) is the sharper demonstration that the documented bound was simply untrue, and it is the case that now actually reaches the fail-open.
THE FIX: the budget now keys its de-duplication on whether the ledger entry is a LIVE signal for the Stop being decided - an open generation claim, a live legacy claimant, or an outcome inside the FM_CLAUDE_AUTOARM_EPOCH_FRESH window - and accounts anything else as no generation at all, exactly like an absent ledger. Behavior for a live or fresh claim is deliberately unchanged, per requirement (5).
DELIBERATE DECISION worth knowing when reading the diff: while fixing this I found the legacy live-claim proof (live pid + autoarm role + not fm_autoarm_claim_abandoned) existed in THREE copies in this file. A first attempt at the fix regressed an existing test (test_hook_claude_mode_allows_when_autoarm_owner_alive) precisely because the new live-signal predicate omitted the legacy claimant, which IS a live signal even over an old ledger entry. I therefore extracted that proof into one shared helper (autoarm_legacy_claim_live) used by all three call sites, per this repo's one-owner rule in the firstmate-coding-guidelines skill. That extraction is intentional, not incidental refactoring: it is what makes the new predicate and the two existing users provably agree.
TESTS ADDED (4, in tests/fm-turnend-guard.test.sh, colocated per repo convention): orphaned arming claim advances the bounded block budget; stale failed epoch reaches the bounded attended fail-open exactly once and not before the budget; absent-auto-arm baseline still advances once per blind turn; a live open generation claim is still accounted exactly once (the no-regression pin for requirement 5). I verified both stale-case tests FAIL against the unfixed guard and pass with the fix.
VALIDATION ALREADY RUN LOCALLY: tests/fm-turnend-guard.test.sh 87/87, tests/fm-claude-stop-autoarm.test.sh 40/40, plus fm-cursor-primary, fm-daemon, fm-guard-stale-banner, fm-omp-harness, fm-pi-watch-extension, fm-supervision-instructions, fm-calm-pi-extension all green; bin/fm-lint.sh and bin/fm-doc-audience-check.sh clean.
DOCS: docs/turnend-guard.md's test-coverage sentence was updated to name the new stale-claim accounting coverage. The script header's step-3 statement about the bounded budget needed no edit because it now describes actual behavior for the first time; the new behavior's rationale lives in the function's own comment, per the one-owner rule.
DELIBERATELY OUT OF SCOPE, reported separately rather than fixed: in the exact originally-observed scenario the session remains blocked even with this fix, because of a second distinct cause - bin/fm-claude-stop-autoarm.sh exits early when another live session holds the home lock, so a lock-refused session's auto-arm is permanently inert and can never produce the verified failure episode that failure_episode_verified requires. Unwedging that would mean changing the documented --claude contract (a guard exemption or fail-open for lock-refused sessions), which is above this task's authority and outside requirement (5)'s narrow scope. An existing test (test_hook_claude_mode_budget_without_verified_failure_keeps_blocking) pins that blocking-without-a-verified-failure-episode is the intended contract, so it must not be changed here.
What Changed
bin/fm-turnend-guard.sh:budget_account_current_epochnow de-duplicates the bounded block budget on whether the current.claude-autoarm-epochentry is a live signal for the Stop being decided (open generation claim, live legacy claimant, or an outcome still withinFM_CLAUDE_AUTOARM_EPOCH_FRESH), via a newautoarm_epoch_signal_livecheck — a stale entry (dead/orphaned owner, or a consumed terminal outcome) is now accounted as no generation at all, so the block counter advances instead of freezing.autoarmrole + notfm_autoarm_claim_abandoned) into a sharedautoarm_legacy_claim_livehelper, used byautoarm_epoch_signal_live,autoarm_owns_recovery, andterminal_fail_open.docs/turnend-guard.md: documented the accepted limitation that a session which never acquires the home's session lock can never reach the attended fail-open, and updated the test-coverage sentence to name the new stale-claim accounting coverage.tests/fm-turnend-guard.test.sh: added tests for an orphaned/dead-owner claim advancing the block budget, a stale terminal outcome reaching the bounded fail-open, the absent-auto-arm baseline still advancing once per blind turn, and a live open-generation claim still being accounted exactly once (no regression for the fresh/live-claim path).Risk Assessment
✅ Low: The fix is a narrow, well-reasoned change to one dedup key (autoarm_epoch_signal_live) whose logic I traced by hand against both reproduced failure scenarios (orphaned arming claim, stale terminal failed claim) and the required no-regression case (live open-generation claim), all of which produce correct budget-counter behavior; the shared-helper extraction is a faithful refactor verified identical at all three call sites, the new tests assert real exit codes/output/persisted state rather than source text, and the documented out-of-scope carve-out (lock-refused sessions) is honest and leaves the existing pinned contract test untouched.
Testing
Ran the two smallest relevant executable test suites (tests/fm-turnend-guard.test.sh and tests/fm-claude-stop-autoarm.test.sh) against the target commit — both fully green — and additionally reproduced the reported bug from scratch by running the target's new stale-claim tests against the unfixed base commit in an isolated throwaway worktree, where the counter pinned at count=1 exactly as reported; confirming the fix and its regression tests are genuine and effective. No UI surface is involved (bash CLI hook script), so no visual artifacts apply.
Evidence: New stale-claim test failing against unfixed base commit (1533f47) — reproduces reported symptom
Evidence: fm-turnend-guard.test.sh full run against target commit (368e02c)
Evidence: fm-claude-stop-autoarm.test.sh full run against target commit
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-turnend-guard.test.shat target commit 368e02c: 81/81 pass (one unrelated away-mode-daemon test flaked once on pid-reuse timing and passed cleanly on immediate rerun, confirmed unrelated to this diff)bash tests/fm-claude-stop-autoarm.test.shat target commit: 40/40 pass (companion cooperation-contract script, unmodified by this diff)Reproduced the reported bug directly: checked out base commit 1533f47 into a throwaway git worktree, copied over the target's updated tests/fm-turnend-guard.test.sh, and reran it against the unfixed bin/fm-turnend-guard.sh — new testtest_hook_claude_mode_stale_arming_claim_advances_block_budgetfailed with 'stale arming claim pinned the block budget at 1 on turn 2', exactly matching the reported symptom (count=1 across repeated Stop firings behind a dead-owner claim)Read the full diff of bin/fm-turnend-guard.sh confirming the fix keys epoch de-dup on autoarm_epoch_signal_live (open claim, live legacy claimant, or fresh outcome) rather than the raw generation number, and that the three call sites for the legacy live-claim proof were consolidated into one shared helper (autoarm_legacy_claim_live) as described🔧 **Document** - 1 issue found → auto-fixed ✅
docs/turnend-guard.md- The second, deliberately out-of-scope cause (lock-refused sessions leaving auto-arm permanently inert) has no tracking issue or documented limitation anywhere in docs/turnend-guard.md; worth filing a follow-up issue so it isn't lost.🔧 Fix: Document lock-refused sessions as a known fail-open limitation
✅ Re-checked - no issues remain.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.