fix(snapshot): write the snapshot through a staging file renamed into place

A write that failed partway (disk full, an interrupted run) truncated a previously valid snapshot at the destination while the run reported failed, so the documented backup was lost. The document now goes to a sibling <path>.tmp and is renamed over the destination, which is atomic for a regular file; the staging file is removed when the write fails, and a cleanup that fails itself does not replace the write's error. Both destination forms write through the one helper. Tests fail the staging write and read the previous snapshot back intact, and fail the rename under the dir form to see the staging file gone and the other target written.

Author: Vivswan

src/flows/snapshot.ts

@@ -10,7 +10,7 @@
  * on disk while nothing about it is printed.
  */
 
-import { mkdirSync, writeFileSync } from "node:fs";
+import { mkdirSync, renameSync, rmSync, writeFileSync } from "node:fs";
 import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path";
 import { err, ok, type Result, ResultAsync } from "neverthrow";
 import type { RepoRef, Target } from "../discovery/targets.js";
@@ -228,8 +228,7 @@ async function snapshotTarget(ctx: {
     };
   }
   try {
-    mkdirSync(dirname(path), { recursive: true });
-    writeFileSync(
+    writeReplacing(
       path,
       renderSnapshotYaml(result, { schemaUrl: SNAPSHOT_SCHEMA_URL, timestamp: ctx.timestamp }),
     );
@@ -253,6 +252,28 @@ async function snapshotTarget(ctx: {
   };
 }
 
+/**
+ * Write `text` to `path` through a sibling staging file renamed into place, so
+ * a write that fails partway (disk full, an interrupted run) leaves the
+ * previous snapshot at `path` intact instead of a truncated one; the rename is
+ * atomic for a regular file on POSIX and Windows alike. The staging file is
+ * removed when the write fails.
+ */
+function writeReplacing(path: string, text: string): void {
+  mkdirSync(dirname(path), { recursive: true });
+  const staging = `${path}.tmp`;
+  try {
+    writeFileSync(staging, text);
+    renameSync(staging, path);
+  } catch (error) {
+    // The write's error is the one reported: a directory at the staging path fails both the write and this rm.
+    try {
+      rmSync(staging, { force: true });
+    } catch {}
+    throw error;
+  }
+}
+
 /**
  * A target's file under the snapshot directory, in the repos-dir layout so the
  * directory can later serve as one. SLUG_RE admits "." and "..", which GitHub

test/flows/snapshot.test.ts

@@ -10,7 +10,7 @@
 import { afterEach, beforeEach, describe, expect, test } from "bun:test";
 import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
-import { join } from "node:path";
+import { dirname, join } from "node:path";
 import { parse as parseYaml } from "yaml";
 import { parseRepoSlug } from "../../src/discovery/targets.js";
 import { SectionSelection } from "../../src/engine/section-selection.js";
@@ -241,6 +241,73 @@ function disjointRefusal(snapshotDir: string, reposDir: string): string {
   );
 }
 
+describe("runSnapshot writes through a staging file", () => {
+  const unwritable = (path: string, input: "snapshot-file" | "snapshot-dir", os: string) => ({
+    level: "error" as const,
+    line: `cannot write the snapshot to ${path}: Error: ${os}. Check that the "${input}" input names a writable path`,
+  });
+
+  test("a staging write that fails leaves the previous snapshot intact and reports the write's error", async () => {
+    const api = new MockApi(labelsRoute("o/r", [BUG]));
+    const cfg = fileCfg();
+    const staging = `${cfg.snapshotFile}.tmp`;
+    mkdirSync(dirname(cfg.snapshotFile), { recursive: true });
+    writeFileSync(cfg.snapshotFile, "labels: []\n");
+    // A directory at the staging path fails the write before the rename; the run must not remove it either.
+    mkdirSync(staging);
+    const collected = collectingIo();
+    expect(await run(api, cfg, collected.io)).toBe(1);
+    expect(collected.outputs.result).toBe("failed");
+    expect(collected.lines[0]).toEqual(
+      unwritable(
+        cfg.snapshotFile,
+        "snapshot-file",
+        `EISDIR: illegal operation on a directory, open '${staging}'`,
+      ),
+    );
+    expect(readFileSync(cfg.snapshotFile, "utf8")).toBe("labels: []\n");
+    rmSync(staging, { recursive: true });
+    expect(await run(api, cfg, collectingIo().io)).toBe(0);
+    expect(parseYaml(readFileSync(cfg.snapshotFile, "utf8"))).toEqual(doc(BUG));
+    expect(existsSync(staging)).toBe(false);
+  });
+
+  test("a rename that fails removes the staging file, fails only that target, and leaves the destination as it was", async () => {
+    const api = new MockApi({
+      "GET /repos/o/a": { data: { private: false } },
+      "GET /repos/o/b": { data: { private: false } },
+      ...labelsRoute("o/a", [BUG]),
+      ...labelsRoute("o/b", [DOCS]),
+    });
+    const cfg = dirCfg();
+    const fileA = join(cfg.snapshotDir, "o", "a.yml");
+    const fileB = join(cfg.snapshotDir, "o", "b.yml");
+    // A directory holding a file at o/a's destination lets the staging write succeed and the rename fail.
+    mkdirSync(fileA, { recursive: true });
+    writeFileSync(join(fileA, "keep"), "authored\n");
+    const collected = collectingIo();
+    expect(await run(api, cfg, collected.io)).toBe(1);
+    expect(collected.outputs).toEqual({
+      "skipped-sections": "",
+      result: "failed",
+      "repos-result": JSON.stringify({
+        "o/a": { result: "failed", source: "remote", skippedSections: [] },
+        "o/b": { result: "snapshot", source: "remote", skippedSections: [] },
+      }),
+    });
+    const { level, line } = unwritable(
+      fileA,
+      "snapshot-dir",
+      `EISDIR: illegal operation on a directory, rename '${fileA}.tmp' -> '${fileA}'`,
+    );
+    expect(collected.lines[0]).toEqual({ level, line: `o/a: ${line}` });
+    expect(existsSync(`${fileA}.tmp`)).toBe(false);
+    expect(readFileSync(join(fileA, "keep"), "utf8")).toBe("authored\n");
+    expect(parseYaml(readFileSync(fileB, "utf8"))).toEqual(doc(DOCS));
+    expect(existsSync(`${fileB}.tmp`)).toBe(false);
+  });
+});
+
 describe("runSnapshot refuses a destination that would overwrite an authored file", () => {
   test.each([
     [