feat(bin): add decision-hold answer-time closure and workflow lint gate

[zeeshaanahmad]

Intent

Batch 2 of the eight-batch sequenced upstream merge described in data/map-upstream-firstmate-for-reconciliation/report.md section 6.1, which is the authoritative map. Batch 1 landed as fork PR #20 (e518906). Batch 2 merges the contiguous upstream prefix through d843712 (upstream PR kunchenguid#2570): 9 commits, mapped at about +5 files / +12 hunks of new conflicts, LOW-MEDIUM risk.

STANDING CONSTRAINTS FOR THIS WORK (deliberate, not accidental):

• MERGE, never rebase, never cherry-pick. This is a captain rule from 2026-08-12. Upstream is landed as contiguous prefixes merged at waypoints, so the result is deliberately a two-parent merge commit with a large diff, not a curated patch series. Ancestry preservation is the point: firstmate will merge the resulting PR with -- --merge precisely so a squash cannot flatten it. Do not propose squashing, rebasing, cherry-picking, or splitting this into smaller commits.
• Push to the FORK (origin, zeeshaanahmad/firstmate) only. Pushing to the upstream remote kunchenguid is DISALLOWED.
• This repo HAS working GitHub Actions; the ci step must run and be monitored, not skipped.
• Conflicts must be resolved with a recorded reason per file rather than a blanket take-one-side rule. Because this is the firstmate repo, AGENTS.md, bin/, .agents/skills/, docs/ and tests/ are shared tracked surface under a one-owner rule: where both sides state a contract, the merged result must state it exactly once.

