From 5638448296ccfdd4e1ccda9f2e5b42330a342e71 Mon Sep 17 00:00:00 2001 From: Tibsfox Date: Fri, 10 Apr 2026 11:27:39 -0700 Subject: [PATCH 1/2] perf(state): cache buildStateFrontmatter disk scan per process (#1967) buildStateFrontmatter performs N+1 readdirSync calls (phases dir + each phase subdirectory) every time it's called. Multiple state writes within a single gsd-tools invocation repeat the same scan unnecessarily. Add a module-level Map cache keyed by cwd that stores the disk scan results. The cache auto-clears when the process exits since each gsd-tools CLI invocation is a short-lived process running one command. Closes #1967 Co-Authored-By: Claude Opus 4.6 --- get-shit-done/bin/lib/state.cjs | 57 +++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/get-shit-done/bin/lib/state.cjs b/get-shit-done/bin/lib/state.cjs index 06da9d13c..e97833ff2 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 */ } } From 67aeb049c2086df77f4b8dceafcd212f802ef8c5 Mon Sep 17 00:00:00 2001 From: Tibsfox Date: Sat, 11 Apr 2026 03:35:00 -0700 Subject: [PATCH 2/2] fix(state): invalidate disk scan cache in writeStateMd (#1967) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add _diskScanCache.delete(cwd) at the start of writeStateMd before buildStateFrontmatter is called. This prevents stale reads if multiple state-mutating operations occur within the same Node process — the write may create new PLAN/SUMMARY files that the next frontmatter computation must see. Matters for: - SDK callers that require() gsd-tools.cjs as a module - Future dispatcher extensions handling compound operations - Tests that import state.cjs directly Adds tests/bug-1967-cache-invalidation.test.cjs which exercises two sequential writes in the same process with a new phase directory created between them, asserting the second write sees the new disk state (total_phases: 2, completed_phases: 1) instead of the cached pre-write snapshot (total_phases: 1, completed_phases: 0). Review feedback on #2054 from @trek-e. --- get-shit-done/bin/lib/state.cjs | 4 + tests/bug-1967-cache-invalidation.test.cjs | 100 +++++++++++++++++++++ 2 files changed, 104 insertions(+) create mode 100644 tests/bug-1967-cache-invalidation.test.cjs diff --git a/get-shit-done/bin/lib/state.cjs b/get-shit-done/bin/lib/state.cjs index e97833ff2..9d65d23b8 100644 --- a/get-shit-done/bin/lib/state.cjs +++ b/get-shit-done/bin/lib/state.cjs @@ -921,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)' + ); + }); +});