From b647f44eb05e544c4edcc0795def26fb4dba3587 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 22 May 2026 11:23:50 -0400 Subject: [PATCH] fix(3806): port W005/W006/I001 fixes from validate.ts to verify.cjs (#83) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../3806-cjs-bundle-drift-w005-w006-i001.md | 5 + get-shit-done/bin/lib/verify.cjs | 39 +++-- tests/verify-health.test.cjs | 147 ++++++++++++++++++ 3 files changed, 182 insertions(+), 9 deletions(-) create mode 100644 .changeset/3806-cjs-bundle-drift-w005-w006-i001.md diff --git a/.changeset/3806-cjs-bundle-drift-w005-w006-i001.md b/.changeset/3806-cjs-bundle-drift-w005-w006-i001.md new file mode 100644 index 000000000..77969618e --- /dev/null +++ b/.changeset/3806-cjs-bundle-drift-w005-w006-i001.md @@ -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) diff --git a/get-shit-done/bin/lib/verify.cjs b/get-shit-done/bin/lib/verify.cjs index 2f907c9e9..9c8ae78f2 100644 --- a/get-shit-done/bin/lib/verify.cjs +++ b/get-shit-done/bin/lib/verify.cjs @@ -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
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 diff --git a/tests/verify-health.test.cjs b/tests/verify-health.test.cjs index 8f89b9fc3..841b58192 100644 --- a/tests/verify-health.test.cjs +++ b/tests/verify-health.test.cjs @@ -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) // ─────────────────────────────────────────────────────────────────────────────