feat(captain-hold): port upstream decision-hold to captain-hold collapse (batch 6)

[zeeshaanahmad]

Intent

Bring upstream waypoint 99b21d8 (upstream PR kunchenguid#2728, "collapse decision holds into captain holds") into this fork as batch 6 of eight, ALONE, and as a PORT rather than a text merge. Batches 1-5 have landed; batch 5's waypoint a0cec26 is already in origin/main's ancestry and is the merge base.

WHAT UPSTREAM DID. 99b21d8 renames the whole subsystem across 38 files: it adds bin/fm-captain-hold.sh (877 lines, the real implementation), docs/captain-hold-lifecycle.md, .agents/skills/captain-hold-lifecycle/SKILL.md and tests/fm-captain-hold-lifecycle.test.sh; deletes docs/decision-hold-lifecycle.md and tests/fm-decision-hold-lifecycle.test.sh; and reduces bin/fm-decision-hold.sh to a 232-line transitional shim whose own header says it will be removed in the release after the collapse. This fork's bin/fm-decision-hold.sh was instead the real 951-line implementation carrying behaviour upstream does not have.

THE REQUIRED SHAPE. A merge commit is mandatory: the waypoint must stay in the branch's ancestry because bin/fm-pr-merge.sh asserts it and refuses a batch whose waypoint is missing or no longer an ancestor. So the branch was created off origin/main, upstream was fetched, git merge 99b21d8 was run, and THE PORT WAS PERFORMED AS THE MERGE RESOLUTION - the resolved tree carries upstream's new files plus this fork's behaviours reapplied onto them. Never rebase, squash or cherry-pick this branch; a flattened head must never be pushed. The batch stops exactly at 99b21d8: 3f03533/4d2cb0c/d3342dc/3d125ad are batch 7 and are deliberately NOT in ancestry.

THE STANDING RULE THIS SATISFIES (captain's rule of 2026-08-17). Take upstream's version, but PROVE it covers the behaviour ours covered, and keep ours only for the examined gap, saying so. Upstream's fm-captain-hold.sh is the base and upstream's three genuine improvements in this region (the one general keyed-answer path wiring fm-procevent.sh, fm-procevent-lavish.sh and fm-send.sh together) are taken as written, not reimplemented.

THE SEVEN CONFLICTS AND THEIR DELIBERATE RESOLUTIONS. bin/fm-decision-hold.sh and .agents/skills/decision-hold-lifecycle/SKILL.md were resolved to upstream verbatim (both are byte-identical to 99b21d8) because the retired implementation and the retired policy must not survive as stale second owners of one contract. bin/fm-procevent.sh, bin/fm-send.sh and bin/fm-test-run.sh were resolved by COMBINING both sides. The two modify/delete conflicts (docs/decision-hold-lifecycle.md, tests/fm-decision-hold-lifecycle.test.sh) took upstream's deletion, with their fork-unique content carried into the new doc and the new test rather than dropped.

WHAT HAD TO SURVIVE, AND HOW EACH WAS RESOLVED. (1) Fork PR 5 (a5d8718), the precondition recheck after resolve, was PORTED because upstream has no equivalent anywhere. Its policy half became a sentence plus operating-sequence steps 7-10 in .agents/skills/captain-hold-lifecycle/SKILL.md, and its advisory-line half became print_precondition_reminder/print_answer_outcome in bin/fm-captain-hold.sh, fired from all six command_answer exits. It was deliberately NOT put in the shim's command_resolve even though that is the literal 1:1 site, because the shim is scheduled for deletion one release from now and that would have re-lost the behaviour on a timer; putting it in answer also makes the shim's resolve inherit it for free. It reads the DECLARED blocked-by edges via tasks-axi list --fields deps rather than the live blocked_by set, because tasks-axi done resolves blocked_by to none while deps retains blocked-by: - that is what lets an idempotent replay name the same freed work the closing run did, which is exactly the property the fork's resolve replay path had. The scan is advisory and deliberately never fails an answer. (2) Fork PR 3 (6d074ec) split: its multi-key fold in bin/fm-classify-lib.sh auto-merged untouched, and its --resolve-key half was ported onto upstream's rewritten refusal text so both the new "no captain-held task '' or '-decision-' still open" wording and the fork's "Key(s) actually open for this task right now" diagnostic are present. (3) 918709d's stdin-leak fix needed NO port and this was proven rather than assumed: upstream's command_answers spawns exactly one child and already writes </dev/null on it, and the second child site that batch 5's D2 fix extended - the decline --drop branch - no longer exists at all, because upstream replaced the reserved drop answer with a fourth close-mode column; git grep __drop__ is empty across the merged tree. (4) 918709d's corrupted-binding diagnostics were already covered in the producer (upstream's read_binding fails hard on an unsafe path, wrong schema or invalid origin) but NOT in the consumer, so the fork's loud forwarding in fm-procevent.sh feed_keyed_answers was kept. (5) PR 29's fixture-cleanup guard (|| exit 1 on the tests/lib.sh source line, so a file copied out of tests/ cannot run with undefined helpers and rm -rf its own working directory) was carried onto tests/fm-captain-hold-lifecycle.test.sh, which is now inside tests/fm-test-fixture-cleanup.test.sh's sweep. (6) Compatibility with this home's LIVE records: the main home has open rows in the pre-collapse -decision- shape, so resolve/answer, complete, verify and Bearings must still read and close those exact records after the port.

TESTS ADDED. Three regressions in tests/fm-captain-hold-lifecycle.test.sh carry the ported behaviours: the freed-work reminder (naming exactly the gated work and nothing else, surviving an idempotent replay, naming the resumed item on --release, and printing nothing when the close frees nothing); the corrupted-binding diagnostic forwarding (with a genuinely unbound source alongside it proving the loud path is specific to corruption); and a pre-collapse row and binding created only through the retired command surface that then answers, feeds, completes, verifies and leaves Captain's Call through the collapsed surface alone. Both ported mechanisms were mutation-checked - neutering them turns the corresponding test red - and the scripts were restored byte-for-byte afterwards.

CALLERS. Every caller in this repo was checked with grep -rn 'fm-decision-hold' AGENTS.md bin .agents docs tests. AGENTS.md, bin/fm-brief.sh, bin/fm-teardown.sh and bin/fm-classify-lib.sh were repointed by upstream itself. Two callers remain on the shim, tests/fm-bearings-board.test.sh and tests/fm-cmux-claude-composer-live-e2e.test.sh, and both are DELIBERATELY left alone: they are byte-identical to upstream, upstream kept them on the shim on purpose to keep it covered, and repointing them here would buy nothing while costing a permanent conflict on every future sync.

REPO STYLE CONTEXT FOR THIS DIFF. This repo requires one full sentence per line in tracked Markdown and a plain dash rather than an em dash, BUT a file carried verbatim from upstream is exempt for as long as it is held byte-identical, because reformatting it trades a permanent conflict on every future sync for cosmetics. bin/fm-decision-hold.sh and .agents/skills/decision-hold-lifecycle/SKILL.md are byte-identical to upstream and must stay that way - do not reflow, restyle or "improve" them. The four upstream files the port modified are no longer byte-identical, so the sentence-per-line rule does apply to them; one upstream line in .agents/skills/captain-hold-lifecycle/SKILL.md was split for exactly that reason in a follow-up commit. Every other upstream-carried Markdown file in this merge that was not modified must stay byte-identical too.

VALIDATION ALREADY RUN LOCALLY, ALL GREEN. tests/fm-captain-hold-lifecycle.test.sh 18 ok; fm-send-resolve-key 16 ok; fm-procevent 42 ok; fm-procevent-when 13 ok; fm-bearings-board 7 ok; fm-bearings-snapshot 42 ok; fm-fleet-snapshot-view 15 ok; fm-brief 25 ok; fm-teardown 61 ok; fm-teardown-endpoint-safety 7 ok; fm-classify-decision-key 17 ok; fm-wake-drain-open-decisions 10 ok; fm-test-fixture-cleanup 12 ok; fm-documentation-audiences 4 ok. bin/fm-test-run.sh --check-coverage reports total=164 with the deleted test removed from the table and the new one registered. bin/fm-doc-audience-check.sh reports ok surfaces=74 local_links=269. bin/fm-lint.sh is clean on pinned ShellCheck 0.11.0 and actionlint 1.7.12. bin/fm-test-run.sh --proven-isolated --jobs 8 reports total=24 failed=0. A throwaway-home smoke reproduced this home's two exact live legacy identities through the old surface only and drove them to closed through the new surface alone.

KNOWN PRE-EXISTING RED, NOT CAUSED BY THIS BRANCH. tests/fm-watcher-lock.test.sh's guard-xmode case ("guard repair line did not source the X-mode cadence config") is red locally on main, is filed separately, and is green in CI because it depends on a locally registered Claude Stop hook. Herdr-gated tripwire failures in this environment are environmental. Neither is a finding against this change.

CONSTRAINTS. Never push to the upstream remote; origin only. Never add an agent name as a commit co-author. Do not modify anything under projects/, data/, state/ or config/.

What Changed

• Merged upstream waypoint 99b21d8 (PR feat: collapse decisions into tasks held for the captain kunchenguid/firstmate#2728), which collapses the decision-hold subsystem into bin/fm-captain-hold.sh (new, the real implementation), adds docs/captain-hold-lifecycle.md and .agents/skills/captain-hold-lifecycle/SKILL.md, and reduces bin/fm-decision-hold.sh and .agents/skills/decision-hold-lifecycle/SKILL.md to byte-identical transitional shims of upstream's version (deleting docs/decision-hold-lifecycle.md and tests/fm-decision-hold-lifecycle.test.sh, replaced by tests/fm-captain-hold-lifecycle.test.sh).
• Ported fork-only behaviors onto the new upstream base during merge resolution: the precondition-recheck reminder (freed-blocked-work notice) as print_precondition_reminder/print_answer_outcome in fm-captain-hold.sh plus new operating-sequence steps in the captain-hold-lifecycle skill; corrupted-binding diagnostic forwarding in fm-procevent.sh's feed_keyed_answers; and the --resolve-key diagnostic wording in fm-send.sh, combined with upstream's rewritten refusal message and its new task-id-or-legacy-key resolution logic.
• Repointed all in-repo callers (AGENTS.md, bin/fm-brief.sh, bin/fm-teardown.sh, bin/fm-classify-lib.sh, bin/fm-test-run.sh, .github/workflows/ci.yml test count) from fm-decision-hold.sh/decision-hold-lifecycle to fm-captain-hold.sh/captain-hold-lifecycle, while two upstream-owned callers were deliberately left on the shim; added tests/fm-captain-hold-lifecycle.test.sh regressions for the ported reminder and corrupted-binding behaviors plus a pre-collapse-row compatibility case, and made a small follow-up documentation/count correction on top of the merge.

Risk Assessment

✅ Low: This fix round's changes are confined to comments, docs, and one new well-constructed regression test that verifies real behavior (task state, stdout suppression across the answer vs answers paths) rather than source text; the previously requested fixes (reminder-scope wording, five-exits count) were applied accurately and match the actual code paths, byte-identical upstream files remain untouched, and the only new issue found is a cosmetic stale count in prose.

Testing

Ran the test suites covering every conflict-resolution surface named in the port's intent (captain-hold lifecycle, send --resolve-key, classify-decision-key, bearings-board, fixture-cleanup guard, procevent) — all passed — and independently confirmed via git the merge is a genuine two-parent merge with the waypoint in ancestry, the shim files are byte-identical to upstream, the drop path is gone, and the three named callers were repointed off the retired surface; a manual mutation check on the ported freed-work reminder confirmed the new regression test actually detects its absence (then the file was restored byte-for-byte). No issues found.

Evidence: fm-captain-hold-lifecycle.test.sh — full behavioral test list (19/19 ok)

ok - report-only unresolved captain call is reproduced and completion refuses before loss
ok - the completion gate attests captain-held inventory and transfers open status decisions
ok - answer records the captain's words, closes idempotently, and releases routed work
ok - answering names the work it frees so its real preconditions get re-checked
ok - the keyed-answer intake stays silent on the reminder while a direct answer still prints it
ok - release frees held work with the captain's words recorded and the body preserved
ok - a deferred captain call leaves the live Captain's Call until its date and stays answerable
ok - an out-of-band close is recordable with the captain's word and nothing else
ok - ended visual review follows the same captain-hold completion owner
ok - resolved findings and decision-like prose do not create captain-held tasks
ok - terminal single-owner stale status decisions do not block empty inventory
ok - main-home and secondmate-home captain calls remain correctly routed
ok - a bound channel's captured answers close their captain-held tasks at answer time
ok - a channel source with no decision binding closes nothing
ok - a corrupted binding record forwards its diagnostic instead of silently acting as unbound
ok - legacy identities, metadata, bindings, and the shim keep working
ok - rows and bindings created through the old surface close through the new one
ok - the chat channel feeds the same keyed-answer intake a captured review does
ok - completion and verification validate origins before constructing paths
FM_TEST_END exit=0 duration_ms=88413

Evidence: Mutation check: neutered freed-work reminder turns the test red, then restored

$ (edited print_precondition_reminder to `return 0`)
$ bash bin/fm-test-run.sh tests/fm-captain-hold-lifecycle.test.sh
ok - report-only unresolved captain call is reproduced and completion refuses before loss
ok - the completion gate attests captain-held inventory and transfers open status decisions
ok - answer records the captain's words, closes idempotently, and releases routed work
not ok - answering did not remind the caller to re-check the freed work's real preconditions: answered: sample-route-call (missing: 'recheck freed task preconditions per .agents/skills/captain-hold-lifecycle/SKILL.md:')
FM_TEST_END exit=1

$ git checkout -- bin/fm-captain-hold.sh
$ git diff HEAD -- bin/fm-captain-hold.sh | wc -l
0

Evidence: Merge structure and byte-identical shim verification

$ git merge-base --is-ancestor 99b21d8 HEAD && echo OK
OK
$ git diff 99b21d8 HEAD -- bin/fm-decision-hold.sh | wc -l
0
$ git diff 99b21d8 HEAD -- .agents/skills/decision-hold-lifecycle/SKILL.md | wc -l
0
$ git show --no-patch --format='%H %P' 1101d8e
1101d8e141c6b405c33b847739c7a1b85e4cf00d e3fbc9ee8429199233642c466dcf9fe17d59c25f 99b21d82db4ecd02f3dbbe0a1d49e2d8c5630351

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

⏭️ **Rebase** - skipped

Step was skipped.

⚠️ **Review** - 1 info

• ⚠️ bin/fm-captain-hold.sh:818 - The ported freed-work reminder (print_precondition_reminder/print_answer_outcome) only reaches anyone when fm-captain-hold.sh answer is invoked directly at a terminal. Every real automated channel closes calls through the plural command_answers instead: its per-key "$0" answer ... subprocess call redirects stdout to /dev/null (line 818), and both real callers additionally wrap the whole answers invocation in >/dev/null 2>&1 (bin/fm-send.sh:507-508 for chat --resolve-key, bin/fm-procevent.sh:196-197 for captured-result adapters and the Lavish bearings board). This mirrors the pre-existing fire-and-forget suppression already used for the old fm-decision-hold.sh answers calls, so it is not a new regression, but it does mean the newly-ported advisory reminder - which .agents/skills/captain-hold-lifecycle/SKILL.md's operating-sequence step 7 frames as something answer provides after any freeing close ("answer names those tasks for you") - is in practice unreachable through the two real-world paths a captain answer actually arrives on (chat, the Lavish board/procevent), and only ever displays for an agent that happens to run the interactive answer command by hand. Worth confirming whether this narrower, interactive-only scope was the deliberate intent, since it limits the ported feature's practical value for its most common real use.
• ℹ️ bin/fm-captain-hold.sh:531 - The comment ("the reminder cannot be forgotten on one of its six exits") and the PR description both claim the reminder fires from six command_answer exits. command_answer actually has exactly five successful return paths that call print_answer_outcome (lines 572, 586, 605, 613, 624); there is no sixth. Not a functional gap - every real successful exit is covered - just a stale count.
• ℹ️ bin/fm-captain-hold.sh:864 - Two minor error-handling rough edges surfaced under adversarial/malformed input, neither affecting a documented golden path: (a) a dangling flag value, e.g. fm-captain-hold.sh answer <id> --decision-file with no path following it, exits 1 with zero diagnostic output because set -e trips mid-arg-parse on the trailing shift; (b) verify_hold_durable "$(resolve_entry ...)" (command_complete:864, command_verify:918) loses resolve_entry's fail message inside the command substitution when a key doesn't resolve, so execution falls through with an empty id and prints a second, more confusing error instead of one clean message.

🔧 Fix: Fix reminder docs to reflect answers-intake scope; correct five-exit count
1 info still open:

• ℹ️ docs/captain-hold-lifecycle.md:81 - This fix round inserted a new regression-description sentence ('A further regression proves the keyed-answer intake's silence is deliberate...', line 83) into the Verification record's paragraph, but left the paragraph's lead-in unchanged: 'Three further regressions carry behaviors this fork had before the collapse.' The paragraph now describes four regressions (routed-call reminder naming at line 82, keyed-answer intake silence at line 83, corrupted-binding diagnostic at line 84, pre-collapse row/binding at line 85), not three. Same class of stale-count slip as the already-fixed six-exits-miscount finding from the prior round. Purely a documentation accuracy issue; change 'Three' to 'Four'.

✅ **Test** - passed

✅ No issues found.

• bash bin/fm-test-run.sh tests/fm-captain-hold-lifecycle.test.sh — 19/19 ok, covering the freed-work reminder, corrupted-binding forwarding, and legacy pre-collapse row/binding compatibility through the new surface
• bash bin/fm-test-run.sh tests/fm-send-resolve-key.test.sh tests/fm-classify-decision-key.test.sh tests/fm-bearings-board.test.sh — 16/16, 17/17, 7/7 ok respectively; fm-bearings-board exercises one of the two callers deliberately left on the fm-decision-hold.sh shim
• bash bin/fm-test-run.sh tests/fm-test-fixture-cleanup.test.sh — 12/12 ok, including the 128-file lib.sh-sourcing sweep that now covers the new tests/fm-captain-hold-lifecycle.test.sh
• bash bin/fm-test-run.sh tests/fm-procevent.test.sh — 42/42 ok ('all procevent tests passed'), covering the combined-conflict-resolution file including corrupted-binding loud forwarding
• bash bin/fm-test-run.sh --check-coverage — total=164, confirms the deleted test is out and the new test is registered
• git merge-base --is-ancestor 99b21d8 HEAD and absence-check on 3f03533/4d2cb0c/d3342dc/3d125ad — waypoint ancestry required by fm-pr-merge.sh holds, batch 7 waypoints are not pulled in
• git diff 99b21d8 HEAD -- bin/fm-decision-hold.sh .agents/skills/decision-hold-lifecycle/SKILL.md — 0 lines, confirming byte-identical retirement of the shim and its policy doc
• git show --no-patch --format='%H %P' 1101d8e — confirms a true two-parent merge commit (e3fbc9e + 99b21d8), not a squash/rebase/cherry-pick
• git grep -n __drop__ — empty, confirming the reserved drop answer path (and its associated stdin-leak fix) no longer exists post-port
• manual mutation check: neutered print_precondition_reminder() in bin/fm-captain-hold.sh, reran the lifecycle suite (the reminder test went red: "answering did not remind the caller to re-check..."), then git checkout -- bin/fm-captain-hold.sh and confirmed git diff HEAD is empty — proves the ported-behavior test is real, not vacuous, and the file was restored byte-for-byte
• grep -rln 'fm-decision-hold' AGENTS.md bin .agents docs tests and grep -n 'captain-hold-lifecycle' bin/fm-brief.sh — confirms bin/fm-brief.sh, bin/fm-teardown.sh, bin/fm-classify-lib.sh were repointed away from the retired surface, leaving only the two documented shim callers plus expected self/doc references

✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.