diff --git a/.changeset/sunny-seals-munch.md b/.changeset/sunny-seals-munch.md new file mode 100644 index 000000000..1839b9e1c --- /dev/null +++ b/.changeset/sunny-seals-munch.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3955 +--- +**`migrate-config` and health repairs no longer write outside the scoped project under `GSD_PROJECT`** — planning-path composition now goes through the project-aware resolver everywhere, so a scoped migration no longer rewrites another project's `config.json`, `project_exists` answers for the project actually being queried, and `validate.health --repair` keeps its writes in one directory. (#3749) diff --git a/src/config.cts b/src/config.cts index a757ebf6b..6775fd338 100644 --- a/src/config.cts +++ b/src/config.cts @@ -1204,7 +1204,11 @@ function cmdConfigPath(cwd: string, _raw: boolean, workstreamContext: Workstream */ function cmdMigrateConfig(cwd: string, raw: boolean): void { const ws = process.env['GSD_WORKSTREAM'] || null; - const report = migrateOnDisk(cwd, ws || undefined); + // #3749: resolve the migration target through the project-aware resolver so + // GSD_PROJECT scopes the write; migrateOnDisk itself cannot (see its + // configPathOverride note). + const scopedConfigPath = path.join(planningDir(cwd, ws || undefined), 'config.json'); + const report = migrateOnDisk(cwd, ws || undefined, scopedConfigPath); // #3760: deduplicated on (path, reason), so a repeated invocation stays quiet. if (report.skipped.length > 0) { diff --git a/src/configuration.cts b/src/configuration.cts index 815ed1be3..30651d25b 100644 --- a/src/configuration.cts +++ b/src/configuration.cts @@ -318,8 +318,14 @@ function mergeDefaults(parsed: Record): Record return deepMergeConfig(defaults, parsed); } -function migrateOnDisk(cwd: string, workstream?: string): MigrateOnDiskResult { - const configPath = join(planningDir(cwd, workstream), 'config.json'); +function migrateOnDisk(cwd: string, workstream?: string, configPathOverride?: string): MigrateOnDiskResult { + // #3749: the caller (cmdMigrateConfig in config.cts) supplies the config + // path resolved through planning-workspace's PROJECT-aware planningDir. + // This module cannot import that sibling (#3571 install-layout contract), + // and its own local planningDir above is deliberately workstream-only — + // resolving here through the local copy made migrate-config under + // GSD_PROJECT rewrite the ROOT config instead of the scoped one. + const configPath = configPathOverride ?? join(planningDir(cwd, workstream), 'config.json'); let raw: string; try { raw = readFileSync(configPath, 'utf-8'); diff --git a/src/health-diagnostic.cts b/src/health-diagnostic.cts index f72829f66..873c58aa3 100644 --- a/src/health-diagnostic.cts +++ b/src/health-diagnostic.cts @@ -119,7 +119,7 @@ const CONSISTENCY_RULES: Rule[] = [ // eslint-disable-next-line @typescript-eslint/no-require-imports import planningWorkspaceMod = require('./planning-workspace.cjs'); -const { planningRoot, planningDir } = planningWorkspaceMod; +const { planningDir } = planningWorkspaceMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import configLoaderMod = require('./config-loader.cjs'); const { CONFIG_DEFAULTS } = configLoaderMod; @@ -216,7 +216,13 @@ interface RepairPaths { * document for the read side. */ function repairPaths(cwd: string): RepairPaths { - const rootBase = planningRoot(cwd); + // #3749: the project root is PROJECT-aware but deliberately NOT workstream- + // scoped — under GSD_PROJECT, config.json/MILESTONES.md/milestones belong to + // `.planning//` (the same base init.new-project's config_path + // names); under GSD_WORKSTREAM they stay at the workstream PARENT, keeping + // the documented root-vs-workstream split. planningDir(cwd, null) is exactly + // that base: project env honored, ws env suppressed. + const rootBase = planningDir(cwd, null); const wsBase = planningDir(cwd); return { rootBase, diff --git a/src/init.cts b/src/init.cts index 8da6308d9..46352541d 100644 --- a/src/init.cts +++ b/src/init.cts @@ -1313,7 +1313,7 @@ function cmdInitNewProject(cwd: string, raw: boolean, options: Record = { - project_exists: pathExistsInternal(cwd, '.planning/PROJECT.md'), + project_exists: pathExistsInternal(cwd, toPosixPath(path.relative(cwd, path.join(planningDir(cwd), 'PROJECT.md')))), planning_exists: fs.existsSync(planningRoot(cwd)), ...getInitGitState(cwd), // #2376: absolute — see comment on phase_dir in cmdInitExecutePhase. The @@ -1565,7 +1565,7 @@ function cmdInitResume(cwd: string, raw: boolean): void { const result: Record = { state_exists: fs.existsSync(path.join(planningDir(cwd), 'STATE.md')), roadmap_exists: fs.existsSync(path.join(planningDir(cwd), 'ROADMAP.md')), - project_exists: pathExistsInternal(cwd, '.planning/PROJECT.md'), + project_exists: pathExistsInternal(cwd, toPosixPath(path.relative(cwd, path.join(planningDir(cwd), 'PROJECT.md')))), planning_exists: fs.existsSync(planningRoot(cwd)), // #2376: absolute — see comment on phase_dir in cmdInitExecutePhase. @@ -2295,7 +2295,7 @@ function cmdInitMilestoneOp(cwd: string, raw: boolean): void { archived_milestones: archivedMilestones, archive_count: archivedMilestones.length, - project_exists: pathExistsInternal(cwd, '.planning/PROJECT.md'), + project_exists: pathExistsInternal(cwd, toPosixPath(path.relative(cwd, path.join(planningDir(cwd), 'PROJECT.md')))), roadmap_exists: fs.existsSync(path.join(planningDir(cwd), 'ROADMAP.md')), state_exists: fs.existsSync(path.join(planningDir(cwd), 'STATE.md')), archive_exists: fs.existsSync(path.join(planningRoot(cwd), 'archive')), @@ -2711,7 +2711,7 @@ function cmdInitManager(cwd: string, raw: boolean): void { waiting_signal: waitingSignal, all_complete: completedCount === nonBacklogPhases.length && nonBacklogPhases.length > 0, - project_exists: pathExistsInternal(cwd, '.planning/PROJECT.md'), + project_exists: pathExistsInternal(cwd, toPosixPath(path.relative(cwd, path.join(planningDir(cwd), 'PROJECT.md')))), roadmap_exists: true, state_exists: true, manager_flags: managerFlags, @@ -3231,7 +3231,7 @@ function cmdInitProgress(cwd: string, raw: boolean, options: Record]` + // — PROJECT.md, config.json live here; #3749). Workstream-free by construction: + // planningDir(cwd, null) honors GSD_PROJECT and suppresses GSD_WORKSTREAM. + const rootBase = planningDir(cwd, null); const _slashRuntime = resolveRuntime(cwd); const slash = (name: string) => formatGsdSlash(name, _slashRuntime) as string; diff --git a/tests/configuration-migrate-config.test.cjs b/tests/configuration-migrate-config.test.cjs index 4430f1565..bec908122 100644 --- a/tests/configuration-migrate-config.test.cjs +++ b/tests/configuration-migrate-config.test.cjs @@ -610,3 +610,58 @@ test('mergeDefaults clones defaults without JSON serialization fragility (#321)' }); }); } + +// ─── #3749: migrate-config must be project-aware under GSD_PROJECT ────────── +describe('migrate-config — GSD_PROJECT scoping (#3749)', () => { + test('migrates the SCOPED config.json, never the root one', (t) => { + const tmpDir = createTempProject('gsd-3749-migrate-'); + t.after(() => cleanup(tmpDir)); + fs.mkdirSync(path.join(tmpDir, '.planning', 'second-product'), { recursive: true }); + // Root carries the legacy key; the scoped config is canonical. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify({ depth: 'quick' }), + ); + const scopedPath = path.join(tmpDir, '.planning', 'second-product', 'config.json'); + fs.writeFileSync(scopedPath, JSON.stringify({ granularity: 'standard' })); + + // Scoped config has no legacy keys → the honest outcome is a no-op that + // writes NOTHING (issue #3749 expectation: "reports that the scoped config + // has no legacy keys and writes nothing"). Either way the root file must + // not be touched. + const r = runMigrateConfig(tmpDir, [], { GSD_PROJECT: 'second-product' }); + assert.equal(r.status, 0, `stderr: ${r.stderr}`); + + const root = JSON.parse(fs.readFileSync(path.join(tmpDir, '.planning', 'config.json'), 'utf8')); + assert.equal(root['depth'], 'quick', '#3749: the root project\'s config must be untouched'); + assert.equal(root['granularity'], undefined, '#3749: no migration may be written into the root config'); + }); + + test('a scoped config WITH legacy keys is the migration target', (t) => { + const tmpDir = createTempProject('gsd-3749-migrate2-'); + t.after(() => cleanup(tmpDir)); + fs.mkdirSync(path.join(tmpDir, '.planning', 'second-product'), { recursive: true }); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify({ granularity: 'fine' }), + ); + const scopedPath = path.join(tmpDir, '.planning', 'second-product', 'config.json'); + fs.writeFileSync(scopedPath, JSON.stringify({ depth: 'quick' })); + + const r = runMigrateConfig(tmpDir, [], { GSD_PROJECT: 'second-product' }); + assert.equal(r.status, 0, `stderr: ${r.stderr}`); + const parsed = JSON.parse(r.stdout); + + const scoped = JSON.parse(fs.readFileSync(scopedPath, 'utf8')); + assert.equal(scoped['granularity'], 'coarse', '#3749: the SCOPED config must be migrated'); + assert.equal(scoped['depth'], undefined, 'legacy key removed from the scoped config'); + const root = JSON.parse(fs.readFileSync(path.join(tmpDir, '.planning', 'config.json'), 'utf8')); + assert.equal(root['granularity'], 'fine', '#3749: root config must remain byte-equivalent'); + if (parsed['wrote']) { + assert.ok( + String(parsed['wrote']).includes(path.join('.planning', 'second-product')), + `#3749: wrote must name the scoped file, got ${parsed['wrote']}`, + ); + } + }); +}); diff --git a/tests/health-diagnostic.test.cjs b/tests/health-diagnostic.test.cjs index 48ef4b892..c9d3fc0f5 100644 --- a/tests/health-diagnostic.test.cjs +++ b/tests/health-diagnostic.test.cjs @@ -559,3 +559,52 @@ describe('applyRepairs — REAL diagnostics (rows 15-16)', () => { assert.ok(detail.error, 'the details row must carry the thrown error message'); }); }); + +// ─── #3749: repair paths must be project-aware under GSD_PROJECT ──────────── +describe('health repair — GSD_PROJECT scoping (#3749)', () => { + test('createConfig writes config.json beside the SCOPED planning dir, not the root', (t) => { + const tmpDir = createTempDir('gsd-3749-repair-'); + t.after(() => cleanup(tmpDir)); + fs.mkdirSync(path.join(tmpDir, '.planning', 'second-product'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, '.planning', 'second-product', 'PROJECT.md'), '# Second Product\n'); + + const diagnostics = [ + fakeDiagnostic('W003', REMEDY_ACTION.CREATE_CONFIG, REMEDY_RISK.NONE), + ]; + const prevProject = process.env['GSD_PROJECT']; + process.env['GSD_PROJECT'] = 'second-product'; + t.after(() => { + if (prevProject === undefined) delete process.env['GSD_PROJECT']; + else process.env['GSD_PROJECT'] = prevProject; + }); + const result = applyRepairs(tmpDir, diagnostics, true, false); + t.after(() => { delete process.env['GSD_PROJECT']; if (prevProject !== undefined) process.env['GSD_PROJECT'] = prevProject; }); + + assert.deepEqual(result.applied, ['W003']); + const scopedConfig = path.join(tmpDir, '.planning', 'second-product', 'config.json'); + assert.ok(fs.existsSync(scopedConfig), '#3749: config.json must be created inside the scoped project'); + assert.ok(!fs.existsSync(path.join(tmpDir, '.planning', 'config.json')), + '#3749: the repair must not create a root config.json under GSD_PROJECT'); + }); + + test('under GSD_WORKSTREAM (no project) config stays OUT of the workstream dir', (t) => { + const tmpDir = createTempDir('gsd-3749-repair-ws-'); + t.after(() => cleanup(tmpDir)); + fs.mkdirSync(path.join(tmpDir, '.planning', 'workstreams', 'alpha'), { recursive: true }); + + const diagnostics = [ + fakeDiagnostic('W003', REMEDY_ACTION.CREATE_CONFIG, REMEDY_RISK.NONE), + ]; + const prev = process.env['GSD_WORKSTREAM']; + process.env['GSD_WORKSTREAM'] = 'alpha'; + const result = applyRepairs(tmpDir, diagnostics, true, false); + if (prev === undefined) delete process.env['GSD_WORKSTREAM']; + else process.env['GSD_WORKSTREAM'] = prev; + + assert.deepEqual(result.applied, ['W003']); + assert.ok(fs.existsSync(path.join(tmpDir, '.planning', 'config.json')), + 'the documented root-vs-workstream split keeps config.json at the workstream PARENT'); + assert.ok(!fs.existsSync(path.join(tmpDir, '.planning', 'workstreams', 'alpha', 'config.json')), + 'config.json must not move into the workstream dir'); + }); +}); diff --git a/tests/init.test.cjs b/tests/init.test.cjs index 7fe92f14d..31bcb4f1a 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -4707,3 +4707,38 @@ describe('#3581: init.progress next_phase prefers the roadmap frontier', () => { assert.equal(out.next_phase, null, 'all-complete milestone: no frontier (completion flow owns the answer)'); }); }); + +// ─── #3749: project_exists must follow project_path under GSD_PROJECT ─────── +describe('init.new-project — GSD_PROJECT scoping (#3749)', () => { + test('project_exists tracks the namespaced PROJECT.md, not the root one', (t) => { + const tmpDir = createTempProject('gsd-3749-init-'); + t.after(() => cleanup(tmpDir)); + fs.mkdirSync(path.join(tmpDir, '.planning', 'second-product'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, '.planning', 'second-product', 'PROJECT.md'), '# Second Product\n'); + + const r1 = runGsdTools(['query', 'init.new-project'], tmpDir, { GSD_PROJECT: 'second-product' }); + assert.ok(r1.success, r1.error); + const out1 = JSON.parse(r1.output); + assert.equal(out1['project_exists'], true, + `#3749: project_path (${out1['project_path']}) names an existing file — project_exists must be true`); + // project_path is POSIX-normalized by toPosixPath — compare with a literal + // forward-slash path, not path.join (which yields backslashes on Windows). + assert.ok(String(out1['project_path']).includes('.planning/second-product')); + + // An unrelated root PROJECT.md must not change the verdict. + fs.writeFileSync(path.join(tmpDir, '.planning', 'PROJECT.md'), '# unrelated\n'); + const r2 = runGsdTools(['query', 'init.new-project'], tmpDir, { GSD_PROJECT: 'second-product' }); + assert.ok(r2.success, r2.error); + assert.equal(JSON.parse(r2.output)['project_exists'], true, + '#3749: verdict must not flip when an unrelated root file appears'); + }); + + test('without GSD_PROJECT the root PROJECT.md still answers project_exists', (t) => { + const tmpDir = createTempProject('gsd-3749-init2-'); + t.after(() => cleanup(tmpDir)); + fs.writeFileSync(path.join(tmpDir, '.planning', 'PROJECT.md'), '# Root Project\n'); + const r = runGsdTools(['query', 'init.new-project'], tmpDir); + assert.ok(r.success, r.error); + assert.equal(JSON.parse(r.output)['project_exists'], true, 'default (unscoped) behavior unchanged'); + }); +});