THE PROOF PASS, which was the substantive work of this batch:
The map classified upstream 362c508 (kunchenguid#2490, keyed-answer path) as a DUPLICATE of fork PRs #3 and #5, and separately recorded as an UNVERIFIED CLAIM that it could not tell whether kunchenguid#2490 covers fork PR #5's precondition-recheck behaviour. Both questions were required to be settled by content, with the evidence recorded, before any conflict was resolved. Both were settled, and both went the opposite way from the map's label:

• The fork does NOT already carry fix(decisions): close decision holds at answer time via one general keyed-answer path kunchenguid/firstmate#2490. The fork's bin/fm-decision-hold.sh exposed only complete/resolve/decline; upstream adds answer, answers, bind, unbind and binding. The only two "answers" matches in the fork's bin/fm-send.sh and bin/fm-procevent.sh are English prose inside comments, not the capability. fix(decisions): close decision holds at answer time via one general keyed-answer path kunchenguid/firstmate#2490 is therefore genuinely new capability, not a duplicate.
• Upstream does NOT carry fork PR fix(bin): require precondition recheck after decision-hold resolve #5. print_precondition_reminder occurs 3 times in the fork and 0 times upstream, and upstream's skill operating sequence contains no precondition-recheck steps.
  Consequence, and the governing decision for every conflict in this batch: the two sides are complementary, so each conflict was resolved as a UNION that preserves both contracts stated exactly once. This is deliberately NOT a side-pick.

PER-FILE RESOLUTIONS AND WHY (all deliberate):

• bin/fm-send.sh: took upstream's case body, which records each key into RESOLVE_STATUS_KEYS. That is load-bearing rather than cosmetic: the close path consumes that variable, so keeping the fork's no-op branch would have left it empty and silently closed no decisions at all. Fork PR fix(bin): show registered decision key and fold every stated key #3's diagnostic value (naming the keys actually open for the task) was folded into upstream's single surviving refusal, so the refusal contract is stated once and the pre-existing explanatory comment above it remains true. The two-branch if/else on resolve_open_keys is intentional: it distinguishes "other keys are open" from "nothing is open".
• .agents/skills/decision-hold-lifecycle/SKILL.md: took upstream's step 7, which names the new answer command and the already-closed-by-channel case, and kept fork PR fix(bin): require precondition recheck after decision-hold resolve #5's steps 8 through 11 that upstream lacks. Two integration edits were required by the merge itself and are intentional: answer was added to the unrouted list in step 8 because it routes no work and so must not trigger the precondition recheck, and upstream's cross-reference "confirm it in step 8" was renumbered to step 12 to match the merged numbering.
• docs/architecture.md: took upstream's evidence-storage description on the FACTS, not on policy preference. The fork's sentence said validation evidence stays in the crew branch; that is stale. Verified empirically that no-mistakes/evidence exists on both remotes, shares no common ancestor with main (a real orphan branch), and holds the evidence files, while the fork's own main tracks zero .no-mistakes/ paths. Upstream's docs: reconcile test evidence storage documentation kunchenguid/firstmate#2548 and docs: clarify test evidence branch storage kunchenguid/firstmate#2549 corrected exactly this. The fork's merge-time static guard clause was deliberately KEPT in the same sentence, because that guard is fork-unique and the immediately following sentence already depends on it having been introduced.
• bin/fm-brief.sh: kept the fork's structured 6-part scout report contract and added upstream's new hint (feat: let live scouts host Lavish review loops kunchenguid/firstmate#2563) that a live scout may host its own Lavish review loop. These are orthogonal additions, so neither side loses anything; the fork's structure was not replaced by upstream's one-line prose version.
• docs/decision-hold-lifecycle.md: the conflict was a recorded verification-evidence line where each side had recorded a count measured on its own tree. Neither number is correct for the merged tree, so rather than picking a side the check was re-run and the measured result recorded (surfaces=70 local_links=259, against the fork's 69/257 and upstream's 68/253). The actionlint lines were added because the newly merged workflow gate now emits them.

WHAT MUST NOT BE REGRESSED:
Fork PR #18's pause_state_class rewrite fixes a false-stale bug upstream has not fixed. Batch 2 deliberately does not touch that region and this merge leaves it untouched. Separately, two captain decisions recorded 2026-08-24 govern LATER batches only and must not be applied here: taking upstream's watch-triage de-flake (batch 4) and adopting upstream's decision-hold to captain-hold rename (batch 6).

SCOPE:
Batch 2 only. MERGE_HEAD is exactly d843712 and nothing beyond it was pulled in; the batch deliberately does not stop short at upstream commit 6 (88d0f2e), which the map measured as costing more conflicts than going one commit further.

VERIFICATION ALREADY PERFORMED:
bin/fm-lint.sh exits 0 with ShellCheck 0.11.0 (pinned) and actionlint 1.7.12 (pinned), 3 workflow files valid. Ten test suites exit 0, including fm-decision-hold-lifecycle, fm-send-resolve-key, fm-brief, fm-teardown, fm-crew-state, fm-peek-remote, fm-send-remote-delivery, fm-lint-workflows, fm-fleet-snapshot-view and fm-bearings-snapshot. Both sides' behaviour is proven by independently authored passing tests: "a bound channel's captured answers close their captain holds at answer time" for upstream's new path, and "a refusal names the key(s) actually open for this task" for fork PR #3's diagnostic. bin/fm-decision-hold.sh auto-merged without a conflict marker and was checked semantically: fork PR #5's reminder survives at both call sites, both are inside command_resolve only, and all five upstream subcommands are present. CLAUDE.md converting from a symlink to the @AGENTS.md pointer file is upstream kunchenguid#2512 and is intended; CI's pointer check from kunchenguid#2515 passes locally on all three assertions.

What Changed

• bin/fm-decision-hold.sh gains answer, answers, bind, unbind, and binding subcommands, and bin/fm-send.sh's keyed-answer path now records each resolved key into RESOLVE_STATUS_KEYS (closing captain holds at answer time) while a merged single refusal still names the specific keys open for the task; .agents/skills/decision-hold-lifecycle/SKILL.md and docs/decision-hold-lifecycle.md document the merged step sequence, unrouted-command list, and refreshed verification counts.
• Adds a pinned workflow-YAML lint gate: new bin/fm-lint-workflows.sh and bin/fm-install-actionlint.sh, wired into bin/fm-lint.sh and .github/workflows/ci.yml, plus new tests/fm-lint-workflows.test.sh and expanded tests/fm-lint.test.sh coverage.
• bin/fm-crew-state.sh, bin/fm-peek.sh, and bin/fm-remote-secondmate-control.sh report remote secondmate delivery and state more accurately; bin/fm-brief.sh notes that a live scout may host its own review loop; docs/architecture.md corrects the evidence-storage description to match the orphan evidence branch; CLAUDE.md converts from a symlink to a real @AGENTS.md pointer file; new/updated tests cover remote peek, remote send delivery, crew state, and decision-hold binding paths end to end.

Risk Assessment

✅ Low: All three prior findings across two fix rounds (stdin leak, swallowed corruption diagnostic, tempfile permission hygiene) were verified as correctly and minimally fixed, matching established sibling patterns with no regressions; the fix-round diff is a single precise line, shellcheck and bash syntax are clean, and no user-intent constraints (required/forbidden behaviors, must-not-regress items) are affected.

Testing

All targeted behavior suites relevant to this batch's union-merge conflict resolutions and its two post-verification review fixes pass (49 existing assertions plus one new one, exit 0 throughout). Added and validated one focused regression test for the previously-uncovered corrupted-binding-diagnostic-forwarding fix in fm-procevent.sh, confirming it fails before the fix and passes after via a real revert/restore cycle. The stdin-leak hardening in command_answers remains only indirectly exercised (the underlying subprocess doesn't currently read stdin, so no observable regression could be constructed), which is disclosed here as a coverage limitation rather than a defect. Worktree is clean except for the intentional new test addition.

Evidence: tests/fm-decision-hold-lifecycle.test.sh full run (post-fix, including new regression test)

ok - report-only unresolved decision is reproduced and completion refuses before loss
ok - non-forced scout teardown always requires durable inventory verification
ok - a declined decision closes with a recorded answer and no routed work
ok - a decision closed outside the script is repairable and then clears teardown
ok - an unanswered decision still blocks completion and resists both unrouted close paths
ok - captain holds are idempotent, distinct, teardown-safe, Bearings-visible, and durably routed before close
ok - completion and verification validate origins before constructing paths
ok - ended visual review follows the same decision-hold completion owner
ok - resolved findings and decision-like prose do not create false holds
ok - terminal single-owner stale status decisions do not block empty inventory
ok - main-home and secondmate-home captain holds remain correctly routed
ok - resolve matches first/middle/last in quoted blocked_by and rejects a genuinely absent id
ok - a bound channel's captured answers close their captain holds 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 - the answer path keeps every guard the unrouted close path already had
ok - the chat channel feeds the same keyed-answer intake a captured review does
EXIT:0

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 2 issues found → auto-fixed (2) ✅

• ⚠️ bin/fm-decision-hold.sh:787 - bin/fm-decision-hold.sh:787 — command_answers reads keyed-answer lines from stdin in a while IFS=$'\t' read -r key answer label; do ... done loop with no input redirection of its own, and inside that loop it recursively invokes "$0" answer "$origin" "$key" --decision-file "$tmp" without redirecting that child's stdin away (e.g. < /dev/null). The child inherits the same stdin pipe/fd as the outer read loop. command_answer -> close_unrouted_hold -> tasks_axi() (bin/fm-decision-hold.sh:205-207) runs tasks-axi "$@" with no stdin redirect either, so if any tasks-axi subcommand in that chain ever reads from stdin (a prompt, pager, or unexpected TTY-less behavior), it would silently consume bytes intended for the outer loop, truncating a multi-key answer batch (e.g. the 5-choice batch the intent's own verification section describes) with no error surfaced — command_answers only reports closed/skipped counts for the lines it actually saw. This is the exact anti-pattern this same merge batch's sibling calls (bin/fm-crew-state.sh:181, bin/fm-peek.sh:34, bin/fm-send.sh:514,585) defend against by adding < /dev/null to subprocess invocations. Mechanical, non-controversial fix: add < /dev/null to the recursive "$0" answer ... call.
• ℹ️ bin/fm-procevent.sh:184 - bin/fm-procevent.sh:184 — feed_keyed_answers does origin=$("$SCRIPT_DIR/fm-decision-hold.sh" binding "$id" 2>/dev/null) || return 1, discarding stderr and collapsing two distinct outcomes into one silent skip: a source with no binding (intentional, documented no-op) versus a corrupted binding record. fm-decision-hold.sh's own read_binding (around line 693) is deliberately designed so a wrong-schema or invalid-origin binding file is a loud fail rather than a silent unbound, per its own comment ('feeding nothing is the safe direction only when it is a deliberate choice, never when it is a corrupted record'). In the automated procevent capture path — the only caller that matters in production — that diagnostic is thrown away and cmd_start (line 416) only prints on success ('answers-fed: ...'), never on failure, so a corrupted binding degrades silently to the manual-wake fallback with zero signal anywhere that it was corruption rather than an intentionally unbound source. Not a data-loss bug (the capture and its wake are preserved either way per the documented best-effort contract), just a lost diagnostic for an edge case that atomic bind/unbind writes make unlikely to occur in normal operation.

