Merge pull request #3722 from jeremymcs/fix/state-machine-wave1-critical

fix(gsd): critical state machine data integrity (wave 1/5)

src/resources/extensions/gsd/state.ts

@@ -189,7 +189,7 @@ export async function getActiveMilestoneId(basePath: string): Promise<string | n
       const byId = new Map(allMilestones.map(m => [m.id, m]));
       for (const id of sortedIds) {
         const m = byId.get(id)!;
-        if (m.status === "complete" || m.status === "done" || m.status === "parked") continue;
+        if (isClosedStatus(m.status) || m.status === "parked") continue;
         return m.id;
       }
       return null;
@@ -442,13 +442,10 @@ export async function deriveStateFromDb(basePath: string): Promise<GSDState> {
       continue;
     }
 
-    // Check roadmap: all slices done means milestone is complete
-    const slices = getMilestoneSlices(m.id);
-    if (slices.length > 0 && slices.every(s => isStatusDone(s.status))) {
-      // All slices done but no summary — still counts as complete for dep resolution
-      // if a summary file exists
-      // Note: without summary file, the milestone is in validating/completing state, not complete
-    }
+    // Milestones with all slices done but no SUMMARY file are in
+    // validating/completing state — intentionally NOT added to
+    // completeMilestoneIds.  The SUMMARY file (checked above) is the
+    // terminal artifact that proves completion per #864.
   }
 
   // Phase 2: Build registry and find active milestone
