infra: atomically replace sync JSON writes (#60589)

gumadeiras · web-flow · commit 300fb36879c2 · 2026-04-03T20:21:44.000-04:00
Merged via squash. Prepared head SHA: cb8ed77 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -104,6 +104,7 @@ Docs: https://docs.openclaw.ai
 - WhatsApp/watchdog: reset watchdog timeout after reconnect so quiet channels no longer enter a tight reconnect loop from stale message timestamps carried across connection runs. (#60007) Thanks @MonkeyLeeT.
 - Agents/fallback: persist selected fallback overrides before retry attempts start, prefer persisted overrides during live-session reconciliation, and keep provider-scoped auth-profile failover from snapping retries back to stale primary selections.
 - Agents/MCP: sort MCP tools deterministically by name so the tools block in API requests is stable across turns, preventing unnecessary prompt-cache busting from non-deterministic `listTools()` order. (#58037) Thanks @bcherny.
+- Infra/json-file: preserve symlink-backed JSON stores and Windows overwrite fallback when atomically saving small sync JSON state files. (#60589) Thanks @gumadeiras.
 
 ## 2026.4.2
 
diff --git a/src/infra/json-file.test.ts b/src/infra/json-file.test.ts
@@ -1,9 +1,20 @@
 import fs from "node:fs";
 import path from "node:path";
-import { describe, expect, it } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
 import { withTempDir } from "../test-helpers/temp-dir.js";
 import { loadJsonFile, saveJsonFile } from "./json-file.js";
 
+const SAVED_PAYLOAD = { enabled: true, count: 2 };
+const PREVIOUS_JSON = '{"enabled":false}\n';
+
+function escapeRegExp(value: string): string {
+  return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
+}
+
+function writeExistingJson(pathname: string) {
+  fs.writeFileSync(pathname, PREVIOUS_JSON, "utf8");
+}
+
 async function withJsonPath<T>(
   run: (params: { root: string; pathname: string }) => Promise<T> | T,
 ): Promise<T> {
@@ -13,6 +24,10 @@ async function withJsonPath<T>(
 }
 
 describe("json-file helpers", () => {
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
   it.each([
     {
       name: "missing files",
@@ -40,11 +55,11 @@ describe("json-file helpers", () => {
   it("creates parent dirs, writes a trailing newline, and loads the saved object", async () => {
     await withTempDir({ prefix: "openclaw-json-file-" }, async (root) => {
       const pathname = path.join(root, "nested", "config.json");
-      saveJsonFile(pathname, { enabled: true, count: 2 });
+      saveJsonFile(pathname, SAVED_PAYLOAD);
 
       const raw = fs.readFileSync(pathname, "utf8");
       expect(raw.endsWith("\n")).toBe(true);
-      expect(loadJsonFile(pathname)).toEqual({ enabled: true, count: 2 });
+      expect(loadJsonFile(pathname)).toEqual(SAVED_PAYLOAD);
 
       const fileMode = fs.statSync(pathname).mode & 0o777;
       const dirMode = fs.statSync(path.dirname(pathname)).mode & 0o777;
@@ -64,15 +79,103 @@ describe("json-file helpers", () => {
     },
     {
       name: "existing JSON files",
-      setup: (pathname: string) => {
-        fs.writeFileSync(pathname, '{"enabled":false}\n', "utf8");
-      },
+      setup: writeExistingJson,
     },
   ])("writes the latest payload for $name", async ({ setup }) => {
     await withJsonPath(({ pathname }) => {
       setup(pathname);
-      saveJsonFile(pathname, { enabled: true, count: 2 });
-      expect(loadJsonFile(pathname)).toEqual({ enabled: true, count: 2 });
+      saveJsonFile(pathname, SAVED_PAYLOAD);
+      expect(loadJsonFile(pathname)).toEqual(SAVED_PAYLOAD);
+    });
+  });
+
+  it("writes through a sibling temp file before replacing the destination", async () => {
+    await withJsonPath(({ pathname }) => {
+      writeExistingJson(pathname);
+      const renameSpy = vi.spyOn(fs, "renameSync");
+
+      saveJsonFile(pathname, SAVED_PAYLOAD);
+
+      const renameCall = renameSpy.mock.calls.find(([, target]) => target === pathname);
+      expect(renameCall?.[0]).toMatch(new RegExp(`^${escapeRegExp(pathname)}\\..+\\.tmp$`));
+      expect(renameSpy).toHaveBeenCalledWith(renameCall?.[0], pathname);
+      expect(loadJsonFile(pathname)).toEqual(SAVED_PAYLOAD);
+    });
+  });
+
+  it.runIf(process.platform !== "win32")(
+    "preserves symlink destinations when replacing existing JSON files",
+    async () => {
+      await withTempDir({ prefix: "openclaw-json-file-" }, async (root) => {
+        const targetDir = path.join(root, "target");
+        const targetPath = path.join(targetDir, "config.json");
+        const linkPath = path.join(root, "config-link.json");
+        fs.mkdirSync(targetDir, { recursive: true });
+        writeExistingJson(targetPath);
+        fs.symlinkSync(targetPath, linkPath);
+
+        saveJsonFile(linkPath, SAVED_PAYLOAD);
+
+        expect(fs.lstatSync(linkPath).isSymbolicLink()).toBe(true);
+        expect(loadJsonFile(targetPath)).toEqual(SAVED_PAYLOAD);
+        expect(loadJsonFile(linkPath)).toEqual(SAVED_PAYLOAD);
+      });
+    },
+  );
+
+  it.runIf(process.platform !== "win32")(
+    "creates a missing target file through an existing symlink",
+    async () => {
+      await withTempDir({ prefix: "openclaw-json-file-" }, async (root) => {
+        const targetDir = path.join(root, "target");
+        const targetPath = path.join(targetDir, "config.json");
+        const linkPath = path.join(root, "config-link.json");
+        fs.mkdirSync(targetDir, { recursive: true });
+        fs.symlinkSync(targetPath, linkPath);
+
+        saveJsonFile(linkPath, SAVED_PAYLOAD);
+
+        expect(fs.lstatSync(linkPath).isSymbolicLink()).toBe(true);
+        expect(loadJsonFile(targetPath)).toEqual(SAVED_PAYLOAD);
+        expect(loadJsonFile(linkPath)).toEqual(SAVED_PAYLOAD);
+      });
+    },
+  );
+
+  it.runIf(process.platform !== "win32")(
+    "does not create missing target directories through an existing symlink",
+    async () => {
+      await withTempDir({ prefix: "openclaw-json-file-" }, async (root) => {
+        const missingTargetDir = path.join(root, "missing-target");
+        const targetPath = path.join(missingTargetDir, "config.json");
+        const linkPath = path.join(root, "config-link.json");
+        fs.symlinkSync(targetPath, linkPath);
+
+        expect(() => saveJsonFile(linkPath, SAVED_PAYLOAD)).toThrow(
+          expect.objectContaining({ code: "ENOENT" }),
+        );
+        expect(fs.existsSync(missingTargetDir)).toBe(false);
+        expect(fs.lstatSync(linkPath).isSymbolicLink()).toBe(true);
+      });
+    },
+  );
+
+  it("falls back to copy when rename-based overwrite fails", async () => {
+    await withJsonPath(({ root, pathname }) => {
+      writeExistingJson(pathname);
+      const copySpy = vi.spyOn(fs, "copyFileSync");
+      const renameSpy = vi.spyOn(fs, "renameSync").mockImplementationOnce(() => {
+        const err = new Error("EPERM") as NodeJS.ErrnoException;
+        err.code = "EPERM";
+        throw err;
+      });
+
+      saveJsonFile(pathname, SAVED_PAYLOAD);
+
+      expect(renameSpy).toHaveBeenCalledOnce();
+      expect(copySpy).toHaveBeenCalledOnce();
+      expect(loadJsonFile(pathname)).toEqual(SAVED_PAYLOAD);
+      expect(fs.readdirSync(root)).toEqual(["config.json"]);
     });
   });
 });
diff --git a/src/infra/json-file.ts b/src/infra/json-file.ts
@@ -1,23 +1,129 @@
+import { randomUUID } from "node:crypto";
 import fs from "node:fs";
 import path from "node:path";
 
-export function loadJsonFile(pathname: string): unknown {
+const JSON_FILE_MODE = 0o600;
+const JSON_DIR_MODE = 0o700;
+
+function trySetSecureMode(pathname: string) {
+  try {
+    fs.chmodSync(pathname, JSON_FILE_MODE);
+  } catch {
+    // best-effort on platforms without chmod support
+  }
+}
+
+function trySyncDirectory(pathname: string) {
+  let fd: number | undefined;
+  try {
+    fd = fs.openSync(path.dirname(pathname), "r");
+    fs.fsyncSync(fd);
+  } catch {
+    // best-effort; some platforms/filesystems do not support syncing directories.
+  } finally {
+    if (fd !== undefined) {
+      try {
+        fs.closeSync(fd);
+      } catch {
+        // best-effort cleanup
+      }
+    }
+  }
+}
+
+function readSymlinkTargetPath(linkPath: string): string {
+  const target = fs.readlinkSync(linkPath);
+  return path.resolve(path.dirname(linkPath), target);
+}
+
+function resolveJsonWriteTarget(pathname: string): { targetPath: string; followsSymlink: boolean } {
+  let currentPath = pathname;
+  const visited = new Set<string>();
+  let followsSymlink = false;
+
+  for (;;) {
+    let stat: fs.Stats;
+    try {
+      stat = fs.lstatSync(currentPath);
+    } catch (error) {
+      if ((error as NodeJS.ErrnoException).code !== "ENOENT") {
+        throw error;
+      }
+      return { targetPath: currentPath, followsSymlink };
+    }
+
+    if (!stat.isSymbolicLink()) {
+      return { targetPath: currentPath, followsSymlink };
+    }
+
+    if (visited.has(currentPath)) {
+      const err = new Error(
+        `Too many symlink levels while resolving ${pathname}`,
+      ) as NodeJS.ErrnoException;
+      err.code = "ELOOP";
+      throw err;
+    }
+
+    visited.add(currentPath);
+    followsSymlink = true;
+    currentPath = readSymlinkTargetPath(currentPath);
+  }
+}
+
+function renameJsonFileWithFallback(tmpPath: string, pathname: string) {
   try {
-    if (!fs.existsSync(pathname)) {
-      return undefined;
+    fs.renameSync(tmpPath, pathname);
+    return;
+  } catch (error) {
+    const code = (error as NodeJS.ErrnoException).code;
+    // Windows does not reliably support rename-based overwrite for existing files.
+    if (code === "EPERM" || code === "EEXIST") {
+      fs.copyFileSync(tmpPath, pathname);
+      fs.rmSync(tmpPath, { force: true });
+      return;
     }
+    throw error;
+  }
+}
+
+function writeTempJsonFile(pathname: string, payload: string) {
+  const fd = fs.openSync(pathname, "w", JSON_FILE_MODE);
+  try {
+    fs.writeFileSync(fd, payload, "utf8");
+    fs.fsyncSync(fd);
+  } finally {
+    fs.closeSync(fd);
+  }
+}
+
+export function loadJsonFile<T = unknown>(pathname: string): T | undefined {
+  try {
     const raw = fs.readFileSync(pathname, "utf8");
-    return JSON.parse(raw) as unknown;
+    return JSON.parse(raw) as T;
   } catch {
     return undefined;
   }
 }
 
 export function saveJsonFile(pathname: string, data: unknown) {
-  const dir = path.dirname(pathname);
-  if (!fs.existsSync(dir)) {
-    fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
+  const { targetPath, followsSymlink } = resolveJsonWriteTarget(pathname);
+  const tmpPath = `${targetPath}.${randomUUID()}.tmp`;
+  const payload = `${JSON.stringify(data, null, 2)}\n`;
+
+  if (!followsSymlink) {
+    fs.mkdirSync(path.dirname(targetPath), { recursive: true, mode: JSON_DIR_MODE });
+  }
+  try {
+    writeTempJsonFile(tmpPath, payload);
+    trySetSecureMode(tmpPath);
+    renameJsonFileWithFallback(tmpPath, targetPath);
+    trySetSecureMode(targetPath);
+    trySyncDirectory(targetPath);
+  } finally {
+    try {
+      fs.rmSync(tmpPath, { force: true });
+    } catch {
+      // best-effort cleanup when rename does not happen
+    }
   }
-  fs.writeFileSync(pathname, `${JSON.stringify(data, null, 2)}\n`, "utf8");
-  fs.chmodSync(pathname, 0o600);
 }
