diff --git a/get-shit-done/bin/lib/state.cjs b/get-shit-done/bin/lib/state.cjs index 06da9d13c..9d65d23b8 100644 --- a/get-shit-done/bin/lib/state.cjs +++ b/get-shit-done/bin/lib/state.cjs @@ -7,6 +7,11 @@ const path = require('path'); const { escapeRegex, loadConfig, getMilestoneInfo, getMilestonePhaseFilter, normalizeMd, planningDir, planningPaths, output, error, atomicWriteFileSync } = require('./core.cjs'); const { extractFrontmatter, reconstructFrontmatter } = require('./frontmatter.cjs'); +// Cache disk scan results from buildStateFrontmatter per cwd per process (#1967). +// Avoids re-reading N+1 directories on every state write when the phase structure +// hasn't changed within the same gsd-tools invocation. +const _diskScanCache = new Map(); + /** Shorthand — every state command needs this path */ function getStatePath(cwd) { return planningPaths(cwd).state; @@ -737,28 +742,40 @@ function buildStateFrontmatter(bodyContent, cwd) { try { const phasesDir = planningPaths(cwd).phases; if (fs.existsSync(phasesDir)) { - const isDirInMilestone = getMilestonePhaseFilter(cwd); - const phaseDirs = fs.readdirSync(phasesDir, { withFileTypes: true }) - .filter(e => e.isDirectory()).map(e => e.name) - .filter(isDirInMilestone); - let diskTotalPlans = 0; - let diskTotalSummaries = 0; - let diskCompletedPhases = 0; + // Use cached disk scan when available — avoids N+1 readdirSync calls + // on repeated buildStateFrontmatter invocations within the same process (#1967) + let cached = _diskScanCache.get(cwd); + if (!cached) { + const isDirInMilestone = getMilestonePhaseFilter(cwd); + const phaseDirs = fs.readdirSync(phasesDir, { withFileTypes: true }) + .filter(e => e.isDirectory()).map(e => e.name) + .filter(isDirInMilestone); + let diskTotalPlans = 0; + let diskTotalSummaries = 0; + let diskCompletedPhases = 0; - for (const dir of phaseDirs) { - const files = fs.readdirSync(path.join(phasesDir, dir)); - const plans = files.filter(f => f.match(/-PLAN\.md$/i)).length; - const summaries = files.filter(f => f.match(/-SUMMARY\.md$/i)).length; - diskTotalPlans += plans; - diskTotalSummaries += summaries; - if (plans > 0 && summaries >= plans) diskCompletedPhases++; + for (const dir of phaseDirs) { + const files = fs.readdirSync(path.join(phasesDir, dir)); + const plans = files.filter(f => f.match(/-PLAN\.md$/i)).length; + const summaries = files.filter(f => f.match(/-SUMMARY\.md$/i)).length; + diskTotalPlans += plans; + diskTotalSummaries += summaries; + if (plans > 0 && summaries >= plans) diskCompletedPhases++; + } + cached = { + totalPhases: isDirInMilestone.phaseCount > 0 + ? Math.max(phaseDirs.length, isDirInMilestone.phaseCount) + : phaseDirs.length, + completedPhases: diskCompletedPhases, + totalPlans: diskTotalPlans, + completedPlans: diskTotalSummaries, + }; + _diskScanCache.set(cwd, cached); } - totalPhases = isDirInMilestone.phaseCount > 0 - ? Math.max(phaseDirs.length, isDirInMilestone.phaseCount) - : phaseDirs.length; - completedPhases = diskCompletedPhases; - totalPlans = diskTotalPlans; - completedPlans = diskTotalSummaries; + totalPhases = cached.totalPhases; + completedPhases = cached.completedPhases; + totalPlans = cached.totalPlans; + completedPlans = cached.completedPlans; } } catch { /* intentionally empty */ } } @@ -904,6 +921,10 @@ function releaseStateLock(lockPath) { * each other's changes (race condition with read-modify-write cycle). */ function writeStateMd(statePath, content, cwd) { + // Invalidate disk scan cache before computing new frontmatter — the write + // may create new PLAN/SUMMARY files that buildStateFrontmatter must see. + // Safe for any calling pattern, not just short-lived CLI processes (#1967). + if (cwd) _diskScanCache.delete(cwd); const synced = syncStateFrontmatter(content, cwd); const lockPath = acquireStateLock(statePath); try { diff --git a/tests/bug-1967-cache-invalidation.test.cjs b/tests/bug-1967-cache-invalidation.test.cjs new file mode 100644 index 000000000..83ce83b19 --- /dev/null +++ b/tests/bug-1967-cache-invalidation.test.cjs @@ -0,0 +1,100 @@ +/** + * Regression tests for #1967 cache invalidation. + * + * The disk scan cache in buildStateFrontmatter must be invalidated on + * writeStateMd to prevent stale reads if multiple state-mutating + * operations occur within the same Node process. This matters for: + * - SDK callers that require() gsd-tools.cjs as a module + * - Future dispatcher extensions that handle compound operations + * - Tests that import state.cjs directly + */ + +'use strict'; + +const { test, describe, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); + +const state = require('../get-shit-done/bin/lib/state.cjs'); + +describe('buildStateFrontmatter cache invalidation (#1967)', () => { + let tmpDir; + let planningDir; + let phasesDir; + let statePath; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-1967-cache-')); + planningDir = path.join(tmpDir, '.planning'); + phasesDir = path.join(planningDir, 'phases'); + fs.mkdirSync(phasesDir, { recursive: true }); + + // Create a minimal config and STATE.md + fs.writeFileSync( + path.join(planningDir, 'config.json'), + JSON.stringify({ project_code: 'TEST' }) + ); + + statePath = path.join(planningDir, 'STATE.md'); + fs.writeFileSync(statePath, [ + '# State', + '', + '**Current Phase:** 1', + '**Status:** executing', + '**Total Phases:** 2', + '', + ].join('\n')); + + // Start with one phase directory containing one PLAN + const phase1 = path.join(phasesDir, '01-foo'); + fs.mkdirSync(phase1); + fs.writeFileSync(path.join(phase1, '01-1-PLAN.md'), '---\nphase: 1\nplan: 1\n---\n# Plan\n'); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + test('writeStateMd invalidates cache so subsequent reads see new disk state', () => { + // First write — populates cache via buildStateFrontmatter + const content1 = fs.readFileSync(statePath, 'utf-8'); + state.writeStateMd(statePath, content1, tmpDir); + + // Create a NEW phase directory AFTER the first write + // Without cache invalidation, the second write would still see only 1 phase + const phase2 = path.join(phasesDir, '02-bar'); + fs.mkdirSync(phase2); + fs.writeFileSync(path.join(phase2, '02-1-PLAN.md'), '---\nphase: 2\nplan: 1\n---\n# Plan\n'); + fs.writeFileSync(path.join(phase2, '02-1-SUMMARY.md'), '---\nstatus: complete\n---\n# Summary\n'); + + // Second write in the SAME process — must see the new phase + const content2 = fs.readFileSync(statePath, 'utf-8'); + state.writeStateMd(statePath, content2, tmpDir); + + // Read back and parse frontmatter to verify it reflects 2 phases, not 1 + const result = fs.readFileSync(statePath, 'utf-8'); + const fmMatch = result.match(/^---\n([\s\S]*?)\n---/); + assert.ok(fmMatch, 'STATE.md should have frontmatter after writeStateMd'); + + const fm = fmMatch[1]; + // Should show 2 total phases (the new disk state), not 1 (stale cache) + const totalPhasesMatch = fm.match(/total_phases:\s*(\d+)/); + assert.ok(totalPhasesMatch, 'frontmatter should contain total_phases'); + assert.strictEqual( + parseInt(totalPhasesMatch[1], 10), + 2, + 'total_phases should reflect new disk state (2), not stale cache (1)' + ); + + // Should show 1 completed phase (phase 2 has SUMMARY) + const completedMatch = fm.match(/completed_phases:\s*(\d+)/); + assert.ok(completedMatch, 'frontmatter should contain completed_phases'); + assert.strictEqual( + parseInt(completedMatch[1], 10), + 1, + 'completed_phases should reflect new disk state (1 complete), not stale cache (0)' + ); + }); +});