fix(spawn): refuse a push-mode brief whose task text forbids the push or PR - #10
Merged
Merged
Conversation
…ndates
fm-brief.sh's no-mistakes and direct-PR definitions of done both end in a
pushed branch and an open PR. The scaffold cannot see the {TASK} text
firstmate fills in afterwards, so a task text that forbids exactly that
shipped as a self-contradicting brief, and the generated section is what the
worker follows: on 2026-08-22 three workers opened PRs the captain had not
approved after their own task text said not to.
A ship spawn already refuses when the brief's recorded mode disagrees with
the flag. Extend that same check so the brief must also agree with itself: on
a push mode, read only the "# Task" section and refuse when it forbids the
push or opening the PR, naming local-only as the mode whose contract already
stops at a branch the captain reviews first.
The default-branch qualifier is stripped before matching and only the opening
verbs count, so "never push to the default branch" and "do not merge the PR"
stay compatible with direct-PR. The check runs before any worktree, endpoint,
or task metadata exists and reads only local files, so it is independent of
the selected harness and runtime backend.
Run against every ship brief this fleet has scaffolded, the first form of the check read three lines it should not have: a force-push ban, a history-rewrite ban, and prose that merely describes a no-push stop point. Strip a force-push ban the same way the default-branch qualifier is stripped - both bound how the mandate is carried out rather than refusing it - and require the refusing token to open a line or a clause, so a refusal written mid sentence reads as prose about pushing rather than an instruction not to. On the same corpus the bounded check now fires on exactly the push-mode briefs that carry the contradiction and on nothing else. Each bound is pinned by a regression row that fails when it is removed.
…bility Both refusals read only local files and run before any worktree, endpoint, or task metadata exists, which is what makes them uniform across every supported harness and runtime backend, on a relaunch, and on each pair of a batch spawn. Record that once in the script header rather than per adapter.
The first detector read five phrasings as stop points that do not refuse
delivery at all: a ban scoped to what may be pushed, an unrelated product
feature that happens to be called push, a sequencing condition that explicitly
permits the PR afterwards, a PR routed at a named target, and a ban on a
PR-related file. With no override, each of those hard-blocked a legitimate
spawn.
Both objects must now end the clause or continue into a word that keeps the
delivery action itself as the object, so "do not push secrets", "no push
notifications", and "do not create a PR template file" no longer read as
refusals. A line that also permits the delivery later ("do not push until the
tests are green, then open the PR as usual") sequences the mandate rather than
refusing it and is dropped before matching, and a refusal may carry its own
subject ("you cannot push until the captain signs off").
Recall is unchanged: across the 49 ship briefs this fleet has scaffolded the
detector still fires on exactly the 7 carrying the contradiction and on nothing
else.
Regressions cover all five reported false positives, add true prohibitions that
name only the push and only the PR, and pin each new bound - removing any one of
them fails its row. Every launching row now asserts it reached a marker the
backend prints, so a row can no longer pass on a spawn that stopped earlier,
and every refusing row asserts that marker is absent.
AGENTS.md gains one section 7 line routing a stop point before the remote to
local-only, and the "Validate end-to-end" section every AGENTS.md owes, both
pointing at the authoritative scripts and docs rather than restating them.
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
2 participants
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
ADDENDUM 2 - four further accepted review rounds, all already applied in the four commits b2e5ba0, a8aaa25, 33a3559, bf28d89 that this run is validating. Each round was escalated as an ask-user finding rather than self-answered, each was ruled a correction required by accepted intent, and each was applied by the pipeline itself. The previous run then died on a pipeline-agent crash ("step review failed: agent review: claude exited: exit status 1") after committing round 5, custody was returned with
no-mistakes axi sync --recoverexactly as branch_sync.next_action directed, and no hand edit was made to the recovered work.Round 2 - the permit rule must require a delivery object. The first permit rule blanked the whole line on any
then <verb>, which silently disarmed the refusal on the literal 2026-08-22 incident line whenever an ordinary trailing sentence followed it ("- Do NOT push and do NOT open a PR. Run the tests, then create a summary in the status file." and "... Start the dev server, then open the preview at 390px." both launched). The permitting verb must now take the delivery itself as its object, so ordinary sequencing continues the task instead of permitting the push. The same round closed three verified recall holes: "- Do not push anything.", "- No pushing and no PR.", and theyetphrasings.Round 3 - a bare comma must not terminate a delivery object. Treating every comma as a clause end hard-blocked six lines that refuse nothing about delivery ("- No user-visible push, the sync is silent.", "- No push, no pull, no fetch in the test harness.", "- Do not add a command that can push, it is unsafe.", "- Do not worry about the push; the captain handles it.", "- Do not let the release push, or the deploy, run twice.", "- Do not name the file PR.md."). A comma now counts only with explicit following negative delivery context, which preserves the two load-bearing comma true positives ("no push, no PR." and "... no-mistakes, push, or open a PR without captain approval."). The
fileverb now requires an article,yetwas added to both object lists, and the --relaunch applicability wording in the fm-spawn.sh header was corrected: the refusals CREATE no worktree, endpoint, or task metadata, but on the relaunch path an existing metadata record and live endpoint are read before the delivery block, so the earlier "before any ... exists" phrasing was wrong.Round 4 - the push object must carry explicit Git delivery meaning. Accepting any determiner meant ordinary programming senses of push hard-blocked a spawn ("- Do not push to the analytics queue on every keystroke.", "- Never push to npm from CI.", "- Do not push it to the array; use concat.", "- Do not push this event to the dataLayer.", "- Do not push and pop the same stack twice."). The object vocabulary was narrowed to explicit Git delivery nouns and the generic continuations that proved nothing were dropped. This applied the same standard the original work used when it deleted its own unexercised routing strip: the review's mutation check showed 14 accepted tokens were exercised by no regression at all, and
to- the token that fired onpush to npm- was one of them.Round 5 - close the
anything/untilbypass. Those two words were accepted as the object regardless of what followed, which re-admitted the exact class round 4 had just released ("- Do not push anything to the CDN.", "- Do not push anything into the global store.", "- Do not push until the animation finishes.", "- Do not push until the queue drains."), plus "- Do not push to the repo cache on read." through a bare-prefix target match and "- Do not push or pop the PR badge." through the second-mention rule. The smallest targeted correction was applied rather than another word-list widening.Verified state of the head this run is validating, re-measured rather than assumed, using the real GNU grep the script runs under (note that an ad-hoc probe through ugrep silently fails open on a pattern this size, which is why every measurement here used /usr/bin/grep):
yetphrasings.Ceilings deliberately left documented rather than grown into machinery, all of which fail in the launch direction (the pre-change status quo) and none of which affect any live brief: a refusal written mid sentence ("Implement it, but do not push and do not open a PR."); a genuine refusal followed on the same line by a permitting verb over a PR-shaped noun ("Do NOT push. Then create a PR template file."); push targets with an adjective or an unlisted determiner ("Never push the feature branch.", "Do not push our changes.", "Do not push commits."); and a refusal written below a line starting with "# " inside a fenced code block in the task text, which the section-scoped read stops at. The accepted instruction in each case was explicitly not to add an override, a general natural-language parser, or broader section extraction.
ADDENDUM - accepted corrections from the previous run's review gate, now implemented on top of the original work described below. The previous run (01M0R7M0EDYN2H4VKPA9NVY2VK) parked at the review gate with two ask-user findings, both were escalated rather than self-answered, both were ruled corrections required by accepted intent, the pipeline's own fix commit was then rejected by the fork's pre-commit hook, and that fix round was discarded. Everything in this addendum was therefore reapplied by hand on top of 305f0b0d between runs, with branch custody released (branch_sync state user_owned), never while a run was active.
Correction 1 - detector tightened rather than given an override. The reviewer proved five phrasings that do not refuse delivery at all were being read as stop points and hard-blocked, with no escape: "- Do not push secrets or .env files to the remote.", "- No push notifications in this milestone; skip the service worker.", "- Do not push until the tests are green, then open the PR as usual.", "Do not open a PR against upstream; open it against the fork.", and "- Do not create a PR template file." All five were independently re-verified as firing before the fix. The accepted resolution was explicitly to tighten detection rather than add an override, so that only an actual prohibition on the selected delivery action refuses, while scoped objects, temporary conditions that explicitly permit the push afterwards, and target-specific wording that still allows the configured fork are all allowed.
Correction 2 - operator-facing rule added to AGENTS.md section 7. One concise line now routes a stop point before the remote (a running preview, or the captain's own review ahead of any push) to --mode local-only instead of contradicting a push-mode generated brief, with the authoritative detail left in bin/fm-spawn.sh, bin/fm-brief.sh, and docs/architecture.md rather than restated. This was deliberately NOT relocated elsewhere to avoid editing AGENTS.md.
Correction 3 - the required "Validate end-to-end" section. Firstmate's AGENTS.md never had one, and the fork's pre-commit hook (check-agents-e2e) blocks any commit touching AGENTS.md until it does, which is what killed the previous run's fix commit. The section was added because every project AGENTS.md is required to own its runnable end-to-end proof, not to get past the hook: the hook was neither weakened, bypassed, nor worked around, and --no-verify was explicitly rejected. It is three sentences that point at bin/fm-lint.sh, bin/fm-test-run.sh, the scaffold/spawn/teardown scripts, docs/tmux-backend.md, and docs/verification/, rather than copying any contract.
Correction 4 - regression strength. Regressions now cover all five reported false positives, plus true prohibitions that name only the push and only the PR, plus a refusal carrying its own subject. Each new bound is pinned by mutation check: removing the permit rule, loosening the push object, or loosening the PR object each fails its own row. Every launching row now asserts it reached a marker the backend prints after the delivery-agreement check, so a row can no longer pass vacuously on a spawn that stopped earlier, and every refusing row asserts that marker is absent - which also proves a refused spawn never reaches the backend at all.
The one review finding marked no-op (launch-rows-weak-assert) is the same weakness Correction 4 closes.
=== ORIGINAL INTENT (unchanged, still the acceptance context) ===
Fix the generated ship-brief contract that causes a worker to push and open a PR even when the task text explicitly says to stop at a running preview and wait for captain approval.
Required approach, as specified:
What the investigation established (context a reviewer reading only the diff would not have):
What was implemented (deliberate choices a reviewer should not flag as mistakes):
Verification evidence:
Baseline evidence that must be preserved and not hidden: four test scripts fail in this local environment and are pre-existing and unrelated to this change - tests/fm-arm-pretool-check.test.sh, tests/fm-calm-pi-extension.test.sh, tests/fm-kimi-harness.test.sh, and tests/fm-test-run.test.sh. They were verified to fail identically on the base commit ba5f4c4 with every file from this branch reverted, they live in four unrelated subsystems (the watcher pretool guard, the Pi calm extension, Kimi hook install, and a missing local ruby needed to parse a workflow as YAML), and they were deliberately left alone under this repo's blocking scope-discipline rule that a change touches only the files its task names. They are reported as baseline, not fixed here and not concealed.
What Changed
bin/fm-spawn.shextends its existing delivery-agreement block from "the brief and the task record must agree" to "the brief must also agree with itself": on--mode no-mistakesordirect-PRit reads only the brief's# Tasksection and refuses the spawn when that text forbids the push or opening the PR that the generated definition of done mandates, naming--mode local-onlyin the error. A newbrief_task_forbids_deliveryhelper does the matching with clause-scoped extended regular expressions that require a Git delivery object (so ordinary senses like "push to the analytics queue", "no push notifications", or "do not create a PR template file" still launch), strips bounded phrasings (force-push, push-to-default-branch), drops lines that permit the delivery later ("... then open the PR as usual"), and fails closed - eachawk/tr/sed/grepstage runs into its own variable and any non-1exit refuses instead of reading its own failure as agreement. The check runs before any worktree, endpoint, or task metadata is created.bin/fm-spawn.shheader (mechanics, including--relaunchand batch-spawn applicability), a cross-reference in thebin/fm-brief.shheader, one sentence indocs/architecture.md's delivery-modes section, and one operator line inAGENTS.mdsection 7 routing a pre-remote stop point tolocal-only.AGENTS.mdalso gains the required## Validate end-to-endsection pointing atbin/fm-lint.sh,bin/fm-test-run.sh, the scaffold/spawn/teardown scripts,docs/tmux-backend.md, anddocs/verification/.tests/fm-task-delivery.test.shgrows regressions that scaffold real briefs throughbin/fm-brief.sh, fill the{TASK}placeholder, and drivebin/fm-spawn.sh: true prohibitions (push-only, PR-only, comma-joined, subject-carrying,yetphrasings) must refuse and write no task metadata, while scoped/programming-sense/permitting phrasings and the direct-PR control cases must launch and reach a backend marker printed after the delivery check. The three generated definitions of done are asserted byte-identical to before.Risk Assessment
✅ Low: The fix round closes the reported fail-open across every classifier stage with correct status capture, its regressions genuinely discriminate rather than pass vacuously, and I independently re-verified that classification on the documented corpus is unchanged; the only residual is an unexercised guard on two stages, which fails in the refusing direction.
Testing
Ran the smallest relevant automated set (tests/fm-task-delivery.test.sh, plus tests/fm-brief.test.sh and tests/fm-documentation-audiences.test.sh through bin/fm-test-run.sh) - all pass - and then proved the intent at the product level with two reproducible CLI transcripts driven only through bin/fm-brief.sh and bin/fm-spawn.sh: the 2026-08-22 cosmic-sets-import brief is refused with its own task line quoted and local-only named, nothing is created and the backend is never reached, while the same text under local-only and an ordinary direct-PR brief both launch; the identical brief still launches on base commit 69e445c, and a sweep of the 49 real push-mode ship briefs in the live firstmate home refuses exactly the 7 that carry the contradiction and nothing else. This change is CLI-only with no rendered UI surface, so the reviewer-visible evidence is the CLI transcripts rather than screenshots. Worktree is clean; evidence lives only under the evidence directory.
Evidence: End-to-end CLI transcript: contradictory brief refused, local-only and ordinary direct-PR launch, false/true positive matrix
Source: End-to-end CLI transcript: contradictory brief refused, local-only and ordinary direct-PR launch, false/true positive matrix
Evidence: Counterfactual against base 69e445c plus live-fleet recall sweep (49 push-mode briefs, 7 refused)
Source: Counterfactual against base 69e445c plus live-fleet recall sweep (49 push-mode briefs, 7 refused)
Evidence: Reproducible demo driver used for the end-to-end transcript
Source: Reproducible demo driver used for the end-to-end transcript
Evidence: Reproducible driver for the counterfactual and live-fleet recall sweep
Source: Reproducible driver for the counterfactual and live-fleet recall sweep
Evidence: Key excerpt: the refusal a captain actually sees
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-spawn.sh:1751- The refusal gate discards its matcher's exit status:n=$(printf ... | sed ... | grep -nE "$DELIVERY_NO_PUSH_RE" | head -n 1 | cut -d: -f1)takes the status ofcut, so grep exit 1 (no match) and grep exit >1 (regex engine error) are indistinguishable, andbrief_task_forbids_deliveryreturns 1 in both cases. Any grep that cannot compile this ~2020-character ERE therefore launches the contradictory brief silently - the exact 2026-08-22 failure this change exists to stop, with no diagnostic. This is not hypothetical: the intent itself records that ugrep fails open on this pattern, and I reproduced it (ugrep -nE "$DELIVERY_NO_PUSH_RE"on the canonical "- Do NOT push and do NOT open a PR." line exits 2 while /usr/bin/grep exits 0). A safety classifier should fail closed. Suggested minimal fix: capture grep's status via PIPESTATUS or run grep into a variable first, and on status >1 print an error andexit 1rather than treating it as "no contradiction found".bin/fm-spawn.sh:1716- Several accepted alternatives in the new vocabulary are covered by no regression row:raise/submitin DELIVERY_REFUSE_OPEN (line 1722),now/here/yourselfand thefor|unless|before|without|at|onhold prefixes in DELIVERY_PR_OBJECT (line 1720),permission/green light/go-aheadin DELIVERY_HOLD (line 1716),unless|without|forin DELIVERY_CONDITION (line 1718), and thewesubject in DELIVERY_REFUSE_TOKEN. Round 4 of this same work deleted accepted tokens on exactly this ground ("an unexercised strip in a safety classifier is exactly the kind of thing that rots"), so the standard is applied unevenly. No concrete failure follows - these widen refusal rather than launch - so this is a note, not a blocker.🔧 Fix: fail closed when delivery-refusal matcher errors
1 warning still open:
bin/fm-spawn.sh:1752- The fail-closed fix only covers the grep stage of the classifier pipeline.hits=$(printf '%s\n' "$lc" | sed -E "/$DELIVERY_PERMIT_RE/s/.*//" | grep -nE "$DELIVERY_NO_PUSH_RE") || st=$?takes the pipeline status, which without pipefail is grep's status only. If the sed stage cannot run (a sed without -E, sed missing from PATH, a permit pattern the engine will not compile) it prints nothing and exits non-zero, grep then exits 1 on empty input,st=1is classified as a genuine no-match, and the contradictory brief launches - exactly the fail-open the round exists to close. Verified:printf 'do not push and do not open a pr.\n' | sed --bogus-flag -E "/x/s/.*//" | grep -nE 'push'yields st=1 with no hits. Two sibling stages have no status handling at all:lc=$(printf ... | tr ... | sed -E '...')at line 1746 and theawktask extraction at line 1745 (set -e is suppressed inside the function because the call site is the left operand of||), so a failure in either produces empty text and the same silent launch. Noteset -o pipefaildoes NOT fix this - pipefail returns the rightmost non-zero status, so grep's 1 still masks sed's failure. Minimal fix: run the permit strip into a variable and check its status (and the tr/awk stages') before greping, treating any stage failure as the same >1 refusal path already added at line 1788. The new header sentence at line 18-20 ("That check fails closed: a matcher that cannot run its pattern refuses the spawn rather than reading its own failure as agreement") currently overstates the behavior.🔧 Fix: fail closed on every delivery-classifier stage
1 info still open:
tests/fm-task-delivery.test.sh:447- The fix round added fail-closed guards for four classifier stages (awk task extraction, tr case-fold, sed bounded-push strip, sed permit strip) plus grep, but the regression ROWS table covers only three: grep, and the two sed stages. Therm -f "$fakebin/grep" "$fakebin/sed" "$fakebin/tr" "$fakebin/awk"line shows all four were intended. The awk and tr guards at bin/fm-spawn.sh:1758 and 1760 are therefore new, unexercised safety code - exactly the standard this work applied to itself when it deleted an unexercised routing strip ("an unexercised strip in a safety classifier is exactly the kind of thing that rots"). Adding two ROWS entries (awk|/^# Taskandtr|[:upper:]) reuses the existing stub_stage helper and needs no new machinery. No concrete failure follows - both guards widen refusal rather than launch - so this is a note, not a blocker.✅ **Test** - passed
✅ No issues found.
./bin/fm-test-run.sh tests/fm-task-delivery.test.sh- 10 cases pass, including the 57-row contradiction matrix, the fail-closed classifier-stage rows, and the byte-identical definitions-of-done assertions./bin/fm-test-run.sh tests/fm-brief.test.sh tests/fm-documentation-audiences.test.sh- scaffolder and maintained-prose suites pass at this headManual end-to-end CLI transcript:bin/fm-brief.sh <id> proj --mode direct-PR-> fill{TASK}with the real cosmic-sets-import prohibition ->bin/fm-spawn.sh <id> <proj> claude --mode direct-PR --yolo off(refuses, no metadata, backend never reached) vs--mode local-only(launches)Manual counterfactual: identical filled brief run againstgit show 69e445c:bin/fm-spawn.sh(launches, reaches the fake backend) vs this branch (refuses)Manual live-fleet recall sweep: every ~/.firstmate/data brief with a recordedDelivery contract: mode=driven through this branch's fm-spawn.sh with its own mode - 53 total, 49 push mode, 7 refused (agentic-portability-impl, cosmic-brand-accents, cosmic-catalogue-threshold, cosmic-homepage-pin, cosmic-sets-import, fm-autocompact-175k-b8, fm-config-symlinked)Manual false-positive/true-positive matrix through the real spawn: 12 programming-sense and scoped-object lines launch, 10 delivery prohibitions refuse✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.