fix(gsd): write DB before disk in validate-milestone to match engine pattern (#2742)

* fix(gsd): write DB before disk in validate-milestone to match engine pattern

validate-milestone.ts wrote the VALIDATION.md file to disk before
inserting the assessment row into the DB. Every other handler in the
engine (complete-task, complete-slice) does DB-first, disk-second with
rollback compensation. The inverted order meant a crash between disk
write and DB insert would leave an orphaned file with no DB record —
a state that is harder to detect and recover from than the inverse
(DB row exists, file missing → projection rendering can regenerate).

Fix: reorder to DB-first, disk-second. On disk write failure, delete
the DB row via DELETE FROM assessments so state stays consistent.

Add two handler-level tests verifying:
1. Both DB row and disk file exist after success
2. DB row is rolled back (deleted) when disk write fails

Closes #2725

* fix(test): use file-as-directory to trigger disk failure cross-platform

chmod 0o444 does not prevent writes on Windows. Replace with
replacing the milestone directory with a regular file, so
saveFile's mkdirSync/write fails on all platforms.

Fixes windows-portability CI failure.

src/resources/extensions/gsd/tests/validate-milestone-write-order.test.ts

@@ -0,0 +1,90 @@
+import { describe, it, afterEach } from "node:test";
+import assert from "node:assert/strict";
+import { mkdirSync, existsSync, rmSync, writeFileSync } from "node:fs";
+import { join } from "node:path";
+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 { clearPathCache } from "../paths.js";
+import { clearParseCache } from "../files.js";
+
+function makeTmpBase(): string {
+  const base = join(tmpdir(), `gsd-val-handler-${randomUUID()}`);
+  mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true });
+  return base;
+}
+
+const VALID_PARAMS = {
+  milestoneId: "M001",
+  verdict: "pass" as const,
+  remediationRound: 0,
+  successCriteriaChecklist: "- [x] All pass",
+  sliceDeliveryAudit: "| S01 | delivered |",
+  crossSliceIntegration: "No issues",
+  requirementCoverage: "All covered",
+  verdictRationale: "Everything checks out",
+};
+
+describe("handleValidateMilestone write ordering (#2725)", () => {
+  let base: string;
+
+  afterEach(() => {
+    clearPathCache();
+    clearParseCache();
+    try { closeDatabase(); } catch { /* */ }
+    if (base) {
+      try { rmSync(base, { recursive: true, force: true }); } catch { /* */ }
+    }
+  });
+
+  it("writes DB row and disk file on success", async () => {
+    base = makeTmpBase();
+    const dbPath = join(base, ".gsd", "gsd.db");
+    openDatabase(dbPath);
+    insertMilestone({ id: "M001" });
+
+    const result = await handleValidateMilestone(VALID_PARAMS, base);
+    assert.ok(!("error" in result), `unexpected error: ${"error" in result ? result.error : ""}`);
+
+    // DB row exists
+    const adapter = _getAdapter()!;
+    const row = adapter.prepare(
+      `SELECT status, scope FROM assessments WHERE milestone_id = 'M001' AND scope = 'milestone-validation'`,
+    ).get() as { status: string; scope: string } | undefined;
+    assert.ok(row, "assessment row should exist in DB");
+    assert.equal(row!.status, "pass");
+
+    // Disk file exists
+    const filePath = join(base, ".gsd", "milestones", "M001", "M001-VALIDATION.md");
+    assert.ok(existsSync(filePath), "VALIDATION.md should exist on disk");
+  });
+
+  it("rolls back DB row when disk write fails", async () => {
+    base = makeTmpBase();
+    const dbPath = join(base, ".gsd", "gsd.db");
+    openDatabase(dbPath);
+    insertMilestone({ id: "M001" });
+
+    // Force disk write failure by replacing the milestone directory with a
+    // regular file. saveFile() will fail because it cannot write inside a
+    // non-directory. This works cross-platform (chmod is ignored on Windows).
+    const milestoneDir = join(base, ".gsd", "milestones", "M001");
+    rmSync(milestoneDir, { recursive: true, force: true });
+    writeFileSync(milestoneDir, "not-a-directory");
+
+    const result = await handleValidateMilestone(VALID_PARAMS, base);
+
+    // Should return error
+    assert.ok("error" in result, "should return error when disk write fails");
+    assert.ok(result.error.includes("disk render failed"));
+
+    // DB row should have been rolled back (deleted)
+    const adapter = _getAdapter()!;
+    const row = adapter.prepare(
+      `SELECT * FROM assessments WHERE milestone_id = 'M001' AND scope = 'milestone-validation'`,
+    ).get();
+    assert.equal(row, undefined, "assessment row should be deleted after disk-write rollback");
+  });
+});

src/resources/extensions/gsd/tools/validate-milestone.ts

@@ -76,7 +76,7 @@ export async function handleValidateMilestone(
     return { error: `verdict must be one of: ${VALIDATION_VERDICTS.join(", ")}` };
   }
 
-  // ── Filesystem render ──────────────────────────────────────────────────
+  // ── Resolve paths and render markdown ────────────────────────────────
   const validationMd = renderValidationMarkdown(params);
 
   let validationPath: string;
@@ -89,16 +89,11 @@ export async function handleValidateMilestone(
     validationPath = join(manualDir, `${params.milestoneId}-VALIDATION.md`);
   }
 
-  try {
-    await saveFile(validationPath, validationMd);
-  } catch (renderErr) {
-    process.stderr.write(
-      `gsd-db: validate_milestone — disk render failed: ${(renderErr as Error).message}\n`,
-    );
-    return { error: `disk render failed: ${(renderErr as Error).message}` };
-  }
-
-  // ── DB write — store in assessments table ──────────────────────────────
+  // ── DB write first — matches complete-task/complete-slice pattern ───
+  // Write DB before disk so a crash between the two leaves a recoverable
+  // state: the DB row exists but the file is missing, which projection
+  // rendering can regenerate. The inverse (file exists, no DB row) is
+  // harder to detect and recover from (#2725).
   const validatedAt = new Date().toISOString();
 
   transaction(() => {
@@ -115,6 +110,23 @@ export async function handleValidateMilestone(
     });
   });
 
+  // ── Filesystem render (outside transaction) ────────────────────────────
+  // If disk render fails, roll back the DB row so state stays consistent.
+  try {
+    await saveFile(validationPath, validationMd);
+  } catch (renderErr) {
+    process.stderr.write(
+      `gsd-db: validate_milestone — disk render failed, rolling back DB row: ${(renderErr as Error).message}\n`,
+    );
+    const rollbackAdapter = _getAdapter();
+    if (rollbackAdapter) {
+      rollbackAdapter.prepare(
+        `DELETE FROM assessments WHERE milestone_id = :mid AND scope = 'milestone-validation'`,
+      ).run({ ":mid": params.milestoneId });
+    }
+    return { error: `disk render failed: ${(renderErr as Error).message}` };
+  }
+
   invalidateStateCache();
   clearPathCache();
   clearParseCache();