* 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 <sim@local>
This commit is contained in:
5
.changeset/patient-yaks-click.md
Normal file
5
.changeset/patient-yaks-click.md
Normal file
@@ -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)
|
||||
41
src/init.cts
41
src/init.cts
@@ -889,6 +889,14 @@ function cmdInitExecutePhase(
|
||||
|
||||
const wf = (config.workflow ?? {}) as Record<string, unknown>;
|
||||
|
||||
// #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<string, unknown> = {
|
||||
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<string, unknown>;
|
||||
|
||||
// #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<string, unknown> = {
|
||||
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<string, unknown> = {
|
||||
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']) {
|
||||
|
||||
@@ -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}`);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user