From f06933173781f06c1575ed82f40205b1318472fb Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 1 Jun 2026 15:41:56 -0400 Subject: [PATCH] Enhancement(#41): /gsd-ship extracts per-commit gate_status into a PR-body TDD Audit + squash trailer (#585) * feat(#41): extract per-commit gate_status into ship PR body TDD Audit /gsd:ship's generate_pr_body now reconstructs the TDD gate trail that a squash-merge would otherwise discard. A new TDD Audit section walks the merge-base..HEAD commit range (merges excluded), reads each commit's gate_status: trailer via Git's native trailer machinery, pairs each test: commit with its following feat:/fix: implementation commit, and counts commits lacking a recognized trailer as missing. A single aggregate `gate_status: skill=N, fallback=N, exempt=N, missing=N` trailer is emitted as the final line of the PR body so a GitHub squash-merge carries the audit footprint into the base branch. Hardening (per adversarial review): impl pairing is restricted to feat:/fix: (refactor/docs/chore are skipped, never mistaken for GREEN); the gate_status cell is normalized to a known token and never rendered raw; commits with multiple gate_status trailers are treated as missing; every table cell escapes pipes and strips CR/LF; records guard against delimiter-injection from adversarial commit messages. Scoped additively: no changes to commands, agents, templates, or SDK. Closes #41 Co-Authored-By: Claude Opus 4.8 * docs(#41): add changeset for ship TDD Audit enhancement Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/feat-41-ship-tdd-audit.md | 7 ++ docs/ship-pr-body-sections.md | 13 ++- get-shit-done/workflows/ship.md | 47 +++++++++ ...eat-41-ship-tdd-audit-gate-status.test.cjs | 98 +++++++++++++++++++ 4 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 .changeset/feat-41-ship-tdd-audit.md create mode 100644 tests/feat-41-ship-tdd-audit-gate-status.test.cjs diff --git a/.changeset/feat-41-ship-tdd-audit.md b/.changeset/feat-41-ship-tdd-audit.md new file mode 100644 index 000000000..e71450539 --- /dev/null +++ b/.changeset/feat-41-ship-tdd-audit.md @@ -0,0 +1,7 @@ +--- +type: Added +pr: 585 +--- +**`/gsd-ship` PR bodies now include a TDD Audit section** — `generate_pr_body` walks the `merge-base..HEAD` commit range (merges excluded), reads each commit's `gate_status:` Git trailer (`skill` | `fallback` | `exempt`), pairs each `test:` commit with its following `feat:`/`fix:` implementation commit in a table, and counts commits without a recognized trailer as `missing`. + +A single aggregate `gate_status: skill=N, fallback=N, exempt=N, missing=N` trailer is emitted as the final line of the PR body, so a GitHub squash-merge carries the per-phase TDD audit footprint into the base branch instead of losing it with the deleted PR branch. diff --git a/docs/ship-pr-body-sections.md b/docs/ship-pr-body-sections.md index 16795b9a6..dedad1158 100644 --- a/docs/ship-pr-body-sections.md +++ b/docs/ship-pr-body-sections.md @@ -13,8 +13,19 @@ Every generated `/gsd-ship` PR body keeps the required core sections: - `Requirements Addressed` - `Verification` - `Key Decisions` +- `TDD Audit` -Custom sections are append-only. They render after `Key Decisions`; they cannot replace, remove, or reorder the core sections. +Custom sections are append-only. They render after `Key Decisions` (and before the `TDD Audit`); they cannot replace, remove, or reorder the core sections. + +### TDD Audit section + +The `TDD Audit` section is always appended last. It walks the commits in the `merge-base..HEAD` range (merges excluded), reads each commit's `gate_status:` Git trailer (`skill` | `fallback` | `exempt`), and pairs each `test:` commit with its following `feat:`/`fix:` implementation commit in a table. Commits that carry no recognized trailer are counted as `missing`. + +The section closes with a single aggregate trailer line that a GitHub squash-merge carries into the base branch: + +``` +gate_status: skill=3, fallback=1, exempt=0, missing=0 +``` ## Configure Sections During Onboarding diff --git a/get-shit-done/workflows/ship.md b/get-shit-done/workflows/ship.md index 7cd6c9317..eb2bf4447 100644 --- a/get-shit-done/workflows/ship.md +++ b/get-shit-done/workflows/ship.md @@ -191,6 +191,53 @@ Example configured sections: } ] ``` + +**8. TDD Audit section:** + +Reconstruct the per-commit TDD gate trail before squash-merge discards it. Walk the PR branch's own commits (merges excluded) and read each commit's `gate_status:` trailer with Git's native trailer machinery — never a raw `%B` grep, which would also match the string written in prose: + +```bash +# Anchor on the merge-base so a stale local ${BASE_BRANCH} ref cannot over-count. +RANGE_BASE=$(git merge-base "${BASE_BRANCH}" HEAD) +git log "${RANGE_BASE}..HEAD" --no-merges --reverse \ + --format='%H%x1f%s%x1f%(trailers:key=gate_status,valueonly,separator=%x2c)%x1e' +``` + +Records are separated by `\x1e`; the fields inside each are `\x1f`-separated — ``, ``, ``. + +Pair commits by their conventional-commit type (the `type:` prefix of the subject): + +- A `test:` commit is the RED row. Pair it with the next following **implementation** commit — a `feat:` or `fix:` — as its **Impl commit** (the GREEN step), skipping over any intervening `refactor:`, `docs:`, or `chore:` commits so they are never mistaken for the GREEN step. +- A `refactor:`, `docs:`, or `chore:` commit that is not consumed as an Impl pairing is a standalone row with Impl commit `—`. +- A `feat:`/`fix:` commit with no preceding unpaired `test:` is a standalone row. + +Surface each commit's `gate_status:` value, normalized to exactly one of `skill`, `fallback`, `exempt`, or `missing` — never the raw trailer text. A commit whose trailer is absent, whose value is none of the first three, or which carries more than one `gate_status:` trailer (ambiguous) is counted as **missing** and still listed. This section is informational; it never blocks the ship. + +Harden every table cell against injection, not just subjects: escape `|` as `\|` and strip `\r`/`\n` from both commit subjects and the rendered `gate_status` value. Prefer NUL (`-z` / `%x00`) record separation, and reject any record whose fields contain the `\x1f`/`\x1e` delimiters, so an adversarial commit message cannot corrupt record or field boundaries. + +```markdown +## TDD Audit + +| Test commit | Impl commit | gate_status | +|---|---|---| +| `a1b2c3d` test: failing parser test | `e4f5g6h` feat: implement parser | skill | +| `i7j8k9l` test: failing export test | `m0n1o2p` feat: implement export | fallback | +| `q3r4s5t` refactor: extract helper | — | exempt | + +Aggregate: 2 skill, 1 fallback, 1 exempt — 0 missing. +``` + +This `## TDD Audit` section is the final body section — it renders after the configured `pr_body_sections`, immediately before the aggregate trailer — so the frozen core sections and the append-only configured sections both keep their existing order. + +**9. Aggregate gate_status trailer (final line):** + +After every other section — including any configured `pr_body_sections` — emit the audit aggregate as a single Git trailer on the **final line** of the PR body, preceded by a blank line so it parses as a valid trailer: + +``` +gate_status: skill=2, fallback=1, exempt=1, missing=0 +``` + +Use the exact key order `skill=`, `fallback=`, `exempt=`, `missing=` so downstream tooling parses it stably. Keeping it last means a GitHub squash-merge that defaults its commit message to the PR description carries the aggregate into `${BASE_BRANCH}`, preserving the audit footprint in `git log` after the PR branch is deleted. (Best-effort: it depends on the repo's squash-message default; the in-body `## TDD Audit` section is the source of truth regardless.) diff --git a/tests/feat-41-ship-tdd-audit-gate-status.test.cjs b/tests/feat-41-ship-tdd-audit-gate-status.test.cjs new file mode 100644 index 000000000..444a8d503 --- /dev/null +++ b/tests/feat-41-ship-tdd-audit-gate-status.test.cjs @@ -0,0 +1,98 @@ +'use strict'; + +// feat(#41): /gsd-ship generate_pr_body emits a TDD Audit table + an aggregate +// `gate_status:` trailer so the per-commit TDD gate trail survives squash-merge. +// These assertions pin the shipped workflow prose in get-shit-done/workflows/ship.md. + +const fs = require('node:fs'); +const path = require('node:path'); +const assert = require('node:assert/strict'); +const { describe, test } = require('node:test'); + +const repoRoot = path.resolve(__dirname, '..'); +function readRepoFile(relativePath) { + return fs.readFileSync(path.join(repoRoot, relativePath), 'utf8'); +} + +describe('feat-41: ship.md TDD Audit gate_status extraction', () => { + const workflow = readRepoFile('get-shit-done/workflows/ship.md'); + + test('adds a "## TDD Audit" section to the generated PR body', () => { + assert.match(workflow, /## TDD Audit/); + }); + + test('extracts gate_status via Git native trailer machinery, not a raw body grep', () => { + assert.match(workflow, /trailers:key=gate_status/); + }); + + test('scopes the scan to the merge-base..HEAD range', () => { + assert.match(workflow, /merge-base/); + assert.match(workflow, /\.\.HEAD/); + assert.match(workflow, /BASE_BRANCH/); + }); + + test('excludes merge commits from the audit', () => { + assert.match(workflow, /--no-merges/); + }); + + test('renders a Test commit / Impl commit / gate_status table', () => { + assert.match(workflow, /Test commit[\s\S]*Impl commit[\s\S]*gate_status/); + }); + + test('pairs conventional-commit test: rows with their impl commit', () => { + assert.match(workflow, /test:/); + assert.match(workflow, /pair/i); + }); + + test('escapes pipe characters in commit subjects so the table is not broken', () => { + assert.match(workflow, /[Ee]scape[\s\S]{0,60}\|/); + }); + + test('counts commits lacking a recognized gate_status trailer as missing', () => { + assert.match(workflow, /missing/); + }); + + test('is informational and never blocks the ship', () => { + assert.match(workflow, /informational|never block|non-blocking/i); + }); + + test('emits the aggregate trailer in the exact, stable key order', () => { + assert.match( + workflow, + /gate_status:\s*skill=[^,]*,\s*fallback=[^,]*,\s*exempt=[^,]*,\s*missing=/, + ); + }); + + test('places the aggregate trailer on the final line so squash-merge carries it', () => { + assert.match(workflow, /squash/i); + assert.match(workflow, /final line|last line/i); + }); + + test('does not disturb the frozen #3167 core section order (Key Decisions precedes the new section)', () => { + assert.match(workflow, /## Key Decisions[\s\S]*## TDD Audit/); + }); + + // Hardening assertions added after adversarial review. + + test('pairs test: rows only with feat:/fix: impl commits, skipping refactor/docs/chore', () => { + assert.match(workflow, /feat:[\s\S]{0,20}fix:/); + assert.match(workflow, /skipping[\s\S]{0,80}(refactor|docs|chore)/i); + }); + + test('normalizes the gate_status cell to a known token, never raw trailer text', () => { + assert.match(workflow, /normaliz[a-z]*[\s\S]{0,120}missing/i); + assert.match(workflow, /never the raw/i); + }); + + test('treats a commit with multiple gate_status trailers as missing', () => { + assert.match(workflow, /more than one[\s\S]{0,40}gate_status/i); + }); + + test('hardens every table cell against pipe/newline injection', () => { + assert.match(workflow, /strip[\s\S]{0,20}\\r/); + }); + + test('guards record/field delimiters against adversarial commit messages', () => { + assert.match(workflow, /NUL|%x00|delimiter/i); + }); +});