* fix(#2237): fail loud on ambiguous bare-number phase directory collision When two unrelated projects share a .planning/phases/ tree, a bare phase number silently resolved to the first 0N-* directory found — risking cross-project file writes. The fix detects multiple matches for the same phase number and surfaces an ambiguous_matches result instead of silently taking the first. Changes: - src/phase-locator.cts: searchPhaseInDir uses filter() + ambiguity check - src/phase.cts: cmdFindPhase same pattern - src/init.cts: cmdInitPhaseOp surfaces ambiguous_matches in the result - tests/phase-locator.test.cjs: 3 regression tests * docs: backfill changeset PR number (#2262) * merge: keep up to date with next * fix: regenerate stale capability-registry after next merge
This commit is contained in:
5
.changeset/tidy-zebras-hum.md
Normal file
5
.changeset/tidy-zebras-hum.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 2262
|
||||
---
|
||||
**Phase-directory resolution fails loud on cross-project collisions** — when two unrelated GSD projects share a `.planning/phases/` tree, a bare phase number silently resolved to the first `0N-*` directory found. The fix detects multiple matches and surfaces an `ambiguous_matches` result. (#2237)
|
||||
16
src/init.cts
16
src/init.cts
@@ -979,6 +979,22 @@ function cmdInitPhaseOp(cwd: string, phase: string, raw: boolean): void {
|
||||
const config = loadConfig(cwd);
|
||||
let phaseInfo = guardedFindPhase(cwd, phase, config.project_code);
|
||||
|
||||
// #2237: surface ambiguous phase-directory collisions instead of silently
|
||||
// taking the first match when unrelated projects share a .planning/phases/ tree.
|
||||
if (phaseInfo?.['ambiguous_matches']) {
|
||||
const matches = phaseInfo['ambiguous_matches'] as string[];
|
||||
const result: Record<string, unknown> = {
|
||||
phase_found: false,
|
||||
phase_dir: null,
|
||||
phase_number: null,
|
||||
phase_name: null,
|
||||
ambiguous_matches: matches,
|
||||
warning: `Phase ${phase} is ambiguous: ${matches.length} directories match (${matches.map((m: string) => `"${m}"`).join(', ')}). Set a distinct project_code in .planning/config.json to scope resolution.`,
|
||||
};
|
||||
output(withProjectRoot(cwd, result), raw);
|
||||
return;
|
||||
}
|
||||
|
||||
if (phaseInfo?.['archived']) {
|
||||
const roadmapPhase = guardedGetRoadmapPhase(cwd, phase, config.project_code);
|
||||
if (roadmapPhase?.['found']) {
|
||||
|
||||
@@ -44,6 +44,7 @@ interface PhaseSearchResult {
|
||||
has_verification: boolean;
|
||||
has_reviews: boolean;
|
||||
archived?: string;
|
||||
ambiguous_matches?: string[];
|
||||
}
|
||||
|
||||
interface ArchivedPhaseDir {
|
||||
@@ -58,8 +59,31 @@ interface ArchivedPhaseDir {
|
||||
function searchPhaseInDir(baseDir: string, relBase: string, normalized: string): PhaseSearchResult | null {
|
||||
try {
|
||||
const dirs = readSubdirectories(baseDir, true);
|
||||
const match = dirs.find(d => phaseTokenMatches(d, normalized));
|
||||
if (!match) return null;
|
||||
const matches = dirs.filter(d => phaseTokenMatches(d, normalized));
|
||||
if (matches.length === 0) return null;
|
||||
|
||||
// #2237: fail loud when multiple directories match the same bare phase
|
||||
// number — this happens when unrelated projects share a .planning/phases/
|
||||
// tree. Silently taking the first match risks cross-project file writes.
|
||||
if (matches.length > 1) {
|
||||
return {
|
||||
found: false,
|
||||
directory: '',
|
||||
phase_number: normalized,
|
||||
phase_name: null,
|
||||
phase_slug: null,
|
||||
plans: [],
|
||||
summaries: [],
|
||||
incomplete_plans: [],
|
||||
has_research: false,
|
||||
has_context: false,
|
||||
has_verification: false,
|
||||
has_reviews: false,
|
||||
ambiguous_matches: matches,
|
||||
};
|
||||
}
|
||||
|
||||
const match = matches[0];
|
||||
|
||||
const phaseToken = extractPhaseToken(match);
|
||||
const phaseNumber = phaseToken || normalized;
|
||||
|
||||
@@ -416,8 +416,20 @@ function cmdFindPhase(cwd: string, phase: string, raw: boolean): void {
|
||||
.map((e) => e.name)
|
||||
.sort((a, b) => comparePhaseNum(a, b));
|
||||
|
||||
const match = dirs.find((d) => phaseTokenMatches(d, normalized));
|
||||
if (!match) continue;
|
||||
// #2237: fail loud when multiple directories match the same bare phase
|
||||
// number — prevents cross-project file writes when unrelated projects
|
||||
// share a .planning/phases/ tree.
|
||||
const matches = dirs.filter((d) => phaseTokenMatches(d, normalized));
|
||||
if (matches.length === 0) continue;
|
||||
if (matches.length > 1) {
|
||||
output({
|
||||
...notFound,
|
||||
ambiguous_matches: matches,
|
||||
warning: `Phase ${normalized} is ambiguous: ${matches.length} directories match (${matches.map(m => `"${m}"`).join(', ')}). Set a distinct project_code in .planning/config.json to scope resolution.`,
|
||||
}, raw, '');
|
||||
return;
|
||||
}
|
||||
const match = matches[0];
|
||||
|
||||
const dirMatch =
|
||||
match.match(
|
||||
|
||||
@@ -411,3 +411,45 @@ describe('searchPhaseInDir: direct filesystem search', () => {
|
||||
assert.strictEqual(result, null);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── #2237: ambiguous phase-directory collision ─────────────────────────────
|
||||
describe('#2237: ambiguous phase-directory collision', () => {
|
||||
let tmpDir;
|
||||
afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } });
|
||||
|
||||
test('searchPhaseInDir returns ambiguous_matches when two dirs share phase number', () => {
|
||||
tmpDir = createTempProject('gsd-pl-collision-');
|
||||
const phasesDir = path.join(tmpDir, '.planning', 'phases');
|
||||
// Two unrelated projects with same phase number
|
||||
fs.mkdirSync(path.join(phasesDir, '04-alpha-feature'), { recursive: true });
|
||||
fs.mkdirSync(path.join(phasesDir, '04-beta-feature'), { recursive: true });
|
||||
const result = phaseLocator.searchPhaseInDir(phasesDir, '.planning/phases', '04');
|
||||
assert.ok(result, 'should return a result (not null)');
|
||||
assert.strictEqual(result.found, false, 'ambiguous result must not be "found"');
|
||||
assert.ok(result.ambiguous_matches, 'must have ambiguous_matches');
|
||||
assert.strictEqual(result.ambiguous_matches.length, 2);
|
||||
assert.ok(result.ambiguous_matches.includes('04-alpha-feature'));
|
||||
assert.ok(result.ambiguous_matches.includes('04-beta-feature'));
|
||||
});
|
||||
|
||||
test('findPhaseInternal propagates ambiguous result', () => {
|
||||
tmpDir = createTempProject('gsd-pl-collision-');
|
||||
const phasesDir = path.join(tmpDir, '.planning', 'phases');
|
||||
fs.mkdirSync(path.join(phasesDir, '05-gamma'), { recursive: true });
|
||||
fs.mkdirSync(path.join(phasesDir, '05-delta'), { recursive: true });
|
||||
const result = phaseLocator.findPhaseInternal(tmpDir, '5');
|
||||
assert.ok(result, 'should return a result');
|
||||
assert.strictEqual(result.found, false);
|
||||
assert.ok(result.ambiguous_matches?.length >= 2);
|
||||
});
|
||||
|
||||
test('single match still resolves normally (no false positive)', () => {
|
||||
tmpDir = createTempProject('gsd-pl-collision-');
|
||||
const phasesDir = path.join(tmpDir, '.planning', 'phases');
|
||||
fs.mkdirSync(path.join(phasesDir, '03-only-phase'), { recursive: true });
|
||||
const result = phaseLocator.searchPhaseInDir(phasesDir, '.planning/phases', '03');
|
||||
assert.ok(result);
|
||||
assert.strictEqual(result.found, true);
|
||||
assert.ok(!result.ambiguous_matches, 'single match must not set ambiguous_matches');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user