From cc29be8c9bcdfaecb90f0ab13124c8f5362a6741 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 18 Feb 2026 04:44:23 +0100 Subject: [PATCH] fix: serialize sandbox registry writes --- CHANGELOG.md | 8 + src/agents/sandbox/registry.test.ts | 219 ++++++++++++++++++++++++++++ src/agents/sandbox/registry.ts | 177 ++++++++++++++-------- 3 files changed, 346 insertions(+), 58 deletions(-) create mode 100644 src/agents/sandbox/registry.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a561b2b00..311eb67cfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ Docs: https://docs.openclaw.ai ### Changes +### Fixes + +- Sandbox/Registry: serialize container and browser registry writes with shared file locks and atomic replacement to prevent lost updates and delete rollback races from desyncing `sandbox list`, `prune`, and `recreate --all`. Thanks @kexinoh. + +## 2026.2.17 + +### Changes + - Agents/Anthropic: add opt-in 1M context beta header support for Opus/Sonnet via model `params.context1m: true` (maps to `anthropic-beta: context-1m-2025-08-07`). - Agents/Models: support Anthropic Sonnet 4.6 (`anthropic/claude-sonnet-4-6`) across aliases/defaults with forward-compat fallback when upstream catalogs still only expose Sonnet 4.5. - Commands/Subagents: add `/subagents spawn` for deterministic subagent activation from chat commands. (#18218) Thanks @JoshuaLelon. diff --git a/src/agents/sandbox/registry.test.ts b/src/agents/sandbox/registry.test.ts new file mode 100644 index 0000000000..a3479bb57f --- /dev/null +++ b/src/agents/sandbox/registry.test.ts @@ -0,0 +1,219 @@ +import { mkdtempSync } from "node:fs"; +import fs from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const TEST_STATE_DIR = mkdtempSync(path.join(tmpdir(), "openclaw-sandbox-registry-")); +const SANDBOX_REGISTRY_PATH = path.join(TEST_STATE_DIR, "containers.json"); +const SANDBOX_BROWSER_REGISTRY_PATH = path.join(TEST_STATE_DIR, "browsers.json"); + +vi.mock("./constants.js", () => ({ + SANDBOX_STATE_DIR: TEST_STATE_DIR, + SANDBOX_REGISTRY_PATH, + SANDBOX_BROWSER_REGISTRY_PATH, +})); + +import type { SandboxBrowserRegistryEntry, SandboxRegistryEntry } from "./registry.js"; +import { + readBrowserRegistry, + readRegistry, + removeBrowserRegistryEntry, + removeRegistryEntry, + updateBrowserRegistry, + updateRegistry, +} from "./registry.js"; + +type WriteDelayConfig = { + containerName: string; + browserName: string; + containerDelayMs: number; + browserDelayMs: number; +}; + +let writeDelayConfig: WriteDelayConfig = { + containerName: "container-a", + browserName: "browser-a", + containerDelayMs: 0, + browserDelayMs: 0, +}; + +const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); +const realFsWriteFile = fs.writeFile; + +function writeText(content: Parameters[1]): string { + if (typeof content === "string") { + return content; + } + if (content instanceof ArrayBuffer) { + return Buffer.from(content).toString("utf-8"); + } + if (ArrayBuffer.isView(content)) { + return Buffer.from(content.buffer, content.byteOffset, content.byteLength).toString("utf-8"); + } + return ""; +} + +beforeEach(() => { + writeDelayConfig = { + containerName: "container-a", + browserName: "browser-a", + containerDelayMs: 0, + browserDelayMs: 0, + }; + vi.spyOn(fs, "writeFile").mockImplementation(async (...args) => { + const [target, content] = args; + if (typeof target !== "string") { + return realFsWriteFile(...args); + } + + const payload = writeText(content); + if ( + target.includes("containers.json") && + payload.includes(`"containerName":"${writeDelayConfig.containerName}"`) && + writeDelayConfig.containerDelayMs > 0 + ) { + await delay(writeDelayConfig.containerDelayMs); + } + + if ( + target.includes("browsers.json") && + payload.includes(`"containerName":"${writeDelayConfig.browserName}"`) && + writeDelayConfig.browserDelayMs > 0 + ) { + await delay(writeDelayConfig.browserDelayMs); + } + return realFsWriteFile(...args); + }); +}); + +afterEach(async () => { + vi.restoreAllMocks(); + await fs.rm(TEST_STATE_DIR, { recursive: true, force: true }); + await fs.mkdir(TEST_STATE_DIR, { recursive: true }); +}); + +afterAll(async () => { + await fs.rm(TEST_STATE_DIR, { recursive: true, force: true }); +}); + +function browserEntry( + overrides: Partial = {}, +): SandboxBrowserRegistryEntry { + return { + containerName: "browser-a", + sessionKey: "agent:main", + createdAtMs: 1, + lastUsedAtMs: 1, + image: "openclaw-browser:test", + cdpPort: 9222, + ...overrides, + }; +} + +function containerEntry(overrides: Partial = {}): SandboxRegistryEntry { + return { + containerName: "container-a", + sessionKey: "agent:main", + createdAtMs: 1, + lastUsedAtMs: 1, + image: "openclaw-sandbox:test", + ...overrides, + }; +} + +async function seedContainerRegistry(entries: SandboxRegistryEntry[]) { + await fs.writeFile(SANDBOX_REGISTRY_PATH, `${JSON.stringify({ entries }, null, 2)}\n`, "utf-8"); +} + +async function seedBrowserRegistry(entries: SandboxBrowserRegistryEntry[]) { + await fs.writeFile( + SANDBOX_BROWSER_REGISTRY_PATH, + `${JSON.stringify({ entries }, null, 2)}\n`, + "utf-8", + ); +} + +describe("registry race safety", () => { + it("keeps both container updates under concurrent writes", async () => { + writeDelayConfig = { + containerName: "container-a", + browserName: "browser-a", + containerDelayMs: 80, + browserDelayMs: 0, + }; + + await Promise.all([ + updateRegistry(containerEntry({ containerName: "container-a" })), + updateRegistry(containerEntry({ containerName: "container-b" })), + ]); + + const registry = await readRegistry(); + expect(registry.entries).toHaveLength(2); + expect( + registry.entries + .map((entry) => entry.containerName) + .slice() + .toSorted(), + ).toEqual(["container-a", "container-b"]); + }); + + it("prevents concurrent container remove/update from resurrecting deleted entries", async () => { + await seedContainerRegistry([containerEntry({ containerName: "container-x" })]); + writeDelayConfig = { + containerName: "container-x", + browserName: "browser-a", + containerDelayMs: 80, + browserDelayMs: 0, + }; + + await Promise.all([ + removeRegistryEntry("container-x"), + updateRegistry(containerEntry({ containerName: "container-x", configHash: "updated" })), + ]); + + const registry = await readRegistry(); + expect(registry.entries).toHaveLength(0); + }); + + it("keeps both browser updates under concurrent writes", async () => { + writeDelayConfig = { + containerName: "container-a", + browserName: "browser-a", + containerDelayMs: 0, + browserDelayMs: 80, + }; + + await Promise.all([ + updateBrowserRegistry(browserEntry({ containerName: "browser-a" })), + updateBrowserRegistry(browserEntry({ containerName: "browser-b", cdpPort: 9223 })), + ]); + + const registry = await readBrowserRegistry(); + expect(registry.entries).toHaveLength(2); + expect( + registry.entries + .map((entry) => entry.containerName) + .slice() + .toSorted(), + ).toEqual(["browser-a", "browser-b"]); + }); + + it("prevents concurrent browser remove/update from resurrecting deleted entries", async () => { + await seedBrowserRegistry([browserEntry({ containerName: "browser-x" })]); + writeDelayConfig = { + containerName: "container-a", + browserName: "browser-x", + containerDelayMs: 0, + browserDelayMs: 80, + }; + + await Promise.all([ + removeBrowserRegistryEntry("browser-x"), + updateBrowserRegistry(browserEntry({ containerName: "browser-x", configHash: "updated" })), + ]); + + const registry = await readBrowserRegistry(); + expect(registry.entries).toHaveLength(0); + }); +}); diff --git a/src/agents/sandbox/registry.ts b/src/agents/sandbox/registry.ts index 6e1b0398f6..2948b8d3eb 100644 --- a/src/agents/sandbox/registry.ts +++ b/src/agents/sandbox/registry.ts @@ -1,4 +1,7 @@ +import crypto from "node:crypto"; import fs from "node:fs/promises"; +import path from "node:path"; +import { acquireSessionWriteLock } from "../session-write-lock.js"; import { SANDBOX_BROWSER_REGISTRY_PATH, SANDBOX_REGISTRY_PATH, @@ -33,86 +36,144 @@ type SandboxBrowserRegistry = { entries: SandboxBrowserRegistryEntry[]; }; -export async function readRegistry(): Promise { +type RegistryReadMode = "strict" | "fallback"; + +async function withRegistryLock(registryPath: string, fn: () => Promise): Promise { + const lock = await acquireSessionWriteLock({ sessionFile: registryPath }); try { - const raw = await fs.readFile(SANDBOX_REGISTRY_PATH, "utf-8"); - const parsed = JSON.parse(raw) as SandboxRegistry; - if (parsed && Array.isArray(parsed.entries)) { - return parsed; - } - } catch { - // ignore + return await fn(); + } finally { + await lock.release(); } - return { entries: [] }; +} + +async function readRegistryFromFile( + registryPath: string, + mode: RegistryReadMode, +): Promise<{ entries: T[] }> { + try { + const raw = await fs.readFile(registryPath, "utf-8"); + const parsed = JSON.parse(raw) as { entries?: unknown }; + if (parsed && Array.isArray(parsed.entries)) { + return { entries: parsed.entries as T[] }; + } + if (mode === "fallback") { + return { entries: [] }; + } + throw new Error(`Invalid sandbox registry format: ${registryPath}`); + } catch (error) { + const code = (error as { code?: string } | null)?.code; + if (code === "ENOENT") { + return { entries: [] }; + } + if (mode === "fallback") { + return { entries: [] }; + } + if (error instanceof Error) { + throw error; + } + throw new Error(`Failed to read sandbox registry file: ${registryPath}`, { cause: error }); + } +} + +async function writeRegistryFile( + registryPath: string, + registry: { entries: T[] }, +): Promise { + await fs.mkdir(SANDBOX_STATE_DIR, { recursive: true }); + const payload = `${JSON.stringify(registry, null, 2)}\n`; + const registryDir = path.dirname(registryPath); + const tempPath = path.join( + registryDir, + `${path.basename(registryPath)}.${crypto.randomUUID()}.tmp`, + ); + await fs.writeFile(tempPath, payload, "utf-8"); + try { + await fs.rename(tempPath, registryPath); + } catch (error) { + await fs.rm(tempPath, { force: true }); + throw error; + } +} + +export async function readRegistry(): Promise { + return await readRegistryFromFile(SANDBOX_REGISTRY_PATH, "fallback"); +} + +async function readRegistryForWrite(): Promise { + return await readRegistryFromFile(SANDBOX_REGISTRY_PATH, "strict"); } async function writeRegistry(registry: SandboxRegistry) { - await fs.mkdir(SANDBOX_STATE_DIR, { recursive: true }); - await fs.writeFile(SANDBOX_REGISTRY_PATH, `${JSON.stringify(registry, null, 2)}\n`, "utf-8"); + await writeRegistryFile(SANDBOX_REGISTRY_PATH, registry); } export async function updateRegistry(entry: SandboxRegistryEntry) { - const registry = await readRegistry(); - const existing = registry.entries.find((item) => item.containerName === entry.containerName); - const next = registry.entries.filter((item) => item.containerName !== entry.containerName); - next.push({ - ...entry, - createdAtMs: existing?.createdAtMs ?? entry.createdAtMs, - image: existing?.image ?? entry.image, - configHash: entry.configHash ?? existing?.configHash, + await withRegistryLock(SANDBOX_REGISTRY_PATH, async () => { + const registry = await readRegistryForWrite(); + const existing = registry.entries.find((item) => item.containerName === entry.containerName); + const next = registry.entries.filter((item) => item.containerName !== entry.containerName); + next.push({ + ...entry, + createdAtMs: existing?.createdAtMs ?? entry.createdAtMs, + image: existing?.image ?? entry.image, + configHash: entry.configHash ?? existing?.configHash, + }); + await writeRegistry({ entries: next }); }); - await writeRegistry({ entries: next }); } export async function removeRegistryEntry(containerName: string) { - const registry = await readRegistry(); - const next = registry.entries.filter((item) => item.containerName !== containerName); - if (next.length === registry.entries.length) { - return; - } - await writeRegistry({ entries: next }); + await withRegistryLock(SANDBOX_REGISTRY_PATH, async () => { + const registry = await readRegistryForWrite(); + const next = registry.entries.filter((item) => item.containerName !== containerName); + if (next.length === registry.entries.length) { + return; + } + await writeRegistry({ entries: next }); + }); } export async function readBrowserRegistry(): Promise { - try { - const raw = await fs.readFile(SANDBOX_BROWSER_REGISTRY_PATH, "utf-8"); - const parsed = JSON.parse(raw) as SandboxBrowserRegistry; - if (parsed && Array.isArray(parsed.entries)) { - return parsed; - } - } catch { - // ignore - } - return { entries: [] }; -} - -async function writeBrowserRegistry(registry: SandboxBrowserRegistry) { - await fs.mkdir(SANDBOX_STATE_DIR, { recursive: true }); - await fs.writeFile( + return await readRegistryFromFile( SANDBOX_BROWSER_REGISTRY_PATH, - `${JSON.stringify(registry, null, 2)}\n`, - "utf-8", + "fallback", ); } +async function readBrowserRegistryForWrite(): Promise { + return await readRegistryFromFile( + SANDBOX_BROWSER_REGISTRY_PATH, + "strict", + ); +} + +async function writeBrowserRegistry(registry: SandboxBrowserRegistry) { + await writeRegistryFile(SANDBOX_BROWSER_REGISTRY_PATH, registry); +} + export async function updateBrowserRegistry(entry: SandboxBrowserRegistryEntry) { - const registry = await readBrowserRegistry(); - const existing = registry.entries.find((item) => item.containerName === entry.containerName); - const next = registry.entries.filter((item) => item.containerName !== entry.containerName); - next.push({ - ...entry, - createdAtMs: existing?.createdAtMs ?? entry.createdAtMs, - image: existing?.image ?? entry.image, - configHash: entry.configHash ?? existing?.configHash, + await withRegistryLock(SANDBOX_BROWSER_REGISTRY_PATH, async () => { + const registry = await readBrowserRegistryForWrite(); + const existing = registry.entries.find((item) => item.containerName === entry.containerName); + const next = registry.entries.filter((item) => item.containerName !== entry.containerName); + next.push({ + ...entry, + createdAtMs: existing?.createdAtMs ?? entry.createdAtMs, + image: existing?.image ?? entry.image, + configHash: entry.configHash ?? existing?.configHash, + }); + await writeBrowserRegistry({ entries: next }); }); - await writeBrowserRegistry({ entries: next }); } export async function removeBrowserRegistryEntry(containerName: string) { - const registry = await readBrowserRegistry(); - const next = registry.entries.filter((item) => item.containerName !== containerName); - if (next.length === registry.entries.length) { - return; - } - await writeBrowserRegistry({ entries: next }); + await withRegistryLock(SANDBOX_BROWSER_REGISTRY_PATH, async () => { + const registry = await readBrowserRegistryForWrite(); + const next = registry.entries.filter((item) => item.containerName !== containerName); + if (next.length === registry.entries.length) { + return; + } + await writeBrowserRegistry({ entries: next }); + }); }