fix(spawn): render no-mistakes invocation for the resolved harness - #1440
Open
JaredHuynhning wants to merge 4 commits into
Open
fix(spawn): render no-mistakes invocation for the resolved harness#1440JaredHuynhning wants to merge 4 commits into
JaredHuynhning wants to merge 4 commits into
Conversation
JaredHuynhning
force-pushed
the
fm/fm-codex-brief-skill-invocation-fresh
branch
from
August 1, 2026 05:49
8c14feb to
cf5daad
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
1 participant
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
Generate every Firstmate ship brief with no-mistakes invocation wording that matches the concrete resolved worker harness: Codex uses $no-mistakes; Claude, Grok, and Kimi use /no-mistakes; OpenCode, Pi, pi-signed, and unknown mappings use natural language. Keep harness-adapters as the single owner of those per-harness facts and render only after fm-spawn resolves the actual harness. Fail closed, with the task and missing FM_NO_MISTAKES_INVOCATION token named, when a current generated no-mistakes validation contract has lost its token. Preserve pre-existing work without guessing: an exact pre-token Firstmate scaffold is recognized by its historical contract signature, warns with the task name, renders only a private owner-only state/.launch-brief.md through the resolved harness mapping, and leaves the durable legacy brief unchanged; ordinary handwritten briefs without that validation contract remain unaffected, while any other malformed tokenless validation contract refuses. Store rendered briefs under the private firstmate home rather than predictable /tmp, remove them on aborted spawn and teardown, and state this exact legacy behavior in the PR. Prove the Codex, Claude, natural-language fallback, malformed-current refusal, exact legacy Codex and Claude relaunch, file permissions, aborted-spawn cleanup, and teardown cleanup through red-first behavior tests.
What Changed
bin/fm-brief.shnow emits the harness-neutral__FM_NO_MISTAKES_INVOCATION__token in no-mistakes ship scaffolds, andbin/fm-spawn.shrenders it only after it resolves the concrete harness, reading the per-harness form from theharness-adaptersskill ($no-mistakesfor codex,/no-mistakesfor claude, grok and kimi, natural language for opencode, pi, pi-signed and any unmatched or unreadable mapping). The rendered copy is written owner-only (umask 077) tostate/<id>.launch-brief.mdunder the private firstmate home rather than/tmp, and is removed by a spawn that aborts before the launch command reaches the pane and bybin/fm-teardown.sh(including secondmate children); a spawn that fails after delivery keeps it so the live pane retains its instructions.state/<id>.launch-brief.md, and leaves the durabledata/<id>/brief.mdunchanged. Ordinary handwritten briefs with no validation contract launch exactly as before and get no private copy.harness-adaptersbecomes the machine-readable owner of the invocation forms,AGENTS.mdrecords the new state file and the leave-the-token rule, andstuck-crewmate-recoverypoints relaunches at the retained launch brief. Nine behaviour tests cover codex, claude, the natural-language fallback, the malformed-current refusal, exact legacy codex and claude relaunch, file permissions, aborted-spawn cleanup and teardown cleanup; the Test gate notes two of the nine are not strictly red-first on base and were instead confirmed load-bearing by mutation, with the other seven failing red. Review raised a case where abort cleanup deleted a live kimi pane's brief after a post-delivery timeout, which was fixed by retaining the brief once the launch is delivered and folding the two renderers into one.Risk Assessment
✅ Low: The follow-up commit closes the round-1 retention gap at the correct boundary (the flag is set immediately after the launch command reaches the pane, and always after the meta write, so no retained brief can become teardown-unreachable), the renderer dedup is behaviour-preserving with both branches still covered, the reworded CI-ready sentence keeps the token in invocation position only, and both the abort and retain directions now have pinned behavioural tests.
Testing
Ran the three changed behaviour suites plus
tests/fm-brief.test.sh(all green), verified red-first by re-running each new test against the basebin/scripts (7 of 9 go red; the other two were proven load-bearing by mutating the change), and then drove fm-spawn end to end through a fake pane to capture the actual product surface: the launch command handed to the worker, the rendered brief path and 0600 mode under the private firstmate home, and the exact per-harness invocation wording for Codex, Claude and OpenCode, the malformed-contract refusal, the exact-legacy warning with an unchanged durable brief, the untouched handwritten-brief path, and the aborted-spawn and teardown cleanups. The change is shell and brief-text only with no rendered UI surface, so the reviewer-visible artifact is the CLI transcript of the real launch commands and rendered brief text rather than a screenshot.Evidence: End-to-end launch brief transcript (all intent branches)
===== harness: codex (spawn exit 0) ===== launch command handed to the pane: codex --dangerously-bypass-approvals-and-sandbox -c "notify=[...]" "$('<repo>/bin/fm-operational-input.sh' encode launch-brief < '<fm-home>/state/demo-codex.launch-brief.md')" mode: 600 56:Firstmate will then instruct you to invoke$no-mistakesto validate and ship a PR. 68:Once you have invoked$no-mistakesand it reports CI green ... You are finished. --- durable source brief still tokenized --- token occurrences: 3 ===== harness: claude (spawn exit 0) ===== mode: 600 56:Firstmate will then instruct you to invoke/no-mistakesto validate and ship a PR. ===== harness: opencode (spawn exit 0) ===== mode: 600 56:Firstmate will then instruct you to invoke theno-mistakesskill by name in natural language to validate and ship a PR. ===== exact pre-token legacy scaffold, harness: codex (spawn exit 0) ===== warning: task demo-legacy-codex has a legacy no-mistakes brief without FM_NO_MISTAKES_INVOCATION; rendering a private launch copy for resolved harness codex and leaving <fm-home>/data/demo-legacy-codex/brief.md unchanged rendered brief path: <fm-home>/state/demo-legacy-codex.launch-brief.md (mode 600) 56:Firstmate will then instruct you to run$no-mistakesto validate and ship a PR. --- durable legacy brief left unchanged --- 56:Firstmate will then instruct you to run /no-mistakes to validate and ship a PR. ===== malformed current validation contract (token deleted) ===== spawn exit: 1 error: refusing task demo-malformed launch for harness codex because ship brief <fm-home>/data/demo-malformed/brief.md is missing FM_NO_MISTAKES_INVOCATION worker launch attempted: no state/ rendered brief created: 0 ===== ordinary handwritten brief without a validation contract ===== spawn exit: 0 launched brief path: <firstmate-home>/data/demo-handwritten/brief.md private rendered copies in state/: 0 ===== aborted spawn (launch delivery fails) ===== spawn exit: 1 leftover state/<id>.launch-brief.md: removedEvidence: Per-harness invocation mapping resolved from harness-adapters
harness rendered invocation wording ------- --------------------------- codex$no-mistakesclaude/no-mistakesgrok/no-mistakeskimi/no-mistakesopencode theno-mistakesskill by name in natural language pi theno-mistakesskill by name in natural language pi-signed theno-mistakesskill by name in natural language some-future-harness theno-mistakesskill by name in natural language adapters missing, harness=codex -> theno-mistakesskill by name in natural languageEvidence: Red-first check against base commit 3772964
test_token_stripped_current_ship_brief_refuses_launch not ok - expected exit 1, got 0 test_legacy_claude_ship_brief_migrates_private_launch_copy not ok - legacy migration warning did not name the task test_legacy_codex_ship_brief_migrates_private_launch_copy not ok - legacy migration warning did not name the task test_aborted_spawn_removes_private_launch_brief ok (vacuous on base; mutation-proven on the change) test_claude_launch_brief_uses_slash_no_mistakes_invocation ok (base scaffold already literal /no-mistakes; mutation-proven) test_opencode_launch_brief_uses_harness_agnostic_wording not ok - did not use the harness-agnostic fallback test_codex_launch_brief_uses_dollar_no_mistakes_invocation not ok - did not use the dollar-prefixed invocation test_teardown_removes_private_launch_brief not ok - teardown left the private rendered launch brief test_kimi_post_delivery_failure_retains_private_launch_brief not ok - fixture did not generate a tokenized ship briefEvidence: Mutation checks proving the two non-red tests are load-bearing
mutation A (abort cleanup removed): not ok - aborted spawn orphaned its private rendered launch brief mutation B (all harnesses fall back to natural language): not ok - claude launch brief did not use the harness-adapters slash invocationEvidence: Targeted suite logs
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 4 issues found → auto-fixed ✅
bin/fm-spawn.sh:300- bin/fm-spawn.sh:300 | spawn_abort_cleanup rm -f's LAUNCH_BRIEF on every non-zero exit, including kimi's post-delivery failure paths that intentionally keep the pane and meta alive. kimi has no BRIEF in its launch template - line 1831 submits "Read the brief at $BRIEF_REAL", which is now state/<id>.launch-brief.md. When kimi_wait_for_delivery (1408) times out after ~20s despite the pointer having landed, kimi_spawn_fail (1419) only appendsfailed:and prints "inspect window $T", then exit 1 deletes the brief out from under the live agent; data/<id>/brief.md is no fallback because it still holds the literal FM_NO_MISTAKES_INVOCATION. Same deletion hits a still-running task if an operator re-spawns that id and the second run fails after line 1492. Fix: clear LAUNCH_BRIEF once the pointer submit is attempted (or in kimi_spawn_fail, alongside the retained pane and meta), keeping removal for genuine pre-delivery aborts.bin/fm-spawn.sh:986- bin/fm-spawn.sh:986 | brief_is_recognized_legacy_no_mistakes pins all three historical lines, but line 3 was reworded in 295c13d (2026-07-06) and line 2 in 362fb54 (2026-06-26), later than the unchanged line 1. A ship brief scaffolded before 2026-07-06 fails the exact-signature match yet still matches brief_carries_no_mistakes_validation, so it is hard-refused instead of migrated. Fail-closed and loud, and the intent explicitly authorises refusing anything outside the exact signature, so no change required - noted only because a ~4-week-old in-flight task is still plausibly respawnable.bin/fm-spawn.sh:618- bin/fm-spawn.sh:618 | the fallback "theno-mistakesskill by name in natural language" is worded as a verb object, but fm-brief.sh:355 substitutes the token as a sentence subject, rendering "After theno-mistakesskill by name in natural language reports CI green ..." for opencode, pi, pi-signed and unknown harnesses. The test only asserts the phrase is present. Reword fm-brief.sh:355 (e.g. "After no-mistakes reports CI green") so the token stays in invocation positions only.bin/fm-spawn.sh:657- bin/fm-spawn.sh:657 | render_launch_brief_for_harness and render_legacy_launch_brief_for_harness differ only in the awk substitution rule, and write_private_launch_brief wraps each in an identical if/else to capture a status thatset -eualready propagates from the simple-command call site at 1493 (its trailingif [ "$status" -ne 0 ]returns 0 on success only by fall-through). Collapsing to one awk parameterised by the match rule would drop the legacy flag threading entirely.🔧 Fix: retain launch brief for live panes, dedup renderer
✅ Re-checked - no issues remain.
tests/fm-spawn-dispatch-profile.test.sh:274- Two of the nine new behaviour tests are not strictly red-first against the base commit:test_aborted_spawn_removes_private_launch_briefpasses vacuously on base (base never creates a private brief, so the absence assertion is trivially true) andtest_claude_launch_brief_uses_slash_no_mistakes_invocationpasses on base (the pre-change scaffold already contains a literal/no-mistakes). Both were confirmed load-bearing on the change by mutation: removing the abort-timermand forcing the natural-language fallback each make the respective test fail. The other seven new tests fail red on base as intended.bin/fm-test-run.sh tests/fm-spawn-dispatch-profile.test.sh(all pass, including the 7 new launch-brief tests)bin/fm-test-run.sh tests/fm-teardown.test.sh tests/fm-kimi-harness.test.sh(all pass, includingtest_teardown_removes_private_launch_briefandtest_kimi_post_delivery_failure_retains_private_launch_brief)bin/fm-test-run.sh tests/fm-brief.test.sh(scaffold generation with the new token)Red-first check: revertedbin/fm-spawn.sh,bin/fm-brief.sh,bin/fm-teardown.shto base 3772964 and ran each new test in isolation - 7 of 9 fail redMutation check on the change: removed the abort-timerm -f -- "$LAUNCH_BRIEF"and separately forced the natural-language fallback for all harnesses, confirmingtest_aborted_spawn_removes_private_launch_briefandtest_claude_launch_brief_uses_slash_no_mistakes_invocationboth failManual end-to-end spawn through the fake tmux pane for codex, claude and opencode: captured the literal launch command, the rendered brief path and mode, the rendered invocation lines, and the untouched tokenized durable briefManual end-to-end spawn of the exact pre-token legacy scaffold for codex and claude: captured the warning, the private rendered copy, and the unchanged durable legacy briefManual end-to-end spawn of a token-stripped current validation contract (refusal, no launch, no state file) and an ordinary handwritten brief (launches unchanged, no private copy)Manual end-to-end aborted spawn with a failing launch delivery, confirming the private brief is removedDirect probe ofno_mistakes_invocation_for_harnessfor codex, claude, grok, kimi, opencode, pi, pi-signed, an unknown harness, and an unreadable harness-adapters file.agents/skills/harness-adapters/SKILL.md:185- harness-adapters/SKILL.md now declares the "## no-mistakes skill invocation" bullets the machine-readable authority that fm-spawn parses, but the per-harness fact tables still carry duplicate "Skill invocation" rows (claude line 185, codex 215, grok 320, kimi 377). The copies currently agree, so nothing is stale, but a maintainer who edits only a table row would silently leave fm-spawn rendering the old form or falling back to natural language. Worth a follow-up that reduces those rows to a pointer at the owner section. Left alone here as an out-of-scope consolidation.✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.