diff --git a/src/resources/extensions/gsd/milestone-validation-gates.ts b/src/resources/extensions/gsd/milestone-validation-gates.ts new file mode 100644 index 000000000..4dcd522b6 --- /dev/null +++ b/src/resources/extensions/gsd/milestone-validation-gates.ts @@ -0,0 +1,56 @@ +/** + * Milestone validation quality gate persistence. + * + * #2945 Bug 4: validate-milestone was writing VALIDATION.md to disk and + * inserting an assessment row, but never persisted structured quality_gates + * records in the DB. This module inserts milestone-level validation gates + * that correspond to the validation checks performed. + * + * Gate IDs for milestone validation: + * MV01 — Success criteria checklist + * MV02 — Slice delivery audit + * MV03 — Cross-slice integration + * MV04 — Requirement coverage + * + * These use the existing quality_gates table with scope "milestone". + */ + +import { _getAdapter } from "./gsd-db.js"; + +/** Milestone validation gate IDs. */ +const MILESTONE_GATE_IDS = ["MV01", "MV02", "MV03", "MV04"] as const; + +/** + * Insert milestone-level quality_gates records for a validation run. + * + * Each gate is inserted with status "complete" and a verdict derived + * from the overall milestone validation verdict. Individual gate-level + * verdicts are not available (the handler receives a single verdict), + * so all gates share the overall verdict. + */ +export function insertMilestoneValidationGates( + milestoneId: string, + sliceId: string, + verdict: string, + evaluatedAt: string, +): void { + const db = _getAdapter(); + if (!db) return; + + const gateVerdict = verdict === "pass" ? "pass" : "flag"; + + for (const gateId of MILESTONE_GATE_IDS) { + db.prepare( + `INSERT OR REPLACE INTO quality_gates + (milestone_id, slice_id, gate_id, scope, task_id, status, verdict, rationale, findings, evaluated_at) + VALUES (:mid, :sid, :gid, 'milestone', '', 'complete', :verdict, :rationale, '', :evaluated_at)`, + ).run({ + ":mid": milestoneId, + ":sid": sliceId, + ":gid": gateId, + ":verdict": gateVerdict, + ":rationale": `Milestone validation verdict: ${verdict}`, + ":evaluated_at": evaluatedAt, + }); + } +} diff --git a/src/resources/extensions/gsd/tests/state-corruption-2945.test.ts b/src/resources/extensions/gsd/tests/state-corruption-2945.test.ts new file mode 100644 index 000000000..a7da901bc --- /dev/null +++ b/src/resources/extensions/gsd/tests/state-corruption-2945.test.ts @@ -0,0 +1,405 @@ +/** + * Regression tests for issue #2945: State corruption in milestone/slice completion workflow. + * + * Covers all 4 sub-bugs: + * Bug 1: ROADMAP corrupted by inline UAT content in table rows + * Bug 2: complete-milestone event replay bypasses task validation + * Bug 3: Worktree directory not cleaned up after mergeAndExit + * Bug 4: Quality gate records not written by validate-milestone + */ + +import { describe, test, beforeEach, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync, existsSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { + openDatabase, + closeDatabase, + insertMilestone, + insertSlice, + insertTask, + getMilestoneSlices, + getSliceTasks, + getGateResults, +} from "../gsd-db.ts"; +import { renderRoadmapContent } from "../workflow-projections.ts"; +import type { MilestoneRow, SliceRow } from "../gsd-db.ts"; +import type { AutoSession } from "../auto/session.ts"; + +// ─── Fixture helpers ──────────────────────────────────────────────────────── + +function tempDbPath(): string { + const dir = mkdtempSync(join(tmpdir(), "gsd-2945-")); + return join(dir, "test.db"); +} + +function cleanupDb(dbPath: string): void { + closeDatabase(); + try { + const dir = join(dbPath, ".."); + rmSync(dir, { recursive: true, force: true }); + } catch { + // best effort + } +} + +function createTempProject(): { basePath: string } { + const basePath = mkdtempSync(join(tmpdir(), "gsd-2945-project-")); + mkdirSync(join(basePath, ".gsd", "milestones", "M001"), { recursive: true }); + return { basePath }; +} + +function makeMilestoneRow(overrides: Partial = {}): MilestoneRow { + return { + id: "M001", + title: "Test Milestone", + vision: "Build a test milestone", + status: "active", + depends_on: [], + created_at: new Date().toISOString(), + completed_at: null, + success_criteria: ["SC1", "SC2"], + key_risks: [], + proof_strategy: [], + verification_contract: "", + verification_integration: "", + verification_operational: "", + verification_uat: "", + definition_of_done: [], + requirement_coverage: "", + boundary_map_markdown: "", + ...overrides, + }; +} + +function makeSliceRow(id: string, overrides: Partial = {}): SliceRow { + return { + id, + milestone_id: "M001", + title: `Slice ${id}`, + goal: `Goal for ${id}`, + demo: `Demo for ${id}`, + risk: "medium", + status: "pending", + sequence: parseInt(id.replace("S", ""), 10) || 0, + depends: [], + created_at: new Date().toISOString(), + completed_at: null, + full_summary_md: "", + full_uat_md: "", + success_criteria: "", + proof_level: "", + integration_closure: "", + observability_impact: "", + replan_triggered_at: null, + ...overrides, + }; +} + +// ═══════════════════════════════════════════════════════════════════════════════ +// Bug 1: ROADMAP corrupted by inline UAT content +// ═══════════════════════════════════════════════════════════════════════════════ + +describe("#2945 Bug 1: ROADMAP table cell corruption by UAT content", () => { + + test("renderRoadmapContent does NOT inject full_uat_md into table rows when demo is empty", () => { + const milestone = makeMilestoneRow(); + + const longUatContent = `### Preconditions +- Database initialized +- Service running + +### Steps +1. Open the application +2. Navigate to settings +3. Enable dark mode + +### Expected +- Theme changes to dark +- All components update`; + + const slices: SliceRow[] = [ + makeSliceRow("S01", { + status: "complete", + demo: "", // empty demo + full_uat_md: longUatContent, // full UAT content in DB + }), + makeSliceRow("S02", { + status: "pending", + demo: "Advanced stuff works", + }), + ]; + + const content = renderRoadmapContent(milestone, slices); + + // The roadmap table row for S01 should NOT contain UAT content + assert.ok( + !content.includes("Preconditions"), + "roadmap table row must not contain UAT preconditions", + ); + assert.ok( + !content.includes("Navigate to settings"), + "roadmap table row must not contain UAT steps", + ); + + // Each table row should be a reasonable length (under 200 chars) + const lines = content.split("\n"); + const s01Row = lines.find(l => l.includes("| S01 |")); + assert.ok(s01Row, "S01 should appear as a table row"); + assert.ok( + s01Row!.length < 200, + `S01 row should be under 200 chars, got ${s01Row!.length}: ${s01Row!.slice(0, 100)}...`, + ); + + // S02 should still be visible + assert.ok(content.includes("| S02 |"), "S02 must still be visible in roadmap table"); + }); + + test("renderRoadmapContent uses 'TBD' fallback when demo is empty, not full_uat_md", () => { + const milestone = makeMilestoneRow(); + const slices: SliceRow[] = [ + makeSliceRow("S01", { demo: "", full_uat_md: "Long UAT content here" }), + ]; + + const content = renderRoadmapContent(milestone, slices); + assert.ok( + content.includes("TBD"), + "empty demo should fallback to 'TBD', not full_uat_md", + ); + assert.ok( + !content.includes("Long UAT content here"), + "full_uat_md should never appear in roadmap table", + ); + }); + + test("renderRoadmapContent preserves demo field when present", () => { + const milestone = makeMilestoneRow(); + const slices: SliceRow[] = [ + makeSliceRow("S01", { demo: "Basic functionality works", full_uat_md: "Full UAT" }), + ]; + + const content = renderRoadmapContent(milestone, slices); + assert.ok( + content.includes("Basic functionality works"), + "demo field should be used when present", + ); + assert.ok( + !content.includes("Full UAT"), + "full_uat_md should not be used when demo is present", + ); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════════ +// Bug 2: complete-milestone event replay bypasses task validation +// ═══════════════════════════════════════════════════════════════════════════════ + +describe("#2945 Bug 2: workflow-reconcile bypasses task validation for complete_slice", () => { + let dbPath: string; + + beforeEach(() => { + dbPath = tempDbPath(); + openDatabase(dbPath); + }); + + afterEach(() => { + cleanupDb(dbPath); + }); + + test("replaySliceComplete must not mark slice done when tasks are pending", async () => { + // Set up: M001 with S01 that has 2 tasks, one pending + insertMilestone({ id: "M001" }); + insertSlice({ id: "S01", milestoneId: "M001" }); + insertTask({ id: "T01", sliceId: "S01", milestoneId: "M001", status: "complete", title: "Done task" }); + insertTask({ id: "T02", sliceId: "S01", milestoneId: "M001", status: "pending", title: "Pending task" }); + + // Import and call replaySliceComplete directly + const { replaySliceComplete } = await import("../workflow-reconcile.ts"); + replaySliceComplete("M001", "S01", new Date().toISOString()); + + // The slice should NOT be marked done because T02 is still pending + const slices = getMilestoneSlices("M001"); + const s01 = slices.find(s => s.id === "S01"); + assert.ok(s01, "S01 should exist"); + assert.notStrictEqual( + s01!.status, + "done", + "replaySliceComplete must not mark slice as done when tasks are pending", + ); + assert.notStrictEqual( + s01!.status, + "complete", + "replaySliceComplete must not mark slice as complete when tasks are pending", + ); + }); + + test("replaySliceComplete marks slice done when all tasks are complete", async () => { + insertMilestone({ id: "M001" }); + insertSlice({ id: "S01", milestoneId: "M001" }); + insertTask({ id: "T01", sliceId: "S01", milestoneId: "M001", status: "complete", title: "Done task" }); + insertTask({ id: "T02", sliceId: "S01", milestoneId: "M001", status: "done", title: "Also done" }); + + const { replaySliceComplete } = await import("../workflow-reconcile.ts"); + replaySliceComplete("M001", "S01", new Date().toISOString()); + + const slices = getMilestoneSlices("M001"); + const s01 = slices.find(s => s.id === "S01"); + assert.ok(s01, "S01 should exist"); + assert.strictEqual( + s01!.status, + "done", + "replaySliceComplete should mark slice as done when all tasks are complete", + ); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════════ +// Bug 3: Worktree directory not cleaned up after mergeAndExit +// ═══════════════════════════════════════════════════════════════════════════════ + +describe("#2945 Bug 3: mergeAndExit must teardown worktree after successful merge", () => { + + test("_mergeWorktreeMode calls teardownAutoWorktree after successful merge", async () => { + // Test the WorktreeResolver to verify teardown is called after merge. + // We use a mock-based approach since actual worktrees require a git repo. + let teardownCalled = false; + let teardownMilestoneId = ""; + + const mockSession = { + basePath: "/mock/worktree/M001", + originalBasePath: "/mock/project", + isolationDegraded: false, + gitService: {} as unknown, + } as unknown as AutoSession; + + const mockDeps = { + isInAutoWorktree: () => true, + shouldUseWorktreeIsolation: () => true, + getIsolationMode: () => "worktree" as const, + mergeMilestoneToMain: () => ({ pushed: false, codeFilesChanged: true }), + syncWorktreeStateBack: () => ({ synced: [] }), + teardownAutoWorktree: (basePath: string, mid: string) => { + teardownCalled = true; + teardownMilestoneId = mid; + }, + createAutoWorktree: () => "", + enterAutoWorktree: () => "", + getAutoWorktreePath: () => null, + autoCommitCurrentBranch: () => {}, + getCurrentBranch: () => "main", + autoWorktreeBranch: () => "gsd/M001", + resolveMilestoneFile: () => "/mock/roadmap.md", + readFileSync: () => "# Roadmap content", + GitServiceImpl: class {} as unknown as new (p: string, c: unknown) => unknown, + loadEffectiveGSDPreferences: () => undefined, + invalidateAllCaches: () => {}, + captureIntegrationBranch: () => {}, + }; + + // Import and create resolver + // We test the behavior contract: after a successful merge, teardown must be called + const { WorktreeResolver } = await import("../worktree-resolver.ts"); + const resolver = new WorktreeResolver(mockSession, mockDeps); + + const ctx = { notify: () => {} }; + resolver.mergeAndExit("M001", ctx); + + assert.ok( + teardownCalled, + "teardownAutoWorktree must be called after successful merge in worktree mode", + ); + assert.strictEqual( + teardownMilestoneId, + "M001", + "teardown must be called with the correct milestone ID", + ); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════════ +// Bug 4: Quality gate records not written by validate-milestone +// ═══════════════════════════════════════════════════════════════════════════════ + +describe("#2945 Bug 4: validate-milestone must persist quality_gates records", () => { + let dbPath: string; + let basePath: string; + + beforeEach(() => { + dbPath = tempDbPath(); + openDatabase(dbPath); + const proj = createTempProject(); + basePath = proj.basePath; + }); + + afterEach(() => { + cleanupDb(dbPath); + try { rmSync(basePath, { recursive: true, force: true }); } catch {} + }); + + test("handleValidateMilestone persists quality_gates records in DB", async () => { + // Set up milestone with slices + insertMilestone({ id: "M001" }); + insertSlice({ id: "S01", milestoneId: "M001" }); + + const { handleValidateMilestone } = await import("../tools/validate-milestone.ts"); + + const result = await handleValidateMilestone({ + milestoneId: "M001", + verdict: "pass", + remediationRound: 0, + successCriteriaChecklist: "- [x] SC1 met\n- [x] SC2 met", + sliceDeliveryAudit: "All slices delivered", + crossSliceIntegration: "Integration verified", + requirementCoverage: "100% coverage", + verdictRationale: "All checks pass", + }, basePath); + + assert.ok(!("error" in result), `handler should succeed, got: ${JSON.stringify(result)}`); + + // Quality gate records should exist in DB for this milestone + // Use a wildcard slice_id since milestone-level gates use a sentinel + const adapter = (await import("../gsd-db.ts"))._getAdapter()!; + const gates = adapter.prepare( + "SELECT * FROM quality_gates WHERE milestone_id = 'M001'" + ).all(); + + assert.ok( + gates.length > 0, + `validate-milestone must persist quality_gates records in DB, found ${gates.length}`, + ); + }); + + test("handleValidateMilestone records verdict correctly in quality_gates", async () => { + insertMilestone({ id: "M001" }); + insertSlice({ id: "S01", milestoneId: "M001" }); + + const { handleValidateMilestone } = await import("../tools/validate-milestone.ts"); + + await handleValidateMilestone({ + milestoneId: "M001", + verdict: "needs-remediation", + remediationRound: 1, + successCriteriaChecklist: "- [ ] SC1 not met", + sliceDeliveryAudit: "S01 incomplete", + crossSliceIntegration: "Not tested", + requirementCoverage: "50% coverage", + verdictRationale: "Needs work", + remediationPlan: "Fix S01", + }, basePath); + + const adapter = (await import("../gsd-db.ts"))._getAdapter()!; + const gates = adapter.prepare( + "SELECT * FROM quality_gates WHERE milestone_id = 'M001'" + ).all(); + + assert.ok(gates.length > 0, "quality_gates records must exist"); + + // At least one gate should have a non-empty verdict + const withVerdict = gates.filter((g: Record) => g["verdict"] && g["verdict"] !== ""); + assert.ok( + withVerdict.length > 0, + "at least one quality_gate should have a recorded verdict", + ); + }); +}); diff --git a/src/resources/extensions/gsd/tests/validate-milestone-write-order.test.ts b/src/resources/extensions/gsd/tests/validate-milestone-write-order.test.ts index 120751b60..1f07791e0 100644 --- a/src/resources/extensions/gsd/tests/validate-milestone-write-order.test.ts +++ b/src/resources/extensions/gsd/tests/validate-milestone-write-order.test.ts @@ -6,7 +6,7 @@ import { tmpdir } from "node:os"; import { randomUUID } from "node:crypto"; import { handleValidateMilestone } from "../tools/validate-milestone.js"; -import { openDatabase, closeDatabase, _getAdapter, insertMilestone } from "../gsd-db.js"; +import { openDatabase, closeDatabase, _getAdapter, insertMilestone, insertSlice } from "../gsd-db.js"; import { clearPathCache } from "../paths.js"; import { clearParseCache } from "../files.js"; @@ -45,6 +45,7 @@ describe("handleValidateMilestone write ordering (#2725)", () => { const dbPath = join(base, ".gsd", "gsd.db"); openDatabase(dbPath); insertMilestone({ id: "M001" }); + insertSlice({ id: "S01", milestoneId: "M001" }); const result = await handleValidateMilestone(VALID_PARAMS, base); assert.ok(!("error" in result), `unexpected error: ${"error" in result ? result.error : ""}`); @@ -71,6 +72,7 @@ describe("handleValidateMilestone write ordering (#2725)", () => { const dbPath = join(base, ".gsd", "gsd.db"); openDatabase(dbPath); insertMilestone({ id: "M001" }); + insertSlice({ id: "S01", milestoneId: "M001" }); const result = await handleValidateMilestone( { ...VALID_PARAMS, verificationClasses: undefined }, @@ -88,6 +90,7 @@ describe("handleValidateMilestone write ordering (#2725)", () => { const dbPath = join(base, ".gsd", "gsd.db"); openDatabase(dbPath); insertMilestone({ id: "M001" }); + insertSlice({ id: "S01", milestoneId: "M001" }); // Force disk write failure by replacing the milestone directory with a // regular file. saveFile() will fail because it cannot write inside a diff --git a/src/resources/extensions/gsd/tests/workflow-projections.test.ts b/src/resources/extensions/gsd/tests/workflow-projections.test.ts index cf21052e2..b9379ede8 100644 --- a/src/resources/extensions/gsd/tests/workflow-projections.test.ts +++ b/src/resources/extensions/gsd/tests/workflow-projections.test.ts @@ -86,10 +86,12 @@ test('workflow-projections: renderPlanContent falls back to TBD when goal and fu assert.ok(content.includes('**Goal:** TBD')); }); -test('workflow-projections: renderPlanContent falls back to full_summary_md when goal is empty', () => { +test('workflow-projections: renderPlanContent falls back to TBD when goal is empty (full_summary_md ignored #2945)', () => { const slice = makeSlice({ goal: '', full_summary_md: 'Fallback goal text' }); const content = renderPlanContent(slice, []); - assert.ok(content.includes('**Goal:** Fallback goal text')); + // #2945: full_summary_md is no longer used as a fallback — it contains + // multi-line rendered markdown that corrupts single-line fields. + assert.ok(content.includes('**Goal:** TBD'), `expected TBD fallback, got: ${content}`); }); test('workflow-projections: renderPlanContent includes ## Tasks section', () => { diff --git a/src/resources/extensions/gsd/tests/worktree-resolver.test.ts b/src/resources/extensions/gsd/tests/worktree-resolver.test.ts index c3a7f7aba..d227d6efb 100644 --- a/src/resources/extensions/gsd/tests/worktree-resolver.test.ts +++ b/src/resources/extensions/gsd/tests/worktree-resolver.test.ts @@ -481,7 +481,8 @@ test("mergeAndExit resolves roadmap from worktree when missing at project root ( // Should have called mergeMilestoneToMain, not bare teardown assert.equal(findCalls(deps.calls, "mergeMilestoneToMain").length, 1); - assert.equal(findCalls(deps.calls, "teardownAutoWorktree").length, 0); + // #2945 Bug 3: secondary teardown is now called after merge for cleanup + assert.equal(findCalls(deps.calls, "teardownAutoWorktree").length, 1); assert.equal(s.basePath, "/project"); // restored assert.ok(ctx.messages.some((m) => m.msg.includes("merged to main"))); }); diff --git a/src/resources/extensions/gsd/tools/validate-milestone.ts b/src/resources/extensions/gsd/tools/validate-milestone.ts index 305b75c06..5e3f57ee4 100644 --- a/src/resources/extensions/gsd/tools/validate-milestone.ts +++ b/src/resources/extensions/gsd/tools/validate-milestone.ts @@ -1,8 +1,12 @@ /** * validate-milestone handler — the core operation behind gsd_validate_milestone. * - * Persists milestone validation results to the assessments table, - * renders VALIDATION.md to disk, and invalidates caches. + * Persists milestone validation results to the assessments table and + * quality_gates table, renders VALIDATION.md to disk, and invalidates caches. + * + * #2945 Bug 4: Previously only wrote to assessments — quality_gates records + * were never persisted, causing M002+ milestones to have zero gate records + * despite passing validation. */ import { join } from "node:path"; @@ -11,11 +15,13 @@ import { transaction, insertAssessment, deleteAssessmentByScope, + getMilestoneSlices, } from "../gsd-db.js"; import { resolveMilestonePath, clearPathCache } from "../paths.js"; import { saveFile, clearParseCache } from "../files.js"; import { invalidateStateCache } from "../state.js"; import { VALIDATION_VERDICTS, isValidMilestoneVerdict } from "../verdict-parser.js"; +import { insertMilestoneValidationGates } from "../milestone-validation-gates.js"; export interface ValidateMilestoneParams { milestoneId: string; @@ -112,6 +118,18 @@ export async function handleValidateMilestone( scope: 'milestone-validation', fullContent: validationMd, }); + + // #2945 Bug 4: persist quality_gates records alongside the assessment. + // Previously only the assessment was written, leaving M002+ milestones + // with zero quality_gate records despite passing validation. + const slices = getMilestoneSlices(params.milestoneId); + const sliceId = slices.length > 0 ? slices[0].id : "_milestone"; + insertMilestoneValidationGates( + params.milestoneId, + sliceId, + params.verdict, + validatedAt, + ); }); // ── Filesystem render (outside transaction) ──────────────────────────── diff --git a/src/resources/extensions/gsd/types.ts b/src/resources/extensions/gsd/types.ts index dc5250372..25bee6774 100644 --- a/src/resources/extensions/gsd/types.ts +++ b/src/resources/extensions/gsd/types.ts @@ -565,8 +565,8 @@ export interface CompleteSliceParams { // ─── Quality Gates ─────────────────────────────────────────────────────── -export type GateId = "Q3" | "Q4" | "Q5" | "Q6" | "Q7" | "Q8"; -export type GateScope = "slice" | "task"; +export type GateId = "Q3" | "Q4" | "Q5" | "Q6" | "Q7" | "Q8" | "MV01" | "MV02" | "MV03" | "MV04"; +export type GateScope = "slice" | "task" | "milestone"; export type GateStatus = "pending" | "complete" | "omitted"; export type GateVerdict = "pass" | "flag" | "omitted" | ""; diff --git a/src/resources/extensions/gsd/workflow-projections.ts b/src/resources/extensions/gsd/workflow-projections.ts index aae3adcb9..848f70376 100644 --- a/src/resources/extensions/gsd/workflow-projections.ts +++ b/src/resources/extensions/gsd/workflow-projections.ts @@ -29,8 +29,10 @@ export function renderPlanContent(sliceRow: SliceRow, taskRows: TaskRow[]): stri lines.push(`# ${sliceRow.id}: ${sliceRow.title}`); lines.push(""); - lines.push(`**Goal:** ${sliceRow.goal || sliceRow.full_summary_md || "TBD"}`); - lines.push(`**Demo:** After this: ${sliceRow.demo || sliceRow.full_uat_md || "TBD"}`); + // #2945: never use full_summary_md/full_uat_md as display fallbacks — + // they contain multi-line rendered markdown that corrupts single-line fields. + lines.push(`**Goal:** ${sliceRow.goal || "TBD"}`); + lines.push(`**Demo:** After this: ${sliceRow.demo || "TBD"}`); lines.push(""); lines.push("## Tasks"); @@ -113,7 +115,10 @@ export function renderRoadmapContent(milestoneRow: MilestoneRow, sliceRows: Slic } const risk = (slice.risk || "low").toLowerCase(); - const demo = slice.demo || slice.full_uat_md || "TBD"; + // #2945 Bug 1: never use full_uat_md as a table cell fallback — it contains + // multi-line UAT content (preconditions, steps, expected results) that + // corrupts the markdown table and makes subsequent slices invisible. + const demo = slice.demo || "TBD"; lines.push(`| ${slice.id} | ${slice.title} | ${risk} | ${depends} | ${done} | ${demo} |`); } diff --git a/src/resources/extensions/gsd/workflow-reconcile.ts b/src/resources/extensions/gsd/workflow-reconcile.ts index 4704501b0..763f93037 100644 --- a/src/resources/extensions/gsd/workflow-reconcile.ts +++ b/src/resources/extensions/gsd/workflow-reconcile.ts @@ -6,14 +6,47 @@ import { transaction, updateTaskStatus, updateSliceStatus, + getSliceTasks, insertVerificationEvidence, upsertDecision, openDatabase, } from "./gsd-db.js"; +import { isClosedStatus } from "./status-guards.js"; import { writeManifest } from "./workflow-manifest.js"; import { atomicWriteSync } from "./atomic-write.js"; import { acquireSyncLock, releaseSyncLock } from "./sync-lock.js"; +// ─── Replay Helpers ────────────────────────────────────────────────────────── + +/** + * Replay a complete_slice event with task validation. + * + * #2945 Bug 2: The original replay blindly called updateSliceStatus("done") + * without checking whether all tasks in the slice are actually complete. + * During API overload or partial execution, a complete_slice event could + * be logged even when tasks were skipped, causing the milestone completion + * guard to see the slice as "done" and allow premature milestone completion. + * + * This function validates that every task in the slice has a closed status + * before marking the slice as done. If any task is still pending, the slice + * status is left unchanged. + */ +export function replaySliceComplete(milestoneId: string, sliceId: string, ts: string): void { + const tasks = getSliceTasks(milestoneId, sliceId); + // If there are tasks and any are not closed, skip the status update + if (tasks.length > 0) { + const incompleteTasks = tasks.filter(t => !isClosedStatus(t.status)); + if (incompleteTasks.length > 0) { + process.stderr.write( + `[gsd] reconcile: skipping complete_slice replay for ${sliceId} — ` + + `${incompleteTasks.length} task(s) still pending\n`, + ); + return; + } + } + updateSliceStatus(milestoneId, sliceId, "done", ts); +} + // ─── Public Types ───────────────────────────────────────────────────────────── export interface ConflictEntry { @@ -82,7 +115,8 @@ function replayEvents(events: WorkflowEvent[]): void { case "complete_slice": { const milestoneId = p["milestoneId"] as string; const sliceId = p["sliceId"] as string; - updateSliceStatus(milestoneId, sliceId, "done", event.ts); + // #2945 Bug 2: validate tasks before marking slice done + replaySliceComplete(milestoneId, sliceId, event.ts); break; } case "plan_slice": { diff --git a/src/resources/extensions/gsd/worktree-resolver.ts b/src/resources/extensions/gsd/worktree-resolver.ts index c84d44656..6c459ba67 100644 --- a/src/resources/extensions/gsd/worktree-resolver.ts +++ b/src/resources/extensions/gsd/worktree-resolver.ts @@ -432,6 +432,20 @@ export class WorktreeResolver { milestoneId, roadmapContent, ); + + // #2945 Bug 3: mergeMilestoneToMain performs best-effort worktree + // cleanup internally (step 12), but it can silently fail on Windows + // or when the worktree directory is locked. Perform a secondary + // teardown here to ensure the worktree is properly cleaned up. + // This is idempotent — if the worktree was already removed, + // teardownAutoWorktree handles the no-op case gracefully. + try { + this.deps.teardownAutoWorktree(originalBase, milestoneId); + } catch { + // Best-effort — the primary cleanup in mergeMilestoneToMain may + // have already removed the worktree. + } + if (mergeResult.codeFilesChanged) { ctx.notify( `Milestone ${milestoneId} merged to main.${mergeResult.pushed ? " Pushed to remote." : ""}`,