From 8f2e120a29ed405b7649ba24c28806a7ceca5d99 Mon Sep 17 00:00:00 2001 From: Tibsfox Date: Mon, 6 Apr 2026 19:04:25 -0700 Subject: [PATCH 1/3] fix(gsd): tighten verifyExpectedArtifact to prevent rogue-write false positives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes to fail-closed when gsd_complete_task didn't actually run: 1. Legacy branch: require checked checkbox (- [x] **T01:) instead of accepting heading-style matches that only prove the task was planned 2. No plan file: return false instead of falling through 3. DB available but task row missing: return false instead of treating as verified — if the DB is up and the task isn't there, the completion tool never ran Closes #3607 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/resources/extensions/gsd/auto-recovery.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/resources/extensions/gsd/auto-recovery.ts b/src/resources/extensions/gsd/auto-recovery.ts index 86c9d70be..1fabee1a1 100644 --- a/src/resources/extensions/gsd/auto-recovery.ts +++ b/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) } } From 77216f60759e1272adcfc22f593db757ac4e6a61 Mon Sep 17 00:00:00 2001 From: Tibsfox Date: Mon, 6 Apr 2026 22:28:51 -0700 Subject: [PATCH 2/3] test: add regression test for verify-artifact tightened legacy branch Co-Authored-By: Claude Opus 4.6 (1M context) --- .../tests/verify-artifact-tightened.test.ts | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 src/resources/extensions/gsd/tests/verify-artifact-tightened.test.ts diff --git a/src/resources/extensions/gsd/tests/verify-artifact-tightened.test.ts b/src/resources/extensions/gsd/tests/verify-artifact-tightened.test.ts new file mode 100644 index 000000000..1a64299d2 --- /dev/null +++ b/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', + ) + }) +}) From 9e268ed182adbdba7bd71cfc88a76ae52396bed0 Mon Sep 17 00:00:00 2001 From: Tibsfox Date: Mon, 6 Apr 2026 22:59:17 -0700 Subject: [PATCH 3/3] test: update heading-style test to expect false after #3607 tightening --- .../gsd/tests/integration/auto-recovery.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/resources/extensions/gsd/tests/integration/auto-recovery.test.ts b/src/resources/extensions/gsd/tests/integration/auto-recovery.test.ts index 65bb58e5b..11f5a3d48 100644 --- a/src/resources/extensions/gsd/tests/integration/auto-recovery.test.ts +++ b/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", ); });