From 5866bb0b273a92a11b786eac47ff3f592c49b8b3 Mon Sep 17 00:00:00 2001
From: Flux Labs <jeremy@fluxlabs.net>
Date: Sun, 15 Mar 2026 23:10:48 -0500
Subject: [PATCH] fix: parse cache collision causing false loop detection on
 complete-slice (#583)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

cacheKey() used length + first/last 100 chars, which collides when a
checkbox changes [ ] → [x] mid-file (same length, same endpoints).
verifyExpectedArtifact() only cleared the path cache, not the parse
cache, so parseRoadmap() returned stale data with done=false.

- Add clearParseCache() to verifyExpectedArtifact alongside clearPathCache
- Include middle 100-char sample in cacheKey to prevent interior collisions
- Add regression test for the cache collision scenario
---
 src/resources/extensions/gsd/auto-recovery.ts |  9 +++-
 src/resources/extensions/gsd/files.ts         |  8 ++-
 .../gsd/tests/auto-recovery.test.ts           | 50 +++++++++++++++++++
 3 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/src/resources/extensions/gsd/auto-recovery.ts b/src/resources/extensions/gsd/auto-recovery.ts
index ca23efebd..4b304d356 100644
--- a/src/resources/extensions/gsd/auto-recovery.ts
+++ b/src/resources/extensions/gsd/auto-recovery.ts
@@ -11,6 +11,7 @@ import type { ExtensionContext } from "@gsd/pi-coding-agent";
 import {
   clearUnitRuntimeRecord,
 } from "./unit-runtime.js";
+import { clearParseCache } from "./files.js";
 import {
   nativeConflictFiles,
   nativeCommit,
@@ -107,9 +108,13 @@ export function verifyExpectedArtifact(unitType: string, unitId: string, base: s
   // is managed by the hook engine, not the artifact verification system.
   if (unitType.startsWith("hook/")) return true;
 
-  // Clear stale directory listing cache so artifact checks see fresh disk state (#431).
-  // Moved after hook check to avoid unnecessary cache clears for hook units.
+  // Clear stale directory listing cache AND parse cache so artifact checks see
+  // fresh disk state (#431). The parse cache must also be cleared because
+  // cacheKey() uses length + first/last 100 chars — when a checkbox changes
+  // from [ ] to [x], the key collides with the pre-edit version, returning
+  // stale parsed results (e.g., slice.done = false when it's actually true).
   clearPathCache();
+  clearParseCache();
 
   if (unitType === "rewrite-docs") {
     const overridesPath = resolveGsdRootFile(base, "OVERRIDES");
diff --git a/src/resources/extensions/gsd/files.ts b/src/resources/extensions/gsd/files.ts
index dd5877f47..60caf003b 100644
--- a/src/resources/extensions/gsd/files.ts
+++ b/src/resources/extensions/gsd/files.ts
@@ -26,12 +26,16 @@ import { nativeParseRoadmap, nativeExtractSection, nativeParsePlanFile, nativePa
 
 const CACHE_MAX = 50;
 
-/** Fast composite key: length + first/last 100 chars. Unique enough for distinct markdown files. */
+/** Fast composite key: length + first/mid/last 100 chars. The middle sample
+ *  prevents collisions when only a few characters change in the interior of
+ *  a file (e.g., a checkbox [ ] → [x] that doesn't alter length or endpoints). */
 function cacheKey(content: string): string {
   const len = content.length;
   const head = content.slice(0, 100);
+  const midStart = Math.max(0, Math.floor(len / 2) - 50);
+  const mid = len > 200 ? content.slice(midStart, midStart + 100) : '';
   const tail = len > 100 ? content.slice(-100) : '';
-  return `${len}:${head}:${tail}`;
+  return `${len}:${head}:${mid}:${tail}`;
 }
 
 const _parseCache = new Map<string, unknown>();
diff --git a/src/resources/extensions/gsd/tests/auto-recovery.test.ts b/src/resources/extensions/gsd/tests/auto-recovery.test.ts
index c0a5b7478..4ea508ac4 100644
--- a/src/resources/extensions/gsd/tests/auto-recovery.test.ts
+++ b/src/resources/extensions/gsd/tests/auto-recovery.test.ts
@@ -7,6 +7,7 @@ import { randomUUID } from "node:crypto";
 
 import {
   resolveExpectedArtifactPath,
+  verifyExpectedArtifact,
   diagnoseExpectedArtifact,
   buildLoopRemediationSteps,
   completedKeysPath,
@@ -14,6 +15,7 @@ import {
   removePersistedKey,
   loadPersistedKeys,
 } from "../auto-recovery.ts";
+import { parseRoadmap, clearParseCache } from "../files.ts";
 
 function makeTmpBase(): string {
   const base = join(tmpdir(), `gsd-test-${randomUUID()}`);
@@ -270,3 +272,51 @@ test("removePersistedKey is safe when file doesn't exist", () => {
     cleanup(base);
   }
 });
+
+// ─── verifyExpectedArtifact: parse cache collision regression ─────────────
+
+test("verifyExpectedArtifact detects roadmap [x] change despite parse cache", () => {
+  // Regression test: cacheKey collision when [ ] → [x] doesn't change
+  // file length or first/last 100 chars. Without the fix, parseRoadmap
+  // returns stale cached data with done=false even though the file has [x].
+  const base = makeTmpBase();
+  try {
+    // Build a roadmap long enough that the [x] change is outside the first/last 100 chars
+    const padding = "A".repeat(200);
+    const roadmapBefore = [
+      `# M001: Test Milestone ${padding}`,
+      "",
+      "## Slices",
+      "",
+      "- [ ] **S01: First slice** `risk:low`",
+      "",
+      `## Footer ${padding}`,
+    ].join("\n");
+    const roadmapAfter = roadmapBefore.replace("- [ ] **S01:", "- [x] **S01:");
+
+    // Verify lengths are identical (the key collision condition)
+    assert.equal(roadmapBefore.length, roadmapAfter.length);
+
+    // Populate parse cache with the pre-edit roadmap
+    const before = parseRoadmap(roadmapBefore);
+    const sliceBefore = before.slices.find(s => s.id === "S01");
+    assert.ok(sliceBefore);
+    assert.equal(sliceBefore!.done, false);
+
+    // Now write the post-edit roadmap to disk and create required artifacts
+    const roadmapPath = join(base, ".gsd", "milestones", "M001", "M001-ROADMAP.md");
+    writeFileSync(roadmapPath, roadmapAfter);
+    const summaryPath = join(base, ".gsd", "milestones", "M001", "slices", "S01", "S01-SUMMARY.md");
+    writeFileSync(summaryPath, "# Summary\nDone.");
+    const uatPath = join(base, ".gsd", "milestones", "M001", "slices", "S01", "S01-UAT.md");
+    writeFileSync(uatPath, "# UAT\nPassed.");
+
+    // verifyExpectedArtifact should see the [x] despite the parse cache
+    // having the [ ] version. The fix clears the parse cache inside verify.
+    const verified = verifyExpectedArtifact("complete-slice", "M001/S01", base);
+    assert.equal(verified, true, "verifyExpectedArtifact should return true when roadmap has [x]");
+  } finally {
+    clearParseCache();
+    cleanup(base);
+  }
+});