fix(verify): use active milestone phase roots for consistency checks

This commit is contained in:
Tom Boucher
2026-05-06 15:30:45 -04:00
parent a0d95176db
commit 56737c057b
3 changed files with 187 additions and 70 deletions

View File

@@ -222,11 +222,11 @@ function cmdFindPhase(cwd, phase, raw) {
if (fs.existsSync(flatPhasesDir)) searchDirs.push(flatPhasesDir);
try {
const milestonesDir = path.join(planBase, 'milestones');
const entries = fs.readdirSync(milestonesDir, { withFileTypes: true });
const entries = fs.readdirSync(milestonesDir, { withFileTypes: true })
.filter(e => e.isDirectory() && /^v\d+.*-phases$/.test(e.name))
.sort((a, b) => a.name.localeCompare(b.name, undefined, { numeric: true }));
for (const e of entries) {
if (e.isDirectory() && /^v\d+.*-phases$/.test(e.name)) {
searchDirs.push(path.join(milestonesDir, e.name));
}
searchDirs.push(path.join(milestonesDir, e.name));
}
} catch { /* no milestones dir */ }

View File

@@ -396,40 +396,76 @@ function cmdVerifyKeyLinks(cwd, planFilePath, raw) {
}, raw, verified === results.length ? 'valid' : 'invalid');
}
// Returns a Set of phase numbers found on disk, scanning both the flat
// .planning/phases/ layout and the milestone-archive .planning/milestones/v*-phases/ layout.
const PHASE_TOKEN_FROM_DIR_RE = /^(?:[A-Z]{1,6}-)?(\d+[A-Z]?(?:\.\d+)*)(?:-|$)/i;
const MILESTONE_ARCHIVE_DIR_RE = /^v\d+.*-phases$/i;
function listMilestoneArchiveDirs(planBase) {
const milestonesDir = path.join(planBase, 'milestones');
try {
return fs.readdirSync(milestonesDir, { withFileTypes: true })
.filter((e) => e.isDirectory() && MILESTONE_ARCHIVE_DIR_RE.test(e.name))
.map((e) => path.join(milestonesDir, e.name))
.sort((a, b) => path.basename(a).localeCompare(path.basename(b), undefined, { numeric: true }));
} catch {
return [];
}
}
function getActiveMilestoneArchiveDir(planBase) {
const archiveDirs = listMilestoneArchiveDirs(planBase);
if (archiveDirs.length === 0) return null;
// Prefer STATE.md milestone when it maps to an on-disk archive dir.
try {
const statePath = path.join(planBase, 'STATE.md');
if (fs.existsSync(statePath)) {
const state = fs.readFileSync(statePath, 'utf-8');
const m = state.match(/^\s*milestone:\s*([^\r\n#]+)\s*$/mi);
if (m && m[1]) {
const milestone = m[1].trim();
const candidate = path.join(planBase, 'milestones', `${milestone}-phases`);
if (archiveDirs.includes(candidate)) return candidate;
}
}
} catch { /* intentionally empty */ }
// Fallback when STATE.md is absent/stale: highest (most recent) archive by version-ish name.
return archiveDirs[archiveDirs.length - 1];
}
function collectPhaseRoots(planBase) {
const roots = [];
const flatPhasesDir = path.join(planBase, 'phases');
if (fs.existsSync(flatPhasesDir)) roots.push(flatPhasesDir);
const activeArchive = getActiveMilestoneArchiveDir(planBase);
if (activeArchive) roots.push(activeArchive);
return roots;
}
// Returns a Set of phase numbers found on disk across active phase roots.
function collectDiskPhases(planBase) {
const diskPhases = new Set();
const phaseRoots = collectPhaseRoots(planBase);
const scanDir = (dir) => {
try {
const entries = fs.readdirSync(dir, { withFileTypes: true });
for (const e of entries) {
if (e.isDirectory()) {
const m = e.name.match(/^(\d+[A-Z]?(?:\.\d+)*)/i);
const m = e.name.match(PHASE_TOKEN_FROM_DIR_RE);
if (m) diskPhases.add(m[1]);
}
}
} catch { /* dir absent */ }
};
scanDir(path.join(planBase, 'phases'));
try {
const milestonesDir = path.join(planBase, 'milestones');
const entries = fs.readdirSync(milestonesDir, { withFileTypes: true });
for (const e of entries) {
if (e.isDirectory() && /^v\d+.*-phases$/.test(e.name)) {
scanDir(path.join(milestonesDir, e.name));
}
}
} catch { /* no milestones dir */ }
for (const root of phaseRoots) scanDir(root);
return diskPhases;
}
function cmdValidateConsistency(cwd, raw) {
const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md');
const phasesDir = path.join(planningDir(cwd), 'phases');
const planBase = planningDir(cwd);
const roadmapPath = path.join(planBase, 'ROADMAP.md');
const errors = [];
const warnings = [];
@@ -452,7 +488,7 @@ function cmdValidateConsistency(cwd, raw) {
}
// Get phases on disk (flat layout + milestone-archive layout)
const diskPhases = collectDiskPhases(planningDir(cwd));
const diskPhases = collectDiskPhases(planBase);
// Check: phases in ROADMAP but not on disk
for (const p of roadmapPhases) {
@@ -484,60 +520,53 @@ function cmdValidateConsistency(cwd, raw) {
}
}
// Check: plan numbering within phases
try {
const entries = fs.readdirSync(phasesDir, { withFileTypes: true });
const dirs = entries.filter(e => e.isDirectory()).map(e => e.name).sort();
const phaseRoots = collectPhaseRoots(planBase);
for (const phaseRoot of phaseRoots) {
try {
const entries = fs.readdirSync(phaseRoot, { withFileTypes: true });
const dirs = entries.filter(e => e.isDirectory()).map(e => e.name).sort();
for (const dir of dirs) {
const phaseFiles = fs.readdirSync(path.join(phasesDir, dir));
const plans = phaseFiles.filter(f => f.endsWith('-PLAN.md')).sort();
for (const dir of dirs) {
const phasePath = path.join(phaseRoot, dir);
const phaseLabel = path.relative(planBase, phasePath).replace(/\\/g, '/');
const phaseFiles = fs.readdirSync(phasePath);
const plans = phaseFiles.filter(f => f.endsWith('-PLAN.md')).sort();
// Extract plan numbers
const planNums = plans.map(p => {
const pm = p.match(/-(\d{2})-PLAN\.md$/);
return pm ? parseInt(pm[1], 10) : null;
}).filter(n => n !== null);
// Extract plan numbers
const planNums = plans.map(p => {
const pm = p.match(/-(\d{2})-PLAN\.md$/);
return pm ? parseInt(pm[1], 10) : null;
}).filter(n => n !== null);
for (let i = 1; i < planNums.length; i++) {
if (planNums[i] !== planNums[i - 1] + 1) {
warnings.push(`Gap in plan numbering in ${dir}: plan ${planNums[i - 1]} → ${planNums[i]}`);
for (let i = 1; i < planNums.length; i++) {
if (planNums[i] !== planNums[i - 1] + 1) {
warnings.push(`Gap in plan numbering in ${phaseLabel}: plan ${planNums[i - 1]} → ${planNums[i]}`);
}
}
// Check: plans without summaries (completed plans)
const summaries = phaseFiles.filter(f => f.endsWith('-SUMMARY.md'));
const planIds = new Set(plans.map(p => p.replace('-PLAN.md', '')));
const summaryIds = new Set(summaries.map(s => s.replace('-SUMMARY.md', '')));
// Summary without matching plan is suspicious
for (const sid of summaryIds) {
if (!planIds.has(sid)) {
warnings.push(`Summary ${sid}-SUMMARY.md in ${phaseLabel} has no matching PLAN.md`);
}
}
// Check: frontmatter in plans has required fields
for (const plan of plans) {
const content = fs.readFileSync(path.join(phasePath, plan), 'utf-8');
const fm = extractFrontmatter(content);
if (!fm.wave) {
warnings.push(`${phaseLabel}/${plan}: missing 'wave' in frontmatter`);
}
}
}
// Check: plans without summaries (completed plans)
const summaries = phaseFiles.filter(f => f.endsWith('-SUMMARY.md'));
const planIds = new Set(plans.map(p => p.replace('-PLAN.md', '')));
const summaryIds = new Set(summaries.map(s => s.replace('-SUMMARY.md', '')));
// Summary without matching plan is suspicious
for (const sid of summaryIds) {
if (!planIds.has(sid)) {
warnings.push(`Summary ${sid}-SUMMARY.md in ${dir} has no matching PLAN.md`);
}
}
}
} catch { /* intentionally empty */ }
// Check: frontmatter in plans has required fields
try {
const entries = fs.readdirSync(phasesDir, { withFileTypes: true });
const dirs = entries.filter(e => e.isDirectory()).map(e => e.name);
for (const dir of dirs) {
const phaseFiles = fs.readdirSync(path.join(phasesDir, dir));
const plans = phaseFiles.filter(f => f.endsWith('-PLAN.md'));
for (const plan of plans) {
const content = fs.readFileSync(path.join(phasesDir, dir, plan), 'utf-8');
const fm = extractFrontmatter(content);
if (!fm.wave) {
warnings.push(`${dir}/${plan}: missing 'wave' in frontmatter`);
}
}
}
} catch { /* intentionally empty */ }
} catch { /* intentionally empty */ }
}
const passed = errors.length === 0;
output({ passed, errors, warnings, warning_count: warnings.length }, raw, passed ? 'passed' : 'failed');

View File

@@ -103,6 +103,70 @@ describe('#3164 — validate consistency: milestone-archive layout', () => {
`Got spurious W006 warnings in milestone-archive layout:\n ${w006.join('\n ')}`
);
});
test('prefixed archive dir names (CK-64-...) are recognized as phase 64', () => {
setupMilestoneArchiveProject(tmpDir, {
milestone: 'v1.7',
phases: ['CK-64-secondary-grader-fix'],
roadmapPhases: ['64'],
});
const result = runGsdTools('validate consistency', tmpDir);
assert.ok(result.success, `validate consistency should succeed: ${result.error}`);
const out = JSON.parse(result.output);
const w006 = (out.warnings || []).filter(w => w.includes('Phase 64') && w.includes('no directory'));
assert.deepStrictEqual(
w006, [],
`Prefixed phase dir should count as phase 64, got W006:\n ${w006.join('\n ')}`
);
});
test('consistency scans only active milestone archive and still validates plans/frontmatter', () => {
// Remove default flat phases dir; this project is archive-only.
fs.rmSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true, force: true });
// Old archived milestone should NOT be treated as active on-disk phase roots.
const oldDir = path.join(tmpDir, '.planning', 'milestones', 'v1.6-phases', '64-legacy');
fs.mkdirSync(oldDir, { recursive: true });
fs.writeFileSync(path.join(oldDir, '64-01-PLAN.md'), '# legacy plan\n');
// Active milestone includes intentionally malformed plan numbering/frontmatter.
const activeDir = path.join(tmpDir, '.planning', 'milestones', 'v1.7-phases', '65-current');
fs.mkdirSync(activeDir, { recursive: true });
fs.writeFileSync(path.join(activeDir, '65-01-PLAN.md'), '# plan 1\n');
fs.writeFileSync(path.join(activeDir, '65-03-PLAN.md'), '# plan 3\n');
fs.writeFileSync(
path.join(tmpDir, '.planning', 'STATE.md'),
'milestone: v1.7\n# Session State\n\nPhase: 65\n'
);
fs.writeFileSync(
path.join(tmpDir, '.planning', 'ROADMAP.md'),
'# Roadmap\n\n## Roadmap v1.7: Current\n\n### Phase 65: Current work\n\nGoal: test.\n'
);
const result = runGsdTools('validate consistency', tmpDir);
assert.ok(result.success, `validate consistency should succeed: ${result.error}`);
const out = JSON.parse(result.output);
const warnings = out.warnings || [];
const phase64Warnings = warnings.filter(w => w.includes('Phase 64 exists on disk but not in ROADMAP.md'));
assert.deepStrictEqual(
phase64Warnings,
[],
`Old archived milestone phase 64 should not be treated as active:\n ${phase64Warnings.join('\n ')}`
);
assert.ok(
warnings.some(w => w.includes('Gap in plan numbering in milestones/v1.7-phases/65-current')),
`Expected plan numbering warning from active archive root, got:\n ${warnings.join('\n ')}`
);
assert.ok(
warnings.some(w => w.includes("milestones/v1.7-phases/65-current/65-01-PLAN.md: missing 'wave'"))
|| warnings.some(w => w.includes("milestones/v1.7-phases/65-current/65-03-PLAN.md: missing 'wave'")),
`Expected frontmatter warning from active archive plans, got:\n ${warnings.join('\n ')}`
);
});
});
describe('#3164 — validate health: milestone-archive layout', () => {
@@ -152,4 +216,28 @@ describe('#3164 — find-phase: milestone-archive layout', () => {
const out = JSON.parse(result.output);
assert.strictEqual(out.found, true, `find-phase 64 should return found:true, got: ${JSON.stringify(out)}`);
});
test('find-phase searches milestone archives in deterministic sorted order', () => {
// Remove flat phases dir so search relies on milestone archives only.
fs.rmSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true, force: true });
const milestonesDir = path.join(tmpDir, '.planning', 'milestones');
const v110 = path.join(milestonesDir, 'v1.10-phases', '64-from-110');
const v12 = path.join(milestonesDir, 'v1.2-phases', '64-from-12');
fs.mkdirSync(v110, { recursive: true });
fs.mkdirSync(v12, { recursive: true });
fs.writeFileSync(path.join(v110, 'PLAN.md'), '# v1.10 plan\n');
fs.writeFileSync(path.join(v12, 'PLAN.md'), '# v1.2 plan\n');
const result = runGsdTools('find-phase 64', tmpDir);
assert.ok(result.success, `find-phase should succeed: ${result.error}`);
const out = JSON.parse(result.output);
assert.strictEqual(out.found, true, `find-phase 64 should return found:true, got: ${JSON.stringify(out)}`);
assert.strictEqual(
out.directory,
'.planning/milestones/v1.2-phases/64-from-12',
`Expected deterministic archive ordering (v1.2 before v1.10), got directory: ${out.directory}`
);
});
});