From 0d4d78550f70a6911de19dc0e3a8622096f1bd78 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 13 Aug 2026 23:50:22 -0400 Subject: [PATCH] fix(#3188): null absent planning-doc paths in init phase queries (#3430) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#3188): null absent planning-doc paths in init phase queries The init phase-op / plan-phase / execute-phase queries emitted a non-null absolute requirements_path / state_path / roadmap_path even when the named file did not exist — built with a bare path.join and no existence check, unlike the conditional sibling fields (patterns_path, context_path, ...) in the same payload. Consumers (e.g. ultraplan-phase.md:104 'requirements_path is not null') therefore read missing files. Each of the three reading sites now returns null when the file is absent and its absolute path when present. The project/milestone-bootstrap and doc-ingest emitters that use these paths as write-targets for not-yet-created files are intentionally unchanged. * chore(#3188): backfill changeset pr: 3430 --------- Co-authored-by: sim --- .changeset/patient-yaks-click.md | 5 ++ src/init.cts | 41 +++++++++++---- tests/init.test.cjs | 90 ++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 9 deletions(-) create mode 100644 .changeset/patient-yaks-click.md diff --git a/.changeset/patient-yaks-click.md b/.changeset/patient-yaks-click.md new file mode 100644 index 000000000..5f111c6bc --- /dev/null +++ b/.changeset/patient-yaks-click.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3430 +--- +The `init phase-op`, `init plan-phase` and `init execute-phase` queries no longer hand consumers a fully-formed absolute path for a `REQUIREMENTS.md`/`STATE.md`/`ROADMAP.md` that does not exist. Those three fields were built with a bare path.join and no existence check, so a non-null value was indistinguishable from the file actually being there — even as the conditional sibling fields in the same payload (`patterns_path`, `context_path`, ...) already returned null for absent files, and `ultraplan-phase.md` explicitly gates its REQUIREMENTS.md read on `requirements_path is not null`. Each of the three reading sites now returns null when the file is absent and its absolute path when present. The project/milestone-bootstrap and doc-ingest emitters that use these paths as write-targets for not-yet-created files are intentionally unchanged. (#3188) diff --git a/src/init.cts b/src/init.cts index ee7682823..6fd6b70ad 100644 --- a/src/init.cts +++ b/src/init.cts @@ -889,6 +889,14 @@ function cmdInitExecutePhase( const wf = (config.workflow ?? {}) as Record; + // #3188: these paths are null when the file is absent, matching the contract + // the conditional sibling fields (context_path, patterns_path, ...) already + // honour and that ultraplan-phase.md / execute-phase.md gate on. Hoisted so + // the existence check and the emitted path share one source of truth. + const statePath = path.join(planningDir(cwd), 'STATE.md'); + const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); + const requirementsPath = path.join(planningDir(cwd), 'REQUIREMENTS.md'); + const result: Record = { executor_model: resolveModelInternal(cwd, 'gsd-executor'), verifier_model: resolveModelInternal(cwd, 'gsd-verifier'), @@ -963,12 +971,13 @@ function cmdInitExecutePhase( roadmap_exists: fs.existsSync(path.join(planningDir(cwd), 'ROADMAP.md')), config_exists: fs.existsSync(path.join(planningDir(cwd), 'config.json')), // #2376: emit absolute paths — see comment above on phase_dir. - state_path: toPosixPath(path.join(planningDir(cwd), 'STATE.md')), - roadmap_path: toPosixPath(path.join(planningDir(cwd), 'ROADMAP.md')), + // #3188: null when the file is absent (parity with patterns_path/context_path). + state_path: fs.existsSync(statePath) ? toPosixPath(statePath) : null, + roadmap_path: fs.existsSync(roadmapPath) ? toPosixPath(roadmapPath) : null, config_path: toPosixPath(path.join(planningDir(cwd), 'config.json')), // #2376: execute-phase.md's verify_phase_goal step reads this instead of // hardcoding '.planning/REQUIREMENTS.md' into the gsd-verifier spawn prompt. - requirements_path: toPosixPath(path.join(planningDir(cwd), 'REQUIREMENTS.md')), + requirements_path: fs.existsSync(requirementsPath) ? toPosixPath(requirementsPath) : null, }; if (options['validate']) { @@ -1060,6 +1069,12 @@ function cmdInitPlanPhase( const wf = (config.workflow ?? {}) as Record; + // #3188: see cmdInitExecutePhase — null when absent, parity with the + // conditional sibling fields in this same result object. + const statePath = path.join(planningDir(cwd), 'STATE.md'); + const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); + const requirementsPath = path.join(planningDir(cwd), 'REQUIREMENTS.md'); + const result: Record = { researcher_model: resolveModelInternal(cwd, 'gsd-phase-researcher'), planner_model: resolveModelInternal(cwd, 'gsd-planner'), @@ -1107,9 +1122,10 @@ function cmdInitPlanPhase( roadmap_exists: fs.existsSync(path.join(planningDir(cwd), 'ROADMAP.md')), // #2376: absolute — see comment on phase_dir above. - state_path: toPosixPath(path.join(planningDir(cwd), 'STATE.md')), - roadmap_path: toPosixPath(path.join(planningDir(cwd), 'ROADMAP.md')), - requirements_path: toPosixPath(path.join(planningDir(cwd), 'REQUIREMENTS.md')), + // #3188: null when the file is absent (parity with patterns_path below). + state_path: fs.existsSync(statePath) ? toPosixPath(statePath) : null, + roadmap_path: fs.existsSync(roadmapPath) ? toPosixPath(roadmapPath) : null, + requirements_path: fs.existsSync(requirementsPath) ? toPosixPath(requirementsPath) : null, patterns_path: null, }; @@ -1892,6 +1908,12 @@ function cmdInitPhaseOp(cwd: string, phase: string, raw: boolean): void { } } + // #3188: see cmdInitExecutePhase — null when absent, parity with the + // conditional sibling fields in this same result object. + const statePath = path.join(planningDir(cwd), 'STATE.md'); + const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); + const requirementsPath = path.join(planningDir(cwd), 'REQUIREMENTS.md'); + const result: Record = { commit_docs: config.commit_docs, brave_search: @@ -1927,9 +1949,10 @@ function cmdInitPhaseOp(cwd: string, phase: string, raw: boolean): void { planning_exists: fs.existsSync(planningDir(cwd)), // #2376: absolute — see comment on phase_dir above. - state_path: toPosixPath(path.join(planningDir(cwd), 'STATE.md')), - roadmap_path: toPosixPath(path.join(planningDir(cwd), 'ROADMAP.md')), - requirements_path: toPosixPath(path.join(planningDir(cwd), 'REQUIREMENTS.md')), + // #3188: null when the file is absent (parity with context_path/research_path). + state_path: fs.existsSync(statePath) ? toPosixPath(statePath) : null, + roadmap_path: fs.existsSync(roadmapPath) ? toPosixPath(roadmapPath) : null, + requirements_path: fs.existsSync(requirementsPath) ? toPosixPath(requirementsPath) : null, }; if (phaseInfo?.['directory']) { diff --git a/tests/init.test.cjs b/tests/init.test.cjs index 3bb2334f9..3a74e5138 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -12,6 +12,21 @@ const { createFixture, seedPhase } = require('./fixtures/index.cjs'); const { createTempProject, createTempDir } = require('./helpers.cjs'); const { executionContextRefs } = require('../scripts/command-contract-helpers.cjs'); +/** + * #3188: write the canonical flat planning docs so an init-query "present" test + * actually has the file it asserts. The emitter now returns null for + * state_path / roadmap_path / requirements_path when the file is absent; tests + * that exercise the present-case must therefore create the file. Phase + * resolution is directory-based (findPhaseInternal) and unaffected by these. + */ +function writePlanningDocs(tmpDir, { state = true, roadmap = true, requirements = true } = {}) { + const planning = path.join(tmpDir, '.planning'); + fs.mkdirSync(planning, { recursive: true }); + if (state) fs.writeFileSync(path.join(planning, 'STATE.md'), '# State\n'); + if (roadmap) fs.writeFileSync(path.join(planning, 'ROADMAP.md'), '# Roadmap\n'); + if (requirements) fs.writeFileSync(path.join(planning, 'REQUIREMENTS.md'), '# Requirements\n'); +} + describe('init commands', () => { let tmpDir; @@ -32,6 +47,9 @@ describe('init commands', () => { seedPhase(tmpDir, '03-api', { '03-01-PLAN.md': '# Plan', }); + // #3188: these are present-case assertions — the docs must exist on disk + // or the emitter now (correctly) returns null for the *_path fields. + writePlanningDocs(tmpDir); const result = runGsdTools('init execute-phase 03', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); @@ -86,6 +104,8 @@ describe('init commands', () => { '03-VERIFICATION.md': '# Verification', '03-UAT.md': '# UAT', }); + // #3188: present-case assertions — create the planning docs the emitter keys on. + writePlanningDocs(tmpDir); const result = runGsdTools('init plan-phase 03', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); @@ -326,6 +346,8 @@ describe('init commands', () => { '03-VERIFICATION.md': '# Verification', '03-UAT.md': '# UAT', }); + // #3188: present-case assertions — create the planning docs the emitter keys on. + writePlanningDocs(tmpDir); const result = runGsdTools('init phase-op 03', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); @@ -721,6 +743,65 @@ describe('init commands', () => { }); }); +// ───────────────────────────────────────────────────────────────────────────── +// #3188: init execute-phase / plan-phase / phase-op must return null for +// state_path / roadmap_path / requirements_path when the planning doc is +// absent — matching the contract the conditional sibling fields (patterns_path, +// context_path, …) already honour, and that ultraplan-phase.md:104 gates on. +// The WRITING emitters (new-project / new-milestone / ingest-docs) are +// intentionally NOT changed and keep returning a non-null write-target path. +// ───────────────────────────────────────────────────────────────────────────── + +describe('#3188 — init query path fields are null when the planning file is absent', () => { + let tmpDir; + + beforeEach(() => { + // #2376 macOS fix: realpath so absPlanningPath matches process.cwd()-anchored output. + tmpDir = fs.realpathSync(createFixture()); + seedPhase(tmpDir, '03-api', { '03-01-PLAN.md': '# Plan' }); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + // All three READING (projection) sites share the identical field group; the + // absent/present boundary must hold uniformly across them. + const COMMANDS = [ + ['init execute-phase', 'init execute-phase 03'], + ['init plan-phase', 'init plan-phase 03'], + ['init phase-op', 'init phase-op 03'], + ]; + + for (const [label, argv] of COMMANDS) { + test(`${label}: state_path / roadmap_path / requirements_path are null when the docs are absent`, () => { + // No STATE.md / ROADMAP.md / REQUIREMENTS.md written. + const result = runGsdTools(argv, tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.state_path, null, + 'state_path must be null when STATE.md is absent'); + assert.strictEqual(output.roadmap_path, null, + 'roadmap_path must be null when ROADMAP.md is absent'); + assert.strictEqual(output.requirements_path, null, + 'requirements_path must be null when REQUIREMENTS.md is absent'); + }); + + test(`${label}: state_path / roadmap_path / requirements_path are absolute when the docs exist`, () => { + writePlanningDocs(tmpDir); + + const result = runGsdTools(argv, tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.state_path, absPlanningPath(tmpDir, 'STATE.md')); + assert.strictEqual(output.roadmap_path, absPlanningPath(tmpDir, 'ROADMAP.md')); + assert.strictEqual(output.requirements_path, absPlanningPath(tmpDir, 'REQUIREMENTS.md')); + }); + } +}); + // ───────────────────────────────────────────────────────────────────────────── // ROADMAP fallback for init plan-phase / execute-phase / verify-work (#1238) // ───────────────────────────────────────────────────────────────────────────── @@ -2322,6 +2403,9 @@ describe('init handlers honor GSD_WORKSTREAM (ADR-0006 planningPaths consumption path.join(wsDir, 'ROADMAP.md'), '# Roadmap\n\n### Phase 1: Setup\n**Goal:** Bootstrap\n**Requirements**: R-01\n**Plans:** 1 plans\n' ); + // #3188: REQUIREMENTS.md present so the workstream-scoped requirements_path + // present-case assertion holds (STATE/ROADMAP already written above). + fs.writeFileSync(path.join(wsDir, 'REQUIREMENTS.md'), '# Requirements\n'); fs.writeFileSync( path.join(wsDir, 'config.json'), JSON.stringify({}) @@ -2371,6 +2455,8 @@ describe('init handlers honor GSD_WORKSTREAM (ADR-0006 planningPaths consumption // Flat fixture: the workstream fixture exists but we do NOT pass GSD_WORKSTREAM. // Handler should resolve flat .planning/ → state/roadmap/config are flat, // and the workstream phase is NOT found (flat phases/ is empty). + // #3188: create the flat docs so the flat *_path present-case assertions hold. + writePlanningDocs(tmpDir); const result = runGsdTools('init execute-phase 1', tmpDir, { GSD_WORKSTREAM: '', GSD_PROJECT: '' }); assert.ok(result.success, `Command failed: ${result.error}`); @@ -2464,6 +2550,8 @@ describe('init handlers honor GSD_WORKSTREAM (ADR-0006 planningPaths consumption }); test('plan-phase WITHOUT GSD_WORKSTREAM resolves flat paths (boundary control)', () => { + // #3188: create the flat docs so the flat *_path present-case assertions hold. + writePlanningDocs(tmpDir); const result = runGsdTools('init plan-phase 1', tmpDir, { GSD_WORKSTREAM: '', GSD_PROJECT: '' }); assert.ok(result.success, `Command failed: ${result.error}`); @@ -2510,6 +2598,8 @@ describe('init handlers honor GSD_WORKSTREAM (ADR-0006 planningPaths consumption }); test('phase-op WITHOUT GSD_WORKSTREAM does not find workstream-only phase (negative discrimination)', () => { + // #3188: create the flat docs so the flat *_path present-case assertions hold. + writePlanningDocs(tmpDir); const result = runGsdTools('init phase-op 1', tmpDir, { GSD_WORKSTREAM: '', GSD_PROJECT: '' }); assert.ok(result.success, `Command failed: ${result.error}`);