Merge pull request #796 from jeremymcs/fix/793-db-cache-invalidation
fix: invalidateAllCaches() in skip-loop eviction and rebuildState (#793)
This commit is contained in:
commit
5271e51a84
3 changed files with 65 additions and 9 deletions
|
|
@ -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
|
||||
|
|
@ -2448,7 +2448,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",
|
||||
|
|
@ -2487,7 +2487,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);
|
||||
|
|
@ -2495,7 +2495,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",
|
||||
|
|
@ -2581,7 +2581,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;
|
||||
|
|
@ -2608,7 +2608,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;
|
||||
|
|
@ -2628,7 +2628,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;
|
||||
|
|
@ -2669,7 +2669,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;
|
||||
|
|
|
|||
|
|
@ -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<void> {
|
||||
invalidateAllCaches();
|
||||
const state = await deriveState(basePath);
|
||||
const path = resolveGsdRootFile(basePath, "STATE");
|
||||
await saveFile(path, buildStateMarkdown(state));
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue