* test(#3962): shipped TDD-Audit trailer token must round-trip through git Behavioral coverage: extract the trailers:key token from ship.md and prove a real git commit carrying that trailer reads back via %(trailers:key=token,valueonly). gate_status contains an underscore, which git's trailer machinery cannot tokenize, so the audit read was structurally empty. * fix(#3962): rename the TDD-Audit trailer token to gate-status Underscore is not a valid git trailer token character, so %(trailers:key=gate_status,...) could never match. Renames the token at the read, the documented aggregate write, and the section's prose/table header. Self-suppression semantics (#2431) unchanged. * test(#3962): compare outcome against the seam's literal, fix doc token Review follow-ups: the round-trip test compared outcome to 'EXITED' but the process seam emits 'exited'; and the trailer rename is propagated to docs/ship-pr-body-sections.md's read/write examples. * chore(#3962): backfill changeset pr number --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
5
.changeset/sunny-ravens-leap.md
Normal file
5
.changeset/sunny-ravens-leap.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 4174
|
||||
---
|
||||
**TDD Audit no longer reads a git trailer token git cannot parse** — the trailer token is renamed gate_status → gate-status, so the per-commit gate trail becomes machine-readable the moment a producer starts writing it; previously every commit read as missing and the section self-suppressed silently. (#3962)
|
||||
@@ -19,12 +19,12 @@ Custom sections are append-only. They render after `Key Decisions` (and before t
|
||||
|
||||
### 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 `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
|
||||
gate-status: skill=3, fallback=1, exempt=0, missing=0
|
||||
```
|
||||
|
||||
## Configure Sections During Onboarding
|
||||
|
||||
@@ -309,16 +309,16 @@ 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:
|
||||
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'
|
||||
--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 — `<sha>`, `<subject>`, `<gate_status value>`.
|
||||
Records are separated by `\x1e`; the fields inside each are `\x1f`-separated — `<sha>`, `<subject>`, `<gate-status value>`.
|
||||
|
||||
Pair commits by their conventional-commit type (the `type:` prefix of the subject):
|
||||
|
||||
@@ -326,16 +326,16 @@ Pair commits by their conventional-commit type (the `type:` prefix of the subjec
|
||||
- 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.
|
||||
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.
|
||||
|
||||
**Self-suppress when every commit is missing (#2431):** the execute pipeline only writes `gate_status:` trailers when TDD mode is active. If every commit in the scan normalizes to `missing`, skip this section and the aggregate trailer (step 9) entirely — a 100%-missing table is pure noise. Only emit when at least one commit carries a real value (`skill`, `fallback`, or `exempt`).
|
||||
**Self-suppress when every commit is missing (#2431):** the execute pipeline only writes `gate-status:` trailers when TDD mode is active. If every commit in the scan normalizes to `missing`, skip this section and the aggregate trailer (step 9) entirely — a 100%-missing table is pure noise. Only emit when at least one commit carries a real value (`skill`, `fallback`, or `exempt`).
|
||||
|
||||
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.
|
||||
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 |
|
||||
| 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 |
|
||||
@@ -346,12 +346,12 @@ 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)** (only when step 8 was emitted — i.e., at least one real `gate_status` value exists):
|
||||
**9. Aggregate gate-status trailer (final line)** (only when step 8 was emitted — i.e., at least one real `gate-status` value exists):
|
||||
|
||||
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
|
||||
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.)
|
||||
|
||||
@@ -76,7 +76,7 @@ describe('workflow CLI compatibility (#1759)', () => {
|
||||
'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.
|
||||
// `gate-status:` trailer so the per-commit TDD gate trail survives squash-merge.
|
||||
// These assertions pin the shipped workflow prose in gsd-core/workflows/ship.md.
|
||||
|
||||
const fs = require('node:fs');
|
||||
@@ -89,15 +89,42 @@ function readRepoFile(relativePath) {
|
||||
return fs.readFileSync(path.join(repoRoot, relativePath), 'utf8');
|
||||
}
|
||||
|
||||
describe('feat-41: ship.md TDD Audit gate_status extraction', () => {
|
||||
describe('feat-41: ship.md TDD Audit gate-status extraction', () => {
|
||||
const workflow = readRepoFile('gsd-core/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('shipped TDD-Audit trailer token round-trips through git (#3962)', (t) => {
|
||||
// `_` is not a valid git trailer token character, so a `gate-status:`
|
||||
// trailer is invisible to interpret-trailers and %(trailers:key=...) -- the
|
||||
// audit read returned empty for every commit and self-suppression hid it.
|
||||
// Extract the token the shipped workflow actually reads, then prove REAL
|
||||
// git can match a commit carrying that exact trailer.
|
||||
const { createTempGitProject, cleanup } = require('./helpers.cjs');
|
||||
const { runGit } = require('./helpers/process-seam.cjs');
|
||||
const tokenMatch = workflow.match(/trailers:key=([A-Za-z-]+),/);
|
||||
assert.ok(tokenMatch, 'ship.md must read %(trailers:key=<token>,...) with a simple token');
|
||||
const token = tokenMatch[1];
|
||||
assert.ok(!token.includes('_'),
|
||||
`trailer token "${token}" contains "_" -- git ignores such trailers entirely (#3962)`);
|
||||
|
||||
const tmpDir = createTempGitProject('gsd-3962-');
|
||||
t.after(() => cleanup(tmpDir));
|
||||
const msg = `test(2-01): probe\n\n${token}: skill\n`;
|
||||
const c = runGit(['commit', '--allow-empty', '-m', msg], { cwd: tmpDir });
|
||||
assert.equal(c.outcome, 'exited', `probe commit must exit cleanly: ${c.stderr}`);
|
||||
const r = runGit(
|
||||
['log', '-1', `--format=%(trailers:key=${token},valueonly)`],
|
||||
{ cwd: tmpDir },
|
||||
);
|
||||
assert.equal(r.stdout.trim(), 'skill',
|
||||
`a commit carrying "${token}: skill" must read back as "skill" via the shipped token`);
|
||||
});
|
||||
|
||||
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', () => {
|
||||
@@ -110,8 +137,8 @@ describe('feat-41: ship.md TDD Audit gate_status extraction', () => {
|
||||
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('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', () => {
|
||||
@@ -123,7 +150,7 @@ describe('feat-41: ship.md TDD Audit gate_status extraction', () => {
|
||||
assert.match(workflow, /[Ee]scape[\s\S]{0,60}\|/);
|
||||
});
|
||||
|
||||
test('counts commits lacking a recognized gate_status trailer as missing', () => {
|
||||
test('counts commits lacking a recognized gate-status trailer as missing', () => {
|
||||
assert.match(workflow, /missing/);
|
||||
});
|
||||
|
||||
@@ -134,7 +161,7 @@ describe('feat-41: ship.md TDD Audit gate_status extraction', () => {
|
||||
test('emits the aggregate trailer in the exact, stable key order', () => {
|
||||
assert.match(
|
||||
workflow,
|
||||
/gate_status:\s*skill=[^,]*,\s*fallback=[^,]*,\s*exempt=[^,]*,\s*missing=/,
|
||||
/gate-status:\s*skill=[^,]*,\s*fallback=[^,]*,\s*exempt=[^,]*,\s*missing=/,
|
||||
);
|
||||
});
|
||||
|
||||
@@ -145,7 +172,7 @@ describe('feat-41: ship.md TDD Audit gate_status extraction', () => {
|
||||
|
||||
// ─── #2431: TDD Audit section self-suppresses when all commits are missing ─
|
||||
//
|
||||
// The execute pipeline only writes `gate_status:` git trailers when TDD mode
|
||||
// The execute pipeline only writes `gate-status:` git trailers when TDD mode
|
||||
// is active. Without TDD mode, every commit is `missing` and the section is
|
||||
// pure noise. The fix adds a self-suppress instruction: skip the section
|
||||
// entirely when every commit normalizes to `missing`. This is data-driven
|
||||
@@ -159,16 +186,16 @@ describe('feat-41: ship.md TDD Audit gate_status extraction', () => {
|
||||
});
|
||||
|
||||
test('#2431: step 9 (aggregate trailer) is also gated on real values existing', () => {
|
||||
// The aggregate gate_status trailer is the companion to the TDD Audit
|
||||
// section; both must be skipped together when no real gate_status exists.
|
||||
// The aggregate gate-status trailer is the companion to the TDD Audit
|
||||
// section; both must be skipped together when no real gate-status exists.
|
||||
// `\z` is a Perl/Ruby end-of-input anchor with NO meaning in JavaScript — it
|
||||
// matched a literal `z`, so the lazy span silently stopped at the first `z`
|
||||
// whenever `**10.` was absent, truncating the captured step. `$` (no /m flag)
|
||||
// is the JS end-of-input anchor.
|
||||
const step9 = workflow.match(/\*\*9\.\s*Aggregate gate_status trailer[\s\S]*?(?=\*\*10\.|$)/);
|
||||
const step9 = workflow.match(/\*\*9\.\s*Aggregate gate-status trailer[\s\S]*?(?=\*\*10\.|$)/);
|
||||
assert.ok(step9, 'step 9 must exist in the workflow');
|
||||
assert.match(step9[0], /step 8|at least one|real/i,
|
||||
'step 9 must reference step 8 or require at least one real gate_status value (#2431)');
|
||||
'step 9 must reference step 8 or require at least one real gate-status value (#2431)');
|
||||
});
|
||||
|
||||
test('#2431: does NOT read workflow.tdd_mode inline (ADR-857 Phase 6 compliant)', () => {
|
||||
@@ -191,13 +218,13 @@ describe('feat-41: ship.md TDD Audit gate_status extraction', () => {
|
||||
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', () => {
|
||||
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('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', () => {
|
||||
|
||||
Reference in New Issue
Block a user