Merge pull request #3726 from jeremymcs/fix/state-machine-wave4-writes

fix(gsd): write safety — atomic writes and randomized tmp (wave 4/5)

src/resources/extensions/gsd/json-persistence.ts

@@ -1,5 +1,6 @@
-import { existsSync, readFileSync, writeFileSync, mkdirSync, renameSync } from "node:fs";
+import { existsSync, readFileSync, writeFileSync, mkdirSync, renameSync, unlinkSync } from "node:fs";
 import { dirname } from "node:path";
+import { randomBytes } from "node:crypto";
 
 /**
  * Load a JSON file with validation, returning a default on failure.
@@ -51,9 +52,11 @@ export function loadJsonFileOrNull<T>(
 export function saveJsonFile<T>(filePath: string, data: T): void {
   try {
     mkdirSync(dirname(filePath), { recursive: true });
-    const tmp = filePath + ".tmp";
+    // Use randomized tmp suffix to prevent concurrent-write data loss
+    const tmp = `${filePath}.tmp.${randomBytes(4).toString("hex")}`;
     writeFileSync(tmp, JSON.stringify(data, null, 2) + "\n", "utf-8");
     renameSync(tmp, filePath);
+    // No cleanup needed — renameSync atomically removes tmp on success
   } catch {
     // Non-fatal — don't let persistence failures break operation
   }
@@ -66,7 +69,7 @@ export function saveJsonFile<T>(filePath: string, data: T): void {
 export function writeJsonFileAtomic<T>(filePath: string, data: T): void {
   try {
     mkdirSync(dirname(filePath), { recursive: true });
-    const tmp = filePath + ".tmp";
+    const tmp = `${filePath}.tmp.${randomBytes(4).toString("hex")}`;
     writeFileSync(tmp, JSON.stringify(data, null, 2), "utf-8");
     renameSync(tmp, filePath);
   } catch {

src/resources/extensions/gsd/tests/wave4-write-safety-regressions.test.ts

@@ -0,0 +1,70 @@
+// GSD State Machine — Wave 4 Write Safety Regression Tests
+// Validates randomized tmp suffix in json-persistence and atomic writes.
+
+import { describe, test } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, readFileSync, readdirSync, rmSync } from "node:fs";
+import { join } from "node:path";
+import { tmpdir } from "node:os";
+import { saveJsonFile, loadJsonFile } from "../json-persistence.js";
+
+// ── Fix 15: json-persistence uses randomized tmp suffix ──
+
+describe("saveJsonFile atomic write", () => {
+  test("writes JSON file correctly", () => {
+    const tmp = mkdtempSync(join(tmpdir(), "gsd-json-test-"));
+    try {
+      const file = join(tmp, "test.json");
+      saveJsonFile(file, { key: "value" });
+      const content = JSON.parse(readFileSync(file, "utf-8"));
+      assert.deepStrictEqual(content, { key: "value" });
+    } finally {
+      rmSync(tmp, { recursive: true, force: true });
+    }
+  });
+
+  test("no .tmp file left after successful write", () => {
+    const tmp = mkdtempSync(join(tmpdir(), "gsd-json-test-"));
+    try {
+      const file = join(tmp, "test.json");
+      saveJsonFile(file, { data: 123 });
+      const files = readdirSync(tmp);
+      const tmpFiles = files.filter((f: string) => f.includes(".tmp"));
+      assert.strictEqual(tmpFiles.length, 0, "No .tmp files should remain after write");
+    } finally {
+      rmSync(tmp, { recursive: true, force: true });
+    }
+  });
+
+  test("concurrent writes don't corrupt data", () => {
+    const tmp = mkdtempSync(join(tmpdir(), "gsd-json-test-"));
+    try {
+      const file = join(tmp, "shared.json");
+      // Write two different values rapidly — both should succeed without corruption
+      saveJsonFile(file, { writer: "first" });
+      saveJsonFile(file, { writer: "second" });
+      const content = JSON.parse(readFileSync(file, "utf-8"));
+      assert.strictEqual(content.writer, "second");
+    } finally {
+      rmSync(tmp, { recursive: true, force: true });
+    }
+  });
+
+  test("round-trip through loadJsonFile", () => {
+    const tmp = mkdtempSync(join(tmpdir(), "gsd-json-test-"));
+    try {
+      const file = join(tmp, "roundtrip.json");
+      const data = { items: [1, 2, 3], name: "test" };
+      saveJsonFile(file, data);
+      const loaded = loadJsonFile(
+        file,
+        (d): d is typeof data => typeof d === "object" && d !== null && "items" in d,
+        () => ({ items: [], name: "" }),
+      );
+      assert.deepStrictEqual(loaded.items, [1, 2, 3]);
+      assert.strictEqual(loaded.name, "test");
+    } finally {
+      rmSync(tmp, { recursive: true, force: true });
+    }
+  });
+});

src/resources/extensions/gsd/triage-resolution.ts

@@ -10,7 +10,8 @@
  * Also provides detectFileOverlap() for surfacing downstream impact on quick tasks.
  */
 
-import { existsSync, mkdirSync, readFileSync, writeFileSync, unlinkSync } from "node:fs";
+import { existsSync, mkdirSync, readFileSync, unlinkSync } from "node:fs";
+import { atomicWriteSync } from "./atomic-write.js";
 import { join } from "node:path";
 import { createRequire } from "node:module";
 import { gsdRoot, milestonesDir } from "./paths.js";
@@ -65,10 +66,10 @@ export function executeInject(
     const filesSection = content.indexOf("## Files Likely Touched");
     if (filesSection !== -1) {
       const updated = content.slice(0, filesSection) + newTask + "\n\n" + content.slice(filesSection);
-      writeFileSync(planPath, updated, "utf-8");
+      atomicWriteSync(planPath, updated, "utf-8");
     } else {
       // No Files section — append at end
-      writeFileSync(planPath, content.trimEnd() + "\n\n" + newTask + "\n", "utf-8");
+      atomicWriteSync(planPath, content.trimEnd() + "\n\n" + newTask + "\n", "utf-8");
     }
 
     return newId;
@@ -105,7 +106,7 @@ export function executeReplan(
       `will detect it and enter the replanning-slice phase.`,
     ].join("\n");
 
-    writeFileSync(triggerPath, content, "utf-8");
+    atomicWriteSync(triggerPath, content, "utf-8");
 
     // Also write replan_triggered_at column for DB-backed detection
     try {
@@ -183,7 +184,7 @@ export function executeBacktrack(
       `3. Resume auto-mode — the state machine will re-enter discussion for the target`,
     ].join("\n");
 
-    writeFileSync(triggerPath, content, "utf-8");
+    atomicWriteSync(triggerPath, content, "utf-8");
 
     // If we have a valid target, also reset that milestone's completion status
     // so deriveState() will re-enter it as the active milestone.
@@ -194,7 +195,7 @@ export function executeBacktrack(
           // Write a regression marker so the state machine knows this milestone
           // needs re-discussion, not just re-execution
           const regressionPath = join(targetDir, `${targetMilestoneId}-REGRESSION.md`);
-          writeFileSync(regressionPath, [
+          atomicWriteSync(regressionPath, [
             `# Milestone Regression`,
             ``,
             `**From:** ${currentMilestoneId}`,
@@ -361,7 +362,7 @@ export function ensureDeferMilestoneDir(
       ``,
     ].join("\n");
 
-    writeFileSync(
+    atomicWriteSync(
       join(msDir, `${targetMilestone}-CONTEXT-DRAFT.md`),
       draftContent,
       "utf-8",

src/resources/extensions/gsd/undo.ts

@@ -4,9 +4,10 @@
 // handleResetSlice: Reset a slice and all its tasks, re-rendering plan + roadmap.
 
 import type { ExtensionCommandContext, ExtensionAPI } from "@gsd/pi-coding-agent";
-import { existsSync, readFileSync, writeFileSync, unlinkSync, readdirSync } from "node:fs";
+import { existsSync, readFileSync, unlinkSync, readdirSync } from "node:fs";
 import { join, basename } from "node:path";
 import { nativeRevertCommit, nativeRevertAbort } from "./native-git-bridge.js";
+import { atomicWriteSync } from "./atomic-write.js";
 import { parseUnitId } from "./unit-id.js";
 import { deriveState } from "./state.js";
 import { invalidateAllCaches } from "./cache.js";
@@ -393,7 +394,7 @@ export function uncheckTaskInPlan(basePath: string, mid: string, sid: string, ti
   const regex = new RegExp(`^(\\s*-\\s*)\\[x\\](\\s*\\**${tid}\\**[:\\s])`, "mi");
   if (regex.test(content)) {
     content = content.replace(regex, "$1[ ]$2");
-    writeFileSync(planFile, content, "utf-8");
+    atomicWriteSync(planFile, content);
     return true;
   }
   return false;