Skip to content

fix(bin): send no-mistakes workers straight from commit into validation - #4149

Closed
karotkriss wants to merge 3 commits into
kunchenguid:mainfrom
karotkriss:fm/fm-dod-done-before-validation
Closed

karotkriss wants to merge 3 commits into
kunchenguid:mainfrom
karotkriss:fm/fm-dod-done-before-validation

Conversation

@karotkriss

@karotkriss karotkriss commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Intent

Upstream issues #99, #1033 and #3141 report one defect. The generated no-mistakes definition of done in bin/fm-dod-lib.sh (fm_dod_block, no-mistakes arm) tells the worker to append a done line and stop as soon as it believes the work is complete, while the same block later requires the done line only after the PR's checks are green. A worker that obeys the first instruction stops before validation, and done is overloaded for an unvalidated commit and a delivered PR. No test asserts the wording.

Fixes #99
Fixes #1033
Fixes #3141

What Changed

  • Rewrote the no-mistakes arm of fm_dod_block in bin/fm-dod-lib.sh so "done" means committed, validated through /no-mistakes, and delivered as a PR with CI green; the worker is now told to start /no-mistakes itself immediately after its implementation commit instead of appending a done line and stopping to wait for a firstmate instruction.
  • Removed the pre-validation done: {summary} instruction, leaving a single terminal done [at=<epoch>]: PR {url} checks green form in the generated block.
  • Added tests/fm-dod-lib.test.sh asserting the arm keeps its contract line, carries the --intent reference, drops both the pre-validation done line and the "Firstmate will then instruct" wait, and emits exactly one terminal done form; updated tests/fm-brief.test.sh to match the new wording.

Risk Assessment

✅ Low: A well-bounded prompt-wording change to one delivery-contract arm plus behavioral tests that correctly fail-before/pass-after; the prior fix rounds' work (epoch stamp, snapshot-test removal) is verified correct and no source, correctness, or policy issues remain.

Testing

I derived the scenarios from the intent (worker must proceed from commit straight into validation; done must not be overloaded onto an unvalidated commit; exactly one terminal done form) and drove the real product surface - the fm_dod_block shell function and its bin/fm-brief.sh emitter - not just source reads. The emitted no-mistakes DoD block now instructs the worker to start /no-mistakes immediately and explicitly "do not stop, wait for a firstmate instruction, or append any status line for the unvalidated commit," and carries exactly one terminal done [at=&lt;epoch&gt;]: PR {url} checks green line. I confirmed the regression by running the behavioral test against the base lib (fails) versus the target lib (passes), and the full fm-brief.sh emitter suite passes end-to-end. This surface is generated worker-facing text (an intentional generated interface), so the captured block itself is the reviewer-visible artifact; there is no graphical UI. Worktree restored clean after the base/target swap.

  • Live validation: ✅ go - 5 of 5 scenarios driven live against the product
Scenario Result Live Evidence
no-mistakes worker is sent straight from its implementation commit into validation, with no pre-validation done/stop instruction ✅ pass live Emitted block (no-mistakes-dod-block.txt) states 'start /no-mistakes yourself, immediately ... do not stop, wait for a firstmate instruction, or append any status line for the unvalidated commit'; `ba…
the no-mistakes arm carries exactly one terminal done form, done [at=&lt;epoch&gt;]: PR {url} checks green ✅ pass live grep -Fc 'done [at=<epoch>]:' over the emitted block returns 1, matching only the terminal CI-green line; asserted by tests/fm-dod-lib.test.sh
regression reproduces: the pre-fix wording (base dee119b) is caught and the fix clears it ✅ pass live Running tests/fm-dod-lib.test.sh against base lib fails ('still parks the worker to wait for a firstmate instruction', exit 1); against target lib it passes (exit 0)
the real emitter bin/fm-brief.sh renders a no-mistakes brief containing the fixed straight-into-validation wording ✅ pass live bash tests/fm-brief.test.sh runs fm-brief.sh to scaffold a brief.md and asserts it contains 'start /no-mistakes yourself, immediately'; whole suite passes (exit 0)
adversarial: removing the premature done must not also drop the --intent contract from the same block ✅ pass live Emitted block still contains the --intent contract paragraphs; tests/fm-dod-lib.test.sh asserts pass \--intent`` present and passes
Evidence: Emitted no-mistakes Definition-of-done block (as a worker receives it)

Source: Emitted no-mistakes Definition-of-done block (as a worker receives it)

# Definition of done
Delivery contract: mode=no-mistakes
The task is complete only when committed on your branch, validated through /no-mistakes, and delivered as a PR with CI green.
After your implementation commit, start /no-mistakes yourself, immediately, with the `--intent` contract below - do not stop, wait for a firstmate instruction, or append any status line for the unvalidated commit.

You drive no-mistakes by responding to its gates, not by implementing fixes.
Follow the guidance no-mistakes itself provides for the mechanics: it loads when you invoke /no-mistakes, and `no-mistakes axi run --help` plus the `help` lines in each `axi` response are authoritative and version-matched to the installed binary.
When starting no-mistakes, pass `--intent` as only this brief's `## Captain's intent` subsection body, not its heading, plus any later words the captain actually said.
Preserve the actual words without adding speaker labels or direct address; the subsection heading supplies provenance outside the pipeline input.
For a legacy brief with no such subsection, include only words on lines marked `[captain] `, excluding that metadata prefix; never copy its mixed `# Task` wholesale.
If it has no provenance-marked captain words, stop and ask firstmate instead of starting no-mistakes.
Do not include `## Firstmate spec`, later Firstmate build constraints, or your own decisions and tradeoffs.
The `--intent` string you pass must be self-sufficient: that string plus the codebase must let a reader reconstruct roughly the same specification, without depending on a separate report, a PR, or context that lives only in this conversation.
When the captain's intent refers to a report, decision, or PR ("do items 1, 2, 3, and 7 of the report"), write the substance of the referenced items into `--intent` in the captain's terms, not only the pointer; that substance is the captain's ask by reference, while Firstmate's build instructions and your own decisions still stay out.
This replaces the no-mistakes skill's advice to enrich `--intent` with decisions and tradeoffs; that advice does not apply to Firstmate-dispatched work.
Do not hand-edit, commit, or fix findings yourself while a run is active - the pipeline applies every fix.

