Merge pull request #3679 from Tibsfox/fix/verify-artifact-false-positive

fix(gsd): tighten verifyExpectedArtifact to prevent rogue-write false positives

src/resources/extensions/gsd/auto-recovery.ts

@@ -286,7 +286,7 @@ export function verifyExpectedArtifact(
     if (!hasCheckboxTask && !hasHeadingTask) return false;
   }
 
-  // execute-task: DB status is authoritative. Fall back to heading-style plan
+  // execute-task: DB status is authoritative. Fall back to checked-checkbox
   // detection when the DB is unavailable (unmigrated projects).
   if (unitType === "execute-task") {
     const { milestone: mid, slice: sid, task: tid } = parseUnitId(unitId);
@@ -297,20 +297,22 @@ export function verifyExpectedArtifact(
         if (dbTask.status !== "complete" && dbTask.status !== "done") return false;
       } else if (!isDbAvailable()) {
         // LEGACY: Pre-migration fallback for projects without DB.
-        // Fall back to plan heading check (format detection, not reconciliation).
-        // Heading-style entries (### T01 --) count as verified because the
-        // summary file existence (checked above) is the real signal.
+        // Require a CHECKED checkbox — a bare heading or unchecked checkbox
+        // does not prove gsd_complete_task ran. Summary file on disk alone
+        // is not sufficient evidence (could be a rogue write) (#3607).
         const planAbs = resolveSliceFile(base, mid, sid, "PLAN");
         if (planAbs && existsSync(planAbs)) {
           const planContent = readFileSync(planAbs, "utf-8");
           const escapedTid = tid.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
-          const hdRe = new RegExp(`^#{2,4}\\s+${escapedTid}\\s*(?:--|—|:)`, "m");
           const cbRe = new RegExp(`^- \\[[xX]\\] \\*\\*${escapedTid}:`, "m");
-          if (!hdRe.test(planContent) && !cbRe.test(planContent)) return false;
+          if (!cbRe.test(planContent)) return false;
+        } else {
+          return false; // no plan file → cannot verify
         }
+      } else {
+        // DB available but task row not found — completion tool never ran (#3607)
+        return false;
       }
-      // else: DB available but task not found — summary file exists (checked above),
-      // so treat as verified (task may not be imported yet)
     }
   }
 

src/resources/extensions/gsd/tests/integration/auto-recovery.test.ts

@@ -454,7 +454,7 @@ test("verifyExpectedArtifact accepts plan-slice with colon-style heading tasks (
   );
 });
 
-test("verifyExpectedArtifact execute-task passes for heading-style plan entry (#1691)", (t) => {
+test("verifyExpectedArtifact execute-task rejects heading-style plan without checked checkbox (#3607)", (t) => {
   const base = makeTmpBase();
   t.after(() => cleanup(base));
 
@@ -471,10 +471,12 @@ test("verifyExpectedArtifact execute-task passes for heading-style plan entry (#
     "Feature description.",
   ].join("\n"));
   writeFileSync(join(tasksDir, "T01-SUMMARY.md"), "# T01 Summary\n\nDone.");
+  // Heading-style entries no longer count as verified — only checked
+  // checkboxes prove gsd_complete_task ran (#3607).
   assert.strictEqual(
     verifyExpectedArtifact("execute-task", "M001/S01/T01", base),
-    true,
-    "execute-task should pass for heading-style plan entry when summary exists",
+    false,
+    "heading-style without checked checkbox should NOT pass verification",
   );
 });
 

src/resources/extensions/gsd/tests/verify-artifact-tightened.test.ts

@@ -0,0 +1,89 @@
+/**
+ * Regression test for #3607 — tighten verifyExpectedArtifact legacy branch
+ *
+ * The legacy (pre-migration) fallback in verifyExpectedArtifact previously
+ * accepted either a heading match (### T01 --) or a checked checkbox as proof
+ * that gsd_complete_task ran. A heading alone does not prove completion —
+ * it could result from a rogue write.
+ *
+ * The fix removes the hdRe heading regex and requires only a checked checkbox
+ * (cbRe) in the legacy branch, ensuring that only actual tool-completed tasks
+ * are treated as verified.
+ */
+
+import { describe, it } from 'node:test'
+import assert from 'node:assert/strict'
+import { readFileSync } from 'node:fs'
+import { resolve } from 'node:path'
+
+const src = readFileSync(
+  resolve(process.cwd(), 'src', 'resources', 'extensions', 'gsd', 'auto-recovery.ts'),
+  'utf-8',
+)
+
+describe('verifyExpectedArtifact legacy branch tightened (#3607)', () => {
+  it('legacy branch does NOT define hdRe heading regex', () => {
+    // Find the legacy fallback section
+    const legacyIdx = src.indexOf('LEGACY: Pre-migration fallback')
+    assert.ok(legacyIdx !== -1, 'LEGACY comment must exist')
+
+    // Check the code within a reasonable window after the LEGACY comment
+    const legacyBlock = src.slice(legacyIdx, legacyIdx + 600)
+
+    assert.ok(
+      !legacyBlock.includes('hdRe'),
+      'hdRe heading regex must NOT exist in legacy branch — heading alone is not proof of completion',
+    )
+  })
+
+  it('legacy branch requires checked checkbox via cbRe', () => {
+    const legacyIdx = src.indexOf('LEGACY: Pre-migration fallback')
+    assert.ok(legacyIdx !== -1)
+
+    const legacyBlock = src.slice(legacyIdx, legacyIdx + 600)
+
+    assert.ok(
+      legacyBlock.includes('cbRe'),
+      'cbRe checked-checkbox regex must exist in legacy branch',
+    )
+
+    // cbRe must match checked checkboxes [x] or [X]
+    assert.ok(
+      legacyBlock.includes('[xX]'),
+      'cbRe must match both [x] and [X] checkbox variants',
+    )
+  })
+
+  it('legacy branch returns false when no plan file exists', () => {
+    const legacyIdx = src.indexOf('LEGACY: Pre-migration fallback')
+    assert.ok(legacyIdx !== -1)
+
+    const legacyBlock = src.slice(legacyIdx, legacyIdx + 1000)
+
+    // The else branch: no plan file means cannot verify
+    assert.ok(
+      legacyBlock.includes('no plan file'),
+      'missing plan file must be handled with return false',
+    )
+  })
+
+  it('DB available but task not found returns false', () => {
+    const legacyIdx = src.indexOf('LEGACY: Pre-migration fallback')
+    assert.ok(legacyIdx !== -1)
+
+    const legacyBlock = src.slice(legacyIdx, legacyIdx + 1000)
+
+    assert.ok(
+      legacyBlock.includes('DB available but task row not found'),
+      'must handle case where DB is available but task row is missing',
+    )
+
+    // The comment should be followed by a return false
+    const commentIdx = legacyBlock.indexOf('DB available but task row not found')
+    const afterComment = legacyBlock.slice(commentIdx, commentIdx + 200)
+    assert.ok(
+      afterComment.includes('return false'),
+      'missing task row when DB available must return false',
+    )
+  })
+})