@@ -954,7 +951,12 @@ export async function deriveStateFromDb(basePath: string): Promise<GSDState> {
   // ── REPLAN-TRIGGER detection ─────────────────────────────────────────
   if (!blockerTaskId) {
     const sliceRow = getSlice(activeMilestone.id, activeSlice.id);
-    if (sliceRow?.replan_triggered_at) {
+    // Check DB column first, fall back to disk trigger file when DB write
+    // was best-effort and failed (triage-resolution.ts dual-write gap).
+    const dbTriggered = !!sliceRow?.replan_triggered_at;
+    const diskTriggered = !dbTriggered &&
+      !!resolveSliceFile(basePath, activeMilestone.id, activeSlice.id, "REPLAN-TRIGGER");
+    if (dbTriggered || diskTriggered) {
       // Loop protection: if replan_history has entries, replan was already done
       const replanHistory = getReplanHistory(activeMilestone.id, activeSlice.id);
       if (replanHistory.length === 0) {

src/resources/extensions/gsd/tests/wave1-critical-regressions.test.ts

@@ -0,0 +1,49 @@
+// GSD State Machine — Wave 1 Critical Regression Tests
+// Validates fixes for event log format mismatch, skipped milestone status,
+// dead code removal, and replan disk-file fallback.
+
+import { describe, test } from "node:test";
+import assert from "node:assert/strict";
+import { extractEntityKey } from "../workflow-reconcile.js";
+import { isClosedStatus } from "../status-guards.js";
+import type { WorkflowEvent } from "../workflow-events.js";
+
+// ── Fix 1: Event log cmd format — hyphens and underscores both accepted ──
+
+describe("extractEntityKey normalizes cmd format", () => {
+  const baseEvent = { params: {}, ts: "", hash: "", actor: "agent" as const, session_id: "" };
+
+  test("accepts hyphenated complete-task", () => {
+    const event: WorkflowEvent = { ...baseEvent, cmd: "complete-task", params: { taskId: "T01" } };
+    const key = extractEntityKey(event);
+    assert.deepStrictEqual(key, { type: "task", id: "T01" });
+  });
+
+  test("accepts underscored complete_task (legacy)", () => {
+    const event: WorkflowEvent = { ...baseEvent, cmd: "complete_task", params: { taskId: "T01" } };
+    const key = extractEntityKey(event);
+    assert.deepStrictEqual(key, { type: "task", id: "T01" });
+  });
+
+  test("accepts hyphenated complete-slice", () => {
+    const event: WorkflowEvent = { ...baseEvent, cmd: "complete-slice", params: { sliceId: "S01" } };
+    const key = extractEntityKey(event);
+    assert.deepStrictEqual(key, { type: "slice", id: "S01" });
+  });
+
+  test("accepts hyphenated complete-milestone", () => {
+    const event: WorkflowEvent = { ...baseEvent, cmd: "complete-milestone", params: { milestoneId: "M001" } };
+    const key = extractEntityKey(event);
+    assert.deepStrictEqual(key, { type: "milestone", id: "M001" });
+  });
+});
+
+// ── Fix 3: getActiveMilestoneId must skip "skipped" milestones ──
+
+describe("isClosedStatus includes skipped", () => {
+  test("complete is closed", () => assert.ok(isClosedStatus("complete")));
+  test("done is closed", () => assert.ok(isClosedStatus("done")));
+  test("skipped is closed", () => assert.ok(isClosedStatus("skipped")));
+  test("pending is not closed", () => assert.ok(!isClosedStatus("pending")));
+  test("active is not closed", () => assert.ok(!isClosedStatus("active")));
+});

src/resources/extensions/gsd/workflow-events.ts

@@ -19,7 +19,7 @@ export function getSessionId(): string {
 // ─── Event Types ─────────────────────────────────────────────────────────
 
 export interface WorkflowEvent {
-  cmd: string;           // e.g. "complete_task"
+  cmd: string;           // e.g. "complete-task" (canonical: hyphens; legacy: underscores — both accepted by replay)
   params: Record<string, unknown>;
   ts: string;            // ISO 8601
   hash: string;          // content hash (hex, 16 chars)

src/resources/extensions/gsd/workflow-reconcile.ts

@@ -7,7 +7,9 @@ import {
   transaction,
   updateTaskStatus,
   updateSliceStatus,
+  updateMilestoneStatus,
   getSliceTasks,
+  getMilestoneSlices,
   insertVerificationEvidence,
   upsertDecision,
   openDatabase,
@@ -74,7 +76,15 @@ function replayEvents(events: WorkflowEvent[]): void {
   transaction(() => {
   for (const event of events) {
     const p = event.params;
-    switch (event.cmd) {
+    // Normalize cmd format: completion tools write hyphens ("complete-task"),
+    // legacy logs use underscores ("complete_task"). Accept both formats.
+    // Type guard: malformed event lines with non-string cmd are skipped.
+    if (typeof event.cmd !== "string") {
+      logWarning("reconcile", `Event with non-string cmd skipped: ${JSON.stringify(event.cmd)}`);
+      continue;
+    }
+    const cmd = event.cmd.replace(/-/g, "_");
+    switch (cmd) {
       case "complete_task": {
         const milestoneId = p["milestoneId"] as string;
         const sliceId = p["sliceId"] as string;
@@ -119,6 +129,21 @@ function replayEvents(events: WorkflowEvent[]): void {
         replaySliceComplete(milestoneId, sliceId, event.ts);
         break;
       }
+      case "complete_milestone": {
+        const milestoneId = p["milestoneId"] as string;
+        if (!milestoneId) break;
+        // Invariant check: only mark complete if all slices are closed.
+        // Without this guard, a reordered/partial event stream could close
+        // a milestone while work is still incomplete.
+        const mSlices = getMilestoneSlices(milestoneId);
+        const allClosed = mSlices.length === 0 || mSlices.every(s => isClosedStatus(s.status));
+        if (allClosed) {
+          updateMilestoneStatus(milestoneId, "complete", event.ts);
+        } else {
+          logWarning("reconcile", `Skipping complete_milestone replay for ${milestoneId}: not all slices are closed`);
+        }
+        break;
+      }
       case "plan_slice": {
         // plan_slice events are informational — slice should already exist.
         // No DB mutation needed during replay (the slice was inserted at plan time).
@@ -139,7 +164,7 @@ function replayEvents(events: WorkflowEvent[]): void {
         break;
       }
       default:
-        // Unknown commands are silently skipped during replay
+        logWarning("reconcile", `Unknown event cmd during replay: "${event.cmd}" — skipped`);
         break;
     }
   }
@@ -157,8 +182,11 @@ export function extractEntityKey(
   event: WorkflowEvent,
 ): { type: string; id: string } | null {
   const p = event.params;
+  // Normalize cmd format: accept both hyphens and underscores
+  if (typeof event.cmd !== "string") return null;
+  const cmd = event.cmd.replace(/-/g, "_");
 
-  switch (event.cmd) {
+  switch (cmd) {
     case "complete_task":
     case "start_task":
     case "report_blocker":
@@ -172,6 +200,11 @@ export function extractEntityKey(
         ? { type: "slice", id: p["sliceId"] }
         : null;
 
+    case "complete_milestone":
+      return typeof p["milestoneId"] === "string"
+        ? { type: "milestone", id: p["milestoneId"] }
+        : null;
+
     case "plan_slice":
       return typeof p["sliceId"] === "string"
         ? { type: "slice_plan", id: p["sliceId"] }