mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-06 06:41:08 +00:00
infra: atomically replace sync JSON writes (#60589)
Merged via squash.
Prepared head SHA: cb8ed77049
Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com>
Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com>
Reviewed-by: @gumadeiras
This commit is contained in:
committed by
GitHub
parent
628c71103e
commit
300fb36879
@@ -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
|
||||
|
||||
|
||||
@@ -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"]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 {
|
||||
if (!fs.existsSync(pathname)) {
|
||||
return undefined;
|
||||
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 {
|
||||
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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user