One drive call blocks until the next gate or outcome, which routinely outlives what your harness lets a single command run: Claude Code kills a command at ten minutes maximum, while one fix round is capped around thirty minutes and up to three rounds chain.
So background the drive call and poll `no-mistakes axi status` from a separate call instead of sitting in one blocking hold your harness will kill.
Where a harness's own command limit is not established, assume it bounds commands and use that same background-and-poll shape.
A killed or timed-out call is never evidence the daemon died: the daemon accepts your response immediately and runs the round in the background, so the call was only ever waiting for a read while the run kept working.
Reattach and keep going rather than reporting the pipeline blocked; rule 7 owns the checks that decide when a pipeline block is real.

Two firstmate-specific rules layer on top of that guidance:
- ask-user findings are never yours to answer: escalate to firstmate using rule 6's ask-user format and stop.
  Firstmate applies `ask-user-authority` and obtains any required captain decision.
  When the decision comes back, feed it to the gate with `no-mistakes axi respond` and let the pipeline apply it - do not route the question to "the user" or implement the fix yourself.
- NEVER pass `--yes` (or `-y`) to `no-mistakes axi run` or `no-mistakes axi respond`. It is banned fleet-wide.
  It auto-resolves every gate including ask-user findings with no escalation, and answering your own ask-user finding is a hard rule violation.

After /no-mistakes reports CI green (the CI-ready return point - do not wait for it to keep monitoring in the background until merge), append `done [at=<epoch>]: PR {url} checks green` and stop. You are finished.
Evidence: Regression: fail-before-fix / pass-after-fix
===== behavioral test against BASE lib (expect FAIL) =====
not ok - no-mistakes arm still parks the worker to wait for a firstmate instruction
BASE_EXIT=1
===== behavioral test against TARGET lib (expect PASS) =====
ok - no-mistakes arm goes straight into validation with one terminal done: form
TARGET_EXIT=0

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

🔧 **Rebase** - 1 issue found → auto-fixed ✅
  • ⚠️ bin/fm-dod-lib.sh - merge conflict rebasing onto origin/main

🔧 Fix applied.
✅ Re-checked - no issues remain.

🔧 **Review** - 2 issues found → auto-fixed (2) ✅
  • 🚨 tests/fm-dod-lib.test.sh:29 - The new test's terminal-done assertions were never updated for the [at=&lt;epoch&gt;] stamp the rebase adopted into bin/fm-dod-lib.sh (lines 261, 263, 299). Against the real generated blocks: grep -c &#39;done:&#39; returns 0 not 1 (actual line is done [at=&lt;epoch&gt;]: PR {url} checks green), assert_grep &#39;append done: PR {url} checks green and stop&#39; misses, and the direct-PR/local-only heredoc fixtures diverge from the generated output by the missing stamp - so every function in this new file fails as committed. Verified by generating each block with fm_dod_block and comparing. Auto-fixed by inserting the [at=&lt;epoch&gt;] stamp in all four places (line 29 count pattern, line 32 assert_grep, and both fixtures).
  • ⚠️ tests/fm-dod-lib.test.sh:40 - test_direct_pr_and_local_only_arms_are_unchanged pins the direct-PR and local-only arms to exact full-block fixtures. The user intent concerns only the no-mistakes arm ('The generated no-mistakes definition of done ... tells the worker to append a done line and stop'); no requirement needs the two untouched arms guarded, and the exact-snapshot form breaks on any legitimate future wording tweak to those arms. Recommend removing this function (keep only the no-mistakes-arm behavioral test) as the smallest remedy; this is a scope question, not a code fix.

