From 10101f07becc109c2bb5bd3a3c944d1c82621351 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 13 May 2026 19:25:14 -0400 Subject: [PATCH] fix(sdk): reduce validate.health false positives (#3479) * fix(sdk): reduce validate.health false positives (#3473) * chore(changeset): add #3479 fragment --- .changeset/graceful-geese-tumble.md | 5 +++ sdk/src/query/validate.test.ts | 65 +++++++++++++++++++++++++++++ sdk/src/query/validate.ts | 35 ++++++++++++++-- 3 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 .changeset/graceful-geese-tumble.md diff --git a/.changeset/graceful-geese-tumble.md b/.changeset/graceful-geese-tumble.md new file mode 100644 index 000000000..96c45f7ea --- /dev/null +++ b/.changeset/graceful-geese-tumble.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3479 +--- +**`gsd-sdk query validate.health` now avoids three false-positive classes** — it accepts 999.X backlog phase dirs, recognizes milestone-archive phase directories for roadmap presence checks, and canonicalizes descriptor PLAN/SUMMARY filename pairing. diff --git a/sdk/src/query/validate.test.ts b/sdk/src/query/validate.test.ts index a233328f8..4f370f541 100644 --- a/sdk/src/query/validate.test.ts +++ b/sdk/src/query/validate.test.ts @@ -603,6 +603,71 @@ describe('validateHealth', () => { expect(warnings.some(w => w.code === 'W005')).toBe(true); }); + it('does not emit W005 for 999.X backlog phase directory naming (#3473)', async () => { + await createHealthyPlanning(); + await mkdir(join(tmpDir, '.planning', 'phases', '999.1-backlog-sweep'), { recursive: true }); + + const result = await validateHealth([], tmpDir); + const data = result.data as Record; + const warnings = data.warnings as Array>; + const w005ForBacklog = warnings.find( + w => w.code === 'W005' && String(w.message).includes('999.1-backlog-sweep'), + ); + expect(w005ForBacklog).toBeUndefined(); + }); + + it('does not emit W006 when roadmap phase exists in milestones archive dir (#3473)', async () => { + await createHealthyPlanning(); + await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), [ + '# Roadmap', + '', + '## v1.0: Shipped ✅ SHIPPED', + '', + '### Phase 7: Old shipped phase', + '', + '## v1.1: Current', + '', + '### Phase 1: Foundation', + '', + ].join('\n')); + await mkdir(join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '07-old-shipped-phase'), { recursive: true }); + await writeFile( + join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '07-old-shipped-phase', '07-01-PLAN.md'), + '# Plan\n', + ); + + const result = await validateHealth([], tmpDir); + const data = result.data as Record; + const warnings = data.warnings as Array>; + const w006s = warnings.filter(w => w.code === 'W006'); + expect(w006s.some(w => String(w.message).includes('Phase 7'))).toBe(false); + }); + + it('does not emit I001 when plan file has descriptor but summary uses canonical stem (#3473)', async () => { + await createHealthyPlanning(); + await mkdir(join(tmpDir, '.planning', 'phases', '68-bug-surface'), { recursive: true }); + await writeFile(join(tmpDir, '.planning', 'phases', '68-bug-surface', '68-01-scaffolding-PLAN.md'), [ + '---', + 'phase: 68', + 'plan: 01', + 'wave: 1', + 'depends_on: []', + 'files_modified: []', + 'autonomous: true', + '---', + '# Plan', + ].join('\n')); + await writeFile(join(tmpDir, '.planning', 'phases', '68-bug-surface', '68-01-SUMMARY.md'), '# Summary\n'); + + const result = await validateHealth([], tmpDir); + const data = result.data as Record; + const info = data.info as Array>; + const i001ForPhase = info.find( + i => i.code === 'I001' && String(i.message).includes('68-bug-surface/68-01-scaffolding-PLAN.md'), + ); + expect(i001ForPhase).toBeUndefined(); + }); + it('returns early with E010 when CWD equals home directory', async () => { const result = await validateHealth([], homedir()); const data = result.data as Record; diff --git a/sdk/src/query/validate.ts b/sdk/src/query/validate.ts index c1c87d303..e68e072a8 100644 --- a/sdk/src/query/validate.ts +++ b/sdk/src/query/validate.ts @@ -29,6 +29,15 @@ import { resolveBundledAgentsDir } from '../sdk-package-compatibility.js'; /** Max length for key_links regex patterns (ReDoS mitigation). */ const MAX_KEY_LINK_PATTERN_LEN = 512; +/** + * Canonical plan stem used for PLAN/SUMMARY matching. + * Example: `68-01-scaffolding` -> `68-01`. + */ +function canonicalPlanStem(stem: string): string { + const m = stem.match(/^(\d+[A-Z]?(?:\.\d+)*-\d+)/i); + return m ? m[1] : stem; +} + /** * Build a RegExp for must_haves key_links pattern matching. * Long or nested-quantifier patterns fall back to a literal match via escapeRegex. @@ -526,7 +535,7 @@ export const validateHealth: QueryHandler = async (args, projectDir, workstream) try { const entries = await readdir(phasesDir, { withFileTypes: true }); for (const e of entries) { - if (e.isDirectory() && !e.name.match(/^\d{2}(?:\.\d+)*-[\w-]+$/)) { + if (e.isDirectory() && !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)'); } } @@ -540,11 +549,17 @@ export const validateHealth: QueryHandler = async (args, projectDir, workstream) const phaseFiles = await readdir(join(phasesDir, 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 summary of summaries) { + const summaryBase = summary.replace('-SUMMARY.md', '').replace('SUMMARY.md', ''); + summaryBases.add(summaryBase); + summaryBases.add(canonicalPlanStem(summaryBase)); + } for (const plan of plans) { const planBase2 = plan.replace('-PLAN.md', '').replace('PLAN.md', ''); - if (!summaryBases.has(planBase2)) { + const canonicalBase = canonicalPlanStem(planBase2); + if (!summaryBases.has(planBase2) && !summaryBases.has(canonicalBase)) { addIssue('info', 'I001', `${e.name}/${plan} has no SUMMARY.md`, 'May be in progress'); } } @@ -594,6 +609,20 @@ export const validateHealth: QueryHandler = async (args, projectDir, workstream) } } } catch { /* intentionally empty */ } + // Include archived milestone phase directories as valid on-disk locations + // for historical ROADMAP phases. + try { + const milestoneEntries = await readdir(join(planBase, 'milestones'), { withFileTypes: true }); + for (const milestoneEntry of milestoneEntries) { + if (!milestoneEntry.isDirectory() || !/-phases$/i.test(milestoneEntry.name)) continue; + const archivedPhaseEntries = await readdir(join(planBase, 'milestones', milestoneEntry.name), { withFileTypes: true }); + for (const archivedPhase of archivedPhaseEntries) { + if (!archivedPhase.isDirectory()) continue; + const dm = archivedPhase.name.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); + if (dm) diskPhases.add(dm[1]); + } + } + } catch { /* intentionally empty */ } for (const p of roadmapPhases) { const padded = String(parseInt(p, 10)).padStart(2, '0');