From 70d8bbcd177c88160156fd11c39830bbc5d69bb1 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 4 Apr 2026 07:14:55 -0400 Subject: [PATCH] fix(phase-resolution): use exact token matching instead of prefix matches (#1639) * fix(phase-resolution): use exact token matching instead of prefix matches Closes #1635 Co-Authored-By: Claude Opus 4.6 * fix(phase-resolution): add case-insensitive flag to project-code strip regex The strip regex in phaseTokenMatches lacked the `i` flag, so lowercase project-code prefixes (e.g. `ck-01-name`) were not stripped during the fallback comparison. This made `phaseTokenMatches('ck-01-name', '01')` return false when it should return true. Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- get-shit-done/bin/lib/core.cjs | 49 ++++++++++++---- get-shit-done/bin/lib/init.cjs | 4 +- get-shit-done/bin/lib/phase.cjs | 19 ++----- get-shit-done/bin/lib/roadmap.cjs | 4 +- tests/phase.test.cjs | 94 +++++++++++++++++++++++++++++++ 5 files changed, 142 insertions(+), 28 deletions(-) diff --git a/get-shit-done/bin/lib/core.cjs b/get-shit-done/bin/lib/core.cjs index 65b440c4f..476b85da2 100644 --- a/get-shit-done/bin/lib/core.cjs +++ b/get-shit-done/bin/lib/core.cjs @@ -884,20 +884,45 @@ function comparePhaseNum(a, b) { return 0; } +/** + * Extract the phase token from a directory name. + * Supports: '01-name', '1009A-name', '999.6-name', 'CK-01-name', 'PROJ-42-name'. + * Returns the token portion (e.g. '01', '1009A', '999.6', 'PROJ-42') or the full name if no separator. + */ +function extractPhaseToken(dirName) { + // Try project-code-prefixed numeric: CK-01-name → CK-01, CK-01A.2-name → CK-01A.2 + const codePrefixed = dirName.match(/^([A-Z]{1,6}-\d+[A-Z]?(?:\.\d+)*)(?:-|$)/i); + if (codePrefixed) return codePrefixed[1]; + // Try plain numeric: 01-name, 1009A-name, 999.6-name + const numeric = dirName.match(/^(\d+[A-Z]?(?:\.\d+)*)(?:-|$)/i); + if (numeric) return numeric[1]; + // Custom IDs: PROJ-42-name → everything before the last segment that looks like a name + const custom = dirName.match(/^([A-Z][A-Z0-9]*(?:-[A-Z0-9]+)*)(?:-[a-z]|$)/i); + if (custom) return custom[1]; + return dirName; +} + +/** + * Check if a directory name's phase token matches the normalized phase exactly. + * Case-insensitive comparison for the token portion. + */ +function phaseTokenMatches(dirName, normalized) { + const token = extractPhaseToken(dirName); + if (token.toUpperCase() === normalized.toUpperCase()) return true; + // Strip optional project_code prefix from dir and retry + const stripped = dirName.replace(/^[A-Z]{1,6}-(?=\d)/i, ''); + if (stripped !== dirName) { + const strippedToken = extractPhaseToken(stripped); + if (strippedToken.toUpperCase() === normalized.toUpperCase()) return true; + } + return false; +} + function searchPhaseInDir(baseDir, relBase, normalized) { try { const dirs = readSubdirectories(baseDir, true); - // Match: starts with normalized (numeric) OR contains normalized as prefix segment (custom ID) - const match = dirs.find(d => { - if (d.startsWith(normalized)) return true; - // For custom IDs like PROJ-42, match case-insensitively - if (d.toUpperCase().startsWith(normalized.toUpperCase())) return true; - // Strip optional project_code prefix (e.g., 'CK-01-name' → '01-name') and retry - const stripped = d.replace(/^[A-Z]{1,6}-/, ''); - if (stripped.startsWith(normalized)) return true; - if (stripped.toUpperCase().startsWith(normalized.toUpperCase())) return true; - return false; - }); + // Match: exact phase token comparison (not prefix matching) + const match = dirs.find(d => phaseTokenMatches(d, normalized)); if (!match) return null; // Extract phase number and name — supports numeric (01-name), project-code-prefixed (CK-01-name), and custom (PROJ-42-name) @@ -1437,6 +1462,8 @@ module.exports = { normalizePhaseName, comparePhaseNum, searchPhaseInDir, + extractPhaseToken, + phaseTokenMatches, findPhaseInternal, getArchivedPhaseDirs, getRoadmapPhaseInternal, diff --git a/get-shit-done/bin/lib/init.cjs b/get-shit-done/bin/lib/init.cjs index 48e340c1e..b377e293c 100644 --- a/get-shit-done/bin/lib/init.cjs +++ b/get-shit-done/bin/lib/init.cjs @@ -5,7 +5,7 @@ const fs = require('fs'); const path = require('path'); const { execSync } = require('child_process'); -const { loadConfig, resolveModelInternal, findPhaseInternal, getRoadmapPhaseInternal, pathExistsInternal, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, normalizePhaseName, planningPaths, planningDir, planningRoot, toPosixPath, output, error, checkAgentsInstalled } = require('./core.cjs'); +const { loadConfig, resolveModelInternal, findPhaseInternal, getRoadmapPhaseInternal, pathExistsInternal, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, normalizePhaseName, planningPaths, planningDir, planningRoot, toPosixPath, output, error, checkAgentsInstalled, phaseTokenMatches } = require('./core.cjs'); function getLatestCompletedMilestone(cwd) { const milestonesPath = path.join(planningRoot(cwd), 'MILESTONES.md'); @@ -848,7 +848,7 @@ function cmdInitManager(cwd, raw) { try { const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); const dirs = entries.filter(e => e.isDirectory()).map(e => e.name).filter(isDirInMilestone); - const dirMatch = dirs.find(d => d.startsWith(normalized + '-') || d === normalized); + const dirMatch = dirs.find(d => phaseTokenMatches(d, normalized)); if (dirMatch) { const fullDir = path.join(phasesDir, dirMatch); diff --git a/get-shit-done/bin/lib/phase.cjs b/get-shit-done/bin/lib/phase.cjs index 11f49b0ad..41b10e7e5 100644 --- a/get-shit-done/bin/lib/phase.cjs +++ b/get-shit-done/bin/lib/phase.cjs @@ -4,7 +4,7 @@ const fs = require('fs'); const path = require('path'); -const { escapeRegex, loadConfig, normalizePhaseName, comparePhaseNum, findPhaseInternal, getArchivedPhaseDirs, generateSlugInternal, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, toPosixPath, planningDir, withPlanningLock, output, error, readSubdirectories } = require('./core.cjs'); +const { escapeRegex, loadConfig, normalizePhaseName, comparePhaseNum, findPhaseInternal, getArchivedPhaseDirs, generateSlugInternal, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, toPosixPath, planningDir, withPlanningLock, output, error, readSubdirectories, phaseTokenMatches } = require('./core.cjs'); const { extractFrontmatter } = require('./frontmatter.cjs'); const { writeStateMd, stateExtractField, stateReplaceField, stateReplaceFieldWithFallback } = require('./state.cjs'); @@ -41,7 +41,7 @@ function cmdPhasesList(cwd, options, raw) { // If filtering by phase number if (phase) { const normalized = normalizePhaseName(phase); - const match = dirs.find(d => d.startsWith(normalized)); + const match = dirs.find(d => phaseTokenMatches(d, normalized)); if (!match) { output({ files: [], count: 0, phase_dir: null, error: 'Phase not found' }, raw, ''); return; @@ -108,7 +108,7 @@ function cmdPhaseNextDecimal(cwd, basePhase, raw) { const dirs = entries.filter(e => e.isDirectory()).map(e => e.name); // Check if base phase exists - const baseExists = dirs.some(d => d.startsWith(normalized + '-') || d === normalized); + const baseExists = dirs.some(d => phaseTokenMatches(d, normalized)); // Find existing decimal phases for this base const decimalPattern = new RegExp(`^${normalized}\\.(\\d+)`); @@ -163,14 +163,7 @@ function cmdFindPhase(cwd, phase, raw) { const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); const dirs = entries.filter(e => e.isDirectory()).map(e => e.name).sort((a, b) => comparePhaseNum(a, b)); - const match = dirs.find(d => { - if (d.startsWith(normalized)) return true; - if (d.toUpperCase().startsWith(normalized.toUpperCase())) return true; - // Strip optional project_code prefix (e.g., 'CK-01-name' → '01-name') and retry - const stripped = d.replace(/^[A-Z]{1,6}-/, ''); - if (stripped.startsWith(normalized)) return true; - return false; - }); + const match = dirs.find(d => phaseTokenMatches(d, normalized)); if (!match) { output(notFound, raw, ''); return; @@ -221,7 +214,7 @@ function cmdPhasePlanIndex(cwd, phase, raw) { try { const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); const dirs = entries.filter(e => e.isDirectory()).map(e => e.name).sort((a, b) => comparePhaseNum(a, b)); - const match = dirs.find(d => d.startsWith(normalized)); + const match = dirs.find(d => phaseTokenMatches(d, normalized)); if (match) { phaseDir = path.join(phasesDir, match); phaseDirName = match; @@ -605,7 +598,7 @@ function cmdPhaseRemove(cwd, targetPhase, options, raw) { // Find target directory const targetDir = readSubdirectories(phasesDir, true) - .find(d => d.startsWith(normalized + '-') || d === normalized) || null; + .find(d => phaseTokenMatches(d, normalized)) || null; // Guard against removing executed work if (targetDir && !force) { diff --git a/get-shit-done/bin/lib/roadmap.cjs b/get-shit-done/bin/lib/roadmap.cjs index 29f2bfde8..f6b3d1c28 100644 --- a/get-shit-done/bin/lib/roadmap.cjs +++ b/get-shit-done/bin/lib/roadmap.cjs @@ -4,7 +4,7 @@ const fs = require('fs'); const path = require('path'); -const { escapeRegex, normalizePhaseName, planningPaths, withPlanningLock, output, error, findPhaseInternal, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone } = require('./core.cjs'); +const { escapeRegex, normalizePhaseName, planningPaths, withPlanningLock, output, error, findPhaseInternal, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, phaseTokenMatches } = require('./core.cjs'); /** * Search for a phase header (and its section) within the given content string. @@ -157,7 +157,7 @@ function cmdRoadmapAnalyze(cwd, raw) { try { const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); const dirs = entries.filter(e => e.isDirectory()).map(e => e.name); - const dirMatch = dirs.find(d => d.startsWith(normalized + '-') || d === normalized); + const dirMatch = dirs.find(d => phaseTokenMatches(d, normalized)); if (dirMatch) { const phaseFiles = fs.readdirSync(path.join(phasesDir, dirMatch)); diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index 6ba019389..30823ec40 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -2018,6 +2018,100 @@ describe('phase complete milestone-scoped next-phase', () => { }); }); +// ───────────────────────────────────────────────────────────────────────────── +// exact token matching (no prefix collisions) +// ───────────────────────────────────────────────────────────────────────────── + +describe('phase resolution uses exact token matching', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('1009 must NOT match 1009A-feature-consistency when 1009 dir is absent', () => { + // With only 1009A on disk, searching for 1009 should return not-found + // because 1009 !== 1009A (prefix match bug: '1009A-...' starts with '1009') + const phasesDir = path.join(tmpDir, '.planning', 'phases'); + fs.mkdirSync(path.join(phasesDir, '1009A-feature-consistency')); + fs.writeFileSync(path.join(phasesDir, '1009A-feature-consistency', 'PLAN.md'), '# Plan'); + + const result = runGsdTools('find-phase 1009', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.found, false, 'should NOT find phase 1009 when only 1009A exists'); + }); + + test('1009 matches 1009-pipeline-accuracy-fix when both exist', () => { + const phasesDir = path.join(tmpDir, '.planning', 'phases'); + fs.mkdirSync(path.join(phasesDir, '1009-pipeline-accuracy-fix')); + fs.mkdirSync(path.join(phasesDir, '1009A-feature-consistency')); + fs.writeFileSync(path.join(phasesDir, '1009-pipeline-accuracy-fix', 'PLAN.md'), '# Plan'); + + const result = runGsdTools('find-phase 1009', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.found, true, 'should find phase 1009'); + assert.ok( + output.directory.includes('1009-pipeline-accuracy-fix'), + `should match 1009-pipeline-accuracy-fix, got: ${output.directory}` + ); + }); + + test('999.6 must NOT match 999.60-episode-processing when 999.6 dir is absent', () => { + // With only 999.60 on disk, searching for 999.6 should return not-found + // because '999.60-...' starts with '999.6' (prefix match bug) + const phasesDir = path.join(tmpDir, '.planning', 'phases'); + fs.mkdirSync(path.join(phasesDir, '999.60-episode-processing')); + fs.writeFileSync(path.join(phasesDir, '999.60-episode-processing', 'PLAN.md'), '# Plan'); + + const result = runGsdTools('find-phase 999.6', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.found, false, 'should NOT find phase 999.6 when only 999.60 exists'); + }); + + test('999.6 matches 999.6-ground-truth-dataset when both exist', () => { + const phasesDir = path.join(tmpDir, '.planning', 'phases'); + fs.mkdirSync(path.join(phasesDir, '999.6-ground-truth-dataset')); + fs.mkdirSync(path.join(phasesDir, '999.60-episode-processing')); + fs.writeFileSync(path.join(phasesDir, '999.6-ground-truth-dataset', 'PLAN.md'), '# Plan'); + + const result = runGsdTools('find-phase 999.6', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.found, true, 'should find phase 999.6'); + assert.ok( + output.directory.includes('999.6-ground-truth-dataset'), + `should match 999.6-ground-truth-dataset, got: ${output.directory}` + ); + }); + + test('normal non-colliding phases still resolve', () => { + const phasesDir = path.join(tmpDir, '.planning', 'phases'); + fs.mkdirSync(path.join(phasesDir, '01-foundation')); + fs.mkdirSync(path.join(phasesDir, '02-implementation')); + fs.writeFileSync(path.join(phasesDir, '01-foundation', 'PLAN.md'), '# Plan'); + fs.writeFileSync(path.join(phasesDir, '02-implementation', 'PLAN.md'), '# Plan'); + + const r1 = runGsdTools('find-phase 1', tmpDir); + assert.ok(r1.success, `Command failed for phase 1: ${r1.error}`); + const o1 = JSON.parse(r1.output); + assert.strictEqual(o1.found, true, 'should find phase 1'); + assert.ok(o1.directory.includes('01-foundation'), `should match 01-foundation, got: ${o1.directory}`); + + const r2 = runGsdTools('find-phase 2', tmpDir); + assert.ok(r2.success, `Command failed for phase 2: ${r2.error}`); + const o2 = JSON.parse(r2.output); + assert.strictEqual(o2.found, true, 'should find phase 2'); + assert.ok(o2.directory.includes('02-implementation'), `should match 02-implementation, got: ${o2.directory}`); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // milestone complete command // ─────────────────────────────────────────────────────────────────────────────