* fix: resolve 4 state corruption bugs in milestone/slice completion workflow Closes #2945 Bug 1 - ROADMAP corrupted by inline UAT content: renderRoadmapContent and renderPlanContent used slice.full_uat_md as a fallback when the demo field was empty. This injected multi-line UAT content (preconditions, steps, expected results) into table cells, corrupting the markdown table and making subsequent slices invisible to the parser. Fix: use "TBD" fallback instead of full_uat_md. Bug 2 - complete-milestone accepts pending slices via event replay: workflow-reconcile's replayEvents blindly called updateSliceStatus("done") for complete_slice events without validating that all tasks in the slice were actually complete. During API overload or partial execution, this allowed slices with pending tasks to be marked done, which then let complete-milestone succeed. Fix: extract replaySliceComplete function that validates task completion before updating slice status. Bug 3 - Worktree directory not cleaned up after merge: WorktreeResolver._mergeWorktreeMode delegated worktree cleanup to mergeMilestoneToMain's internal best-effort removeWorktree call, which can silently fail. Fix: add secondary teardownAutoWorktree call after successful merge to ensure cleanup. Bug 4 - Quality gate records not written by validate-milestone: handleValidateMilestone wrote to the assessments table and rendered VALIDATION.md to disk, but never persisted quality_gates records in the DB. Fix: insert milestone-level quality gates (MV01-MV04) alongside the assessment record. Extended GateScope and GateId types to support milestone-level validation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: align test type literals with MilestoneRow, SliceRow, and AutoSession Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: update tests for removed full_summary_md fallback and FK constraints - workflow-projections: renderPlanContent test now expects TBD fallback instead of full_summary_md (removed in #2945 to prevent corruption) - validate-milestone: insert slice rows before validation so quality_gates FK constraint (milestone_id, slice_id) is satisfied - worktree-resolver: update teardownAutoWorktree assertion from 0 to 1 to account for secondary cleanup added in #2945 Bug 3 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
GitHub
parent
a02b140f61
commit
4327b4bb3f
10 changed files with 550 additions and 12 deletions
56
src/resources/extensions/gsd/milestone-validation-gates.ts
Normal file
56
src/resources/extensions/gsd/milestone-validation-gates.ts
Normal file
|
|
@ -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,
|
||||
});
|
||||
}
|
||||
}
|
||||
405
src/resources/extensions/gsd/tests/state-corruption-2945.test.ts
Normal file
405
src/resources/extensions/gsd/tests/state-corruption-2945.test.ts
Normal file
|
|
@ -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> = {}): 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> = {}): 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<string, unknown>) => g["verdict"] && g["verdict"] !== "");
|
||||
assert.ok(
|
||||
withVerdict.length > 0,
|
||||
"at least one quality_gate should have a recorded verdict",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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")));
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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) ────────────────────────────
|
||||
|
|
|
|||
|
|
@ -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" | "";
|
||||
|
||||
|
|
|
|||
|
|
@ -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} |`);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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": {
|
||||
|
|
|
|||
|
|
@ -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." : ""}`,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue