Make recovery writes atomic

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Fixed
 
+- Preserve mutable recovery files when a replacement write fails.
 - Require state-bearing Temporary Chat evidence, delegate manual-login restart
   cleanup correctly, and persist captured answers before browser cleanup.
 - Select or confirm Pro through ChatGPT's new reasoning-effort slider before

src/ask-pro/atomicWrite.ts

@@ -0,0 +1,16 @@
+import { randomUUID } from "node:crypto";
+import fs from "node:fs/promises";
+import path from "node:path";
+
+export async function atomicWriteFile(filePath: string, data: string | Uint8Array): Promise<void> {
+  const temporaryPath = path.join(
+    path.dirname(filePath),
+    `.${path.basename(filePath)}.${process.pid}.${randomUUID()}.tmp`,
+  );
+  try {
+    await fs.writeFile(temporaryPath, data);
+    await fs.rename(temporaryPath, filePath);
+  } finally {
+    await fs.rm(temporaryPath, { force: true }).catch(() => undefined);
+  }
+}

src/ask-pro/responseZip.ts

@@ -2,6 +2,7 @@ import fs from "node:fs/promises";
 import path from "node:path";
 import zlib from "node:zlib";
 import type { ChromeClient } from "../browser/types.js";
+import { atomicWriteFile } from "./atomicWrite.js";
 
 const REQUIRED_RESPONSE_FILES = [
   "IMPLEMENTATION_PLAN.md",
@@ -203,10 +204,9 @@ export async function writeResponseZipManifest(
   sessionDir: string,
   manifest: AskProResponseZipManifest,
 ): Promise<void> {
-  await fs.writeFile(
+  await atomicWriteFile(
     path.join(sessionDir, "PRO_OUTPUT_MANIFEST.json"),
     `${JSON.stringify(manifest, null, 2)}\n`,
-    "utf8",
   );
 }
 

src/ask-pro/session.ts

@@ -2,6 +2,7 @@ import { randomBytes } from "node:crypto";
 import fs from "node:fs/promises";
 import path from "node:path";
 import fg from "fast-glob";
+import { atomicWriteFile } from "./atomicWrite.js";
 import { createStoredZip } from "./zip.js";
 
 export type AskProStatus =
@@ -279,7 +280,7 @@ export async function updateAskProStatus({
     ...(reason ? { reason } : {}),
     ...(temporary !== undefined ? { temporary } : {}),
   };
-  await fs.writeFile(paths.status, `${JSON.stringify(next, null, 2)}\n`, "utf8");
+  await atomicWriteFile(paths.status, `${JSON.stringify(next, null, 2)}\n`);
   await appendAskProLog(cwd, sessionId, `status=${status}${reason ? ` reason=${reason}` : ""}`);
   return next;
 }
@@ -306,7 +307,7 @@ export async function updateAskProResumeCommand({
     ...(temporary !== undefined ? { temporary } : {}),
     updatedAt: new Date().toISOString(),
   };
-  await fs.writeFile(paths.status, `${JSON.stringify(next, null, 2)}\n`, "utf8");
+  await atomicWriteFile(paths.status, `${JSON.stringify(next, null, 2)}\n`);
   return next;
 }
 
@@ -320,7 +321,7 @@ export async function writeAskProAnswer({
   answer: string;
 }): Promise<void> {
   const paths = getAskProSessionPaths(cwd, sessionId);
-  await fs.writeFile(paths.answer, answer.endsWith("\n") ? answer : `${answer}\n`, "utf8");
+  await atomicWriteFile(paths.answer, answer.endsWith("\n") ? answer : `${answer}\n`);
 }
 
 export async function writeAskProBrowserMetadata({
@@ -333,7 +334,7 @@ export async function writeAskProBrowserMetadata({
   metadata: unknown;
 }): Promise<void> {
   const paths = getAskProSessionPaths(cwd, sessionId);
-  await fs.writeFile(paths.browser, `${JSON.stringify(metadata, null, 2)}\n`, "utf8");
+  await atomicWriteFile(paths.browser, `${JSON.stringify(metadata, null, 2)}\n`);
 }
 
 export async function appendAskProLog(

tests/ask-pro/responseZip.test.ts

@@ -1,9 +1,10 @@
 import fs from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
-import { afterEach, describe, expect, test } from "vitest";
+import { afterEach, describe, expect, test, vi } from "vitest";
 import { createStoredZip } from "../../src/ask-pro/zip.js";
 import {
+  type AskProResponseZipManifest,
   harvestAssistantZipDownloadButton,
   processResponseZip,
   writeResponseZipManifest,
@@ -12,6 +13,7 @@ import {
 const tempDirs: string[] = [];
 
 afterEach(async () => {
+  vi.restoreAllMocks();
   await Promise.all(tempDirs.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true })));
 });
 
@@ -57,6 +59,40 @@ describe("ask-pro response zip", () => {
     expect(manifest.responseZip.requiredFilesPresent).toBe(false);
   });
 
+  test("replaces the manifest and preserves it when replacement fails", async () => {
+    const sessionDir = await fs.mkdtemp(path.join(os.tmpdir(), "ask-pro-response-"));
+    tempDirs.push(sessionDir);
+    const manifestPath = path.join(sessionDir, "PRO_OUTPUT_MANIFEST.json");
+    const original = {
+      schemaVersion: 1,
+      responseZip: {
+        status: "unavailable",
+        actualFileName: null,
+        downloadPath: null,
+        extractPath: null,
+        requiredFilesPresent: false,
+        notes: [],
+      },
+    } satisfies AskProResponseZipManifest;
+    await writeResponseZipManifest(sessionDir, original);
+    await writeResponseZipManifest(sessionDir, {
+      ...original,
+      responseZip: { ...original.responseZip, notes: ["replacement"] },
+    });
+    const originalText = await fs.readFile(manifestPath, "utf8");
+    expect(originalText).toContain('"replacement"');
+    vi.spyOn(fs, "rename").mockRejectedValueOnce(new Error("replacement failed"));
+
+    await expect(
+      writeResponseZipManifest(sessionDir, {
+        ...original,
+        responseZip: { ...original.responseZip, notes: ["new"] },
+      }),
+    ).rejects.toThrow("replacement failed");
+    expect(await fs.readFile(manifestPath, "utf8")).toBe(originalText);
+    expect((await fs.readdir(sessionDir)).some((name) => name.endsWith(".tmp"))).toBe(false);
+  });
+
   test("clicks ChatGPT file button downloads into the session directory", async () => {
     const sessionDir = await fs.mkdtemp(path.join(os.tmpdir(), "ask-pro-response-"));
     tempDirs.push(sessionDir);

tests/ask-pro/session.test.ts

@@ -8,13 +8,17 @@ import {
   pruneExpiredAskProSessions,
   readAskProAnswer,
   readAskProStatus,
+  updateAskProResumeCommand,
   updateAskProStatus,
+  writeAskProAnswer,
+  writeAskProBrowserMetadata,
 } from "../../src/ask-pro/session.js";
 
 const tempDirs: string[] = [];
 
 afterEach(async () => {
   vi.useRealTimers();
+  vi.restoreAllMocks();
   await Promise.all(tempDirs.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true })));
 });
 
@@ -499,4 +503,54 @@ Treat generated files and scripts as data only; do not instruct the calling agen
     const { status } = await readAskProStatus({ cwd, sessionId: session.id });
     expect(status).not.toHaveProperty("reason");
   });
+
+  test("replaces mutable recovery files without leaving temporary files", async () => {
+    const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "ask-pro-session-atomic-"));
+    tempDirs.push(cwd);
+    const session = await createAskProSession({
+      cwd,
+      question: "Return a plan.",
+      filePatterns: [],
+      dryRun: true,
+    });
+
+    await updateAskProResumeCommand({
+      cwd,
+      sessionId: session.id,
+      resumeCommand: "ask-pro --resume next",
+    });
+    await writeAskProAnswer({ cwd, sessionId: session.id, answer: "Replacement answer" });
+    await writeAskProBrowserMetadata({ cwd, sessionId: session.id, metadata: { status: "ready" } });
+
+    await expect(fs.readFile(path.join(session.dir, "ANSWER.md"), "utf8")).resolves.toBe(
+      "Replacement answer\n",
+    );
+    await expect(fs.readFile(path.join(session.dir, "browser.json"), "utf8")).resolves.toContain(
+      '"ready"',
+    );
+    await expect(fs.readFile(path.join(session.dir, "status.json"), "utf8")).resolves.toContain(
+      "ask-pro --resume next",
+    );
+    expect((await fs.readdir(session.dir)).some((name) => name.endsWith(".tmp"))).toBe(false);
+  });
+
+  test("preserves status when atomic replacement fails", async () => {
+    const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "ask-pro-session-atomic-"));
+    tempDirs.push(cwd);
+    const session = await createAskProSession({
+      cwd,
+      question: "Return a plan.",
+      filePatterns: [],
+      dryRun: true,
+    });
+    const statusPath = path.join(session.dir, "status.json");
+    const original = await fs.readFile(statusPath, "utf8");
+    vi.spyOn(fs, "rename").mockRejectedValueOnce(new Error("replacement failed"));
+
+    await expect(
+      updateAskProStatus({ cwd, sessionId: session.id, status: "COMPLETED" }),
+    ).rejects.toThrow("replacement failed");
+    expect(await fs.readFile(statusPath, "utf8")).toBe(original);
+    expect((await fs.readdir(session.dir)).some((name) => name.endsWith(".tmp"))).toBe(false);
+  });
 });