fix(bin): include commit conventions in generated briefs - #1587
Closed
sbracewell64 wants to merge 1 commit into
Closed
sbracewell64 wants to merge 1 commit into
sbracewell64 wants to merge 1 commit into
Conversation
AGENTS.md section 1 forbids naming an agent as a commit co-author, and bars the fleet's captain-address and nautical conventions from commits, PRs, and anything other tools read. Neither rule reached a worker: bin/fm-brief.sh stated neither, so a generated brief carried the co-author rule only on firstmate-repo tasks, and then only because those briefs separately name the firstmate-coding-guidelines skill, which carries it. Every worker on every other project was silently missed. Both are structurally the same gap. A crewmate does not read this repo's AGENTS.md for another project, and its own harness instructions may actively tell it to append a Co-Authored-By trailer, so the brief is the only place either rule can arrive. Commit 53932fb on fm/platform-landing-battery-windows-reds shipped a Co-Authored-By trailer for exactly this reason, and a separate incident leaked captain address into a commit subject the same way. Render a "# Commit conventions" section from one shared value into all three scaffolds that can reach a commit - ship for all three delivery modes, scout, and the secondmate charter - placed beside each one's delivery instructions rather than in the preamble. In the ship scaffold it is the last thing before "the task is complete only when committed on your branch". The scout copy adds that scratch commits are held to the same bar because a scout can be promoted in place. Rule numbering is untouched, so no cross-reference moves. test_every_committing_variant_carries_commit_conventions generates all five variants and asserts both rules plus the placement constraint. Witnessed red against the unfixed scaffold first: all five generated variants contained zero occurrences of either rule.
Owner
|
Speaking as Kun's firstmate: closing this as stale. It has been waiting on a contributor update for 14+ days with no author push or comment. Reopen if you want to pick it back up. |
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
Make the fleet's no-agent-co-author rule actually reach the workers it governs, by putting it in the brief scaffold itself.
BACKGROUND AND MEASUREMENT. AGENTS.md section 1 states the rule plainly: never add an agent name as a commit co-author. bin/fm-brief.sh, the code the fleet actually runs, contained zero occurrences of it. Measured 2026-08-03: a generated ship brief for an agentic-engineering task had 0 mentions, while a firstmate-repo brief had 1 only because that brief separately names the firstmate-coding-guidelines skill and THAT skill carries the rule. So the rule reached a worker only on firstmate-repo tasks and silently missed every worker on every other project. The consequence is already realised, not hypothetical: commit 53932fb on fm/platform-landing-battery-windows-reds shipped a Co-Authored-By: Claude Opus 5 trailer, caught only because firstmate happened to read the commit before landing. The worker did nothing wrong; it was never told. A worker cannot learn this rule any other way: it does not read this repo's AGENTS.md when working another project, and its own harness instructions may actively tell it to append a Co-Authored-By trailer.
WHAT WAS ASKED. Put the rule in the scaffold itself so it reaches every generated brief for every project, independent of which skills a particular task happens to name. Place it where a worker will actually read it at the moment it matters, next to the commit/delivery instructions, not buried in a preamble. Keep it short and unambiguous. State the rule, not the rationale.
THE NEIGHBOURING RULE, DELIBERATELY IN SCOPE. A closely related defect is registered separately as captain-address-leaked-into-commit: firstmate's captain-address convention leaked into a commit subject. I was told NOT to fix that task's specific instance, but to check whether the same structural gap explains it, and if so to cover both in one place. I checked and it IS the same gap: the scaffold never told workers that fleet conversational conventions must not appear in commits, PRs, or anything crewmates and other tools read, and a crewmate in a firstmate worktree loads CLAUDE.md (a symlink to AGENTS.md) which instructs it to address the user as captain. Both rules are therefore stated together in the one new section. The other task's specific instance was not touched.
DELIBERATE DESIGN DECISIONS. (1) One shared COMMIT_CONVENTIONS value is rendered into all three scaffolds that can reach a commit rather than duplicating prose three times. (2) It is added as its own '# Commit conventions' section placed immediately before each scaffold's delivery instructions; in the ship scaffold it is the last thing before 'the task is complete only when committed on your branch'. (3) Rule numbering is deliberately NOT changed. An earlier stranded attempt at this same rule made it Rule 1 of the Rules section and had to renumber every subsequent rule across three variants and update cross-references such as 'escalate to firstmate (rule 6)'. That is a large fragile diff, and the Rules section is further from the commit instruction than the chosen placement, so the renumbering approach was rejected. (4) Scout gets one extra sentence because a scout's scratch commits are discarded at teardown but firstmate may promote the task in place and carry them into a shipping branch, so they are held to the same bar. (5) Secondmate is included because a secondmate is a firstmate in its own home and commits shared tracked material directly when its fleet is empty. (6) The variable is single-quoted and free of apostrophes and backticks so nothing interpolates at scaffold time and the file stays Bash 3.2 parse-safe, consistent with the existing heredoc-safety guards in this script.
SCOPE LIMITS I WAS GIVEN. Only bin/fm-brief.sh and its colocated tests. Do not restructure the brief scaffold. Do not edit AGENTS.md, which already states the rule correctly and owns it; the scaffold delivers the rule to a different audience rather than restating a contract for firstmate. Do not fix other briefs' content.
TESTING CONSTRAINT AND ITS RESOLVED TENSION. firstmate-coding-guidelines requires that tests exercise behavior through an executable or public interface and never assert implementation-source bytes. The task also requires asserting the rule's presence. These were resolved deliberately: the test generates briefs by invoking bin/fm-brief.sh and asserts against the GENERATED BRIEF, which is this script's observable output, rather than grepping the script's own source. It covers all five variants that can reach a commit (ship no-mistakes, ship direct-PR, ship local-only, scout, secondmate charter) and also asserts the placement constraint, that the section does not appear before the Setup section, because a rule the worker will not read is useless.
NEGATIVE CONTROL, EXPLICITLY REQUIRED AND PERFORMED. The test was written first and witnessed red against the unfixed scaffold before any change was made: the suite went red at the first variant, and because the assertion helper exits on first failure, per-variant evidence was captured separately showing all five generated variants contained zero occurrences of either rule. After the change all five carry both, the fm-brief suite is 21 ok, bin/fm-lint.sh is clean, and six neighbouring suites that touch briefs pass.
MY OWN COMMIT. It carries no Co-Authored-By trailer and no captain address, verified by inspecting the trailer block directly rather than by a loose grep, which false-positives on the message describing the rule.
What Changed
Risk Assessment
✅ Low: Captain, the change is narrowly scoped, satisfies the stated scaffold coverage and placement requirements, and introduces no material source-level risks.
Testing
The focused fm-brief behavior suite passed, and manual end-to-end CLI generation demonstrated both rules and their required placement across all five commit-capable variants; generated briefs and transcripts were captured as reviewer-visible text artifacts, the target commit had no trailers or captain-address subject, and the worktree remained clean.
Evidence: Generated brief excerpts for all five variants
Evidence: Convention placement and target commit evidence
Evidence: Generated no-mistakes ship brief
Evidence: Generated direct-PR ship brief
Evidence: Generated local-only ship brief
Evidence: Generated scout brief
Evidence: Generated secondmate charter
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
Inspectedgit diff 4ee4a0a2790cfaa5e47b30fa462f16546f2ab5b6..7f62937bedeb57c3ffe2c8d9449dfdeca91582e7 -- bin/fm-brief.sh tests/fm-brief.test.shRanbin/fm-brief.sh --helpto verify the public scaffold interfaceRan focused behavior suitetests/fm-brief.test.shGenerated ship briefs throughbin/fm-brief.shfor--mode no-mistakes,--mode direct-PR, and--mode local-onlyGenerated scout and secondmate briefs throughbin/fm-brief.sh --scoutandbin/fm-brief.sh --secondmate alphaInspected generated brief excerpts and line positions withawkandgrep, confirming one convention section per brief after Setup and immediately before delivery instructionsParsed target commit trailers withgit interpret-trailers --parseand inspected its subjectRangit status --shortafter testing to confirm no transient working-tree artifacts remained✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.