mirror of
https://github.com/glittercowboy/get-shit-done
synced 2026-04-25 17:25:23 +02:00
fix(state): invalidate disk scan cache in writeStateMd (#1967)
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.
This commit is contained in:
@@ -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 {
|
||||
|
||||
100
tests/bug-1967-cache-invalidation.test.cjs
Normal file
100
tests/bug-1967-cache-invalidation.test.cjs
Normal file
@@ -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)'
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user