fix(3806): port W005/W006/I001 fixes from validate.ts to verify.cjs (#83)
PR #3479 fixed three false-positive classes in sdk/src/query/validate.ts but the fixes never propagated to get-shit-done/bin/lib/verify.cjs — the hand-maintained CJS runtime bundle that gsd-tools.cjs actually executes. W005: widened phase-dir regex from \d{2} to \d{2,} so 3+-digit prefixes like 999.1-foo are accepted. W006: adds forEachArchivedPhaseToken call after collectDiskPhases so phases whose directories live in a milestone archive are not flagged as missing. I001: adds canonicalPlanStem helper and uses it in summaryBases Set construction so 68-01-scaffolding-PLAN.md correctly matches 68-01-SUMMARY.md. Adds five regression tests (TDD red→green) covering each false-positive path. Fixes #3806 Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
5
.changeset/3806-cjs-bundle-drift-w005-w006-i001.md
Normal file
5
.changeset/3806-cjs-bundle-drift-w005-w006-i001.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3806
|
||||
---
|
||||
**`validate health` no longer emits W005/W006/I001 false positives for multi-digit phase prefixes, milestone-archive phases, and descriptor PLAN/SUMMARY stem pairs** — PR #3479 fixed these three false-positive classes in `sdk/src/query/validate.ts` but the fix never propagated to `get-shit-done/bin/lib/verify.cjs`, which is the runtime bundle that `gsd-tools.cjs validate health` actually executes. This PR ports the three fixes: (1) W005 regex widened from `\d{2}` to `\d{2,}` so `999.1-foo` and other multi-digit backlog directories are accepted; (2) W006 now calls `forEachArchivedPhaseToken` after `collectDiskPhases` so phases whose directories live in a milestone archive are not flagged as missing; (3) I001 now builds the `summaryBases` Set with both raw and canonical plan stems so `68-01-scaffolding-PLAN.md` correctly matches `68-01-SUMMARY.md`. (#3806)
|
||||
@@ -594,6 +594,16 @@ function cmdValidateConsistency(cwd, raw) {
|
||||
output({ passed, errors, warnings, warning_count: warnings.length }, raw, passed ? 'passed' : 'failed');
|
||||
}
|
||||
|
||||
/**
|
||||
* Canonical plan stem used for PLAN/SUMMARY matching.
|
||||
* Mirrors canonicalPlanStem in sdk/src/query/validate.ts (#3479 / #3806).
|
||||
* Example: `68-01-scaffolding` -> `68-01`.
|
||||
*/
|
||||
function canonicalPlanStem(stem) {
|
||||
const m = stem.match(/^(\d+[A-Z]?(?:\.\d+)*-\d+)/i);
|
||||
return m ? m[1] : stem;
|
||||
}
|
||||
|
||||
function cmdValidateHealth(cwd, options, raw) {
|
||||
// Guard: detect if CWD is the home directory (likely accidental)
|
||||
const resolved = path.resolve(cwd);
|
||||
@@ -776,7 +786,7 @@ function cmdValidateHealth(cwd, options, raw) {
|
||||
|
||||
// ─── Check 6: Phase directory naming (NN-name format) ─────────────────────
|
||||
for (const e of phaseDirEntries) {
|
||||
if (!e.name.match(/^\d{2}(?:\.\d+)*-[\w-]+$/)) {
|
||||
if (!e.name.match(/^\d{2,}(?:\.\d+)*-[\w-]+$/)) {
|
||||
addIssue('warning', 'W005', `Phase directory "${e.name}" doesn't follow NN-name format`, 'Rename to match pattern (e.g., 01-setup)');
|
||||
}
|
||||
}
|
||||
@@ -786,11 +796,17 @@ function cmdValidateHealth(cwd, options, raw) {
|
||||
const phaseFiles = phaseDirFiles.get(e.name) || [];
|
||||
const plans = phaseFiles.filter(f => f.endsWith('-PLAN.md') || f === 'PLAN.md');
|
||||
const summaries = phaseFiles.filter(f => f.endsWith('-SUMMARY.md') || f === 'SUMMARY.md');
|
||||
const summaryBases = new Set(summaries.map(s => s.replace('-SUMMARY.md', '').replace('SUMMARY.md', '')));
|
||||
const summaryBases = new Set();
|
||||
for (const s of summaries) {
|
||||
const summaryBase = s.replace('-SUMMARY.md', '').replace('SUMMARY.md', '');
|
||||
summaryBases.add(summaryBase);
|
||||
summaryBases.add(canonicalPlanStem(summaryBase));
|
||||
}
|
||||
|
||||
for (const plan of plans) {
|
||||
const planBase = plan.replace('-PLAN.md', '').replace('PLAN.md', '');
|
||||
if (!summaryBases.has(planBase)) {
|
||||
const canonicalBase = canonicalPlanStem(planBase);
|
||||
if (!summaryBases.has(planBase) && !summaryBases.has(canonicalBase)) {
|
||||
addIssue('info', 'I001', `${e.name}/${plan} has no SUMMARY.md`, 'May be in progress');
|
||||
}
|
||||
}
|
||||
@@ -831,12 +847,13 @@ function cmdValidateHealth(cwd, options, raw) {
|
||||
} catch { /* intentionally empty — agent check is non-blocking */ }
|
||||
|
||||
// ─── Check 8: Run existing consistency checks ─────────────────────────────
|
||||
// Inline subset of cmdValidateConsistency. Note: unlike Check 4 (W002),
|
||||
// this check intentionally filters ROADMAP.md through extractCurrentMilestone
|
||||
// first — shipped milestones (whether collapsed in <details> or not) are
|
||||
// stripped before the heading scan, so archived phase numbers never reach
|
||||
// `roadmapPhases` and W006/W007 cannot fire for them. That is why the
|
||||
// #3652 archive-union added to Check 4 is NOT mirrored here.
|
||||
// Inline subset of cmdValidateConsistency. Unlike Check 4 (W002), this
|
||||
// check filters ROADMAP.md through extractCurrentMilestone first — shipped
|
||||
// milestones are stripped before the heading scan. However, a phase can
|
||||
// appear in the CURRENT milestone AND have its directory inside a milestone
|
||||
// archive (completed + archived). forEachArchivedPhaseToken is therefore
|
||||
// called below to add archived dirs to diskPhases so W006 does not fire
|
||||
// for them. (#3652, #3806)
|
||||
if (fs.existsSync(roadmapPath)) {
|
||||
const roadmapContentRaw = fs.readFileSync(roadmapPath, 'utf-8');
|
||||
const roadmapContent = extractCurrentMilestone(roadmapContentRaw, cwd);
|
||||
@@ -848,6 +865,10 @@ function cmdValidateHealth(cwd, options, raw) {
|
||||
}
|
||||
|
||||
const diskPhases = collectDiskPhases(planBase);
|
||||
// Include archived milestone phase directories as valid on-disk locations.
|
||||
// Mirrors forEachArchivedPhaseToken call in sdk/src/query/validate.ts
|
||||
// Check 8. (#3806)
|
||||
forEachArchivedPhaseToken(planBase, (token) => diskPhases.add(token));
|
||||
|
||||
// Build a set of phases explicitly marked not-yet-started in the ROADMAP
|
||||
// summary list (- [ ] **Phase N:**). These phases are intentionally absent
|
||||
|
||||
@@ -830,6 +830,153 @@ describe('validate health --repair command', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Regression: CJS bundle drift — W005/W006/I001 false positives (#3806)
|
||||
// PR #3479 fixed these in sdk/src/query/validate.ts but never propagated to
|
||||
// get-shit-done/bin/lib/verify.cjs. These tests fail on old verify.cjs and
|
||||
// pass on the fixed version.
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('validate health — #3806 CJS bundle drift regressions', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempProject();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
// W005 regression: \d{2} → \d{2,} so 999.1-foo is accepted (#3806)
|
||||
test('does not emit W005 for a phase directory with a 3-digit prefix (999.1-foo)', () => {
|
||||
writeMinimalProjectMd(tmpDir);
|
||||
// Roadmap with no phases to avoid spurious W006
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'ROADMAP.md'),
|
||||
'# Roadmap\n\nNo phases yet.\n'
|
||||
);
|
||||
writeMinimalStateMd(tmpDir, '# Session State\n\nNo phase refs.\n');
|
||||
writeValidConfigJson(tmpDir);
|
||||
// 999.1-foo should be valid under the widened \d{2,} pattern
|
||||
fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '999.1-foo'), { recursive: true });
|
||||
|
||||
const result = runGsdTools('validate health', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
const w005s = output.warnings.filter(w => w.code === 'W005');
|
||||
assert.strictEqual(
|
||||
w005s.length, 0,
|
||||
`W005 must not fire for "999.1-foo" (3-digit prefix is valid under \\d{2,}), got: ${JSON.stringify(w005s)}`
|
||||
);
|
||||
});
|
||||
|
||||
// W005 regression: additional multi-digit variants
|
||||
test('does not emit W005 for phase directories with 4-digit and 2-digit prefixes', () => {
|
||||
writeMinimalProjectMd(tmpDir);
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'ROADMAP.md'),
|
||||
'# Roadmap\n\nNo phases yet.\n'
|
||||
);
|
||||
writeMinimalStateMd(tmpDir, '# Session State\n\nNo phase refs.\n');
|
||||
writeValidConfigJson(tmpDir);
|
||||
fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '1000-backlog'), { recursive: true });
|
||||
fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '99-done'), { recursive: true });
|
||||
fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '100.2-feature'), { recursive: true });
|
||||
|
||||
const result = runGsdTools('validate health', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
const w005s = output.warnings.filter(w => w.code === 'W005');
|
||||
assert.strictEqual(
|
||||
w005s.length, 0,
|
||||
`W005 must not fire for multi-digit prefix dirs (\\d{2,} pattern), got: ${JSON.stringify(w005s)}`
|
||||
);
|
||||
});
|
||||
|
||||
// W006 regression: archived phases in milestones/*-phases/ must not trigger W006 (#3806)
|
||||
test('does not emit W006 for a ROADMAP phase whose directory lives in a milestone archive', () => {
|
||||
writeMinimalProjectMd(tmpDir);
|
||||
// ROADMAP references Phase 1 in the current section
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'ROADMAP.md'),
|
||||
[
|
||||
'# Roadmap',
|
||||
'',
|
||||
'## v1.0.0',
|
||||
'',
|
||||
'### Phase 1: Setup',
|
||||
'',
|
||||
].join('\n')
|
||||
);
|
||||
writeMinimalStateMd(tmpDir, '# Session State\n\nPhase 1 complete.\n');
|
||||
writeValidConfigJson(tmpDir);
|
||||
// Phase 1 directory is in a milestone archive, NOT in the flat phases/ dir
|
||||
const archiveDir = path.join(tmpDir, '.planning', 'milestones', 'v1.0.0-phases');
|
||||
fs.mkdirSync(path.join(archiveDir, '01-setup'), { recursive: true });
|
||||
// Ensure flat phases dir exists but does NOT contain phase 1
|
||||
fs.mkdirSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true });
|
||||
|
||||
const result = runGsdTools('validate health', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
const w006s = output.warnings.filter(w => w.code === 'W006');
|
||||
assert.strictEqual(
|
||||
w006s.length, 0,
|
||||
`W006 must not fire for Phase 1 when its directory is in a milestone archive, got: ${JSON.stringify(w006s)}`
|
||||
);
|
||||
});
|
||||
|
||||
// I001 regression: FOO-PLAN.md + FOO-SUMMARY.md must match via canonicalPlanStem (#3806)
|
||||
// e.g. 68-01-scaffolding-PLAN.md should match 68-01-SUMMARY.md (canonical stem = "68-01")
|
||||
test('does not emit I001 when PLAN name has a descriptor suffix but SUMMARY uses canonical stem', () => {
|
||||
writeMinimalProjectMd(tmpDir);
|
||||
writeMinimalRoadmap(tmpDir, ['1']);
|
||||
writeMinimalStateMd(tmpDir, '# Session State\n\nPhase 1 in progress.\n');
|
||||
writeValidConfigJson(tmpDir);
|
||||
// Create phase dir with descriptor-named PLAN and canonical-named SUMMARY
|
||||
const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-setup');
|
||||
fs.mkdirSync(phaseDir, { recursive: true });
|
||||
// PLAN has a descriptor suffix; SUMMARY uses the canonical stem only
|
||||
fs.writeFileSync(path.join(phaseDir, '01-01-scaffolding-PLAN.md'), '# Plan\n');
|
||||
fs.writeFileSync(path.join(phaseDir, '01-01-SUMMARY.md'), '# Summary\n');
|
||||
|
||||
const result = runGsdTools('validate health', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
const i001s = output.info.filter(i => i.code === 'I001');
|
||||
assert.strictEqual(
|
||||
i001s.length, 0,
|
||||
`I001 must not fire when SUMMARY stem (01-01) matches the canonical base of PLAN (01-01-scaffolding → 01-01), got: ${JSON.stringify(i001s)}`
|
||||
);
|
||||
});
|
||||
|
||||
// Confirm I001 still fires for a genuinely orphaned plan (no summary at all)
|
||||
test('still emits I001 for a PLAN with no matching SUMMARY at all', () => {
|
||||
writeMinimalProjectMd(tmpDir);
|
||||
writeMinimalRoadmap(tmpDir, ['1']);
|
||||
writeMinimalStateMd(tmpDir, '# Session State\n\nPhase 1 in progress.\n');
|
||||
writeValidConfigJson(tmpDir);
|
||||
const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-setup');
|
||||
fs.mkdirSync(phaseDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n');
|
||||
// No SUMMARY file at all
|
||||
|
||||
const result = runGsdTools('validate health', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.ok(
|
||||
output.info.some(i => i.code === 'I001'),
|
||||
`I001 must still fire for an orphaned PLAN (no SUMMARY exists), got: ${JSON.stringify(output.info)}`
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Graceful degradation when phasesDir is missing (#1973)
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user