From 3991479570820a4091ce8c3c1ec30acac0f69d32 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Mon, 16 Mar 2026 23:21:13 -0500 Subject: [PATCH] fix: invalidateAllCaches() in skip-loop eviction and rebuildState (#793) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: The skip-loop breaker in dispatchNextUnit() called invalidateStateCache() which only clears the in-memory _stateCache. Path caches (nativeTreeCache, dirEntryCache) and parse caches (_parseCache) remained warm. On the next deriveState() call, the nativeBatchParseGsdFiles/cachedLoadFile path used the stale cache to build fileContentCache, returning the same stale unit — looping forever even though disk had the correct [x] state. rebuildState() in doctor.ts had the same bug: it called deriveState() without invalidating caches first, writing stale state to STATE.md. Fix: 1. auto.ts: replace all invalidateStateCache() calls in dispatch hot paths with invalidateAllCaches() so path, parse, and state caches are all cleared before the next deriveState() call. 2. doctor.ts: call invalidateAllCaches() at the top of rebuildState() so STATE.md is always written from fresh disk reads. 3. Remove now-unused invalidateStateCache import from auto.ts. Test: Added #793 test to auto-recovery.test.ts: creates a fixture with T01 active, simulates task completion on disk (plan [x] + summary written), calls invalidateAllCaches(), then asserts deriveState() returns T02 as active (not T01 again). 30 passed, 0 failed. --- src/resources/extensions/gsd/auto.ts | 18 +++---- src/resources/extensions/gsd/doctor.ts | 2 + .../gsd/tests/auto-recovery.test.ts | 54 +++++++++++++++++++ 3 files changed, 65 insertions(+), 9 deletions(-) diff --git a/src/resources/extensions/gsd/auto.ts b/src/resources/extensions/gsd/auto.ts index 8e95668b2..c459833b7 100644 --- a/src/resources/extensions/gsd/auto.ts +++ b/src/resources/extensions/gsd/auto.ts @@ -16,7 +16,7 @@ import type { ExtensionCommandContext, } from "@gsd/pi-coding-agent"; -import { deriveState, invalidateStateCache } from "./state.js"; +import { deriveState } from "./state.js"; import type { BudgetEnforcementMode, GSDState } from "./types.js"; import { loadFile, parseRoadmap, getManifestStatus, resolveAllOverrides } from "./files.js"; import { loadPrompt } from "./prompt-loader.js"; @@ -1465,7 +1465,7 @@ export async function handleAgentEnd( persistCompletedKey(basePath, completionKey); completedKeySet.add(completionKey); } - invalidateStateCache(); + invalidateAllCaches(); } } catch { // Non-fatal — worst case we fall through to normal dispatch which has its own checks @@ -2421,7 +2421,7 @@ async function dispatchNextUnit( unitConsecutiveSkips.delete(idempotencyKey); completedKeySet.delete(idempotencyKey); removePersistedKey(basePath, idempotencyKey); - invalidateStateCache(); + invalidateAllCaches(); ctx.ui.notify( `Skip loop detected: ${unitType} ${unitId} skipped ${skipCount} times without advancing. Evicting completion record and forcing reconciliation.`, "warning", @@ -2460,7 +2460,7 @@ async function dispatchNextUnit( if (verifyExpectedArtifact(unitType, unitId, basePath)) { persistCompletedKey(basePath, idempotencyKey); completedKeySet.add(idempotencyKey); - invalidateStateCache(); + invalidateAllCaches(); // Same consecutive-skip guard as the idempotency path above. const skipCount2 = (unitConsecutiveSkips.get(idempotencyKey) ?? 0) + 1; unitConsecutiveSkips.set(idempotencyKey, skipCount2); @@ -2468,7 +2468,7 @@ async function dispatchNextUnit( unitConsecutiveSkips.delete(idempotencyKey); completedKeySet.delete(idempotencyKey); removePersistedKey(basePath, idempotencyKey); - invalidateStateCache(); + invalidateAllCaches(); ctx.ui.notify( `Skip loop detected: ${unitType} ${unitId} skipped ${skipCount2} times without advancing. Evicting completion record and forcing reconciliation.`, "warning", @@ -2554,7 +2554,7 @@ async function dispatchNextUnit( persistCompletedKey(basePath, reconciledKey); completedKeySet.add(reconciledKey); unitDispatchCount.delete(dispatchKey); - invalidateStateCache(); + invalidateAllCaches(); await new Promise(r => setImmediate(r)); await dispatchNextUnit(ctx, pi); return; @@ -2581,7 +2581,7 @@ async function dispatchNextUnit( persistCompletedKey(basePath, dispatchKey); completedKeySet.add(dispatchKey); unitDispatchCount.delete(dispatchKey); - invalidateStateCache(); + invalidateAllCaches(); await new Promise(r => setImmediate(r)); await dispatchNextUnit(ctx, pi); return; @@ -2601,7 +2601,7 @@ async function dispatchNextUnit( persistCompletedKey(basePath, dispatchKey); completedKeySet.add(dispatchKey); unitDispatchCount.delete(dispatchKey); - invalidateStateCache(); + invalidateAllCaches(); await new Promise(r => setImmediate(r)); await dispatchNextUnit(ctx, pi); return; @@ -2642,7 +2642,7 @@ async function dispatchNextUnit( persistCompletedKey(basePath, repairedKey); completedKeySet.add(repairedKey); unitDispatchCount.delete(dispatchKey); - invalidateStateCache(); + invalidateAllCaches(); await new Promise(r => setImmediate(r)); await dispatchNextUnit(ctx, pi); return; diff --git a/src/resources/extensions/gsd/doctor.ts b/src/resources/extensions/gsd/doctor.ts index c15221d75..7ffb29830 100644 --- a/src/resources/extensions/gsd/doctor.ts +++ b/src/resources/extensions/gsd/doctor.ts @@ -4,6 +4,7 @@ import { join, sep } from "node:path"; import { loadFile, parsePlan, parseRoadmap, parseSummary, saveFile, parseTaskPlanMustHaves, countMustHavesMentionedInSummary } from "./files.js"; import { resolveMilestoneFile, resolveMilestonePath, resolveSliceFile, resolveSlicePath, resolveTaskFile, resolveTaskFiles, resolveTasksDir, milestonesDir, gsdRoot, relMilestoneFile, relSliceFile, relTaskFile, relSlicePath, relGsdRootFile, resolveGsdRootFile } from "./paths.js"; import { deriveState, isMilestoneComplete } from "./state.js"; +import { invalidateAllCaches } from "./cache.js"; import { loadEffectiveGSDPreferences, type GSDPreferences } from "./preferences.js"; import { listWorktrees, resolveGitDir } from "./worktree-manager.js"; import { abortAndReset } from "./git-self-heal.js"; @@ -200,6 +201,7 @@ async function updateStateFile(basePath: string, fixesApplied: string[]): Promis /** Rebuild STATE.md from current disk state. Exported for auto-mode post-hooks. */ export async function rebuildState(basePath: string): Promise { + invalidateAllCaches(); const state = await deriveState(basePath); const path = resolveGsdRootFile(basePath, "STATE"); await saveFile(path, buildStateMarkdown(state)); diff --git a/src/resources/extensions/gsd/tests/auto-recovery.test.ts b/src/resources/extensions/gsd/tests/auto-recovery.test.ts index 597669fe9..1e193cb11 100644 --- a/src/resources/extensions/gsd/tests/auto-recovery.test.ts +++ b/src/resources/extensions/gsd/tests/auto-recovery.test.ts @@ -17,6 +17,8 @@ import { loadPersistedKeys, } from "../auto-recovery.ts"; import { parseRoadmap, clearParseCache } from "../files.ts"; +import { invalidateAllCaches } from "../cache.ts"; +import { deriveState, invalidateStateCache } from "../state.ts"; function makeTmpBase(): string { const base = join(tmpdir(), `gsd-test-${randomUUID()}`); @@ -584,3 +586,55 @@ test("selfHealRuntimeRecords clears stale record when artifact exists at worktre cleanup(mainBase); } }); + +// ─── #793: invalidateAllCaches unblocks skip-loop ───────────────────────── +// When the skip-loop breaker fires, it must call invalidateAllCaches() (not +// just invalidateStateCache()) to clear path/parse caches that deriveState +// depends on. Without this, even after cache invalidation, deriveState reads +// stale directory listings and returns the same unit, looping forever. +test("#793: invalidateAllCaches clears all caches so deriveState sees fresh disk state", async () => { + const base = makeTmpBase(); + try { + const mid = "M001"; + const sid = "S01"; + const planDir = join(base, ".gsd", "milestones", mid, "slices", sid); + const tasksDir = join(planDir, "tasks"); + mkdirSync(tasksDir, { recursive: true }); + mkdirSync(join(base, ".gsd", "milestones", mid), { recursive: true }); + + writeFileSync( + join(base, ".gsd", "milestones", mid, `${mid}-ROADMAP.md`), + `# M001: Test Milestone\n\n**Vision:** test.\n\n## Slices\n\n- [ ] **${sid}: Slice One** \`risk:low\` \`depends:[]\`\n > After this: done.\n`, + ); + const planUnchecked = `# ${sid}: Slice One\n\n**Goal:** test.\n\n## Tasks\n\n- [ ] **T01: Task One** \`est:10m\`\n- [ ] **T02: Task Two** \`est:10m\`\n`; + writeFileSync(join(planDir, `${sid}-PLAN.md`), planUnchecked); + writeFileSync(join(tasksDir, "T01-PLAN.md"), "# T01: Task One\n\n**Goal:** t\n\n## Steps\n- step\n\n## Verification\n- v\n"); + writeFileSync(join(tasksDir, "T02-PLAN.md"), "# T02: Task Two\n\n**Goal:** t\n\n## Steps\n- step\n\n## Verification\n- v\n"); + + // Warm all caches + const state1 = await deriveState(base); + assert.equal(state1.activeTask?.id, "T01", "initial: T01 is active"); + + // Simulate task completion on disk (what the LLM does) + const planChecked = `# ${sid}: Slice One\n\n**Goal:** test.\n\n## Tasks\n\n- [x] **T01: Task One** \`est:10m\`\n- [ ] **T02: Task Two** \`est:10m\`\n`; + writeFileSync(join(planDir, `${sid}-PLAN.md`), planChecked); + writeFileSync(join(tasksDir, "T01-SUMMARY.md"), "---\nid: T01\n---\n# Summary\n"); + + // invalidateStateCache alone: _stateCache cleared but path/parse caches warm + invalidateStateCache(); + + // invalidateAllCaches: all caches cleared — deriveState must re-read disk + invalidateAllCaches(); + const state2 = await deriveState(base); + + // After full invalidation, T01 should be complete and T02 should be next + assert.notEqual(state2.activeTask?.id, "T01", "#793: T01 not re-dispatched after full invalidation"); + + // Verify the caches are truly cleared by calling clearParseCache and clearPathCache + // do not throw (they should be no-ops after invalidateAllCaches already cleared them) + clearParseCache(); // no-op, but should not throw + assert.ok(true, "clearParseCache after invalidateAllCaches is safe"); + } finally { + cleanup(base); + } +});