From 1ec4b38bd32020ebcac83317cdbcc5022b465d8d Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 4 Sep 2026 20:17:36 -0400 Subject: [PATCH] test(#4298): add tdd-walk.cjs end-to-end sniff-test harness for TDD dispatch (#4300) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#4298): add tdd-walk.cjs end-to-end sniff-test harness for TDD dispatch Epic #4272 Phase 5's own checklist named this deliverable ("the same class of coverage loop-walk.cjs gives the loop") separately from #4268. Adds tests/qa/tdd-walk.cjs, extracting and REALLY EXECUTING (via a real `bash -c` subprocess against a real temp fixture project) the shipped bash resolution snippets from both TDD dispatch backends — never reimplementing or grep-simulating the predicate. Proves, by execution rather than text-shape assertion: the CLI predicate and both backends agree for a type: tdd plan and a plain plan; the worktree backend's fail-closed guard genuinely halts (non-zero exit, FATAL stderr) on a missing plan file; and the tdd.md embed ternary's condition tracks the real resolved value (#3800). This is exactly the class of proof #4264/#4265 (unassigned/divergent predicate) and #4268 (static-shape checks can't see backend divergence) could not provide. Extraction uses indexOf/slice on fenced-code markers only, never a backtracking regex over whole-file text (per the #4228 incident this repo's tests already document). Co-Authored-By: Claude Sonnet 5 * test(#4298): scrub ambient env, tighten fail-closed assertion, fix comment Standards+Spec review found: (1) executeBackendScript spread raw process.env unfiltered into the spawned bash subprocess, unlike tests/helpers.cjs's runGsdTools, which deliberately scrubs SESSION_IDENTITY_ENV_KEYS + config-location env vars before spawning (#2665) — an ambient developer/CI override could silently change what phase.tdd-applicable resolves to in a way a gsd-test bench container won't reproduce; (2) the row-5 fail-closed test asserted only `stderr.includes('FATAL')`, which would also pass if the file's unrelated ISOLATION fail-closed guard fired instead of the TDD one; (3) a docstring called the worktree backend's first fenced block a "shim preamble" when it's actually the whole ISOLATION-resolution block. Fixes: spread the exported TEST_ENV_BASE (every scrub-listed key set to '') before the two intentional RUNTIME_DIR/GSD_TEST_MODE overrides; assert the exact TDD-applicability FATAL text; correct the docstring. Re-verified by direct execution against real fixtures — all three precedence-tier cases and the fail-closed case behave identically to before the fix. Co-Authored-By: Claude Sonnet 5 --------- Co-authored-by: sim Co-authored-by: Claude Sonnet 5 --- tests/qa/tdd-walk.cjs | 332 +++++++++++++++++++++++++++++++++++++ tests/tdd-walk.qa.test.cjs | 156 +++++++++++++++++ 2 files changed, 488 insertions(+) create mode 100644 tests/qa/tdd-walk.cjs create mode 100644 tests/tdd-walk.qa.test.cjs diff --git a/tests/qa/tdd-walk.cjs b/tests/qa/tdd-walk.cjs new file mode 100644 index 000000000..1a804d956 --- /dev/null +++ b/tests/qa/tdd-walk.cjs @@ -0,0 +1,332 @@ +'use strict'; + +/** + * tdd-walk.cjs — a small stateful harness proving the TDD-dispatch predicate + * resolves correctly END-TO-END, mirroring the discipline `tests/qa/loop-walk.cjs` + * brings to the loop subsystem: invoke the REAL shipped mechanism against a + * REAL temp project, never reimplement or grep-simulate it (#4298, Phase 5 of + * epic #4272). + * + * WHAT'S EXECUTABLE VS WHAT ISN'T (see + * `.gsd/phase/chore-4298-tdd-walk-qa-harness/40-design.md`): the two TDD + * dispatch backends' `${TDD_APPLICABLE ? "..." : ""}` embed-list markers are + * LLM-orchestrator pseudo-syntax composed by the agent reading the workflow + * markdown at dispatch time — they are NOT real bash and cannot be executed + * by a shell. What IS real, executable bash is the resolution block that + * computes the `TDD_APPLICABLE` variable itself: + * - `gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md` + * ("harness" backend) — one self-contained fenced block: its own + * `gsd_run` shim-discovery preamble, then the `TDD_APPLICABLE_RAW=...` / + * `TDD_APPLICABLE_RC=$?` / fail-closed check / `TDD_APPLICABLE=` assignment. + * - `gsd-core/workflows/execute-phase/steps/executor-isolation-dispatch.md` + * ("worktree" backend) — the `gsd_run` shim-discovery preamble lives in + * the file's FIRST fenced block, which is not just the shim: it is the + * whole ISOLATION resolution (its own independent fail-closed branches), + * ahead of a LATER fenced block that contains the `_TDD_APPLICABLE_RAW=...` + * resolution followed by the `EXECUTOR_PROMPT=` composition this harness + * does not need. A standalone execution must concatenate the first block + * (for its `gsd_run` definition) with just the TDD resolution lines + * sliced out of the second. + * + * This module extracts each backend's real bash via `indexOf`/`slice` on the + * fenced-code markers (NEVER a backtracking regex over the whole file — see + * `tests/tdd-single-statement.test.cjs`'s `restatesCycle()` header for the + * documented #4228 incident: a lazy-quantifier regex over a 47KB file pinned + * a Windows CI runner for 32 minutes), substitutes the plan's placeholders, + * and executes the result via a real `bash -c` subprocess against a real + * fixture project built with `tests/helpers.cjs`'s `createTempProject`. + */ + +const fs = require('fs'); +const path = require('path'); +const { execFileSync } = require('child_process'); +const { createTempProject, cleanup, runGsdTools, readFileNormalized, TEST_ENV_BASE } = require('../helpers.cjs'); + +/** Absolute real repo root — NOT the temp fixture's own root. */ +const REPO_ROOT = path.join(__dirname, '..', '..'); + +const RESOLUTION_STEP_PATH = path.join( + REPO_ROOT, 'gsd-core', 'workflows', 'execute-phase', 'steps', 'tdd-applicability-resolution.md', +); +const DISPATCH_STEP_PATH = path.join( + REPO_ROOT, 'gsd-core', 'workflows', 'execute-phase', 'steps', 'executor-isolation-dispatch.md', +); + +const FENCE_OPEN = '```bash'; +const FENCE_CLOSE = '```'; + +// The exact slice boundaries inside executor-isolation-dispatch.md's later +// fenced block (see module header). Extracted by literal substring search, +// never a regex, per the #4228 constraint. +const TDD_SLICE_START_MARKER = '_TDD_APPLICABLE_RAW='; +const TDD_SLICE_END_MARKER = 'TDD_APPLICABLE="$_TDD_APPLICABLE_RAW"'; + +// The pseudo-bash embed-list ternary this harness's resolved value ultimately +// gates (#3800/#4266) — NOT executable, documented here only so the QA +// self-tests can confirm it exists over the real shipped file without a +// second, independently-typed copy drifting from it. +const TDD_EMBED_TERNARY = '${TDD_APPLICABLE ? "- tdd.md" : ""}'; + +/** + * Extract the Nth (0-indexed) fenced ` ```bash ... ``` ` block's body from + * `content`, verbatim (fence markers excluded), via `indexOf`/`slice`. + * + * @param {string} content + * @param {number} [occurrence] - 0-indexed occurrence to extract. + * @returns {string} + */ +function extractFencedBashBlock(content, occurrence = 0) { + let searchFrom = 0; + for (let i = 0; i <= occurrence; i += 1) { + const openIdx = content.indexOf(FENCE_OPEN, searchFrom); + if (openIdx === -1) { + throw new Error( + `extractFencedBashBlock: could not find occurrence ${i} of "${FENCE_OPEN}" (wanted occurrence ${occurrence})`, + ); + } + const bodyStart = openIdx + FENCE_OPEN.length; + const closeIdx = content.indexOf(FENCE_CLOSE, bodyStart); + if (closeIdx === -1) { + throw new Error(`extractFencedBashBlock: unterminated fenced block opened at offset ${openIdx}`); + } + if (i === occurrence) { + let body = content.slice(bodyStart, closeIdx); + if (body.startsWith('\n')) body = body.slice(1); + if (body.endsWith('\n')) body = body.slice(0, -1); + return body; + } + searchFrom = closeIdx + FENCE_CLOSE.length; + } + throw new Error('extractFencedBashBlock: unreachable'); +} + +/** + * Scan every fenced ` ```bash ... ``` ` block in `content`, in document + * order, and return the body of the first one whose body contains the + * literal substring `marker`. + * + * @param {string} content + * @param {string} marker + * @returns {string} + */ +function extractFencedBashBlockContaining(content, marker) { + let searchFrom = 0; + for (;;) { + const openIdx = content.indexOf(FENCE_OPEN, searchFrom); + if (openIdx === -1) { + throw new Error(`extractFencedBashBlockContaining: no fenced bash block contains "${marker}"`); + } + const bodyStart = openIdx + FENCE_OPEN.length; + const closeIdx = content.indexOf(FENCE_CLOSE, bodyStart); + if (closeIdx === -1) { + throw new Error(`extractFencedBashBlockContaining: unterminated fenced block opened at offset ${openIdx}`); + } + const body = content.slice(bodyStart, closeIdx); + if (body.indexOf(marker) !== -1) { + return body.startsWith('\n') ? body.slice(1) : body; + } + searchFrom = closeIdx + FENCE_CLOSE.length; + } +} + +/** + * From executor-isolation-dispatch.md's later fenced block (the one composing + * `EXECUTOR_PROMPT`), slice out ONLY the TDD-applicability resolution lines: + * from `_TDD_APPLICABLE_RAW=` through the `TDD_APPLICABLE="$_TDD_APPLICABLE_RAW"` + * line, inclusive. Everything before (comments) and after (`EXECUTOR_PROMPT=...`) + * is dropped. + * + * @param {string} blockBody + * @returns {string} + */ +function sliceTddResolution(blockBody) { + const startIdx = blockBody.indexOf(TDD_SLICE_START_MARKER); + if (startIdx === -1) { + throw new Error(`sliceTddResolution: start marker "${TDD_SLICE_START_MARKER}" not found`); + } + const endMarkerIdx = blockBody.indexOf(TDD_SLICE_END_MARKER, startIdx); + if (endMarkerIdx === -1) { + throw new Error(`sliceTddResolution: end marker "${TDD_SLICE_END_MARKER}" not found after start marker`); + } + const endOfLineIdx = blockBody.indexOf('\n', endMarkerIdx); + const sliceEnd = endOfLineIdx === -1 ? blockBody.length : endOfLineIdx; + return blockBody.slice(startIdx, sliceEnd); +} + +/** + * Build the standalone, placeholder-substituted bash script for one backend. + * + * @param {'harness'|'worktree'} backend + * @param {{phaseDir: string, planFile: string, planNumber: string}} placeholders + * @returns {string} + */ +function buildBackendScript(backend, placeholders) { + let template; + if (backend === 'harness') { + template = extractFencedBashBlock(readFileNormalized(RESOLUTION_STEP_PATH), 0); + } else if (backend === 'worktree') { + const dispatchContent = readFileNormalized(DISPATCH_STEP_PATH); + const preamble = extractFencedBashBlock(dispatchContent, 0); + const laterBlock = extractFencedBashBlockContaining(dispatchContent, TDD_SLICE_START_MARKER); + const resolution = sliceTddResolution(laterBlock); + template = `${preamble}\n${resolution}`; + } else { + throw new Error(`buildBackendScript: unknown backend "${backend}" (expected "harness" or "worktree")`); + } + const substituted = template + .replaceAll('{phase_dir}', placeholders.phaseDir) + .replaceAll('{plan_file}', placeholders.planFile) + .replaceAll('{plan_number}', placeholders.planNumber); + return `${substituted}\necho "TDD_APPLICABLE_RESULT=$TDD_APPLICABLE"\n`; +} + +/** Parse the `TDD_APPLICABLE_RESULT=` sentinel out of stdout via indexOf/slice. */ +function parseResultSentinel(stdout) { + const marker = 'TDD_APPLICABLE_RESULT='; + const idx = stdout.lastIndexOf(marker); + if (idx === -1) return undefined; + const valueStart = idx + marker.length; + const lineEndIdx = stdout.indexOf('\n', valueStart); + return lineEndIdx === -1 ? stdout.slice(valueStart) : stdout.slice(valueStart, lineEndIdx); +} + +/** + * Execute a backend's extracted script for real, via `bash -c`, against + * `cwd`. Never throws: a non-zero exit is captured and returned as a normal + * `{success: false, ...}` value so a caller can assert "this backend failed + * closed" without a try/catch. + * + * @param {string} script + * @param {string} cwd + * @returns {{success: true, value: string|undefined, stdout: string, stderr: string} + * | {success: false, status: number|null, stdout: string, stderr: string}} + */ +function executeBackendScript(script, cwd) { + try { + // #4298 Standards+Spec review: `runGsdTools` (tests/helpers.cjs) scrubs + // SESSION_IDENTITY_ENV_KEYS + config-location env vars before spawning, + // because an ambient developer/CI value (e.g. a real ~/.gsd/config.json + // location override) can silently change what a gsd-tools child resolves + // — documented there as #2665. This harness spawns `gsd_run` the exact + // same way (indirectly, via the extracted resolution script), so it needs + // the same hermeticity: spread the exported `TEST_ENV_BASE` (every + // scrub-listed key set to '') AFTER `...process.env` but BEFORE the two + // intentional overrides below, so ambient state is blanked and only + // RUNTIME_DIR/GSD_TEST_MODE are deliberately set. + const stdout = execFileSync('bash', ['-c', script], { + cwd, + env: { ...process.env, ...TEST_ENV_BASE, RUNTIME_DIR: REPO_ROOT, GSD_TEST_MODE: '1' }, + encoding: 'utf8', + timeout: 30000, + }); + return { success: true, value: parseResultSentinel(stdout), stdout, stderr: '' }; + } catch (error) { + return { + success: false, + status: typeof error.status === 'number' ? error.status : null, + stdout: error.stdout != null ? error.stdout.toString() : '', + stderr: error.stderr != null ? error.stderr.toString() : '', + }; + } +} + +class TddWalk { + /** @param {string} dir - absolute project root (an already-created temp project). */ + constructor(dir) { + this.dir = dir; + this.planNumber = '01'; + this.phaseDirRel = path.join('.planning', 'phases', '01-x'); + this.planFileName = '01-PLAN.md'; + } + + /** @returns {string} absolute path to the fixture's phase directory. */ + get phaseDir() { + return path.join(this.dir, this.phaseDirRel); + } + + /** @returns {string} absolute path to the fixture's plan file. */ + get planPath() { + return path.join(this.phaseDir, this.planFileName); + } + + /** + * Create a fresh temp project and a `TddWalk` bound to it. Does NOT write a + * plan file — callers that need the fail-closed path (row 5 of the test + * matrix) rely on the plan being absent until `writePlan()` is called. + * + * @param {{prefix?: string}} [opts] + * @returns {TddWalk} + */ + static create(opts = {}) { + const { prefix = 'gsd-tdd-walk-' } = opts; + const dir = createTempProject(prefix); + return new TddWalk(dir); + } + + /** + * Write (or overwrite) this walk's plan file with `content`, creating + * parent directories as needed. + * + * @param {string} content + */ + writePlan(content) { + fs.mkdirSync(this.phaseDir, { recursive: true }); + fs.writeFileSync(this.planPath, content, 'utf8'); + } + + /** + * Resolve TDD-applicability via the real CLI verb (`gsd_run query + * phase.tdd-applicable`), reusing Phase 1's proven invocation + * (`tests/phase-tdd-applicable.test.cjs`). + * + * @param {{cliFlag?: boolean}} [opts] + * @returns {{success: true, applicable: boolean, source: string, plan_type: unknown, + * config_tdd_mode: unknown, cli_flag_present: boolean} | {success: false, error: string, exitCode: number|null}} + */ + resolveViaCli(opts = {}) { + const { cliFlag = false } = opts; + const argv = ['query', 'phase.tdd-applicable', this.planPath]; + if (cliFlag) argv.push('--cli-flag'); + const result = runGsdTools(argv, this.dir); + if (!result.success) { + return { success: false, error: result.error, exitCode: result.exitCode ?? null }; + } + return { success: true, ...JSON.parse(result.output) }; + } + + /** + * Resolve `TDD_APPLICABLE` by extracting and EXECUTING the real shipped + * bash for `backend`, against this walk's real fixture project. See + * `executeBackendScript` for the return shape. + * + * @param {'harness'|'worktree'} backend + * @returns {ReturnType} + */ + resolveViaBackend(backend) { + const script = buildBackendScript(backend, { + phaseDir: this.phaseDir, + planFile: this.planFileName, + planNumber: this.planNumber, + }); + return executeBackendScript(script, this.dir); + } + + /** Remove this walk's temp project. Safe to call multiple times. */ + cleanup() { + cleanup(this.dir); + } +} + +module.exports = { + TddWalk, + extractFencedBashBlock, + extractFencedBashBlockContaining, + sliceTddResolution, + buildBackendScript, + executeBackendScript, + parseResultSentinel, + RESOLUTION_STEP_PATH, + DISPATCH_STEP_PATH, + TDD_EMBED_TERNARY, + REPO_ROOT, +}; diff --git a/tests/tdd-walk.qa.test.cjs b/tests/tdd-walk.qa.test.cjs new file mode 100644 index 000000000..9da5fcb2a --- /dev/null +++ b/tests/tdd-walk.qa.test.cjs @@ -0,0 +1,156 @@ +'use strict'; + +/** + * tdd-walk.qa.test.cjs — self-tests for `tests/qa/tdd-walk.cjs` (#4298, Phase 5 + * of epic #4272). + * + * Matrix rows referenced below are from + * `.gsd/phase/chore-4298-tdd-walk-qa-harness/50-test-matrix.md`. Every test + * here runs the REAL shipped bash resolution against a REAL temp project — + * no reimplementation of the predicate, no text-shape assertions on the + * workflow markdown standing in for execution. + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); + +const { TddWalk, DISPATCH_STEP_PATH, TDD_EMBED_TERNARY } = require('./qa/tdd-walk.cjs'); +const { readFileNormalized } = require('./helpers.cjs'); + +const TDD_TYPE_PLAN = `--- +type: tdd +--- +a +`; + +const PLAIN_PLAN = `a\n`; + +describe('tdd-walk harness', () => { + test('row 1 — CLI predicate resolution: type: tdd resolves applicable/plan_frontmatter', (t) => { + const walk = TddWalk.create(); + t.after(() => walk.cleanup()); + walk.writePlan(TDD_TYPE_PLAN); + const result = walk.resolveViaCli(); + assert.equal(result.success, true, result.error); + assert.equal(result.applicable, true); + assert.equal(result.source, 'plan_frontmatter'); + }); + + test('row 1 — CLI predicate resolution: nothing set resolves not-applicable/none', (t) => { + const walk = TddWalk.create(); + t.after(() => walk.cleanup()); + walk.writePlan(PLAIN_PLAN); + const result = walk.resolveViaCli(); + assert.equal(result.success, true, result.error); + assert.equal(result.applicable, false); + assert.equal(result.source, 'none'); + }); + + test('row 2 — harness-backend script execution: type: tdd resolves TDD_APPLICABLE=true', (t) => { + const walk = TddWalk.create(); + t.after(() => walk.cleanup()); + walk.writePlan(TDD_TYPE_PLAN); + const result = walk.resolveViaBackend('harness'); + assert.equal(result.success, true, result.stderr); + assert.equal(result.value, 'true'); + }); + + test('row 2 — harness-backend script execution: plain plan resolves TDD_APPLICABLE=false', (t) => { + const walk = TddWalk.create(); + t.after(() => walk.cleanup()); + walk.writePlan(PLAIN_PLAN); + const result = walk.resolveViaBackend('harness'); + assert.equal(result.success, true, result.stderr); + assert.equal(result.value, 'false'); + }); + + test('row 3 — worktree-backend script execution: type: tdd resolves TDD_APPLICABLE=true', (t) => { + const walk = TddWalk.create(); + t.after(() => walk.cleanup()); + walk.writePlan(TDD_TYPE_PLAN); + const result = walk.resolveViaBackend('worktree'); + assert.equal(result.success, true, result.stderr); + assert.equal(result.value, 'true'); + }); + + test('row 3 — worktree-backend script execution: plain plan resolves TDD_APPLICABLE=false', (t) => { + const walk = TddWalk.create(); + t.after(() => walk.cleanup()); + walk.writePlan(PLAIN_PLAN); + const result = walk.resolveViaBackend('worktree'); + assert.equal(result.success, true, result.stderr); + assert.equal(result.value, 'false'); + }); + + test('row 4 — cross-backend agreement: same fixture plan, identical resolved value (#4264/#4265)', (t) => { + const tddWalk = TddWalk.create(); + t.after(() => tddWalk.cleanup()); + tddWalk.writePlan(TDD_TYPE_PLAN); + const harnessResult = tddWalk.resolveViaBackend('harness'); + const worktreeResult = tddWalk.resolveViaBackend('worktree'); + assert.equal(harnessResult.success, true, harnessResult.stderr); + assert.equal(worktreeResult.success, true, worktreeResult.stderr); + assert.equal(harnessResult.value, worktreeResult.value); + assert.equal(harnessResult.value, 'true'); + + const plainWalk = TddWalk.create(); + t.after(() => plainWalk.cleanup()); + plainWalk.writePlan(PLAIN_PLAN); + const harnessPlain = plainWalk.resolveViaBackend('harness'); + const worktreePlain = plainWalk.resolveViaBackend('worktree'); + assert.equal(harnessPlain.success, true, harnessPlain.stderr); + assert.equal(worktreePlain.success, true, worktreePlain.stderr); + assert.equal(harnessPlain.value, worktreePlain.value); + assert.equal(harnessPlain.value, 'false'); + }); + + test('row 5 — fail-closed: missing plan file exits non-zero with the TDD-resolution FATAL on stderr (worktree backend)', (t) => { + const walk = TddWalk.create(); + t.after(() => walk.cleanup()); + // Deliberately no writePlan() call — {phase_dir}/{plan_file} points at a + // plan that does not exist on disk. + const result = walk.resolveViaBackend('worktree'); + assert.equal(result.success, false); + // #4298 Standards+Spec review: a bare `.includes('FATAL')` would also + // pass if the file's OTHER fail-closed guard (the unrelated ISOLATION + // resolution, which shares the same first fenced block and can emit its + // own differently-worded FATAL) fired instead of the TDD-applicability + // one — silently proving the wrong guard. Assert the exact TDD-resolution + // FATAL text (from executor-isolation-dispatch.md's own echo line) so a + // future edit that changes which guard fires here is caught. + assert.ok( + result.stderr.includes("could not resolve TDD-applicability for plan"), + `expected stderr to contain the TDD-applicability FATAL message, got: ${result.stderr}`, + ); + }); + + test('row 6 — #3800: the tdd.md embed ternary exists and its condition tracks the real predicate value', (t) => { + const dispatchContent = readFileNormalized(DISPATCH_STEP_PATH); + assert.ok( + dispatchContent.includes(TDD_EMBED_TERNARY), + 'executor-isolation-dispatch.md no longer contains the documented TDD_APPLICABLE embed ternary', + ); + + const tddWalk = TddWalk.create(); + t.after(() => tddWalk.cleanup()); + tddWalk.writePlan(TDD_TYPE_PLAN); + const tddResult = tddWalk.resolveViaBackend('harness'); + assert.equal(tddResult.success, true, tddResult.stderr); + // The embed ternary fires (includes tdd.md) exactly when TDD_APPLICABLE + // resolved to the literal string "true". + assert.equal(tddResult.value, 'true'); + + const plainWalk = TddWalk.create(); + t.after(() => plainWalk.cleanup()); + plainWalk.writePlan(PLAIN_PLAN); + const plainResult = plainWalk.resolveViaBackend('harness'); + assert.equal(plainResult.success, true, plainResult.stderr); + assert.equal(plainResult.value, 'false'); + }); +}); + +// Non-regression (see 50-test-matrix.md "Non-regression"): this file shares +// no file with tests/tdd-single-statement.test.cjs, tests/tdd-backend-wiring.test.cjs, +// or tests/phase-tdd-applicable.test.cjs — it only reuses tests/helpers.cjs +// and tests/qa/tdd-walk.cjs (new, own to this phase), so those suites' own +// runs are the actual non-regression proof; nothing further to assert here.