fix(bin): forbid agent co-author commit trailers in every generated brief - #4352
Open
NicholasACTran wants to merge 15 commits into
Open
NicholasACTran wants to merge 15 commits into
NicholasACTran wants to merge 15 commits into
Conversation
The captain banned auto-adding an agent name as a commit co-author, but nothing enforced it: six of the last eight merged landvera commits carried the trailer, and bin/fm-brief.sh never mentioned it. Add the hard prohibition to fm-dod-lib.sh's fm_dod_block, the single owner shared by fm-brief.sh and fm-promote.sh, so every ship mode and a promoted scout's ship instructions all carry it. It covers commits a pipeline step makes on the worker's behalf, not just hand-written ones, and it authorizes stripping the trailer only from the task's own unmerged branch, never from anything already on the default branch.
tiago-peixoto
added a commit
to tiago-peixoto/firstmate
that referenced
this pull request
Sep 25, 2026
…time (#57) Cursor injects the trailer after the typed message, and a per-machine cli-config opt-out is not a fleet contract. Every spawn now gives the pane a commit-msg hook that strips known AI trailers at the commit object, chaining the repository's own hooks at run time, failing closed on non-git launches, and cleaning up its read-only strip directory on abort and teardown. A secondmate home must be a git checkout so the strip cannot be skipped there; the upstream worker-account fixture is taught that. Fork patch. No upstream thread of this fleet's own tracks it; the nearest upstream threads are other contributors' brief-rule PRs kunchenguid#1379 and kunchenguid#4352 (open) and kunchenguid#4523 (closed by its author as a mistaken target).
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
Crewmate briefs must forbid the agent co-author commit trailer. Add a hard, unmissable prohibition to the bin/fm-brief.sh scaffold so every generated brief tells the worker never to add a Co-Authored-By agent trailer, phrased so it also reaches how the worker instructs the no-mistakes pipeline agent committing on its behalf (since pipeline-authored fix commits on the worker's branch carry the trailer too). Cover the case where a worker already made such a commit: it may rewrite its OWN unmerged task branch to strip it, but must never touch anything that has reached the default branch. Add a test asserting the scaffold carries this prohibition (asserting the shape, not the exact sentence), since briefs here are generated. Do not rewrite history on any project's default branch (six such commits already merged to landvera main are the captain's call, not this ticket). Do not change no-mistakes itself. One sentence per line in Markdown, plain dash not em dash, shellcheck-clean shell.
What Changed
fm_commit_attribution_blocktobin/fm-dod-lib.shas the single owner of a hard "Commit attribution" prohibition on agent co-author trailers, rendered into all three ship modes' Definition of done plus the scout and secondmate scaffolds inbin/fm-brief.sh; each arm is tailored to branch ownership -direct-PRandlocal-onlyget a mode-specific check point and an authorization to rewrite only the task's own unmergedfm/<id>branch,no-mistakesadditionally tells the worker to carry the ban into--intentandaxi respondfix instructions, forbids any rewrite while a run owns the branch, and routes a surviving post-run trailer to anote:line before the terminaldone:line. Never rewriting the default branch is absolute in every arm, andfm_brief_intent_overlaynow carries the ban through the overlay that otherwise supersedes earlier--intentinstructions.note:as a status verb in the scout and ship brief state lists and classified it inbin/fm-classify-lib.shas nonterminal and not captain-relevant, so a disclosure whose prose contains a legacy free-text token such asmergedneither escalates nor clears a pane's possible-wedge aging.launch-brief.md, and thenote:verb's nonterminal wedge-aging behavior; updateddocs/architecture.mdanddocs/scripts.mdto record the new verb classification andfm-dod-lib.sh's ownership of the ban.🤖 Generated with Claude Code
Risk Assessment
✅ Low: The change adds worker-facing prose to generated briefs plus one narrow, correctly-placed classifier exclusion for the
note:verb; it conforms to the stated intent, renders cleanly in every brief kind, and its behavioral change is covered by a genuine regression test at the shared boundary both daemon call sites already route through.Testing
I stood up isolated firstmate homes and ran the brief scaffolder, the spawn, and the scout promotion the way firstmate does, then read the Markdown the worker actually receives. All five generated brief kinds now carry the hard co-author trailer ban: the three ship modes each name their own check point, the no-mistakes arm is phased (strip pre-run, never touch the branch while a run owns it, disclose afterwards on a note: line) and carries the --intent and axi respond levers, while the scout and secondmate arms carry the ban with no branch, pipeline, or check-point prose. The assembled launch-brief.md keeps the ban and states its substance inline in the intent overlay rather than dangling a reference, and a promoted scout receives the same block. Adversarially, a note: line whose prose says "merged" or "PR ready" reads neither terminal nor captain-relevant and leaves the possible-wedge marker aging, the note: line still surfaces as unread status, and the prescribed disclosure shape leavesdone: PR <url> checks greenfully parseable by the reconciler while the forbidden folded-into-done: variant loses the URL - which is exactly why the instruction places it on its own line. The change has no graphical surface; the end-user artifact is generated Markdown, so evidence is the rendered brief files themselves plus the CLI transcripts. Targeted suites for brief, task delivery, and daemon classification all pass, and the new brief tests fail against the pre-change scaffold and pass against this change.FM_HOME=<tmp> ./bin/fm-brief.sh live-no-mistakes some-proj --mode no-mistakes; rendered block in evidence file brief-ship-no-mistakes.md shows the HARD RULE heading, the --intent/axi respond levers,…./bin/fm-brief.sh ... --mode direct-PR|local-only; attribution-blocks-all-brief-kinds.txt shows "Before you push this branch and before you open or update its PR" and "Before you report this branch…./bin/fm-brief.sh live-scout some-proj --scout; brief-scout.md block names "the scratch commits discarded at teardown" and contains no branch/pipeline/--intent/check-point clause./bin/fm-brief.sh live-sm --secondmate some-proj; brief-secondmate-charter.md block names "a merge you perform yourself under standing merge authority" with no ship-path clauses./bin/fm-spawn.sh live-launch <proj> claude --mode no-mistakes --yolo off; launch-brief-no-mistakes-assembled.md carries the full block and an overlay sentence that spells out the Co-Authored-By rul…./bin/fm-promote.sh promote-live-b --mode no-mistakes --yolo offwith a capture stub for fm-send.sh; promoted-scout-ship-instructions.md contains the identical no-mistakes attribution blockgit archive: briefs render 0 co-author mentions and the new tests/fm-brief.test.sh fails there with "brief did not mention the co-author trailer ban at all"; the same…Evidence: Attribution block as rendered in all five brief kinds
Source: Attribution block as rendered in all five brief kinds
Evidence: Rendered no-mistakes ship brief (full)
Source: Rendered no-mistakes ship brief (full)
Evidence: Rendered direct-PR ship brief (full)
Source: Rendered direct-PR ship brief (full)
Evidence: Rendered local-only ship brief (full)
Source: Rendered local-only ship brief (full)
Evidence: note: disclosure shape and wedge-classification drive
Source: note: disclosure shape and wedge-classification drive
Evidence: Regression: new brief tests before vs after the change
Source: Regression: new brief tests before vs after the change
Pipeline
Updates from git push no-mistakes
... (7 earlier update rounds omitted to keep the PR body within GitHub's 65536-char limit; full history is in the run log.)
🔧 Fix applied.
1 warning still open:
bin/fm-dod-lib.sh:153- The--intentexception added in round 4 is overridden by the last section of the brief the worker actually reads. bin/fm-spawn.sh:2371-2377 appendsfm_brief_intent_overlaytolaunch-brief.mdfor EVERYKIND=ship/MODE=no-mistakesspawn, after the whole Definition of done, and that overlay opens by declaring "This section supersedes every earlier brief instruction about constructing--intent, but not later clarifications actually supplied by the captain" (bin/fm-dod-lib.sh:152), then gives an exhaustive construction rule: "Use the serialized captain intent below plus any later words the captain actually supplied as--intent" (line 153). The DOD's newly amended sentence ("...plus the standing commit-attribution ban above, which belongs in every run's intent", bin/fm-dod-lib.sh:289) is precisely an "earlier brief instruction about constructing--intent", and the ban is not a later captain clarification, so the overlay's two-part rule displaces the three-part one. Concrete sequence: firstmate scaffolds a no-mistakes ship brief, fm-spawn.sh renders launch-brief.md, the worker reads the file top-down, reaches the explicitly-superseding final section, and builds--intentfrom the serialized captain intent alone. The pipeline then never receives the ban, its fix commits carryCo-Authored-By: Claude ..., and the worker is simultaneously forbidden from amending the branch while the run is active (bin/fm-dod-lib.sh:270). That defeats the intent's required phrasing "so it also reaches how the worker instructs the no-mistakes pipeline agent committing on its behalf" for the only mode where a pipeline commits at all. This is the same class of conflict the round-3 decision fixed, but at the stronger, explicitly-superseding site; the fix round only edited the DOD sentence. The narrow remedy is to carry the ban into fm_brief_intent_overlay's construction sentence (and update the wording assertions at tests/fm-task-delivery.test.sh:511,545). Note that the existing brief-shape test cannot see this: tests/fm-brief.test.sh asserts onbrief.md, never on the renderedlaunch-brief.mdthat contains the overlay. This is worker-facing generated contract prose and the placement is the author's call, so ask-user rather than a reviewer rewrite.🔧 Fix applied.
3 issues (2 warnings, 1 info) still open:
bin/fm-brief.sh:285- The User intent requires the prohibition in "the bin/fm-brief.sh scaffold so every generated brief tells the worker never to add a Co-Authored-By agent trailer". bin/fm-brief.sh generates three brief kinds; the change addsfm_commit_attribution_blockto the ship scaffold (bin/fm-dod-lib.sh:259, :273, :283) and the scout scaffold (bin/fm-brief.sh:425), but the secondmate charter (theif [ "$KIND" = secondmate ]scaffold at bin/fm-brief.sh:217-330) carries no attribution rule at all. A secondmate is not a passive router: its own charter tells it to report "a merge you performed yourself under standing merge authority" (bin/fm-brief.sh:280), and it writes docs under its home, so it can author a commit - including a merge commit onto a default branch - with a hand-written message carryingCo-Authored-By: Claude ... <noreply@anthropic.com>, and nothing in its brief forbids it. The gap is also the one place where the change's own renderednote:/States updates were applied selectively (bin/fm-brief.sh:383 and :475 gainednote, line 285 did not), so the omission reads as unexamined rather than deliberate. Whether "every generated brief" was meant to include the secondmate charter, or whether the intent's title ("Crewmate briefs") deliberately scopes it to ship and scout only, is the author's call - a secondmate is consistently distinguished from a crewmate elsewhere in this scaffold (seetest_herdr_lab_contract_applies_to_scouts_but_not_secondmates). The remedy either adds a secondmate arm tofm_commit_attribution_block(extending the change to a third brief kind) or records that the charter is intentionally out of scope; both are scope decisions, not a mechanical correction.tests/fm-brief.test.sh:465- Simplification: the change introduces a second, exact-sentence copy of a rule the new shape-level test already asserts.assert_grep "plus the standing commit-attribution ban above"at tests/fm-brief.test.sh:465 pins a literal fragment of bin/fm-dod-lib.sh:289, while tests/fm-brief.test.sh:276-281 already asserts the same fact at shape level (the--intent-restricting sentence must affirmatively admit the attribution ban, and must not name it only to exclude it). The User intent explicitly asks for the opposite of an exact pin: "Add a test asserting the scaffold carries this prohibition (asserting the shape, not the exact sentence), since briefs here are generated." No intent requirement needs the literal-fragment copy, and keeping it means any future rewording of that sentence breaks two tests for one contract, one of which fails with a message about wording rather than about the rule. I note the surroundingtest_no_mistakes_dod_wordingis itself a pre-existing wording-pinning test, so adding a line there follows local convention - which is why this is the author's call rather than a reviewer fix. Recommended remedy is removing tests/fm-brief.test.sh:465 and leaving the shape-level assertion as the single owner, not hardening or documenting the duplicate.bin/fm-dod-lib.sh:154- The new overlay sentence is a pointer, not a statement: "The Definition of done's commit-attribution ban is the one standing exception to that supersession: carry it into--intentas well". It never says what the ban is. bin/fm-spawn.sh:2369-2379 re-renderslaunch-brief.mdfrom the storedbrief.mdon every spawn AND every relaunch, andbrief.mdis scaffolded once and never regenerated - so a no-mistakes task scaffolded before this change and relaunched after it gets an overlay pointing at a "Definition of done's commit-attribution ban" that is absent from the assembled file. The worker then has a dangling reference and no ban text to carry, which is no worse than the pre-change status quo (the ban simply does not reach--intent), so this is transitional rather than a regression; the new test at tests/fm-task-delivery.test.sh:799 cannot see it because it scaffolds a fresh brief. The narrow remedy is to make the overlay sentence self-sufficient by stating the ban's substance inline (one clause naming theCo-Authored-Byagent trailer) rather than only referencing it, which is worker-facing generated prose and therefore the author's wording call.🔧 Fix applied.
2 issues (1 warning, 1 info) still open:
bin/fm-dod-lib.sh:217- Simplification: the shared ship-mode preamble renders "This holds whether you write the commit yourself or a pipeline step writes it on your behalf while applying a fix." into all three ship modes, including two whose own Definition of done in the same brief states that no pipeline ever runs. A direct-PR brief reads "This task ships direct-PR: you raise the PR yourself, without the no-mistakes pipeline." and "Do NOT run /no-mistakes." (bin/fm-dod-lib.sh:260, :264); a local-only brief reads "This task ships local-only: no remote, no PR, no pipeline." (bin/fm-dod-lib.sh:271). No pipeline step ever commits on those branches, so the sentence describes a case that cannot occur and sits three lines below a flat prohibition on the pipeline it invokes.The User intent scopes this clause to no-mistakes only: "phrased so it also reaches how the worker instructs the no-mistakes pipeline agent committing on its behalf (since pipeline-authored fix commits on the worker's branch carry the trailer too)". No intent requirement needs the clause in direct-PR or local-only. The component was introduced by fix round 2's mode split (7992c99), not by the author, and the new test at tests/fm-brief.test.sh:249 pins it into all three modes, so the clause cannot be narrowed without also narrowing that assertion.
Narrower form that satisfies the intent: move the pipeline sentence into the
no-mistakes)arm (alongside the existing "The pipeline commits on your behalf..." line at bin/fm-dod-lib.sh:225) and move the corresponding assertion out of the shared ship-mode loop into the no-mistakes-specific block at tests/fm-brief.test.sh:258. The remedy removes a component the intent does not require rather than hardening it, so it is the author's call.bin/fm-brief.sh:285- Thenoteverb was added to the declared state set in the ship brief (bin/fm-brief.sh:385) and the scout brief (bin/fm-brief.sh:477), each with the explanatory sentence "Usenote: {fact}for a supervisor-actionable fact that is not a state change; it never replaces a state line." The secondmate charter's own states list at bin/fm-brief.sh:285 was left as "States: working, needs-decision, blocked, $PAUSED_VERB, done, failed."This is not currently reachable as a defect: the secondmate arm of
fm_commit_attribution_block(bin/fm-dod-lib.sh:210-212) deliberately carries no branch, pipeline, or check-point clause and therefore never instructs a secondmate to append anote:line, so nothing in this change makes a secondmate emit the verb it was not told about.note:is nonetheless a real verb on the parent channel a secondmate writes to (status_line_is_unread_surface, bin/fm-classify-lib.sh:1474), so the asymmetry is worth a deliberate yes-or-no rather than an accident. Noting only; no action required unless the author intended "every generated brief" to include the charter's states list.🔧 Fix applied.
1 warning still open:
bin/fm-dod-lib.sh:222- Simplification: the no-mistakes arm now states pipeline coverage of the ban twice. Round 6's selected fix moved the formerly shared preamble sentence "This holds whether you write the commit yourself or a pipeline step writes it on your behalf while applying a fix." (line 222) into theno-mistakes)case, where it lands directly above "The pipeline commits on your behalf, so carry this ban to it through the two channels you already drive: state it in the--intentyou passno-mistakes axi run, and restate it in every fix instruction you send withno-mistakes axi respond." (line 223). Both sentences assert the same fact - the ban reaches a commit a pipeline step authors on the worker's branch - so the arm carries a parallel copy of one rule, which is exactly the second-definition shape this pass is asked to name. The User intent requires the ban be "phrased so it also reaches how the worker instructs the no-mistakes pipeline agent committing on its behalf"; line 223 alone satisfies that, and it is the only one of the two that gives the worker the actual lever. No intent requirement needs line 222 once the clause is mode-scoped - it only existed to carry the fact into the two modes that no longer receive it. The narrower form is deleting line 222. The new assertiongrep -Eiq "pipeline.*on your behalf"at tests/fm-brief.test.sh is satisfied by line 223 by itself, as are the--intentandrespond|fix instruction|gatelever assertions, so no test needs changing. This is worker-facing generated prose introduced by a prior fix round, so the wording call is the author's.🔧 Fix applied.
1 info still open:
tests/fm-brief.test.sh:225- The new test's header comment states "The captain has banned an agent co-author commit trailer outright, and nothing else enforces that ban". That is inaccurate: bin/fm-spawn.sh:1539 already carries an attribution-off policy in every claude launch's per-launch --settings JSON ("attribution":{"commit":"","pr":"","sessionUrl":false}), documented again at bin/fm-spawn.sh:325, which suppresses the Co-Authored-By trailer for spawned claude workers. The brief-level ban is still justified and required by the intent - it is the only control that reaches non-claude backends (the codex launch template at bin/fm-spawn.sh:1553 carries no such settings) and the no-mistakes pipeline agent committing on the worker's branch - but the comment overstates the gap and would mislead a future reader into thinking the settings policy does not exist. Noting only; the code and the assertions are correct.✅ **Test** - passed
✅ No issues found.
FM_HOME=<tmp> ./bin/fm-brief.sh live-no-mistakes some-proj --mode no-mistakes; rendered block in evidence file brief-ship-no-mistakes.md shows the HARD RULE heading, the --intent/axi respond levers,…./bin/fm-brief.sh ... --mode direct-PR|local-only; attribution-blocks-all-brief-kinds.txt shows "Before you push this branch and before you open or update its PR" and "Before you report this branch…./bin/fm-brief.sh live-scout some-proj --scout; brief-scout.md block names "the scratch commits discarded at teardown" and contains no branch/pipeline/--intent/check-point clause./bin/fm-brief.sh live-sm --secondmate some-proj; brief-secondmate-charter.md block names "a merge you perform yourself under standing merge authority" with no ship-path clauses./bin/fm-spawn.sh live-launch <proj> claude --mode no-mistakes --yolo off; launch-brief-no-mistakes-assembled.md carries the full block and an overlay sentence that spells out the Co-Authored-By rul…./bin/fm-promote.sh promote-live-b --mode no-mistakes --yolo offwith a capture stub for fm-send.sh; promoted-scout-ship-instructions.md contains the identical no-mistakes attribution blockgit archive: briefs render 0 co-author mentions and the new tests/fm-brief.test.sh fails there with "brief did not mention the co-author trailer ban at all"; the same…FM_HOME=<tmp> ./bin/fm-brief.sh live-<mode> some-proj --mode no-mistakes|direct-PR|local-onlythen read the rendered## Commit attribution - HARD RULE, no exceptionsblock in each brief.mdFM_HOME=<tmp> ./bin/fm-brief.sh live-scout some-proj --scoutand./bin/fm-brief.sh live-sm --secondmate some-proj, checking each arm carries the ban and borrows no branch/pipeline/check-point prose./bin/fm-spawn.sh live-launch <proj> claude --mode no-mistakes --yolo off(fake tmux, FM_GATE_REFUSE_BYPASS=1 harness hatch) to renderlaunch-brief.mdand read the ban plus the self-contained intent-overlay exception./bin/fm-promote.sh promote-live-b --mode no-mistakes --yolo offwith a capture stub for fm-send.sh, reading the ship instructions actually delivered to the promoted workerLive drive ofstatus_is_captain_relevant,status_is_terminal_verb,status_line_is_unread_surfaceand the reconciler'sdone: PR <url> checks greenextractor with the brief's prescribed note:/done: pair and with adversarialnote:prose containing "merged", "PR ready", "checks green"./tests/fm-brief.test.sh./tests/fm-task-delivery.test.sh./tests/fm-daemon.test.sh(includestest_note_line_is_nonterminal_and_keeps_wedge_aging, which drives classify_stale/handle_wake/housekeeping)Regression reproduction: newtests/fm-brief.test.shrun against agit archiveof base b518a25 (fails) versus the change (passes).agents/skills/firstmate-codexapp/SKILL.md:64- Judgment call, left unchanged: .agents/skills/firstmate-codexapp/SKILL.md:64 gives a Codex Desktop thread template listing status prefixes (working, needs-decision, blocked, paused, done, failed) withoutnote. I read it as still accurate because it scopes itself to "prefixes for status changes" andnote:is explicitly not a state change, and because a Desktop companion thread is not a generated crewmate brief. If the author intends the advertised verb set to be uniform across every worker-facing surface, that line is the remaining place to add it.✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.