diff --git a/.changeset/nimble-herons-rest.md b/.changeset/nimble-herons-rest.md new file mode 100644 index 000000000..f68f1e07c --- /dev/null +++ b/.changeset/nimble-herons-rest.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4295 +--- +**TDD dispatch pointers now cite the tdd.md sections that actually carry the material they promise** — the RED/GREEN/REFACTOR pointer in both `execute-plan.md` and `agents/gsd-executor.md` cited a single section for the commit-scope contract, fail-fast rule, and error handling, but only the commit-scope contract lived there; each is now cited correctly. `agents/gsd-executor.md`'s plan-level gate-enforcement rules, previously restated in full alongside `tdd.md`'s own copy, now point at `tdd.md` as the single owner. (#4267, #4269) diff --git a/agents/gsd-executor.md b/agents/gsd-executor.md index 7f84f769f..eab2ac008 100644 --- a/agents/gsd-executor.md +++ b/agents/gsd-executor.md @@ -399,23 +399,19 @@ When executing task with `tdd="true"`: **1. Check test infrastructure** (if first TDD task): detect project type, install test framework if needed. -**2-4. RED → GREEN → REFACTOR (#3990: stated ONCE):** execute the cycle exactly as the -canonical `gsd-core/references/tdd.md` "Red-Green-Refactor Cycle" section specifies (embedded -when TDD applies) — its commit-scope contract, fail-fast rule, and error handling. The -reference is the single source; do not improvise a variant. +**2-4. RED → GREEN → REFACTOR (#3990: stated ONCE; #4267: cited correctly):** execute the +cycle exactly as the canonical `gsd-core/references/tdd.md` reference specifies (embedded when +TDD applies) — the "Red-Green-Refactor Cycle" section's commit-scope contract, the "Gate +Enforcement Rules" section's "Fail-Fast Rules" subsection, and the "Error Handling" section. +The reference is the single source; do not improvise a variant. -## Plan-Level TDD Gate Enforcement (type: tdd plans) +## Plan-Level TDD Gate Enforcement (type: tdd plans, #4269: stated ONCE) -When the plan frontmatter has `type: tdd`, the entire plan follows the RED/GREEN/REFACTOR cycle as a single feature. Gate sequence is mandatory: - -**Fail-fast rules (#3770):** If a test passes unexpectedly during RED, STOP — do NOT skip RED. A nonzero exit alone is NOT RED either: persist the RED evidence (command, exit code, failing test, expected, actual) and verify with `gsd_run check tdd-red-evidence ` — only `RED_EVIDENCE_OK` (the TARGET test failed an assertion for the behavior) authorizes GREEN; any INVALID_RED (zero tests, fixture crash, parser error, wrong test) blocks it. - -**Gate sequence validation:** After completing the plan, verify in git log: -1. A `test(...)` commit exists (RED gate) -2. A `feat(...)` commit exists after it (GREEN gate) -3. Optionally a `refactor(...)` commit exists after GREEN (REFACTOR gate) - -If RED or GREEN gate commits are missing, add a warning to SUMMARY.md under a `## TDD Gate Compliance` section. +When the plan frontmatter has `type: tdd`, the mandatory RED/GREEN/REFACTOR gate sequence, +its fail-fast rules (including the #3770 INVALID_RED / intentional-RED-evidence requirement +enforced via `gsd_run check tdd-red-evidence`), and the `## TDD Gate Compliance` SUMMARY.md contract are +specified in the canonical `gsd-core/references/tdd.md` "Gate Enforcement Rules" section +(embedded when TDD applies). The reference is the single source; do not improvise a variant. ## MVP+TDD Gate diff --git a/gsd-core/workflows/execute-plan.md b/gsd-core/workflows/execute-plan.md index 9ba3d3d9e..56347cb2c 100644 --- a/gsd-core/workflows/execute-plan.md +++ b/gsd-core/workflows/execute-plan.md @@ -292,12 +292,13 @@ End with: **Total deviations:** N auto-fixed (breakdown). **Impact:** assessment For `type: tdd` plans — RED-GREEN-REFACTOR: 1. **Infrastructure** (first TDD plan only): detect project, install framework, config, verify empty suite -2. **Cycle (#3990: stated ONCE):** execute RED → GREEN → REFACTOR exactly as specified in the -canonical `~/.claude/gsd-core/references/tdd.md` "Red-Green-Refactor Cycle" section — its -commit-scope contract (`test({phase}-{plan})` → `feat({phase}-{plan})` → -`refactor({phase}-{plan})`, RED must fail, GREEN must pass, REFACTOR commits only on change), -its fail-fast rule, and its error handling. The reference is the single source; do not -improvise a variant. +2. **Cycle (#3990: stated ONCE; #4267: cited correctly):** execute RED → GREEN → REFACTOR +exactly as specified in the canonical `~/.claude/gsd-core/references/tdd.md` reference — the +"Red-Green-Refactor Cycle" section's commit-scope contract (`test({phase}-{plan})` → +`feat({phase}-{plan})` → `refactor({phase}-{plan})`, RED must fail, GREEN must pass, REFACTOR +commits only on change), the "Gate Enforcement Rules" section's "Fail-Fast Rules" subsection, +and the "Error Handling" section. The reference is the single source; do not improvise a +variant. diff --git a/tests/no-bare-gsd-tools-command-position.test.cjs b/tests/no-bare-gsd-tools-command-position.test.cjs index 8d5e946c2..0bf31e207 100644 --- a/tests/no-bare-gsd-tools-command-position.test.cjs +++ b/tests/no-bare-gsd-tools-command-position.test.cjs @@ -103,11 +103,11 @@ const BARE_COMMAND_RE = new RegExp( // Each entry MUST carry a one-line reason; the test prints the allowlist on // failure so a reviewer can see exactly what is sanctioned. const PROSE_ALLOWLIST = [ - { file: 'agents/gsd-executor.md', line: 816, reason: 'describes the SDK return envelope of `gsd-tools query commit`; not an instruction to run the bare word' }, + { file: 'agents/gsd-executor.md', line: 812, reason: 'describes the SDK return envelope of `gsd-tools query commit`; not an instruction to run the bare word' }, { file: 'agents/gsd-phase-researcher.md', line: 33, reason: 'package-legitimacy provenance rule names the command as the source of an OK verdict; descriptive' }, { file: 'agents/gsd-roadmapper.md', line: 647, reason: 'parenthetical "e.g." naming SDK queries a user *could* run; not an agent instruction' }, { file: 'agents/gsd-intel-updater.md', line: 40, reason: 'cross-platform note names the `gsd-tools intel ` CLI surface descriptively ("CLI invocations go through..."); not an agent instruction' }, - { file: 'gsd-core/workflows/execute-plan.md', line: 418, reason: 'describes the downstream SDK validation step (`validated downstream by ...`); names the mechanism, does not instruct the agent to type it' }, + { file: 'gsd-core/workflows/execute-plan.md', line: 419, reason: 'describes the downstream SDK validation step (`validated downstream by ...`); names the mechanism, does not instruct the agent to type it' }, ]; // Resolver-snippet definition lines / probes that must never be flagged. A line diff --git a/tests/tdd-reference-correctness.test.cjs b/tests/tdd-reference-correctness.test.cjs new file mode 100644 index 000000000..35de1c9dc --- /dev/null +++ b/tests/tdd-reference-correctness.test.cjs @@ -0,0 +1,121 @@ +'use strict'; + +/** + * #4267 / #4269 — tdd.md pointer citations must name the section that + * actually carries the guidance, and gsd-executor.md's plan-level gate + * restatement collapses to a pointer at tdd.md's "Gate Enforcement Rules". + * + * #4228 introduced pointers in execute-plan.md and gsd-executor.md that all + * cite a single section — "Red-Green-Refactor Cycle" — for three distinct + * pieces of guidance (commit-scope contract, fail-fast rule, error handling). + * Only the commit-scope contract actually lives there; the fail-fast rule is + * in "Fail-Fast Rules" (a subsection of "Gate Enforcement Rules") and error + * handling is in its own "Error Handling" section. (#4267) + * + * Separately, gsd-executor.md's "## Plan-Level TDD Gate Enforcement" section + * fully restates gate-sequence rules that tdd.md's "## Gate Enforcement + * Rules" section already owns (and covers more thoroughly, including the + * actual bash validation script). (#4269) + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const ROOT = path.join(__dirname, '..'); +const read = (p) => fs.readFileSync(path.join(ROOT, p), 'utf8'); + +describe('#4267 — pointer text cites the correct tdd.md section per fact', () => { + test("execute-plan.md's cycle pointer cites three distinct, correctly-named sections", () => { + const plan = read('gsd-core/workflows/execute-plan.md'); + const pointer = plan.slice(plan.indexOf(''), plan.indexOf('')); + assert.ok(/"Red-Green-Refactor Cycle"/.test(pointer), + 'commit-scope contract must cite "Red-Green-Refactor Cycle"'); + assert.ok(/"Fail-Fast Rules"/.test(pointer), + 'fail-fast rule must cite "Fail-Fast Rules" by its literal section name, not just the words "fail-fast"'); + assert.ok(/"Error Handling"/.test(pointer), + 'error handling must cite "Error Handling" by its literal section name'); + }); + + test("gsd-executor.md's cycle pointer cites three distinct, correctly-named sections", () => { + const executor = read('agents/gsd-executor.md'); + const pointer = executor.slice(executor.indexOf(''), executor.indexOf('## Plan-Level') > -1 ? executor.indexOf('## Plan-Level') : executor.indexOf('')); + assert.ok(/"Red-Green-Refactor Cycle"/.test(pointer), + 'commit-scope contract must cite "Red-Green-Refactor Cycle"'); + assert.ok(/"Fail-Fast Rules"/.test(pointer), + 'fail-fast rule must cite "Fail-Fast Rules" by its literal section name, not just the words "fail-fast"'); + assert.ok(/"Error Handling"/.test(pointer), + 'error handling must cite "Error Handling" by its literal section name'); + }); + + test('tdd.md\'s "Fail-Fast Rules" section actually contains fail-fast guidance', () => { + const tdd = read('gsd-core/references/tdd.md'); + const start = tdd.indexOf('### Fail-Fast Rules'); + assert.ok(start !== -1, 'tdd.md must carry a "### Fail-Fast Rules" section'); + const end = tdd.indexOf('### Executor Gate Validation', start); + const section = tdd.slice(start, end === -1 ? undefined : end); + assert.ok(/Unexpected GREEN in RED phase/i.test(section), + 'the Fail-Fast Rules section must actually describe the unexpected-pass-in-RED rule'); + }); + + test('tdd.md\'s "Error Handling" section actually contains error-handling guidance', () => { + const tdd = read('gsd-core/references/tdd.md'); + const start = tdd.indexOf('## Error Handling'); + assert.ok(start !== -1, 'tdd.md must carry an "## Error Handling" section'); + const end = tdd.indexOf('## Commit Pattern for TDD Plans', start); + const section = tdd.slice(start, end === -1 ? undefined : end); + assert.ok(/fail/i.test(section) && /RED|GREEN|REFACTOR/.test(section), + 'the Error Handling section must actually describe fail-in-RED/GREEN/REFACTOR guidance'); + }); +}); + +describe('#4269 — plan-level gate rules are owned once, by tdd.md', () => { + // The OLD gsd-executor.md restatement's distinctive two-line gate-sequence + // pattern (its own phrasing, distinct from tdd.md's). + const OLD_RESTATEMENT_MARKERS = [ + /A `test\(\.\.\.\)` commit exists \(RED gate\)/, + /A `feat\(\.\.\.\)` commit exists after it \(GREEN gate\)/, + ]; + + test('gsd-executor.md no longer restates the plan-level gate rules inline', () => { + const executor = read('agents/gsd-executor.md'); + for (const marker of OLD_RESTATEMENT_MARKERS) { + assert.ok(!marker.test(executor), + `gsd-executor.md must not restate the old gate-sequence wording ${marker} — point at tdd.md's "Gate Enforcement Rules" instead`); + } + assert.ok(/references\/tdd\.md/.test(executor) && /Gate Enforcement Rules/.test(executor), + 'gsd-executor.md must point at tdd.md\'s "Gate Enforcement Rules" section'); + }); + + test('gsd-executor.md\'s plan-level gate section is now a short pointer, not a restatement', () => { + const executor = read('agents/gsd-executor.md'); + const headingIdx = executor.search(/## Plan-Level TDD Gate Enforcement/); + assert.ok(headingIdx !== -1, 'the "## Plan-Level TDD Gate Enforcement" heading must still exist as an anchor'); + const nextHeadingIdx = executor.indexOf('', headingIdx); + const section = executor.slice(headingIdx, nextHeadingIdx === -1 ? undefined : nextHeadingIdx); + const lineCount = section.split('\n').filter((l) => l.trim().length > 0).length; + assert.ok(lineCount <= 6, + `the plan-level gate section must be a short pointer (<=6 non-blank lines), got ${lineCount} lines:\n${section}`); + }); + + test('the gate-sequence rules (fail-fast + git-log validation + TDD Gate Compliance) appear in full only in tdd.md', () => { + const tdd = read('gsd-core/references/tdd.md'); + const executor = read('agents/gsd-executor.md'); + + // tdd.md owns the full contract in its own words. + assert.ok(/Unexpected GREEN in RED phase/.test(tdd), + 'tdd.md must carry the fail-fast rule'); + assert.ok(/git log --oneline -E --grep/.test(tdd), + 'tdd.md must carry the actual git-log gate validation script'); + assert.ok(/## TDD Gate Compliance/.test(tdd), + 'tdd.md must carry the TDD Gate Compliance SUMMARY.md contract'); + + // gsd-executor.md must not carry the actual validation script or the + // old fail-fast prose — only a pointer. + assert.ok(!/git log --oneline -E --grep/.test(executor), + 'gsd-executor.md must not restate the git-log gate validation script — that lives only in tdd.md'); + assert.ok(!/If a test passes unexpectedly during RED, STOP/.test(executor), + 'gsd-executor.md must not restate the fail-fast rule prose — that lives only in tdd.md'); + }); +});