diff --git a/.changeset/eager-moles-squeak.md b/.changeset/eager-moles-squeak.md new file mode 100644 index 000000000..adc62fd62 --- /dev/null +++ b/.changeset/eager-moles-squeak.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4715 +--- +**TDD mode no longer trips on every task in Go, Ruby, Elixir and Python projects** — the RED-commit gate looked only for `*.test.*`, `*.spec.*` and `tests/`, so a commit adding `foo_test.go` was invisible and every behaviour-adding task halted with `TDD GATE TRIPPED: missing RED commit`. It now recognises the conventions the TDD reference already advertises, and matches at the repo root as well as in subdirectories. Rust stays a documented gap: `#[test]` lives in the implementation file, so no path-based gate can see it. (#4379) diff --git a/gsd-core/references/tdd.md b/gsd-core/references/tdd.md index 5915d99bc..6ec87fa65 100644 --- a/gsd-core/references/tdd.md +++ b/gsd-core/references/tdd.md @@ -178,10 +178,32 @@ cargo test # Rust ``` **5. Create first test file:** -Follow project conventions for test location: -- `*.test.ts` / `*.spec.ts` next to source +Follow project conventions for test location. The RED-commit gate +(`workflows/execute-phase.md`) recognises the patterns below at any depth, repo root +included. Note the gate's pathspec is deliberately **wider than the project types the +detection step above enumerates** — it costs nothing to recognise a convention the +onboarding flow does not yet auto-detect, and a project using one should not have its +RED commits go unseen: +- `*.test.*` / `*.spec.*` next to source — JS/TS and anything sharing the convention - `__tests__/` directory - `tests/` directory at root +- `*_test.go` — Go +- `test_*.py` / `*_test.py` — Python (in addition to `tests/`) +- `*_test.exs` — Elixir +- `*_spec.rb` / `*_test.rb` — Ruby + +**Cost of the broad pathspec (#4379).** `*.spec.*` can match a non-test file that happens to carry +the word — `api.spec.json`, `openapi.spec.yaml` — which lets the RED gate pass on a commit touching +only that. This is not new: the previous `**/*.spec.*` already matched those at any nested path, so +dropping the `**/` prefix only extends the same false-positive class to the repo root. It is +accepted rather than narrowed, because narrowing it is a behaviour change to the +currently-supported case and not part of making other languages visible. + +**Known gap — Rust (#4379).** `#[test]` conventionally lives inside the implementation file, so a +Rust RED commit touches `src/*.rs` and no path-based gate can distinguish it from ordinary source. +Widening the pathspec to cover it would match all source and make the gate meaningless. `cargo test` +works; the RED-*commit* gate cannot see it, so a Rust project using `workflow.tdd_mode` should +expect the gate to trip. Framework setup is a one-time cost included in the first TDD plan's RED phase. diff --git a/gsd-core/workflows/execute-phase.md b/gsd-core/workflows/execute-phase.md index eb9464bd8..01c80d2fa 100644 --- a/gsd-core/workflows/execute-phase.md +++ b/gsd-core/workflows/execute-phase.md @@ -223,7 +223,14 @@ if [ "$TDD_MODE" = "true" ]; then PLAN_N=$((10#${PLAN_ID})) PLAN_SCOPE_RE="^[a-z]+\((0*${PHASE_N})-(0*${PLAN_N})\):" # TDD gate's own scope check TDD_MILESTONE_BASE=$(git describe --tags --abbrev=0 2>/dev/null || echo "") - RED_COMMIT=$(git log --oneline -E ${TDD_MILESTONE_BASE:+"$TDD_MILESTONE_BASE..HEAD"} --grep="${PLAN_SCOPE_RE}" -- "**/*.test.*" "**/*.spec.*" "tests/" | head -1) + # #4379: this pathspec IS the gate's definition of "a test file". It was + # JS/TS-only, so Go tripped on every task; and `**/` never matches a + # root-level path, so a root `foo.test.js` was invisible too. Bare globs + # match at every depth. NOT widened to ordinary source — that would make + # the gate pass on any in-scope commit. Trade-offs and the Rust gap: + # references/tdd.md § Create first test file. + + RED_COMMIT=$(git log --oneline -E ${TDD_MILESTONE_BASE:+"$TDD_MILESTONE_BASE..HEAD"} --grep="${PLAN_SCOPE_RE}" -- "*.test.*" "*.spec.*" "tests/" "__tests__/" "*_test.go" "test_*.py" "*_test.py" "*_test.exs" "*_spec.rb" "*_test.rb" | head -1) if [ -z "$RED_COMMIT" ]; then gsd_run query state.update last_gate_trip "${PLAN_ID}/${TASK_ID}" || true echo "TDD GATE TRIPPED: missing RED commit for ${PLAN_ID}/${TASK_ID}" diff --git a/tests/fixtures/compact-content-benchmark-baseline.json b/tests/fixtures/compact-content-benchmark-baseline.json index 77715c550..f80617f3c 100644 --- a/tests/fixtures/compact-content-benchmark-baseline.json +++ b/tests/fixtures/compact-content-benchmark-baseline.json @@ -18,9 +18,9 @@ "reductionPct": 16.51 }, "execute-phase": { - "offTokens": 25952, - "onTokens": 23701, - "reductionPct": 8.67 + "offTokens": 26103, + "onTokens": 23852, + "reductionPct": 8.62 }, "new-project": { "offTokens": 14308, @@ -39,8 +39,8 @@ } }, "aggregate": { - "offTokens": 107440, - "onTokens": 90792, - "reductionPct": 15.5 + "offTokens": 107591, + "onTokens": 90943, + "reductionPct": 15.47 } } diff --git a/tests/safe-resume-gate-anchoring.test.cjs b/tests/safe-resume-gate-anchoring.test.cjs index 87f4acfde..91d1a6fb1 100644 --- a/tests/safe-resume-gate-anchoring.test.cjs +++ b/tests/safe-resume-gate-anchoring.test.cjs @@ -63,7 +63,7 @@ describe('#4003 — safe_resume_gate commit-scope greps', () => { assert.ok(w.includes('PHASE_INT=${PHASE_NUMBER%%.*}; PHASE_FRAC=${PHASE_NUMBER#"$PHASE_INT"}') && w.includes('PHASE_N="$((10#$PHASE_INT))${PHASE_FRAC//./\\\\.}"') && w.includes('PLAN_N=$((10#${PLAN_ID}))'), 'the TDD block derives zero-stripped components (#4619: leading integer segment only, decimal/N-segment tolerant)'); - assert.ok(w.includes('RED_COMMIT=$(git log --oneline -E ${TDD_MILESTONE_BASE:+"$TDD_MILESTONE_BASE..HEAD"} --grep="${PLAN_SCOPE_RE}" -- "**/*.test.*"'), + assert.ok(w.includes('RED_COMMIT=$(git log --oneline -E ${TDD_MILESTONE_BASE:+"$TDD_MILESTONE_BASE..HEAD"} --grep="${PLAN_SCOPE_RE}" -- "*.test.*"'), 'the RED grep must use the same anchored padding-tolerant scope, milestone-bounded'); assert.ok(!w.includes('--grep="^test(${PHASE_NUMBER}-${PLAN_ID})"'), 'the padded-literal RED grep must not remain'); @@ -137,3 +137,100 @@ describe('#4003 — safe_resume_gate commit-scope greps', () => { assert.ok(!unbounded.some((l) => /in prose/.test(l)), 'the anchor still holds without a tag'); }); }); + +// ─── #4379: the RED pathspec is language-agnostic, and matches at root ────── +// +// The pathspec IS this gate's definition of "a test file". It was JS/TS-only, so +// a commit adding `foo_test.go` was invisible and every behaviour-adding task in +// a Go project halted with `TDD GATE TRIPPED: missing RED commit` — while +// references/tdd.md:175-177 advertises `go test ./...` as supported. +// +// These rows drive the REAL pathspec, extracted from the shipped workflow, against +// a real git repo. Re-typing the pattern into the test would only assert that two +// copies of a string agree; running it answers the question the gate actually asks. +describe('#4379 — the TDD RED pathspec is language-agnostic', () => { + // Pull the pathspec out of the workflow rather than restating it: the thing + // under test is what SHIPS, not a copy maintained alongside it. + function shippedRedPathspec() { + const w = fs.readFileSync(WORKFLOW, 'utf8'); + const line = w.split(/\r?\n/).find((l) => l.includes('RED_COMMIT=$(git log')); + assert.ok(line, 'the RED_COMMIT line must exist in execute-phase.md'); + const tail = line.slice(line.indexOf('-- ') + 3, line.lastIndexOf('| head -1)')); + const specs = (tail.match(/"[^"]{1,200}"/g) || []).map((s) => s.slice(1, -1)); + assert.ok(specs.length > 0, `expected quoted pathspecs, parsed none from: ${tail}`); + return specs; + } + + // One seeded repo, one commit, reused by every row below. + function seedRepo(t) { + const dir = createTempGitProject('gsd-4379-'); + t.after(() => cleanup(dir)); + const files = [ + 'foo_test.go', 'pkg/bar_test.go', // Go — incl. ROOT level + 'root.test.js', 'src/a.test.ts', // JS/TS — incl. ROOT level + 'tests/c.py', '__tests__/d.js', // directory conventions + 'test_mod.py', 'mod_test.py', // Python, outside tests/ + 'lib_spec.rb', 'x_test.exs', // Ruby, Elixir + 'src/impl.go', 'src/lib.rs', // NOT tests — must never match + ]; + for (const rel of files) { + const abs = path.join(dir, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, 'x\n'); + } + gitOrThrow(['add', '-A'], { cwd: dir }); + gitOrThrow(['commit', '-m', 'test(1-01): seed every convention'], { cwd: dir }); + return dir; + } + + function matched(dir, specs) { + const out = gitOrThrow(['log', '--format=', '--name-only', '--', ...specs], { cwd: dir }); + return new Set(out.split(/\r?\n/).map((l) => l.trim()).filter(Boolean)); + } + + test('every convention tdd.md advertises is visible to the RED detector', (t) => { + const dir = seedRepo(t); + const hits = matched(dir, shippedRedPathspec()); + for (const rel of [ + 'foo_test.go', 'pkg/bar_test.go', + 'test_mod.py', 'mod_test.py', + 'lib_spec.rb', 'x_test.exs', + ]) { + assert.ok(hits.has(rel), `${rel} must satisfy the RED detector (#4379); matched: ${[...hits].sort().join(', ')}`); + } + }); + + test('a ROOT-level test file is not invisible', (t) => { + // The pre-#4379 pathspec was `**/`-prefixed, and `**/` does not match a path + // with no directory component — so a root `foo.test.js` was missed even in + // the language the gate did support. This is the half the issue did not report. + const dir = seedRepo(t); + const hits = matched(dir, shippedRedPathspec()); + assert.ok(hits.has('root.test.js'), 'a root-level .test.js must match'); + assert.ok(hits.has('foo_test.go'), 'a root-level _test.go must match'); + }); + + test('the existing JS/TS conventions still match (no regression)', (t) => { + const dir = seedRepo(t); + const hits = matched(dir, shippedRedPathspec()); + for (const rel of ['src/a.test.ts', 'tests/c.py', '__tests__/d.js']) { + assert.ok(hits.has(rel), `${rel} must still match after widening`); + } + }); + + test('implementation files never match — the gate must still be able to trip', (t) => { + // A pathspec broad enough to catch ordinary source would make the gate pass on + // ANY in-scope commit, which is worse than the bug being fixed. + // + // `src/lib.rs` carries the Rust consequence: `#[test]` conventionally lives + // INSIDE the implementation file, so a Rust RED commit touches only source and + // no path-based gate can see it. That gap is a direct consequence of this row + // holding — the two cannot both be satisfied — which is why it is documented in + // references/tdd.md rather than "fixed" by widening the pathspec. + const dir = seedRepo(t); + const hits = matched(dir, shippedRedPathspec()); + for (const rel of ['src/impl.go', 'src/lib.rs']) { + assert.ok(!hits.has(rel), `${rel} must NOT be treated as a test file`); + } + }); +});