feat(bin): enforce the cipher needs-decision hook and durably close answered decisions - #22
Merged
morris2spears merged 3 commits intoAug 31, 2026
Conversation
…needs-decision hook bypasses A genuine keyed needs-decision must enter the Cipher hook before firstmate answers it in any form; the iinvy#292 event was settled by filing a follow-up issue and reporting the scope choice without the hook ever running, and its keyed status line stayed open afterwards (firstmate#21). - cipher-hook skill and AGENTS.md now name filing a decision-bearing follow-up, keeping the current PR scoped, and reporting the choice as settled as forms of answering that may not precede the hook. - new `fm-cipher-hook.sh resolve-decision <task-id> <request-id>` appends the durable keyed `resolved` closure once the authenticated Cipher decision comment is recorded, refuses without it, and is idempotent; the skill's durable-answer procedure now ends with it. - the keyed-decision fold honors the key token written at the start of the note (the observed real-world placement), so open and close events match their intended key instead of degrading to "default", and the generated brief now shows the canonical verb-adjacent placement. - regression tests prove a current keyed decision emits exactly one authenticated hook and parks unanswered, the disabled-route fallback stays exit 3, an unacknowledged or comment-less request refuses durable closure, and instruction owners keep the follow-up branch wording. Closes #21 Claude-Session: https://claude.ai/code/session_01W5thFUb7qQqQPpNc9Uyupv
morris2spears
deleted the
fm/firstmate-issue21-needs-decision-hook-enforcement
branch
August 31, 2026 23:30
This was referenced Aug 31, 2026
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 firstmate issue #21: route genuine needs-decision findings through Cipher before answering or filing follow-ups. Observed incident: task iinvy-issue292-resolve-transactionless-entity emitted 'needs-decision: [key=issue292-cross-role] ...' and the primary session settled it (kept kunchenguid#292 scope, filed follow-up iinvy issue kunchenguid#293, reported the choice to the captain) without ever running bin/fm-cipher-hook.sh needs-decision; the keyed status line then stayed open stale, and was only closed by a manual resolved append. Deliberate changes: (1) AGENTS.md needs-decision trigger and the cipher-hook skill now define answering to include filing a decision-bearing follow-up issue, keeping the current PR scoped around the question, and reporting the choice as settled - none may precede the hook. (2) New 'fm-cipher-hook.sh resolve-decision ' subcommand (shell + python resolve_decision_plan) durably appends 'resolved [key=]: Cipher decision accepted ' to the task status - it deliberately refuses (exit 1) unless the acknowledged needs-decision request AND the committed authenticated decision-comment receive record both exist, so a decision can never be marked Cipher-answered without the durable GitHub answer; it is idempotent (silent exit 0 when the keyed decision is already closed). The cipher-hook skill's durable-GitHub-answer procedure now ends with this command. (3) fm-classify-lib.sh keyed-decision fold now also honors a key token written at the start of the note ('needs-decision: [key=x] summary' - the placement the real incident used), so open/close events match their intended key instead of silently degrading to 'default'; a leading bracket that is not a valid key slug stays ordinary prose (falls back to default, deliberately NOT skipping the line). (4) bin/fm-brief.sh rule 6 (ship and scout variants) now shows the canonical placement 'needs-decision [key=]: {summary}' with the token between verb and colon. (5) Regression tests: tests/fm-cipher-hook.test.sh gains test_note_keyed_decision_single_hook_and_park (a note-keyed current decision emits exactly one authenticated HMAC-V2 hook with its intended key, dedupes on replay, and the hook never mutates the status file or touches GitHub - it parks) and test_resolve_decision_requires_and_follows_authenticated_answer (disabled decision route still exits 3 to the existing authority; resolve-decision refuses unacknowledged and comment-less requests; closes durably and idempotently after the authenticated comment). tests/fm-instruction-owners.test.sh pins the new instruction wording. Constraints honored: one sentence per line in tracked Markdown, plain dash only, shellcheck-clean via bin/fm-lint.sh, colocated tests extending existing suites, usage() sed range updated for the longer header (2,47). The disabled-route exit-3 fallback and held-delivery fail-closed/supersede behavior are intentionally unchanged. PR must close issue #21 and is NOT auto-merged even when green (standing note from issue #14): report checks green and stop; Cipher/the captain decide the merge.
What Changed
bin/fm-cipher-hook.sh resolve-decision <task-id> <request-id>(with aresolve_decision_planinbin/fm-cipher-hook.py) that appendsresolved [key=<decision-id>]: Cipher decision accepted <comment-url>to the task status, refuses with exit 1 unless both the acknowledged needs-decision request and the committed authenticated decision-comment receive record exist, and exits 0 silently when the keyed decision is already closed.bin/fm-classify-lib.shkeyed-decision fold to honor a key token written at the start of the note (needs-decision: [key=x] summary), so open and close events match their intended key instead of degrading todefault; a leading bracket that is not a valid key slug stays ordinary prose and still folds todefault, and a bareresolved:line still closes a note-placed keyed decision for backward compatibility.bin/fm-brief.shrule 6 shows the canonicalneeds-decision [key=<decision-slug>]: {summary}placement, and new cases intests/fm-cipher-hook.test.sh,tests/fm-watch-triage.test.sh, andtests/fm-instruction-owners.test.shcover the note-keyed single hook and park, the resolve-decision refusals and idempotent closure, and the new wording.Closes #21.
Risk Assessment
✅ Low: The fold change is now backward compatible with every existing status file (verified by hand-tracing legacy note-placed, explicit-keyed, reopened, and activity-phase cases plus the pre-existing keys.status case), the resolve-decision subcommand is fail-closed on both the acknowledged request and the authenticated comment record, and the only remaining issue is a non-behavioral duplication cleanup.
Testing
Ran the targeted suites for every touched surface (cipher hook, classify-lib keyed-decision fold, instruction owners, generated briefs, and the fleet-snapshot and decision-nudge consumers of the fold) - all green - and, because passing units alone would not show the incident being prevented, drove the real scripts end-to-end against a live HMAC-verifying localhost gateway using the exact status line from the iinvy-issue292 incident. That transcript shows the note-placed key folding toissue292-cross-roleinstead of the olddefault, one authenticated HMAC-V2 delivery with that key that dedupes on replay and neither mutates the status file nor touches GitHub,resolve-decisionrefusing (exit 1, "no authenticated Cipher decision comment is recorded") before the answer exists, and a single durableresolved [key=...]: Cipher decision accepted <comment-url>line after the authenticated comment arrives, with the replay silently doing nothing. The change is CLI/instruction-facing with no rendered UI surface, so the reviewer-visible evidence is CLI transcripts and the generated brief text rather than screenshots. The worktree is clean of test artifacts.Evidence: End-to-end incident replay (before/after fold, hook delivery, refusal, durable closure)
=== the worker's status file (real incident placement) === working: reviewing transactionless entity scope needs-decision: [key=issue292-cross-role] keep #292 scoped to the entity, or absorb the cross-role history change here === BEFORE this change: which decision key does the fold see? === key=default verb=needs-decision === AFTER this change: the fold honors the intended key === key=issue292-cross-role verb=needs-decision === firstmate routes the finding through Cipher (never answers it) === exit=0 -- replay (dedupes, no second delivery) -- exit=0 -- authenticated deliveries seen by the gateway -- decision_id=issue292-cross-role event=needs-decision hmac_v2_valid=True timestamp_fresh=True -- status file untouched by the hook (worker still parked, finding unanswered) -- working: reviewing transactionless entity scope needs-decision: [key=issue292-cross-role] keep #292 scoped to the entity, or absorb the cross-role history change here -- github activity attempted by the hook -- (none) === firstmate tries to settle the decision BEFORE Cipher's durable GitHub answer === $ bin/fm-cipher-hook.sh resolve-decision iinvy-issue292-resolve-transactionless-entity fmch-v1-41cf4139... error: Cipher hook refused: no authenticated Cipher decision comment is recorded for this request exit=1 -- status file still shows the decision open -- issue292-cross-role needs-decision [key=issue292-cross-role] keep #292 scoped to the entity, or absorb the cross-role history change here === Cipher answers with an authenticated decision comment on GitHub === queued: fmcr-v1-00daebf9... $ bin/fm-cipher-hook.sh resolve-decision iinvy-issue292-resolve-transactionless-entity fmch-v1-41cf4139... resolved iinvy-issue292-resolve-transactionless-entity issue292-cross-role exit=0 -- durable closure appended to the task status -- resolved [key=issue292-cross-role]: Cipher decision accepted https://github.com/example/iinvy/issues/292#issuecomment-9921 -- open decisions now -- (none - the keyed decision is durably closed) -- replay is idempotent -- $ bin/fm-cipher-hook.sh resolve-decision iinvy-issue292-resolve-transactionless-entity fmch-v1-41cf4139... exit=0 (silent, no second line) resolved-lines=1Evidence: Generated worker brief rule 6 (ship + scout) and fm-cipher-hook.sh usage
=== ship-variant brief rule 6 (generated by bin/fm-brief.sh) === 6. If a decision belongs above the implementation worker (product choices, destructive actions, ask-user findings), appendneeds-decision [key=<decision-slug>]: {summary of options}- keep the[key=...]token between the verb and the colon - and stop. Firstmate will apply the configured authority and reply with the decision. === scout-variant brief rule 6 === 6. If a decision belongs to a human (product choices, destructive actions), appendneeds-decision [key=<decision-slug>]: {summary of options}- keep the[key=...]token between the verb and the colon - and stop. Firstmate will reply with the decision. === bin/fm-cipher-hook.sh usage (sed range 2,47 renders the new block) ===resolve-decisiondurably closes the answered keyed status decision for an acknowledged needs-decision event. It refuses unless the authenticated decision-comment receive record for that exact request exists, then appends one idempotent "resolved [key=<decision-id>]: Cipher decision accepted <comment-url>" status line while the keyed decision is still open. Usage: fm-cipher-hook.sh needs-decision <task-id> [decision-id] fm-cipher-hook.sh pr-ready <task-id> <pr-url> fm-cipher-hook.sh retry-held fm-cipher-hook.sh resolve-decision <task-id> <request-id> ...Evidence: tests/fm-cipher-hook.test.sh output
Evidence: tests/fm-watch-triage.test.sh output
Evidence: tests/fm-instruction-owners.test.sh output
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-classify-lib.sh:192- The new note-placement key fallback changes the reading of status files that already exist on disk. A historical line 'needs-decision: [key=x] ...' previously folded to key 'default' and was closed by a bare 'resolved:' line; it now folds to key 'x', so that bare closure no longer matches and the decision re-opens on the next fold. Reachable consequences with no new writes: bin/fm-decision-hold.sh:359 fails with 'open structured decision <task>/x has no captain-held inventory entry' because the captain-hold inventory for that origin was recorded under the old 'default' key, and bin/fm-afk-return.sh:76 reports the same decision as a live blocker again. The incident task described in the intent is precisely this shape (note-placed key, closed by a manual 'resolved' append). Either accept the re-open as intended migration cost, or make a bare resolved/captain-held event also close a note-placed key opened before this change.🔧 Fix: keep bare resolved closing note-placed keyed decisions
1 info still open:
bin/fm-classify-lib.sh:211- _fm_decision_key_placement re-implements the whole bracket-parsing body of _fm_decision_key (prefix scan, note fallback, slug validation) just to report where the token sat, and both folds now call it once per status line in addition to _fm_decision_key - so every line costs two extra subshells (the placement helper plus its own status_line_note call) in a hot per-task fold used by fm-fleet-snapshot, fm-decision-hold, and fm-afk-return. Having _fm_decision_key emit '<key>\t<placement>' (or a paired _fm_decision_key_parse) and splitting once at the call sites in status_open_decisions:281 and _fm_status_open_activities_stream:325 removes the duplicate grammar - which otherwise has to be kept in sync by hand - and halves the fork count. Behavior is correct as written; this is cleanup only.✅ **Test** - passed
✅ No issues found.
bash tests/fm-cipher-hook.test.sh(includes the two new tests: note-keyed single hook and park, resolve-decision requires the authenticated answer)bash tests/fm-watch-triage.test.sh(keyed-decision fold, note-placed keys, bare-close backward compatibility)bash tests/fm-instruction-owners.test.sh(new AGENTS.md / cipher-hook / brief wording pins)bash tests/fm-brief.test.sh(brief scaffolds after the rule 6 change)bash tests/fm-decision-hold-lifecycle.test.sh,bash tests/fm-fleet-snapshot-view.test.sh,bash tests/fm-decision-nudge.test.sh(consumers of the open-decision fold)Manual end-to-end CLI replay of the incident against a live localhost HMAC-verifying Cipher gateway: base-vs-target fold comparison,bin/fm-cipher-hook.sh needs-decision(delivery + replay dedupe + park + no GitHub writes),bin/fm-cipher-hook.sh resolve-decisionrefusal before the answer,bin/fm-cipher-receive.sh decision-comment, then durable and idempotent closureManual render of the generated worker brief (ship and scout variants) to confirm the canonicalneeds-decision [key=<decision-slug>]:placement, andbin/fm-cipher-hook.sh --helpto confirm the widened usage() sed range shows the new subcommand✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.