🔧 Fix: Fix stdin leak in keyed-answer replay and forward corrupted-binding diagnostics
1 warning still open:

• ⚠️ bin/fm-procevent.sh:186 - bin/fm-procevent.sh:186 — the fix-round's new corrupted-binding diagnostic path creates err=$(mktemp "${TMPDIR:-/tmp}/fm-procevent-binding-err.XXXXXX") without umask 077 before it, so the temp file capturing fm-decision-hold.sh's stderr (which can contain internal paths from fail "decision binding is unsafe: $path" / "...incompatible schema: $path" / "...invalid origin id: $path") is created with the default (often world-readable) permissions in the shared TMPDIR/tmp. This is inconsistent with the established convention for this exact same pattern elsewhere in the very code this fix models itself on: bin/fm-decision-hold.sh:774-775 uses err=$(umask 077; mktemp "${TMPDIR:-/tmp}/fm-keyed-decision-err.XXXXXX") for an identical subprocess-stderr-capture purpose. Mechanical one-line fix: wrap the mktemp call with umask 077; mktemp ... like the sibling code does.

🔧 Fix: Harden binding-error tempfile permissions with umask 077
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

• bash tests/fm-decision-hold-lifecycle.test.sh (17/17 assertions pass, including new test_corrupted_binding_forwards_its_diagnostic)
• bash tests/fm-send-resolve-key.test.sh (16/16 pass)
• bash tests/fm-procevent.test.sh (17/17 pass)
• Manual regression proof: reverted bin/fm-procevent.sh's feed_keyed_answers to its pre-fix (commit f6742a2) shape and reran the new test in isolation — it failed exactly as expected ('missing: decision binding has an incompatible schema'), then restored the fix and confirmed it passes
• mktemp "${TMPDIR:-/tmp}/fm-perm-check.XXXXXX" under default umask 022 — confirmed file mode is already 600, showing the umask-077 hardening in 54a52d4 is defense-in-depth rather than a functional fix

✅ **Document** - passed

✅ No issues found.

⚠️ **Lint** - 1 warning

• ⚠️ linter found issues (exit code 127)

✅ **Push** - passed

✅ No issues found.