From 3e5bce5dbdfb6976a343579ba5f897ba617fa7a0 Mon Sep 17 00:00:00 2001 From: Tibsfox Date: Mon, 6 Apr 2026 18:46:42 -0700 Subject: [PATCH 1/3] fix(gsd): force re-validation when verdict is needs-remediation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When validation returns needs-remediation, remediation slices are added and executed. But the state machine treated any terminal verdict as ready for completing-milestone, while dispatch correctly blocked completion for needs-remediation — creating a permanent deadlock. Now all three derivation paths (deriveStateFromDb, _deriveStateImpl registry loop, and _deriveStateImpl completion check) treat needs-remediation as requiring re-validation, routing back to validating-milestone instead of completing-milestone. Closes #3596 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/resources/extensions/gsd/state.ts | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/resources/extensions/gsd/state.ts b/src/resources/extensions/gsd/state.ts index bc41ab6ed..d474d896f 100644 --- a/src/resources/extensions/gsd/state.ts +++ b/src/resources/extensions/gsd/state.ts @@ -636,12 +636,15 @@ export async function deriveStateFromDb(basePath: string): Promise { const validationFile = resolveMilestoneFile(basePath, activeMilestone.id, "VALIDATION"); const validationContent = validationFile ? await loadFile(validationFile) : null; const validationTerminal = validationContent ? isValidationTerminal(validationContent) : false; + const verdict = validationContent ? extractVerdict(validationContent) : undefined; const sliceProgress = { done: activeMilestoneSlices.length, total: activeMilestoneSlices.length, }; - if (!validationTerminal) { + // Force re-validation when verdict is needs-remediation — remediation slices + // may have completed since the stale validation was written (#3596). + if (!validationTerminal || verdict === 'needs-remediation') { return { activeMilestone, activeSlice: null, activeTask: null, phase: 'validating-milestone', @@ -1099,22 +1102,25 @@ export async function _deriveStateImpl(basePath: string): Promise { const validationFile = resolveMilestoneFile(basePath, mid, "VALIDATION"); const validationContent = validationFile ? await cachedLoadFile(validationFile) : null; const validationTerminal = validationContent ? isValidationTerminal(validationContent) : false; + const verdict = validationContent ? extractVerdict(validationContent) : undefined; + // needs-remediation is terminal but requires re-validation (#3596) + const needsRevalidation = !validationTerminal || verdict === 'needs-remediation'; if (summaryFile) { // Summary exists → milestone is complete regardless of validation state. // The summary is the terminal artifact (#864). registry.push({ id: mid, title, status: 'complete' }); - } else if (!validationTerminal && !activeMilestoneFound) { - // No summary and no terminal validation → validating-milestone + } else if (needsRevalidation && !activeMilestoneFound) { + // No summary and needs (re-)validation → validating-milestone activeMilestone = { id: mid, title }; activeRoadmap = roadmap; activeMilestoneFound = true; registry.push({ id: mid, title, status: 'active' }); - } else if (!validationTerminal && activeMilestoneFound) { - // No summary and no terminal validation, but another milestone is already active + } else if (needsRevalidation && activeMilestoneFound) { + // Needs (re-)validation, but another milestone is already active registry.push({ id: mid, title, status: 'pending' }); } else if (!activeMilestoneFound) { - // Terminal validation but no summary → completing-milestone + // Terminal validation (pass/needs-attention) but no summary → completing-milestone activeMilestone = { id: mid, title }; activeRoadmap = roadmap; activeMilestoneFound = true; @@ -1299,12 +1305,15 @@ export async function _deriveStateImpl(basePath: string): Promise { const validationFile = resolveMilestoneFile(basePath, activeMilestone.id, "VALIDATION"); const validationContent = validationFile ? await cachedLoadFile(validationFile) : null; const validationTerminal = validationContent ? isValidationTerminal(validationContent) : false; + const verdict = validationContent ? extractVerdict(validationContent) : undefined; const sliceProgress = { done: activeRoadmap.slices.length, total: activeRoadmap.slices.length, }; - if (!validationTerminal) { + // Force re-validation when verdict is needs-remediation — remediation slices + // may have completed since the stale validation was written (#3596). + if (!validationTerminal || verdict === 'needs-remediation') { return { activeMilestone, activeSlice: null, From 64d5232cc008e52ac990925042996b37ce6e8663 Mon Sep 17 00:00:00 2001 From: Tibsfox Date: Mon, 6 Apr 2026 22:23:05 -0700 Subject: [PATCH 2/3] test: add regression test for needs-remediation revalidation guard Co-Authored-By: Claude Opus 4.6 (1M context) --- .../needs-remediation-revalidation.test.ts | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 src/resources/extensions/gsd/tests/needs-remediation-revalidation.test.ts diff --git a/src/resources/extensions/gsd/tests/needs-remediation-revalidation.test.ts b/src/resources/extensions/gsd/tests/needs-remediation-revalidation.test.ts new file mode 100644 index 000000000..4705cffab --- /dev/null +++ b/src/resources/extensions/gsd/tests/needs-remediation-revalidation.test.ts @@ -0,0 +1,48 @@ +/** + * Regression test for #3670 — needs-remediation verdict forces re-validation + * + * When validation returns needs-remediation, the state machine must route + * back to validating-milestone instead of completing-milestone. Without this, + * dispatch blocks completion for needs-remediation while state derives + * completing-milestone, creating a permanent deadlock. + * + * This structural test verifies the verdict === 'needs-remediation' guard + * exists at all three derivation paths in state.ts. + */ + +import { describe, test } from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { dirname, join } from 'node:path'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); + +const source = readFileSync(join(__dirname, '..', 'state.ts'), 'utf-8'); + +describe('needs-remediation revalidation guard (#3670)', () => { + test('verdict === needs-remediation guard exists in state.ts', () => { + const matches = source.match(/verdict\s*===\s*['"]needs-remediation['"]/g); + assert.ok(matches, 'verdict === "needs-remediation" check must exist in state.ts'); + assert.ok(matches.length >= 2, + `Expected at least 2 needs-remediation guards (deriveStateFromDb + _deriveStateImpl), found ${matches.length}`); + }); + + test('needsRevalidation variable is derived from verdict', () => { + assert.match(source, /needsRevalidation.*=.*verdict\s*===\s*['"]needs-remediation['"]/, + 'needsRevalidation should incorporate verdict === "needs-remediation"'); + }); + + test('deriveStateFromDb path uses needs-remediation guard', () => { + assert.match(source, /!validationTerminal\s*\|\|\s*verdict\s*===\s*['"]needs-remediation['"]/, + 'deriveStateFromDb should check !validationTerminal || verdict === "needs-remediation"'); + }); + + test('extractVerdict is called on validation content', () => { + const extractCalls = source.match(/extractVerdict\(validationContent\)/g); + assert.ok(extractCalls, 'extractVerdict should be called on validation content'); + assert.ok(extractCalls.length >= 2, + `Expected at least 2 extractVerdict calls, found ${extractCalls.length}`); + }); +}); From c829956ec386c92391ad3b7420646e6d71709e0d Mon Sep 17 00:00:00 2001 From: Tibsfox Date: Mon, 6 Apr 2026 22:37:52 -0700 Subject: [PATCH 3/3] fix(test): update needs-remediation test to expect validating-milestone phase The fix intentionally changed needs-remediation to route back to validating-milestone instead of completing-milestone. Update the test assertion to match the new expected behavior. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../extensions/gsd/tests/validate-milestone.test.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/resources/extensions/gsd/tests/validate-milestone.test.ts b/src/resources/extensions/gsd/tests/validate-milestone.test.ts index 9647ddb46..633862b83 100644 --- a/src/resources/extensions/gsd/tests/validate-milestone.test.ts +++ b/src/resources/extensions/gsd/tests/validate-milestone.test.ts @@ -163,16 +163,15 @@ test("deriveState returns completing-milestone when VALIDATION exists with termi } }); -test("deriveState treats needs-remediation as terminal — does not re-enter validating-milestone (#832)", async () => { +test("deriveState treats needs-remediation as non-terminal — re-enters validating-milestone (#832)", async () => { const base = makeTmpBase(); try { writeRoadmap(base, "M001", ALL_DONE_ROADMAP); writeValidation(base, "M001", "---\nverdict: needs-remediation\nremediation_round: 0\n---\n\n# Validation\nNeeds fixes."); const state = await deriveState(base); - // needs-remediation is now terminal — milestone needs a SUMMARY to be fully complete - // Without SUMMARY, it enters completing-milestone (not validating-milestone) - assert.notEqual(state.phase, "validating-milestone"); + // needs-remediation routes back to validating-milestone for re-validation + assert.equal(state.phase, "validating-milestone"); assert.equal(state.activeMilestone?.id, "M001"); } finally { cleanup(base);