fix(bin): send no-mistakes workers straight from commit into validation - #4149
Closed
karotkriss wants to merge 3 commits into
Closed
karotkriss wants to merge 3 commits into
karotkriss wants to merge 3 commits into
Conversation
Contributor
Author
|
CI note: on the first run (34480845265), 'Behavior portable serial 1' was cancelled after an intermittent |
karotkriss
force-pushed
the
fm/fm-dod-done-before-validation
branch
from
September 11, 2026 14:55
deaea3f to
efd116e
Compare
karotkriss
force-pushed
the
fm/fm-dod-done-before-validation
branch
from
September 11, 2026 16:03
efd116e to
b769813
Compare
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
force-pushed
the
fm/fm-dod-done-before-validation
branch
from
September 20, 2026 23:54
b769813 to
3cb5d7a
Compare
Contributor
Author
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
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
fm_dod_blockinbin/fm-dod-lib.shso "done" means committed, validated through/no-mistakes, and delivered as a PR with CI green; the worker is now told to start/no-mistakesitself immediately after its implementation commit instead of appending a done line and stopping to wait for a firstmate instruction.done: {summary}instruction, leaving a single terminaldone [at=<epoch>]: PR {url} checks greenform in the generated block.tests/fm-dod-lib.test.shasserting the arm keeps its contract line, carries the--intentreference, drops both the pre-validation done line and the "Firstmate will then instruct" wait, and emits exactly one terminal done form; updatedtests/fm-brief.test.shto 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 terminaldone [at=<epoch>]: PR {url} checks greenline. 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.done [at=<epoch>]: PR {url} checks greenbash tests/fm-brief.test.shruns fm-brief.sh to scaffold a brief.md and asserts it contains 'start /no-mistakes yourself, immediately'; whole suite passes (exit 0)--intentcontract paragraphs; tests/fm-dod-lib.test.sh assertspass \--intent`` present and passesEvidence: 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)
Evidence: Regression: fail-before-fix / pass-after-fix
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-doneassertions were never updated for the[at=<epoch>]stamp the rebase adopted into bin/fm-dod-lib.sh (lines 261, 263, 299). Against the real generated blocks:grep -c 'done:'returns 0 not 1 (actual line isdone [at=<epoch>]: PR {url} checks green),assert_grep 'appenddone: PR {url} checks greenand stop'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=<epoch>]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.
done [at=<epoch>]: PR {url} checks greenbash tests/fm-brief.test.shruns fm-brief.sh to scaffold a brief.md and asserts it contains 'start /no-mistakes yourself, immediately'; whole suite passes (exit 0)--intentcontract paragraphs; tests/fm-dod-lib.test.sh assertspass \--intent`` present and passesbash tests/fm-dod-lib.test.sh(behavioral, passes on target)Swapped in base lib (dee119b) and re-ranbash 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 ranfm_dod_block no-mistakes demo-task-123, capturing the emitted block and grepping fordone [at=<epoch>]: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.