fix(#3990): state the RED/GREEN/REFACTOR cycle once, embed tdd.md conditionally (#4228)

* test(#3990): the RED/GREEN/REFACTOR cycle is stated once, embedded conditionally

* fix(#3990): state the cycle once — pointers in consumers, conditional tdd.md embeds

Emitted-Drift-Ack-Growth: execute-phase.md — #3990 conditions the tdd.md embed on the dispatch being TDD

* chore(#3990): changeset for the single-statement TDD cycle

* chore(#3990): backfill changeset pr number

* fix(#4228): linear cycle check — the lazy-span regex pinned a Windows core for the whole job cap

* test(#3990): allowlist pin tracks the rebased line

* fix(tests): npm-integrity gate names an empty audit output explicitly — empty stdout crashed the parse as a bare SyntaxError

* fix: name an empty npm-audit stdout explicitly — it crashed the parse as a bare SyntaxError

Observed on CI (several branches, all lanes): spawnSync npm ETIMEDOUT with
empty stdout; the empty string survived the recovery path and surfaced as
'SyntaxError: Unexpected end of JSON input', hiding the captured error. The
recovery path now requires non-empty stdout, and an empty result throws with
the captured stdout/stderr/message so the actual error is on the record.
Root cause of the ETIMEDOUT itself is NOT diagnosed here — this change only
stops masking it.

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-09-03 21:41:14 -04:00
committed by GitHub
parent 590edec7a7
commit 97ce61dee2
9 changed files with 134 additions and 24 deletions

View File

@@ -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)

View File

@@ -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 `<behavior>`, create test file, write failing tests, run (MUST fail), commit: `test({phase}-{plan}): add failing test for [feature]`
**3. GREEN:** Read `<implementation>`, 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 <20><><EFBFBD> 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)

View File

@@ -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`'}
</execution_context>

View File

@@ -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

View File

@@ -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 `<behavior>` → failing test(s) → run (MUST fail) → commit: `test({phase}-{plan}): add failing test for [feature]`
3. **GREEN:** Read `<implementation>` → 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.
</tdd_plan_execution>
<precommit_failure_handling>

View File

@@ -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) {

View File

@@ -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 <subcommand>` 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

View File

@@ -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.

View File

@@ -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 <tdd_execution>, workflows/execute-plan.md
* <tdd_plan_execution>), 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),
'<tdd_execution> 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),
'<tdd_plan_execution> 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()}`);
}
});
});