From 08d1767a1b82eedd2ff4dcede8ed17fe3b6ec485 Mon Sep 17 00:00:00 2001 From: Tibsfox Date: Fri, 10 Apr 2026 11:25:19 -0700 Subject: [PATCH 1/2] perf(health): merge four readdirSync passes into one in cmdValidateHealth (#1973) cmdValidateHealth read the phases directory four separate times for checks 6 (naming), 7 (orphaned plans), 7b (validation artifacts), and 8 (roadmap cross-reference). Hoist the directory listing into a single readdirSync call with a shared Map of per-phase file lists. Reduces syscalls from ~3N+1 to N+1 where N is the number of phase directories. Closes #1973 Co-Authored-By: Claude Opus 4.6 --- get-shit-done/bin/lib/verify.cjs | 78 ++++++++++++++++---------------- 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/get-shit-done/bin/lib/verify.cjs b/get-shit-done/bin/lib/verify.cjs index d7721bffa..30b4c6ff3 100644 --- a/get-shit-done/bin/lib/verify.cjs +++ b/get-shit-done/bin/lib/verify.cjs @@ -655,52 +655,55 @@ function cmdValidateHealth(cwd, options, raw) { } catch { /* intentionally empty */ } } - // ─── Check 6: Phase directory naming (NN-name format) ───────────────────── + // ─── Read phase directories once for checks 6, 7, 7b, and 8 (#1973) ────── + let phaseDirEntries = []; + const phaseDirFiles = new Map(); // phase dir name → file list try { - const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - for (const e of entries) { - 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)'); - } + phaseDirEntries = fs.readdirSync(phasesDir, { withFileTypes: true }).filter(e => e.isDirectory()); + for (const e of phaseDirEntries) { + try { + phaseDirFiles.set(e.name, fs.readdirSync(path.join(phasesDir, e.name))); + } catch { phaseDirFiles.set(e.name, []); } } } catch { /* intentionally empty */ } + // ─── Check 6: Phase directory naming (NN-name format) ───────────────────── + for (const e of phaseDirEntries) { + 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)'); + } + } + // ─── Check 7: Orphaned plans (PLAN without SUMMARY) ─────────────────────── - try { - const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - for (const e of entries) { - if (!e.isDirectory()) continue; - const phaseFiles = fs.readdirSync(path.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', ''))); + for (const e of phaseDirEntries) { + 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', ''))); - for (const plan of plans) { - const planBase = plan.replace('-PLAN.md', '').replace('PLAN.md', ''); - if (!summaryBases.has(planBase)) { - addIssue('info', 'I001', `${e.name}/${plan} has no SUMMARY.md`, 'May be in progress'); - } + for (const plan of plans) { + const planBase = plan.replace('-PLAN.md', '').replace('PLAN.md', ''); + if (!summaryBases.has(planBase)) { + addIssue('info', 'I001', `${e.name}/${plan} has no SUMMARY.md`, 'May be in progress'); } } - } catch { /* intentionally empty */ } + } // ─── Check 7b: Nyquist VALIDATION.md consistency ──────────────────────── - try { - const phaseEntries = fs.readdirSync(phasesDir, { withFileTypes: true }); - for (const e of phaseEntries) { - if (!e.isDirectory()) continue; - const phaseFiles = fs.readdirSync(path.join(phasesDir, e.name)); - const hasResearch = phaseFiles.some(f => f.endsWith('-RESEARCH.md')); - const hasValidation = phaseFiles.some(f => f.endsWith('-VALIDATION.md')); - if (hasResearch && !hasValidation) { - const researchFile = phaseFiles.find(f => f.endsWith('-RESEARCH.md')); + for (const e of phaseDirEntries) { + const phaseFiles = phaseDirFiles.get(e.name) || []; + const hasResearch = phaseFiles.some(f => f.endsWith('-RESEARCH.md')); + const hasValidation = phaseFiles.some(f => f.endsWith('-VALIDATION.md')); + if (hasResearch && !hasValidation) { + const researchFile = phaseFiles.find(f => f.endsWith('-RESEARCH.md')); + try { const researchContent = fs.readFileSync(path.join(phasesDir, e.name, researchFile), 'utf-8'); if (researchContent.includes('## Validation Architecture')) { addIssue('warning', 'W009', `Phase ${e.name}: has Validation Architecture in RESEARCH.md but no VALIDATION.md`, 'Re-run /gsd-plan-phase with --research to regenerate'); } - } + } catch { /* intentionally empty */ } } - } catch { /* intentionally empty */ } + } // ─── Check 7c: Agent installation (#1371) ────────────────────────────────── // Verify GSD agents are installed. Missing agents cause Task(subagent_type=...) @@ -733,15 +736,10 @@ function cmdValidateHealth(cwd, options, raw) { } const diskPhases = new Set(); - try { - const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - for (const e of entries) { - if (e.isDirectory()) { - const dm = e.name.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); - if (dm) diskPhases.add(dm[1]); - } - } - } catch { /* intentionally empty */ } + for (const e of phaseDirEntries) { + const dm = e.name.match(/^(\d+[A-Z]?(?:\.\d+)*)/i); + if (dm) diskPhases.add(dm[1]); + } // Build a set of phases explicitly marked not-yet-started in the ROADMAP // summary list (- [ ] **Phase N:**). These phases are intentionally absent From 053269823bc95d8c9dda3f74fdbfd1974757ee78 Mon Sep 17 00:00:00 2001 From: Tibsfox Date: Sat, 11 Apr 2026 03:24:49 -0700 Subject: [PATCH 2/2] test(health): add degradation test for missing phasesDir (#1973) Covers the behavior change from independent per-check degradation to coupled degradation when the hoisted readdirSync throws. Asserts that cmdValidateHealth completes without throwing and emits zero phase directory warnings (W005, W006, W007, W009, I001) when phasesDir doesn't exist. Review feedback on #2053 from @trek-e. --- tests/verify-health.test.cjs | 44 ++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/verify-health.test.cjs b/tests/verify-health.test.cjs index 50aa0dc75..ddeea35f0 100644 --- a/tests/verify-health.test.cjs +++ b/tests/verify-health.test.cjs @@ -767,3 +767,47 @@ describe('validate health --repair command', () => { assert.strictEqual(output.repairable_count, 0, `Expected no repairable issues for W002: ${JSON.stringify(output)}`); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// Graceful degradation when phasesDir is missing (#1973) +// ───────────────────────────────────────────────────────────────────────────── + +describe('validate health — missing phasesDir', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('completes without throwing and emits zero phase-directory warnings when phasesDir does not exist', () => { + // Setup: valid PROJECT, ROADMAP, STATE, config but NO phases directory + writeMinimalProjectMd(tmpDir); + writeMinimalRoadmap(tmpDir, ['1', '2']); + writeMinimalStateMd(tmpDir); + writeValidConfigJson(tmpDir); + + // Remove the phases directory if it exists + const phasesDir = path.join(tmpDir, '.planning', 'phases'); + if (fs.existsSync(phasesDir)) { + fs.rmSync(phasesDir, { recursive: true, force: true }); + } + + // Should complete without throwing + const result = runGsdTools('validate health', tmpDir); + assert.ok(result.success, `Command should succeed when phasesDir is missing: ${result.error}`); + + const output = JSON.parse(result.output); + + // Assert no phase-directory warnings fired + const phaseDirCodes = ['W005', 'W006', 'W007', 'W009', 'I001']; + const issues = output.issues || []; + for (const code of phaseDirCodes) { + const matches = issues.filter(i => i.code === code); + assert.strictEqual(matches.length, 0, `Expected no ${code} issues when phasesDir is missing, got: ${JSON.stringify(matches)}`); + } + }); +});