Skip to content

feat(brief): add a concise current-state PR-description contract#871

Open
willronchetti wants to merge 1 commit into
kunchenguid:mainfrom
willronchetti:fm/fm-concise-pr-bodies-sonnet-6v
Open

feat(brief): add a concise current-state PR-description contract#871
willronchetti wants to merge 1 commit into
kunchenguid:mainfrom
willronchetti:fm/fm-concise-pr-bodies-sonnet-6v

Conversation

@willronchetti

Copy link
Copy Markdown

Intent

Update Firstmate's shared tracked instructions so PR descriptions written by ship workers are concise, current-state summaries reviewable in 3-5 minutes, instead of chronological implementation diaries. The contract must: describe final behavior not the sequence of attempts/commits; use short sections/bullets; cover intent, material behavior changes, reviewer-relevant safety/risk, validation, deployment/rollback where applicable, and genuinely unresolved considerations; omit chronological CI diaries, superseded claims, repeated failure/fix narratives, exhaustive internal helper/event lists, debugging transcripts, and duplicated evidence; keep exact commands/examples only when needed; and on later updates rewrite stale sections instead of appending another historical addendum. It must reach ordinary ship workers in both the no-mistakes and direct-PR delivery paths, but explicitly NOT scout or secondmate briefs (which never produce a PR body), and NOT local-only (which never opens a PR either). AGENTS.md must stay concise: only a short one-line cross-reference/reinforcement was added there, per the firstmate-coding-guidelines one-owner rule - the full contract text lives exactly once, as a shared PR_DESCRIPTION_CONTRACT variable in bin/fm-brief.sh, interpolated into both the direct-PR and no-mistakes Definition-of-done heredocs so there is a single source of truth. Every existing safety/merge-authority/validation/status/worktree-isolation/delivery-mode instruction was preserved untouched. A new targeted test (test_pr_description_contract_in_pr_producing_modes_only) was added to tests/fm-brief.test.sh asserting the contract appears in generated no-mistakes and direct-PR briefs and is absent from local-only, scout, and secondmate briefs; this follows the suite's existing targeted-assertion style rather than full-text snapshots. bin/fm-lint.sh (shellcheck) and the full fm-brief.test.sh, fm-instruction-owners.test.sh, fm-secondmate-safety.test.sh, fm-subagent-pretool-check.test.sh, and fm-tangle-guard.test.sh suites were all run and pass.

What Changed

  • Added a single-source PR_DESCRIPTION_CONTRACT variable in bin/fm-brief.sh, interpolated into the Definition-of-done heredocs for both the no-mistakes and direct-PR modes, directing ship workers to write PR descriptions as concise 3–5 minute current-state summaries (intent, material behavior changes, safety/risk, validation, deploy/rollback, open considerations) rather than chronological diaries — and to rewrite stale sections in place instead of appending addenda. The contract is deliberately omitted from local-only, scout, and secondmate briefs, which never open a PR.
  • Added a one-line cross-reference in AGENTS.md pointing to bin/fm-brief.sh as the owner of the contract, keeping a single source of truth.
  • Added test_pr_description_contract_in_pr_producing_modes_only to tests/fm-brief.test.sh, asserting the contract appears in no-mistakes and direct-PR briefs and is absent from local-only, scout, and secondmate briefs.

Risk Assessment

✅ Low: A small, well-scoped instruction/documentation change that adds a single-source PR-description contract to only the two PR-producing ship modes, preserves all existing instructions, and adds targeted test coverage; it fully satisfies the stated intent with no correctness, security, or regression concerns.

Testing

Ran the new targeted test and the four other named suites (fm-brief, fm-instruction-owners, fm-secondmate-safety, fm-subagent-pretool-check, fm-tangle-guard) — all pass. Beyond the assertions, I drove bin/fm-brief.sh directly to produce actual briefs for all five delivery paths: the identical PR-description contract (concise current-state summary, 3-5 minute read target, chronological-diary exclusion, rewrite-in-place update rule) renders in both the no-mistakes and direct-PR briefs and is completely absent (0 occurrences) from local-only, scout, and secondmate briefs, confirming the single-source-of-truth interpolation reaches exactly the PR-producing modes and no others. Skipped fm-lint.sh/shellcheck per the no-linters rule. No working-tree artifacts were left behind.

Evidence: Rendered PR-description contract across all five delivery modes

no-mistakes & direct-PR briefs both render the full '# PR description' contract; local-only, scout, and secondmate briefs each show 0 occurrences of '# PR description'.

=== Rendered 'PR description' section in the no-mistakes brief (demo-nomistakes) ===
# PR description
Write the PR description (or update it) as a concise, current-state summary: what a reviewer needs to approve the change, not a log of how you got there.
Target a 3-5 minute read: short sections and bullets, not a wall of prose.
Cover intent, the material behavior changes, reviewer-relevant safety or risk, how it was validated, deployment or rollback notes when they apply, and any genuinely unresolved considerations.
Leave out chronological CI diaries, superseded claims, repeated failure-then-fix narratives, exhaustive internal helper or event lists, debugging transcripts, and duplicated evidence.
Keep an exact command or example only when a reviewer or operator will actually need it to act.
If you update the description later, rewrite the stale section in place rather than appending another dated addendum on top of an earlier one.

=== Rendered 'PR description' section in the direct-PR brief (demo-directpr) ===
# PR description
Write the PR description (or update it) as a concise, current-state summary: what a reviewer needs to approve the change, not a log of how you got there.
Target a 3-5 minute read: short sections and bullets, not a wall of prose.
Cover intent, the material behavior changes, reviewer-relevant safety or risk, how it was validated, deployment or rollback notes when they apply, and any genuinely unresolved considerations.
Leave out chronological CI diaries, superseded claims, repeated failure-then-fix narratives, exhaustive internal helper or event lists, debugging transcripts, and duplicated evidence.
Keep an exact command or example only when a reviewer or operator will actually need it to act.
If you update the description later, rewrite the stale section in place rather than appending another dated addendum on top of an earlier one.

=== Modes that must NOT carry the contract ===
  demo-localonly: '# PR description' occurrences = 0
  demo-scout: '# PR description' occurrences = 0
  demo-secondmate: '# PR description' occurrences = 0

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.

  • bash tests/fm-brief.test.sh (full suite, incl. new test_pr_description_contract_in_pr_producing_modes_only)
  • bash tests/fm-instruction-owners.test.sh
  • bash tests/fm-secondmate-safety.test.sh
  • bash tests/fm-subagent-pretool-check.test.sh
  • bash tests/fm-tangle-guard.test.sh
  • Generated real briefs via bin/fm-brief.sh for no-mistakes, direct-PR, local-only, --scout, and --secondmate modes and inspected the rendered # PR description section presence/absence
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

no-mistakes and direct-PR ship briefs now instruct the worker to write
PR descriptions as a 3-5 minute current-state summary instead of a
chronological diary of attempts and commits. local-only, scout, and
secondmate briefs are unaffected since none of them produce a PR body.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant