diff --git a/.changeset/patient-jaguars-rally.md b/.changeset/patient-jaguars-rally.md new file mode 100644 index 000000000..75605916e --- /dev/null +++ b/.changeset/patient-jaguars-rally.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4228 +--- +**Non-TDD executor dispatches no longer embed the full RED/GREEN/REFACTOR protocol three times over** — the cycle is stated once in the canonical `gsd-core/references/tdd.md`, consumers carry pointers, and both dispatch paths load the reference only when the dispatch is actually TDD. (#3990) diff --git a/agents/gsd-executor.md b/agents/gsd-executor.md index 9f964b362..fb27422a0 100644 --- a/agents/gsd-executor.md +++ b/agents/gsd-executor.md @@ -399,13 +399,10 @@ When executing task with `tdd="true"`: **1. Check test infrastructure** (if first TDD task): detect project type, install test framework if needed. -**2. RED:** Read ``, create test file, write failing tests, run (MUST fail), commit: `test({phase}-{plan}): add failing test for [feature]` - -**3. GREEN:** Read ``, write minimal code to pass, run (MUST pass), commit: `feat({phase}-{plan}): implement [feature]` - -**4. REFACTOR (if needed):** Clean up, run tests (MUST still pass), commit only if changes: `refactor({phase}-{plan}): clean up [feature]` - -**Error handling:** RED doesn't fail ��� investigate. GREEN doesn't pass → debug/iterate. REFACTOR breaks → undo. +**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. ## Plan-Level TDD Gate Enforcement (type: tdd plans) diff --git a/gsd-core/workflows/execute-phase.md b/gsd-core/workflows/execute-phase.md index c57512b92..7918764b9 100644 --- a/gsd-core/workflows/execute-phase.md +++ b/gsd-core/workflows/execute-phase.md @@ -757,7 +757,7 @@ increases monotonically across waves. `{status}` is `complete` (success), - `~/.claude/gsd-core/workflows/execute-plan.md` - `~/.claude/gsd-core/templates/summary.md` - `~/.claude/gsd-core/references/checkpoints.md` - - `~/.claude/gsd-core/references/tdd.md` + ${TDD_APPLICABLE ? '- `~/.claude/gsd-core/references/tdd.md`' : ''} # #3990: only when this dispatch is TDD (plan type: tdd, a tdd="true" task, or TDD_MODE=true) - `~/.claude/gsd-core/references/worktree-path-safety.md` ${CONTEXT_WINDOW < 200000 ? '' : '- `~/.claude/gsd-core/references/executor-examples.md`'} diff --git a/gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md b/gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md index c8b6c348b..e17d93311 100644 --- a/gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md +++ b/gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md @@ -181,7 +181,7 @@ the executor workflow from repository search. SUMMARY commit semantics and the gitignored-planning skip contract) - summary.md template - checkpoints.md -- tdd.md +${TDD_APPLICABLE ? "- tdd.md" : ""} # #3990: only when this dispatch is TDD (plan type: tdd, or TDD_MODE=true) - worktree-path-safety.md - agents/gsd-executor.md (the ROLE DEFINITION you are executing — its steps 0/0a/0b per-commit HEAD/cwd-drift/path-guard discipline applies to every diff --git a/gsd-core/workflows/execute-plan.md b/gsd-core/workflows/execute-plan.md index a0d6c6a4b..9ba3d3d9e 100644 --- a/gsd-core/workflows/execute-plan.md +++ b/gsd-core/workflows/execute-plan.md @@ -292,13 +292,12 @@ 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. **RED:** Read `` → failing test(s) → run (MUST fail) → commit: `test({phase}-{plan}): add failing test for [feature]` -3. **GREEN:** Read `` → minimal code → run (MUST pass) → commit: `feat({phase}-{plan}): implement [feature]` -4. **REFACTOR:** Clean up → tests MUST pass → commit: `refactor({phase}-{plan}): clean up [feature]` - -Errors: RED doesn't fail → investigate test/existing feature. GREEN doesn't pass → debug, iterate. REFACTOR breaks → undo. - -See `~/.claude/gsd-core/references/tdd.md` for structure. +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. diff --git a/scripts/npm-audit-baseline.cjs b/scripts/npm-audit-baseline.cjs index 324bd0167..c12e182c8 100644 --- a/scripts/npm-audit-baseline.cjs +++ b/scripts/npm-audit-baseline.cjs @@ -105,15 +105,32 @@ function runPackageLockAudit(cwd) { break; } catch (e) { // `npm audit` exits non-zero when advisories are present; the JSON is - // still on stdout in that case. Recover and let the caller classify. - if (e && typeof e.stdout !== 'undefined' && e.stdout !== undefined && e.stdout !== null) { - out = Buffer.isBuffer(e.stdout) ? e.stdout.toString('utf-8') : String(e.stdout); + // still on stdout in that case. Recover and let the caller classify — + // but only when stdout actually CARRIES the JSON: an audit that was + // killed or aborted can exit non-zero with EMPTY stdout, and accepting + // the empty string here surfaces as a bare `SyntaxError: Unexpected + // end of JSON input` at the parse below, hiding the captured error + // (observed on CI 2026-09-03, both lanes; cause undetermined). + const recovered = e && typeof e.stdout !== 'undefined' && e.stdout !== null + ? (Buffer.isBuffer(e.stdout) ? e.stdout.toString('utf-8') : String(e.stdout)) + : ''; + if (recovered.trim()) { + out = recovered; lastErr = null; break; } lastErr = e; } } + if (!out || !out.trim()) { + const detail = lastErr + ? [lastErr.stdout, lastErr.stderr, String(lastErr.message)].filter(Boolean).join('\n').slice(0, 500) + : '(no error captured)'; + throw new Error( + `npm audit --json produced no output in ${cwd}. ` + + `Detail from the failed invocation:\n${detail}` + ); + } if (lastErr) throw lastErr; const parsed = JSON.parse(out); if (parsed && parsed.metadata && parsed.metadata.vulnerabilities) { diff --git a/tests/no-bare-gsd-tools-command-position.test.cjs b/tests/no-bare-gsd-tools-command-position.test.cjs index 53236433c..8d5e946c2 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: 819, 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: 816, 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: 419, 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: 418, 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/npm-integrity-gate.test.cjs b/tests/npm-integrity-gate.test.cjs index 9ab0e9df6..76156601d 100644 --- a/tests/npm-integrity-gate.test.cjs +++ b/tests/npm-integrity-gate.test.cjs @@ -244,15 +244,33 @@ function auditProductionVulns(cwd) { break; } catch (e) { // `npm audit` exits non-zero when advisories are present; the JSON is - // still on stdout in that case. Recover and let the assertion classify. - if (e && typeof e.stdout !== 'undefined' && e.stdout !== undefined && e.stdout !== null) { - out = Buffer.isBuffer(e.stdout) ? e.stdout.toString('utf-8') : String(e.stdout); + // still on stdout in that case. Recover and let the assertion classify — + // but only when stdout actually CARRIES the JSON. A killed or aborted + // audit exits non-zero with EMPTY stdout (npm writes plain-text errors + // to stderr); accepting the empty string here used to surface as + // `SyntaxError: Unexpected end of JSON input` at the parse below, hiding + // the real cause. Keep the candidate loop going and let the explicit + // empty-output throw below name npm's stderr instead. + const recovered = e && typeof e.stdout !== 'undefined' && e.stdout !== null + ? (Buffer.isBuffer(e.stdout) ? e.stdout.toString('utf-8') : String(e.stdout)) + : ''; + if (recovered.trim()) { + out = recovered; lastErr = null; break; } lastErr = e; } } + if (!out || !out.trim()) { + const detail = lastErr + ? [lastErr.stdout, lastErr.stderr, String(lastErr.message)].filter(Boolean).join('\n').slice(0, 500) + : '(no error captured)'; + throw new Error( + `npm audit --json produced no output. ` + + `Detail from the failed invocation:\n${detail}` + ); + } if (lastErr) throw lastErr; const parsed = JSON.parse(out); // `null` is reserved for the "node_modules missing → skip" signal above. diff --git a/tests/tdd-single-statement.test.cjs b/tests/tdd-single-statement.test.cjs new file mode 100644 index 000000000..9feb10442 --- /dev/null +++ b/tests/tdd-single-statement.test.cjs @@ -0,0 +1,74 @@ +'use strict'; + +/** + * #3990 — the RED/GREEN/REFACTOR cycle is stated ONCE. + * + * The cycle used to be restated verbatim in three files (references/tdd.md, + * agents/gsd-executor.md , workflows/execute-plan.md + * ), and tdd.md was embedded into EVERY executor dispatch + * unconditionally — so non-TDD tasks paid for the whole cycle three times. + * The contract now: one canonical statement in tdd.md, consumers carry + * pointers, and the embed lists load tdd.md only when the dispatch is TDD. + * Deployed text IS the runtime-loaded product; shape assertions are the check. + */ + +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'); + +// #4228 diagnosis: the first draft used a regex with two lazy [^]*? spans to +// detect a restated RED/GREEN pair — on a 47KB agent file that is a +// superlinear backtracking walk which completed on linux but pinned a Windows +// CI core for the whole 32-minute job cap (the lane's three cancellations at +// 21m/32m/41m all traced here). IndexOf on disjoint anchors is linear and +// cannot backtrack: a restatement exists iff a RED commit-scope line and a +// LATER GREEN commit-scope line both appear outside the canonical reference. +function restatesCycle(text) { + const red = text.indexOf('commit: `test({phase}-{plan})'); + if (red === -1) return false; + const green = text.indexOf('commit: `feat({phase}-{plan})', red); + return green !== -1; +} + +describe('#3990 — one statement of the cycle', () => { + test('the cycle is stated in full only in the canonical reference', () => { + const tdd = read('gsd-core/references/tdd.md'); + assert.ok(/## Red-Green-Refactor Cycle/.test(tdd), + 'tdd.md carries the canonical Red-Green-Refactor Cycle section'); + assert.ok(/Commit: `test\(\{phase\}-\{plan\}\)/.test(tdd) && /Commit: `feat\(\{phase\}-\{plan\}\)/.test(tdd), + 'the canonical section carries the commit-scope contract'); + }); + + test('the executor carries a pointer, not a third restatement', () => { + const executor = read('agents/gsd-executor.md'); + assert.ok(!restatesCycle(executor), + ' must not restate the numbered RED/GREEN commit protocol — point at tdd.md'); + assert.ok(/references\/tdd\.md/.test(executor), + 'the executor points at the canonical reference'); + }); + + test('execute-plan carries a pointer, not a second restatement', () => { + const plan = read('gsd-core/workflows/execute-plan.md'); + assert.ok(!restatesCycle(plan), + ' must not restate the numbered RED/GREEN commit protocol'); + assert.ok(/references\/tdd\.md/.test(plan.slice(plan.indexOf('tdd_plan_execution'))), + 'the plan-execution section points at the canonical reference'); + }); + + test('both embed lists load tdd.md only when the dispatch is TDD', () => { + const main = read('gsd-core/workflows/execute-phase.md'); + const wt = read('gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md'); + for (const [name, text] of [['execute-phase.md', main], ['executor-isolation-dispatch.md', wt]]) { + // The LIST entry line — not any prose line that mentions tdd.md (the TDD + // gate's own prose cites it too). + const line = text.split('\n').find((l) => /tdd\.md/.test(l) && /TDD_APPLICABLE/.test(l)); + assert.ok(line, `${name} still lists tdd.md as a conditional embed entry`); + assert.ok(/TDD_APPLICABLE \?/.test(line), + `${name}'s tdd.md entry must be conditional on TDD_APPLICABLE (#3990), got: ${line.trim()}`); + } + }); +});