From c602b244c6e06126f0c7c8b30a3581b2bfd9ade6 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 30 Mar 2026 16:29:39 -0400 Subject: [PATCH] fix: invalidate stale milestone validation on roadmap reassessment (#2957) (#3242) When roadmap reassessment adds/modifies/removes slices after a needs-remediation validation verdict, the prior milestone-validation DB row and VALIDATION.md file are now cleared. This forces deriveState() to return phase: 'validating-milestone' once the new slices complete, instead of dead-ending at 'completing-milestone' with a stale verdict. Co-authored-by: Claude Opus 4.6 --- .../gsd/tests/reassess-handler.test.ts | 117 ++++++++++++++++++ .../extensions/gsd/tools/reassess-roadmap.ts | 36 ++++++ 2 files changed, 153 insertions(+) diff --git a/src/resources/extensions/gsd/tests/reassess-handler.test.ts b/src/resources/extensions/gsd/tests/reassess-handler.test.ts index 38908433f..2f8e2aa36 100644 --- a/src/resources/extensions/gsd/tests/reassess-handler.test.ts +++ b/src/resources/extensions/gsd/tests/reassess-handler.test.ts @@ -9,6 +9,7 @@ import { closeDatabase, insertMilestone, insertSlice, + insertAssessment, getSlice, getMilestoneSlices, getAssessment, @@ -323,3 +324,119 @@ test('handleReassessRoadmap returns structured error payloads with actionable me cleanup(base); } }); + +// ─── Bug #2957: Stale VALIDATION survives roadmap remediation ──────────── + +test('handleReassessRoadmap invalidates stale milestone-validation when roadmap changes (#2957)', async () => { + const base = makeTmpBase(); + openDatabase(join(base, '.gsd', 'gsd.db')); + + try { + // Seed: M001 with S01-S04 all complete, plus a stale VALIDATION with needs-remediation + insertMilestone({ id: 'M001', title: 'Test Milestone', status: 'active' }); + insertSlice({ id: 'S01', milestoneId: 'M001', title: 'Slice One', status: 'complete', demo: 'Demo' }); + insertSlice({ id: 'S02', milestoneId: 'M001', title: 'Slice Two', status: 'complete', demo: 'Demo' }); + insertSlice({ id: 'S03', milestoneId: 'M001', title: 'Slice Three', status: 'complete', demo: 'Demo' }); + insertSlice({ id: 'S04', milestoneId: 'M001', title: 'Slice Four', status: 'complete', demo: 'Demo' }); + + // Insert milestone-validation assessment with needs-remediation verdict (stale) + const validationPath = join('.gsd', 'milestones', 'M001', 'M001-VALIDATION.md'); + insertAssessment({ + path: validationPath, + milestoneId: 'M001', + sliceId: null, + taskId: null, + status: 'needs-remediation', + scope: 'milestone-validation', + fullContent: '---\nverdict: needs-remediation\nremediation_round: 0\n---\n\n# Validation\nNeeds remediation.', + }); + + // Verify the validation row exists before reassess + const adapter = _getAdapter()!; + const before = adapter.prepare( + `SELECT * FROM assessments WHERE milestone_id = 'M001' AND scope = 'milestone-validation'`, + ).get() as Record | undefined; + assert.ok(before, 'milestone-validation row should exist before reassess'); + + // Now reassess the roadmap: add remediation slice S05 + // This simulates the scenario from #2957 where validation produced needs-remediation + // and then roadmap was reassessed to add a remediation slice + const result = await handleReassessRoadmap({ + milestoneId: 'M001', + completedSliceId: 'S04', + verdict: 'on-track', + assessment: 'S04 completed. Adding remediation slice S05.', + sliceChanges: { + modified: [], + added: [ + { + sliceId: 'S05', + title: 'Remediation Slice', + risk: 'low', + depends: ['S04'], + demo: 'Fix the issues found during validation.', + }, + ], + removed: [], + }, + }, base); + + assert.ok(!('error' in result), `unexpected error: ${'error' in result ? result.error : ''}`); + + // The stale milestone-validation row must be deleted after roadmap changes + const after = adapter.prepare( + `SELECT * FROM assessments WHERE milestone_id = 'M001' AND scope = 'milestone-validation'`, + ).get() as Record | undefined; + assert.equal(after, undefined, 'milestone-validation row should be deleted after roadmap changes — stale validation must not survive remediation (#2957)'); + } finally { + cleanup(base); + } +}); + +test('handleReassessRoadmap does NOT invalidate validation when no roadmap structural changes (#2957)', async () => { + const base = makeTmpBase(); + openDatabase(join(base, '.gsd', 'gsd.db')); + + try { + // Seed: M001 with slices, plus a validation with pass verdict + insertMilestone({ id: 'M001', title: 'Test Milestone', status: 'active' }); + insertSlice({ id: 'S01', milestoneId: 'M001', title: 'Slice One', status: 'complete', demo: 'Demo' }); + insertSlice({ id: 'S02', milestoneId: 'M001', title: 'Slice Two', status: 'pending', demo: 'Demo' }); + + // Insert milestone-validation assessment with pass verdict + const validationPath = join('.gsd', 'milestones', 'M001', 'M001-VALIDATION.md'); + insertAssessment({ + path: validationPath, + milestoneId: 'M001', + sliceId: null, + taskId: null, + status: 'pass', + scope: 'milestone-validation', + fullContent: '---\nverdict: pass\nremediation_round: 0\n---\n\n# Validation\nAll good.', + }); + + // Reassess with no structural changes (empty added/modified/removed) + const result = await handleReassessRoadmap({ + milestoneId: 'M001', + completedSliceId: 'S01', + verdict: 'confirmed', + assessment: 'S01 completed. No changes needed.', + sliceChanges: { + modified: [], + added: [], + removed: [], + }, + }, base); + + assert.ok(!('error' in result), `unexpected error: ${'error' in result ? result.error : ''}`); + + // Validation should still exist when no structural changes occurred + const adapter = _getAdapter()!; + const row = adapter.prepare( + `SELECT * FROM assessments WHERE milestone_id = 'M001' AND scope = 'milestone-validation'`, + ).get() as Record | undefined; + assert.ok(row, 'milestone-validation row should survive when no structural changes occurred'); + } finally { + cleanup(base); + } +}); diff --git a/src/resources/extensions/gsd/tools/reassess-roadmap.ts b/src/resources/extensions/gsd/tools/reassess-roadmap.ts index 040aacf56..933fabec5 100644 --- a/src/resources/extensions/gsd/tools/reassess-roadmap.ts +++ b/src/resources/extensions/gsd/tools/reassess-roadmap.ts @@ -1,4 +1,5 @@ import { join } from "node:path"; +import { existsSync, unlinkSync } from "node:fs"; import { clearParseCache } from "../files.js"; import { isClosedStatus } from "../status-guards.js"; import { isNonEmptyString } from "../validation.js"; @@ -10,6 +11,7 @@ import { insertSlice, updateSliceFields, insertAssessment, + deleteAssessmentByScope, deleteSlice, } from "../gsd-db.js"; import { invalidateStateCache } from "../state.js"; @@ -200,6 +202,21 @@ export async function handleReassessRoadmap( for (const removedId of params.sliceChanges.removed) { deleteSlice(params.milestoneId, removedId); } + + // ── Invalidate stale milestone validation (#2957) ────────────── + // When roadmap structure changes (slices added/modified/removed), + // any prior milestone-validation verdict is stale. Delete the DB + // row so deriveState() returns phase: 'validating-milestone' once + // the new slices complete, rather than advancing directly to + // 'completing-milestone' with a stale needs-remediation verdict. + const hasStructuralChanges = + params.sliceChanges.added.length > 0 || + params.sliceChanges.modified.length > 0 || + params.sliceChanges.removed.length > 0; + + if (hasStructuralChanges) { + deleteAssessmentByScope(params.milestoneId, "milestone-validation"); + } }); } catch (err) { return { error: `db write failed: ${(err as Error).message}` }; @@ -218,6 +235,25 @@ export async function handleReassessRoadmap( completedSliceId: params.completedSliceId, }); + // ── Remove stale VALIDATION file from disk (#2957) ──────────── + const hasStructuralChanges = + params.sliceChanges.added.length > 0 || + params.sliceChanges.modified.length > 0 || + params.sliceChanges.removed.length > 0; + + if (hasStructuralChanges) { + const validationFile = join( + basePath, ".gsd", "milestones", params.milestoneId, + `${params.milestoneId}-VALIDATION.md`, + ); + try { + if (existsSync(validationFile)) unlinkSync(validationFile); + } catch { + // Best-effort: DB row is already deleted, so state derivation + // will not see the file-based verdict as authoritative. + } + } + // ── Invalidate caches ───────────────────────────────────────── invalidateStateCache(); clearParseCache();