diff --git a/.changeset/sturdy-bears-sprint.md b/.changeset/sturdy-bears-sprint.md new file mode 100644 index 000000000..9903cd78d --- /dev/null +++ b/.changeset/sturdy-bears-sprint.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4318 +--- +**`state advance-plan` no longer strands you when `## Current Position` has lost its labeled plan-position lines** — the failure now returns reason `plan_position_unreadable` with the phase directory's on-disk plan/summary counts and the exact labeled lines to re-insert, instead of a bare unparseable error with no recovery path. (#4093) diff --git a/docs/CLI-TOOLS.md b/docs/CLI-TOOLS.md index 4fb30fd1e..25e1a0d7e 100644 --- a/docs/CLI-TOOLS.md +++ b/docs/CLI-TOOLS.md @@ -93,6 +93,10 @@ node gsd-tools.cjs state patch --field1 val1 --field2 val2 # Increment plan counter node gsd-tools.cjs state advance-plan +# When no labeled plan position can be parsed (e.g. ## Current Position drifted +# to pure narrative prose), declines with reason "plan_position_unreadable" plus +# the disk-derived counts and the exact labeled lines to re-insert; STATE.md is +# left byte-identical. # Record execution metrics node gsd-tools.cjs state record-metric --phase N --plan M --duration Xmin [--tasks N] [--files N] diff --git a/gsd-core/bin/lib/tdd-red-evidence.cjs b/gsd-core/bin/lib/tdd-red-evidence.cjs new file mode 100644 index 000000000..b3644010e --- /dev/null +++ b/gsd-core/bin/lib/tdd-red-evidence.cjs @@ -0,0 +1,133 @@ +"use strict"; +/** + * TDD RED-evidence classification (#3770). + * + * The `type: tdd` executor gate previously accepted ANY nonzero test command as + * RED: syntax errors, zero-test discovery, fixture crashes, parser errors, and + * unrelated assertions all authorized production edits (GREEN). This module + * defines the compact RED evidence the gate now requires — the TARGET test's + * identity plus a matching assertion failure — and classifies a persisted test + * run into exactly one verdict: + * + * RED_EVIDENCE_OK — nonzero exit AND the target test failed as a REAL test + * (distinctly named, TAP-reported failure). The ONLY + * verdict that may advance to GREEN. + * INVALID_RED — everything else, with a machine-readable reason: + * unexpected_green | zero_tests_discovered | + * nonzero_exit_without_test_failure | + * fixture_or_load_failure | no_target_test_failure | + * invalid_record | unreadable_record (router arm). + * + * Everything here is PURE — no fs, no spawn, no clock — so the executor can + * persist the record (command, exit code, failing test, expected, actual) and + * validate it via `gsd_run check tdd-red-evidence `. + * + * TAP parsing reuses the proven primitives from prohibition-enforcement + * (`parseNodeTestSummary`, `tapFailedTestNames`) — the same contract the + * prohibition probe's fail-first prover already relies on (#1259). + */ +Object.defineProperty(exports, "__esModule", { value: true }); +exports.classifyRedEvidence = classifyRedEvidence; +exports.buildRedEvidenceRecord = buildRedEvidenceRecord; +const prohibition_enforcement_cjs_1 = require("./prohibition-enforcement.cjs"); +/** Basename of a path-like string ('' for non-strings) — separators `/` and `\`. */ +function baseOf(p) { + return typeof p === 'string' ? (p.split(/[\\/]/).pop() ?? p) : ''; +} +/** Coerce and validate the raw record's scalar fields. Returns null exit_code only when absent/non-numeric. */ +function readInput(input) { + const command = typeof input?.command === 'string' ? input.command : ''; + const output = typeof input?.output === 'string' ? input.output : ''; + const targetTest = typeof input?.targetTest === 'string' ? input.targetTest.trim() : ''; + const exitCode = typeof input?.exitCode === 'number' && Number.isFinite(input.exitCode) ? input.exitCode : null; + if (!command || !targetTest || exitCode === null) + return null; + return { command, exitCode, output, targetTest }; +} +/** + * Classify a persisted RED-phase test run. Fail-closed: malformed input, an + * unparseable/empty TAP summary, a file-named (load/crash) failure, or a + * failure that is not the target test's are all INVALID_RED — only a nonzero + * exit WITH the distinctly-named target test failing is RED_EVIDENCE_OK. + * Never throws. + */ +function classifyRedEvidence(input) { + const parsed = readInput(input); + if (!parsed) { + return { + verdict: 'INVALID_RED', + reason: 'invalid_record', + evidence: { + command: typeof input?.command === 'string' ? input.command : '', + exit_code: null, + target_test: '', + tests: 0, + pass: 0, + fail: 0, + failing_tests: [], + }, + }; + } + const { command, exitCode, output, targetTest } = parsed; + const summary = (0, prohibition_enforcement_cjs_1.parseNodeTestSummary)(output); + const failing = (0, prohibition_enforcement_cjs_1.tapFailedTestNames)(output); + const evidence = { + command, + exit_code: exitCode, + target_test: targetTest, + tests: summary.tests, + pass: summary.pass, + fail: summary.fail, + failing_tests: failing, + }; + // Existing fail-fast rule, now machine-checked: exit 0 during RED is an + // unexpected GREEN — the feature may already exist or the test is wrong. + if (exitCode === 0) { + return { verdict: 'INVALID_RED', reason: 'unexpected_green', evidence }; + } + // Zero-test discovery: the discovery pattern / fixture matched no tests. + // A run that executed nothing cannot prove anything about the behavior. + if (summary.tests === 0) { + return { verdict: 'INVALID_RED', reason: 'zero_tests_discovered', evidence }; + } + // Nonzero exit but TAP reports no failing test: harness/setup/parser crash + // whose failure never reached a test assertion (or unparseable output). + if (summary.fail === 0 || failing.length === 0) { + return { verdict: 'INVALID_RED', reason: 'nonzero_exit_without_test_failure', evidence }; + } + // Fixture/load failure: every failing entry is named like the target FILE — + // node reports a load-time crash (throw-on-require, syntax error, ENOENT + // fixture) as a file-named `not ok 1 - `, never the target test. + const targetBase = baseOf(input?.targetFile ?? ''); + const distinctlyNamed = failing.filter((n) => (targetBase ? baseOf(n) !== targetBase : true)); + if (distinctlyNamed.length === 0) { + return { verdict: 'INVALID_RED', reason: 'fixture_or_load_failure', evidence }; + } + // Unrelated failure: real tests ran and failed, but none is the target test + // the plan named — an unrelated assertion must not authorize GREEN. + if (!distinctlyNamed.includes(targetTest)) { + return { verdict: 'INVALID_RED', reason: 'no_target_test_failure', evidence }; + } + return { verdict: 'RED_EVIDENCE_OK', reason: 'target_test_failed', evidence }; +} +/** + * Project a classification into the persisted record shape — command, exit + * code, failing test, expected, actual, verdict, reason — so the evidence + * survives past the terminal and the gate can re-verify it deterministically. + * Pure: JSON-serializable, no timestamps (the record's mtime/commit carries time). + */ +function buildRedEvidenceRecord(input, result) { + const failingTest = result.evidence.failing_tests.find((n) => n === result.evidence.target_test) ?? + result.evidence.failing_tests[0] ?? + null; + return { + command: result.evidence.command, + exit_code: result.evidence.exit_code, + failing_test: failingTest, + target_test: result.evidence.target_test, + expected: typeof input?.expected === 'string' ? input.expected : null, + actual: typeof input?.actual === 'string' ? input.actual : null, + verdict: result.verdict, + reason: result.reason, + }; +} diff --git a/src/state.cts b/src/state.cts index 17cbb799b..b44a2ef42 100644 --- a/src/state.cts +++ b/src/state.cts @@ -927,11 +927,16 @@ function stateReplaceFieldWithFallback(content: string, primary: string, fallbac * Current Position `Phase:` line, milestone-archived layouts) keep today's * behavior rather than being newly refused. * - * Returns `{ dir, outstanding }` where `outstanding` is empty when every plan - * on disk is summarized (vacuously so for a zero-plan phase — #3168's - * zero-plan-phase posture). + * Returns `{ dir, outstanding, planCount, summaryCount }` where `outstanding` + * is empty when every plan on disk is summarized (vacuously so for a zero-plan + * phase — #3168's zero-plan-phase posture). `planCount`/`summaryCount` are the + * countable disk facts behind `outstanding` (live plan files; summaries after + * the #3345 blocked filter) — #4093's recovery decline reports them to a + * caller whose STATE.md has lost its labeled plan position, so the suggested + * repair values are computed from the SAME set `outstanding` was, and can + * never disagree with a count-based decision either. */ -function scanOutstanding(phasesDir: string, dir: string): { dir: string; outstanding: string[] } | null { +function scanOutstanding(phasesDir: string, dir: string): { dir: string; outstanding: string[]; planCount: number; summaryCount: number } | null { const phaseDirPath = path.join(phasesDir, dir); const scan = scanPhasePlans(phaseDirPath); if (scan.scope !== SCOPE.COMPLETE) return null; @@ -942,13 +947,13 @@ function scanOutstanding(phasesDir: string, dir: string): { dir: string; outstan (f) => !planDependencyGraphMod.isSummaryFileBlocked(path.join(phaseDirPath, f)), ); const outstanding = coreUtilsMod.findUnsummarizedPlans(scan.planFiles, countableSummaries); - return { dir, outstanding }; + return { dir, outstanding, planCount: scan.planFiles.length, summaryCount: countableSummaries.length }; } function unsummarizedPlansForPositionPhase( cwd: string, positionPhase: string, -): { dir: string; outstanding: string[] } | null { +): { dir: string; outstanding: string[]; planCount: number; summaryCount: number } | null { const phasesDir = planningPaths(cwd).phases; // #3185 (ADR-3180 Decision 1): "which phase directories exist" is owned by // listMilestonePhaseDirs — no hand-rolled readdirSync here. The owner @@ -1003,7 +1008,12 @@ function cmdStateAdvancePlan(cwd: string, raw: boolean): void { // named here so the post-lock output path can report it without re-deriving. // Holder (not a bare let) so TypeScript's closure-unaware narrowing cannot // collapse the post-lock read to `never` — the callback assigns it. - const outstandingRef: { value: { dir: string; outstanding: string[] } | null } = { value: null }; + const outstandingRef: { value: { dir: string; outstanding: string[]; planCount: number; summaryCount: number } | null } = { value: null }; + // #4093: the position phase token the callback resolved (Current Position + // `Phase:` line first, frontmatter `current_phase` as fallback), carried out + // so the generic parse-failure decline can derive recovery facts from disk + // without re-reading STATE.md outside the lock. Same holder idiom as above. + const positionPhaseRef: { value: string | null } = { value: null }; const wrote = readModifyWriteStateMd(statePath, (content) => { // advance-plan has no phase argument of its own — the phase it advances is // whatever ## Current Position names. Compare that against the milestone @@ -1013,6 +1023,17 @@ function cmdStateAdvancePlan(cwd: string, raw: boolean): void { const body = stripFrontmatter(content); const positionScope = matchCurrentPositionSection(body) ?? body; const positionPhase = parseProsePhaseField(stateExtractField(positionScope, 'Phase')).phase; + // #4093: a Current Position section with ZERO labeled fields has no + // `Phase:` line either; frontmatter `current_phase` is the documented + // survivor of body drift (the reporter's document still carried it, and + // `buildStateFrontmatter` re-derives it from the body only when the body + // HAS the line). It feeds the recovery DECLINE only — never a write. + let fmPhase: string | null = null; + if (positionPhase === null) { + const fmToken = extractFrontmatter(content, statePath)['current_phase']; + fmPhase = typeof fmToken === 'string' && fmToken.trim() !== '' ? fmToken.trim() : null; + } + positionPhaseRef.value = positionPhase ?? fmPhase; if (positionPhase !== null) { milestoneConflict = milestoneLockMod.checkMilestonePosition(cwd, positionPhase); if (milestoneConflict) { @@ -1100,7 +1121,58 @@ function cmdStateAdvancePlan(cwd: string, raw: boolean): void { }, raw, undefined); return; } - output({ error: advancePlanShapeError() }, raw, undefined); + // #4093: the generic terminus — no accepted labeled plan-position shape + // parsed anywhere in the document (the reporter's case: ## Current + // Position drifted to pure narrative prose with zero labeled fields). + // Every OTHER refusal above carries a machine-readable reason and the + // evidence to act on; this one stranded the caller at a bare sentence + // with no recovery path. Give it the same posture: a `reason` the caller + // can branch on, plus — when the position phase can be resolved and its + // directory scanned — the disk-derived facts and the exact labeled lines + // to re-insert. Nothing is WRITTEN: STATE.md is returned byte-identical + // (the callback already returned the original content for this path), + // so the decline is idempotent and no repair is guessed into the file — + // the caller (human or agent) applies the suggested lines and re-runs. + // Disk is the recovery source per #4067's posture; the values below are + // computed from the SAME `scanOutstanding` counts the plans_outstanding + // guard uses, so the two declines can never disagree about a phase. + const positionToken = positionPhaseRef.value; + const diskFacts = positionToken !== null + ? unsummarizedPlansForPositionPhase(cwd, positionToken) + : null; + if (diskFacts === null) { + // No resolvable phase (no Phase: line, no current_phase frontmatter, or + // no matching phase directory / incomplete scan): keep today's shape + // error, plus the reason so callers can tell this refusal from the + // ambiguous_* ones without string-matching the sentence. + output({ error: advancePlanShapeError(), reason: 'plan_position_unreadable' }, raw, undefined); + return; + } + const planCount = diskFacts.planCount; + const summarized = diskFacts.summaryCount; + // A summarized count below the plan count means the next plan to execute + // is summarized+1; an equal count means the phase is done on disk and the + // position line should say so (current = total; the next advance-plan run + // takes the #4067-guarded phase-complete branch from it). Zero plan files + // means disk has no opinion — suggest nothing rather than `1 of 0`. + const payload: Record = { + error: advancePlanShapeError(), + reason: 'plan_position_unreadable', + phase_dir: diskFacts.dir, + disk: { plan_count: planCount, summarized_count: summarized }, + }; + if (planCount > 0) { + const current = summarized < planCount ? summarized + 1 : planCount; + payload['suggested'] = { + current_plan: current, + total_plans: planCount, + lines: [`Current Plan: ${current}`, `Total Plans in Phase: ${planCount}`], + }; + payload['error'] = + `${advancePlanShapeError()} Disk for phase ${diskFacts.dir}: ${summarized} of ${planCount} plan(s) summarized. ` + + `Re-insert a labeled plan position at the top of ## Current Position (e.g. Current Plan: ${current} with Total Plans in Phase: ${planCount}), then re-run.`; + } + output(payload, raw, undefined); return; } diff --git a/tests/lint-compiled-artifact-sync.test.cjs b/tests/lint-compiled-artifact-sync.test.cjs index e3ef77898..0cee4336d 100644 --- a/tests/lint-compiled-artifact-sync.test.cjs +++ b/tests/lint-compiled-artifact-sync.test.cjs @@ -183,8 +183,20 @@ describe('fix-2657: compiled .cjs artifacts are gitignored, not tracked (ADR-457 }); test('lint-compiled-artifact-sync exits 0 with nothing left to check', () => { + // #4093 CI: this spawn is NOT the PROBE class the shared default below + // describes. The script's "nothing left to check" path still runs a FULL + // `tsc -p tsconfig.build.json` compile to a throwaway outDir whenever any + // compiled artifact remains tracked (ten are, deliberately — ADR-457's + // staged end state), and under CI shard load that compile can exceed the + // 15s probe budget, dying to a SIGTERM with empty piped stdout (observed + // twice on ubuntu shard 1/3). Per helpers/timeouts.cjs's own rule, a call + // site that genuinely differs from its class — "a real `tsc` compile" — + // keeps its own local constant with its own justifying comment; see + // tests/ensure-runtime-build.test.cjs's BUILD_TIMEOUT_MS for the other + // instance. 60s is that same class, sized for the cold-cache CI case. + const TSC_COMPILE_TIMEOUT_MS = 60000; const args = [path.join(REPO_ROOT, 'scripts', 'lint-compiled-artifact-sync.cjs')]; - const result = run(process.execPath, args); + const result = run(process.execPath, args, { timeoutMs: TSC_COMPILE_TIMEOUT_MS }); assert.equal(result.status, 0, describeFailure(process.execPath, args, result)); }); }); diff --git a/tests/state.test.cjs b/tests/state.test.cjs index 0fb0f3808..099f9a24d 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -2096,6 +2096,156 @@ describe('cmdStateAdvancePlan (state advance-plan)', () => { assert.ok(updated.includes('Plan: 2 of 3'), 'Plan counter should advance to 2 of 3'); }); }); + + describe('cmdStateAdvancePlan #4093 zero-labeled-fields recovery decline', () => { + // The reporter's exact shape: ## Current Position has drifted to pure + // narrative prose — zero matches for Phase:/Plan:/Current Plan:/Total + // Plans in Phase: anywhere in the file, bold or plain — while frontmatter + // still carries current_phase and the phase directory still holds the + // plan/summary set that IS the position. advance-plan must not strand the + // caller at a bare "cannot parse" error: the decline carries a + // machine-readable reason plus the disk-derived facts needed to repair. + const narrativeFixture = [ + '---', + 'status: Executing', + 'current_phase: 01', + 'current_phase_name: Implementation', + 'last_activity: 2026-09-01', + '---', + '', + '# Project State', + '', + '## Current Position', + '', + '**Phase 1 plan 2** (auth flow): EXECUTED — token round-trip verified end to end.', + "Follow-ups captured in plan 3's tasks.", + '', + '## Session', + '', + 'Session ID: abc', + '', + ].join('\n'); + + const seedPhaseDir = (dir, planCount, summaryCount) => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', dir); + fs.mkdirSync(phaseDir, { recursive: true }); + for (let i = 1; i <= planCount; i++) { + fs.writeFileSync(path.join(phaseDir, `01-0${i}-PLAN.md`), `# plan ${i}\n`); + } + for (let i = 1; i <= summaryCount; i++) { + fs.writeFileSync(path.join(phaseDir, `01-0${i}-SUMMARY.md`), `# summary ${i}\n`); + } + return phaseDir; + }; + + test('#4093 declines with disk-derived repair guidance when Current Position has zero labeled fields', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), narrativeFixture); + seedPhaseDir('01-impl', 2, 1); + + const result = runGsdTools('state advance-plan', tmpDir); + assert.ok(result.success, `Command should exit 0: ${result.error}`); + + const out = JSON.parse(result.output); + assert.ok(typeof out.error === 'string' && /cannot read the plan position/i.test(out.error), + `error should still name the unreadable plan position; got: ${JSON.stringify(out)}`); + assert.strictEqual(out.reason, 'plan_position_unreadable', + `reason should be plan_position_unreadable; got: ${JSON.stringify(out)}`); + assert.strictEqual(out.phase_dir, '01-impl', + `phase_dir should name the disk phase directory; got: ${JSON.stringify(out)}`); + assert.strictEqual(out.disk.plan_count, 2, + `disk.plan_count should count the 2 plan files; got: ${JSON.stringify(out.disk)}`); + assert.strictEqual(out.disk.summarized_count, 1, + `disk.summarized_count should count the 1 summary; got: ${JSON.stringify(out.disk)}`); + assert.strictEqual(out.suggested.current_plan, 2, + `next plan after 1 summarized of 2 is 2; got: ${JSON.stringify(out.suggested)}`); + assert.strictEqual(out.suggested.total_plans, 2, + `suggested total is the on-disk plan count; got: ${JSON.stringify(out.suggested)}`); + assert.ok(out.suggested.lines.includes('Current Plan: 2') && out.suggested.lines.includes('Total Plans in Phase: 2'), + `suggested lines should name the legacy pair to re-insert; got: ${JSON.stringify(out.suggested)}`); + + const after = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.strictEqual(after, narrativeFixture, + 'a recovery decline must leave STATE.md byte-identical'); + }); + + test('#4093 resolves the position phase from the Phase line when frontmatter has none', () => { + const noFm = [ + '# Project State', + '', + '## Current Position', + '', + 'Phase: 2 — Build out', + '', + 'All narrative from here; the labeled plan lines were displaced by executor notes.', + '', + ].join('\n') + '\n'; + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), noFm); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '02-second'); + fs.mkdirSync(phaseDir, { recursive: true }); + for (let i = 1; i <= 3; i++) fs.writeFileSync(path.join(phaseDir, `02-0${i}-PLAN.md`), `# plan ${i}\n`); + fs.writeFileSync(path.join(phaseDir, '02-01-SUMMARY.md'), '# summary 1\n'); + + const result = runGsdTools('state advance-plan', tmpDir); + assert.ok(result.success, `Command should exit 0: ${result.error}`); + + const out = JSON.parse(result.output); + assert.strictEqual(out.reason, 'plan_position_unreadable', `got: ${JSON.stringify(out)}`); + assert.strictEqual(out.phase_dir, '02-second', + `phase should resolve from the Phase: prose line; got: ${JSON.stringify(out)}`); + assert.strictEqual(out.suggested.current_plan, 2, `got: ${JSON.stringify(out.suggested)}`); + assert.strictEqual(out.suggested.total_plans, 3, `got: ${JSON.stringify(out.suggested)}`); + }); + + test('#4093 names the reason even when no phase can be resolved from disk or frontmatter', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '# Project State\n\n## Current Position\n\nPure narrative, no labeled lines anywhere.\n', + ); + + const result = runGsdTools('state advance-plan', tmpDir); + assert.ok(result.success, `Command should exit 0: ${result.error}`); + + const out = JSON.parse(result.output); + assert.strictEqual(out.reason, 'plan_position_unreadable', `got: ${JSON.stringify(out)}`); + assert.ok(/cannot read the plan position/i.test(out.error), + `the accepted-shape sentence must survive; got: ${out.error}`); + assert.strictEqual(out.phase_dir, undefined, + `no resolvable phase means no phase_dir; got: ${JSON.stringify(out)}`); + assert.strictEqual(out.disk, undefined, `no resolvable phase means no disk block`); + }); + + test('#4093 omits suggested values when the phase directory has no plan files', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), narrativeFixture); + seedPhaseDir('01-impl', 0, 0); + + const result = runGsdTools('state advance-plan', tmpDir); + assert.ok(result.success, `Command should exit 0: ${result.error}`); + + const out = JSON.parse(result.output); + assert.strictEqual(out.reason, 'plan_position_unreadable', `got: ${JSON.stringify(out)}`); + assert.strictEqual(out.phase_dir, '01-impl', `got: ${JSON.stringify(out)}`); + assert.strictEqual(out.disk.plan_count, 0, `got: ${JSON.stringify(out.disk)}`); + assert.strictEqual(out.suggested, undefined, + `zero plans on disk means nothing to suggest; got: ${JSON.stringify(out)}`); + }); + + test('#4093 gives the same recovery decline for a present-but-unreadable plan field', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + narrativeFixture.replace("Follow-ups captured in plan 3's tasks.", 'Plan: TBD'), + ); + seedPhaseDir('01-impl', 2, 1); + + const result = runGsdTools('state advance-plan', tmpDir); + assert.ok(result.success, `Command should exit 0: ${result.error}`); + + const out = JSON.parse(result.output); + assert.strictEqual(out.reason, 'plan_position_unreadable', `got: ${JSON.stringify(out)}`); + assert.strictEqual(out.phase_dir, '01-impl', `got: ${JSON.stringify(out)}`); + assert.strictEqual(out.suggested.current_plan, 2, `got: ${JSON.stringify(out.suggested)}`); + assert.strictEqual(out.advanced, undefined, 'an unreadable position must never be advanced'); + }); + }); }); describe('cmdStateRecordMetric (state record-metric)', () => {