🔧 Fix applied.
1 warning still open:

  • ⚠️ tests/fm-dod-lib.test.sh:40 - test_direct_pr_and_local_only_arms_are_unchanged pins the direct-PR and local-only arms to exact full-block fixtures. The user intent concerns only the no-mistakes arm ('tells the worker to append a done line and stop'); fm_dod_block dispatches by a case statement, so editing the no-mistakes arm cannot alter the sibling arms, meaning this snapshot guards nothing for the stated change while breaking on any legitimate future wording tweak to those untouched arms. No intent requirement needs it. Recommend removing this function (keeping only the no-mistakes-arm behavioral test) as the smallest remedy; this is a scope question, not a code fix.

🔧 Fix applied.
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • Live validation: ✅ go - 5 of 5 scenarios driven live against the product
Scenario Result Live Evidence
no-mistakes worker is sent straight from its implementation commit into validation, with no pre-validation done/stop instruction ✅ pass live Emitted block (no-mistakes-dod-block.txt) states 'start /no-mistakes yourself, immediately ... do not stop, wait for a firstmate instruction, or append any status line for the unvalidated commit'; `ba…
the no-mistakes arm carries exactly one terminal done form, done [at=&lt;epoch&gt;]: PR {url} checks green ✅ pass live grep -Fc 'done [at=<epoch>]:' over the emitted block returns 1, matching only the terminal CI-green line; asserted by tests/fm-dod-lib.test.sh
regression reproduces: the pre-fix wording (base dee119b) is caught and the fix clears it ✅ pass live Running tests/fm-dod-lib.test.sh against base lib fails ('still parks the worker to wait for a firstmate instruction', exit 1); against target lib it passes (exit 0)
the real emitter bin/fm-brief.sh renders a no-mistakes brief containing the fixed straight-into-validation wording ✅ pass live bash tests/fm-brief.test.sh runs fm-brief.sh to scaffold a brief.md and asserts it contains 'start /no-mistakes yourself, immediately'; whole suite passes (exit 0)
adversarial: removing the premature done must not also drop the --intent contract from the same block ✅ pass live Emitted block still contains the --intent contract paragraphs; tests/fm-dod-lib.test.sh asserts pass \--intent`` present and passes
  • bash tests/fm-dod-lib.test.sh (behavioral, passes on target)
  • Swapped in base lib (dee119b) and re-ran bash tests/fm-dod-lib.test.sh -> fails with 'still parks the worker to wait for a firstmate instruction'; restored target -> passes (fail-before-fix/pass-after-fix)
  • Sourced bin/fm-dod-lib.sh and ran fm_dod_block no-mistakes demo-task-123, capturing the emitted block and grepping for done [at=&lt;epoch&gt;]: occurrences (exactly 1)
  • bash tests/fm-brief.test.sh (real emitter renders brief.md containing 'start /no-mistakes yourself, immediately')
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

@karotkriss karotkriss closed this Sep 10, 2026
@karotkriss karotkriss reopened this Sep 10, 2026
@karotkriss

Copy link
Copy Markdown
Contributor Author

CI note: on the first run (34480845265), 'Behavior portable serial 1' was cancelled after an intermittent bin/fm-wake-lib.sh: trap: line 2: unexpected EOF parse error during tests/fm-watch-triage.test.sh - unrelated to this diff and filed separately; it passed on the re-run. On the re-run (https://github.com/kunchenguid/firstmate/actions/runs/34485070521), 'Behavior portable parallel 1' timed out at the job's 10-minute cap (started 13:49:37Z, cancelled 13:59:50Z) - the known cap issue until #4123 merges - with 278 passing test results and zero failures before the cutoff. Every other check is green, so the validation gate is closed on the evidence that the shard's executed tests passed.

The generated no-mistakes Definition of done opened by telling the worker
to append done: {summary} and stop, with validation deferred to a later
firstmate instruction, while the same block's terminal contract is
done: PR {url} checks green. Workers obeying the first instruction stopped
before validation, and done: was overloaded for an unvalidated commit and
a delivered PR.

The no-mistakes arm now sends the worker directly from its implementation
commit into the /no-mistakes run, with the --intent contract already in the
same block and exactly one terminal done: form. The direct-PR and
local-only arms are unchanged and now pinned by fixture in
tests/fm-dod-lib.test.sh.

Fixes kunchenguid#99
Fixes kunchenguid#1033
Fixes kunchenguid#3141
@karotkriss
karotkriss force-pushed the fm/fm-dod-done-before-validation branch from b769813 to 3cb5d7a Compare September 20, 2026 23:54
@karotkriss karotkriss changed the title fix(bin): send no-mistakes workers from commit straight into validation fix(bin): send no-mistakes workers straight from commit into validation Sep 20, 2026
@karotkriss

Copy link
Copy Markdown
Contributor Author

Superseded: main has since made the first done report the deliberate pipeline handoff, and #3135 is the existing PR for #99, #1033 and #3141; closing.

@karotkriss karotkriss closed this Sep 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant