From 266ded1d5bdf3960cfc1aadbd02c9dd415ec7298 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 8 May 2026 13:04:59 +0100 Subject: [PATCH] refactor: move sandbox registry migration to doctor --- .../check-database-first-legacy-stores.mjs | 1 - src/agents/sandbox/registry.test.ts | 2 +- src/agents/sandbox/registry.ts | 220 +-------------- .../doctor-sandbox-registry-migration.ts | 260 ++++++++++++++++++ src/commands/doctor-sandbox.ts | 12 +- ...rns-sandbox-enabled-without-docker.test.ts | 2 +- 6 files changed, 269 insertions(+), 228 deletions(-) create mode 100644 src/commands/doctor-sandbox-registry-migration.ts diff --git a/scripts/check-database-first-legacy-stores.mjs b/scripts/check-database-first-legacy-stores.mjs index 854946d4541..3a4fb972db7 100644 --- a/scripts/check-database-first-legacy-stores.mjs +++ b/scripts/check-database-first-legacy-stores.mjs @@ -132,7 +132,6 @@ const allowedExactPaths = new Set([ "extensions/skill-workshop/src/state-migrations.ts", "extensions/qqbot/src/state-migrations.ts", "extensions/telegram/src/state-migrations.ts", - "src/agents/sandbox/registry.ts", "src/infra/state-migrations.ts", "src/plugin-state/plugin-state-store.sqlite.ts", "src/tasks/task-flow-registry.store.sqlite.ts", diff --git a/src/agents/sandbox/registry.test.ts b/src/agents/sandbox/registry.test.ts index a495c5a96f0..ab489624cdf 100644 --- a/src/agents/sandbox/registry.test.ts +++ b/src/agents/sandbox/registry.test.ts @@ -35,8 +35,8 @@ vi.mock("./constants.js", () => ({ SANDBOX_BROWSERS_DIR, })); +import { migrateLegacySandboxRegistryFiles } from "../../commands/doctor-sandbox-registry-migration.js"; import { - migrateLegacySandboxRegistryFiles, readBrowserRegistry, readRegistry, readRegistryEntry, diff --git a/src/agents/sandbox/registry.ts b/src/agents/sandbox/registry.ts index c93218fa46b..27d71b19655 100644 --- a/src/agents/sandbox/registry.ts +++ b/src/agents/sandbox/registry.ts @@ -1,4 +1,3 @@ -import fs from "node:fs/promises"; import path from "node:path"; import type { Insertable } from "kysely"; import { z } from "zod"; @@ -14,14 +13,7 @@ import { type OpenClawStateDatabase, type OpenClawStateDatabaseOptions, } from "../../state/openclaw-state-db.js"; -import { safeParseJsonWithSchema } from "../../utils/zod-parse.js"; -import { - SANDBOX_BROWSER_REGISTRY_PATH, - SANDBOX_BROWSERS_DIR, - SANDBOX_CONTAINERS_DIR, - SANDBOX_REGISTRY_PATH, - SANDBOX_STATE_DIR, -} from "./constants.js"; +import { SANDBOX_STATE_DIR } from "./constants.js"; export type SandboxRegistryEntry = { containerName: string; @@ -60,40 +52,14 @@ type RegistryEntry = { type RegistryEntryPayload = RegistryEntry & Record; -type RegistryFile = { - entries: RegistryEntryPayload[]; -}; - type LegacyRegistryKind = "containers" | "browsers"; -type LegacyRegistryTarget = { - kind: LegacyRegistryKind; - registryPath: string; - shardedDir: string; -}; - -export type LegacySandboxRegistryInspection = LegacyRegistryTarget & { - exists: boolean; - valid: boolean; - entries: number; -}; - -export type LegacySandboxRegistryMigrationResult = LegacyRegistryTarget & { - status: "missing" | "migrated" | "removed-empty" | "quarantined-invalid"; - entries: number; - quarantinePath?: string; -}; - const RegistryEntrySchema = z .object({ containerName: z.string(), }) .passthrough(); -const RegistryFileSchema = z.object({ - entries: z.array(RegistryEntrySchema), -}); - function normalizeSandboxRegistryEntry(entry: SandboxRegistryEntry): SandboxRegistryEntry { return { ...entry, @@ -103,23 +69,6 @@ function normalizeSandboxRegistryEntry(entry: SandboxRegistryEntry): SandboxRegi }; } -async function readLegacyRegistryFile(registryPath: string): Promise { - try { - const raw = await fs.readFile(registryPath, "utf-8"); - const parsed = safeParseJsonWithSchema(RegistryFileSchema, raw) as RegistryFile | null; - return parsed; - } catch (error) { - const code = (error as { code?: string } | null)?.code; - if (code === "ENOENT") { - return { entries: [] }; - } - if (error instanceof Error) { - throw error; - } - throw new Error(`Failed to read sandbox registry file: ${registryPath}`, { cause: error }); - } -} - export async function readRegistry(): Promise { const entries = readRegistryEntries("containers"); return { @@ -232,173 +181,6 @@ function upsertRegistryEntry( ); } -async function quarantineLegacyRegistry(registryPath: string): Promise { - const quarantinePath = `${registryPath}.invalid-${Date.now()}`; - await fs.rename(registryPath, quarantinePath).catch(async (error) => { - const code = (error as { code?: string } | null)?.code; - if (code !== "ENOENT") { - await fs.rm(registryPath, { force: true }); - } - }); - return quarantinePath; -} - -async function legacyShardPaths(dir: string): Promise { - try { - const names = await fs.readdir(dir); - return names - .filter((name) => name.endsWith(".json")) - .toSorted() - .map((name) => path.join(dir, name)); - } catch (error) { - const code = (error as { code?: string } | null)?.code; - if (code === "ENOENT") { - return []; - } - throw error; - } -} - -async function readLegacyShardFile(shardPath: string): Promise { - try { - const raw = await fs.readFile(shardPath, "utf-8"); - return safeParseJsonWithSchema(RegistryEntrySchema, raw) as RegistryEntryPayload | null; - } catch (error) { - const code = (error as { code?: string } | null)?.code; - if (code === "ENOENT") { - return null; - } - throw error; - } -} - -async function inspectMonolithicLegacyRegistry(target: LegacyRegistryTarget): Promise<{ - exists: boolean; - valid: boolean; - entries: RegistryEntryPayload[]; -}> { - try { - await fs.access(target.registryPath); - } catch (error) { - const code = (error as { code?: string } | null)?.code; - if (code === "ENOENT") { - return { exists: false, valid: true, entries: [] }; - } - throw error; - } - - const registry = await readLegacyRegistryFile(target.registryPath); - return { - exists: true, - valid: Boolean(registry), - entries: registry?.entries ?? [], - }; -} - -async function inspectShardedLegacyRegistry(target: LegacyRegistryTarget): Promise<{ - exists: boolean; - valid: boolean; - entries: RegistryEntryPayload[]; - invalidPath?: string; -}> { - const shardPaths = await legacyShardPaths(target.shardedDir); - const entries: RegistryEntryPayload[] = []; - for (const shardPath of shardPaths) { - const entry = await readLegacyShardFile(shardPath); - if (!entry) { - return { exists: true, valid: false, entries, invalidPath: shardPath }; - } - entries.push(entry); - } - return { exists: shardPaths.length > 0, valid: true, entries }; -} - -async function migrateTargetIfNeeded( - target: LegacyRegistryTarget, -): Promise { - const monolithic = await inspectMonolithicLegacyRegistry(target); - if (!monolithic.valid) { - const quarantinePath = await quarantineLegacyRegistry(target.registryPath); - return { ...target, status: "quarantined-invalid", entries: 0, quarantinePath }; - } - const sharded = await inspectShardedLegacyRegistry(target); - if (!sharded.valid) { - const quarantinePath = sharded.invalidPath - ? await quarantineLegacyRegistry(sharded.invalidPath) - : undefined; - return { ...target, status: "quarantined-invalid", entries: 0, quarantinePath }; - } - - if (!monolithic.exists && !sharded.exists) { - return { ...target, status: "missing", entries: 0 }; - } - - const entries = [...monolithic.entries, ...sharded.entries]; - if (entries.length === 0) { - await fs.rm(target.registryPath, { force: true }); - await fs.rm(`${target.registryPath}.lock`, { force: true }); - await fs.rm(target.shardedDir, { recursive: true, force: true }); - return { ...target, status: "removed-empty", entries: 0 }; - } - - runOpenClawStateWriteTransaction((database) => { - for (const entry of entries) { - if (!getRegistryEntry(database, target.kind, entry.containerName)) { - upsertRegistryEntry(database, target.kind, entry); - } - } - }, sandboxRegistryDbOptions()); - - await fs.rm(target.registryPath, { force: true }); - await fs.rm(`${target.registryPath}.lock`, { force: true }); - await fs.rm(target.shardedDir, { recursive: true, force: true }); - return { ...target, status: "migrated", entries: entries.length }; -} - -function legacyRegistryTargets(): LegacyRegistryTarget[] { - return [ - { - kind: "containers", - registryPath: SANDBOX_REGISTRY_PATH, - shardedDir: SANDBOX_CONTAINERS_DIR, - }, - { - kind: "browsers", - registryPath: SANDBOX_BROWSER_REGISTRY_PATH, - shardedDir: SANDBOX_BROWSERS_DIR, - }, - ]; -} - -export async function inspectLegacySandboxRegistryFiles(): Promise< - LegacySandboxRegistryInspection[] -> { - const inspections: LegacySandboxRegistryInspection[] = []; - for (const target of legacyRegistryTargets()) { - const monolithic = await inspectMonolithicLegacyRegistry(target); - const sharded = monolithic.valid - ? await inspectShardedLegacyRegistry(target) - : { exists: false, valid: true, entries: [] }; - inspections.push({ - ...target, - exists: monolithic.exists || sharded.exists, - valid: monolithic.valid && sharded.valid, - entries: monolithic.entries.length + sharded.entries.length, - }); - } - return inspections; -} - -export async function migrateLegacySandboxRegistryFiles(): Promise< - LegacySandboxRegistryMigrationResult[] -> { - const results: LegacySandboxRegistryMigrationResult[] = []; - for (const target of legacyRegistryTargets()) { - results.push(await migrateTargetIfNeeded(target)); - } - return results; -} - export async function readRegistryEntry( containerName: string, ): Promise { diff --git a/src/commands/doctor-sandbox-registry-migration.ts b/src/commands/doctor-sandbox-registry-migration.ts new file mode 100644 index 00000000000..023f37ebf68 --- /dev/null +++ b/src/commands/doctor-sandbox-registry-migration.ts @@ -0,0 +1,260 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { z } from "zod"; +import { + SANDBOX_BROWSER_REGISTRY_PATH, + SANDBOX_BROWSERS_DIR, + SANDBOX_CONTAINERS_DIR, + SANDBOX_REGISTRY_PATH, +} from "../agents/sandbox/constants.js"; +import { + readBrowserRegistry, + readRegistryEntry, + updateBrowserRegistry, + updateRegistry, + type SandboxBrowserRegistryEntry, + type SandboxRegistryEntry, +} from "../agents/sandbox/registry.js"; +import { safeParseJsonWithSchema } from "../utils/zod-parse.js"; + +type RegistryEntry = { + containerName: string; +}; + +type RegistryEntryPayload = RegistryEntry & Record; + +type RegistryFile = { + entries: RegistryEntryPayload[]; +}; + +type LegacyRegistryKind = "containers" | "browsers"; + +type LegacyRegistryTarget = { + kind: LegacyRegistryKind; + registryPath: string; + shardedDir: string; +}; + +export type LegacySandboxRegistryInspection = LegacyRegistryTarget & { + exists: boolean; + valid: boolean; + entries: number; +}; + +export type LegacySandboxRegistryMigrationResult = LegacyRegistryTarget & { + status: "missing" | "migrated" | "removed-empty" | "quarantined-invalid"; + entries: number; + quarantinePath?: string; +}; + +const RegistryEntrySchema = z + .object({ + containerName: z.string(), + }) + .passthrough(); + +const RegistryFileSchema = z.object({ + entries: z.array(RegistryEntrySchema), +}); + +async function readLegacyRegistryFile(registryPath: string): Promise { + try { + const raw = await fs.readFile(registryPath, "utf-8"); + const parsed = safeParseJsonWithSchema(RegistryFileSchema, raw) as RegistryFile | null; + return parsed; + } catch (error) { + const code = (error as { code?: string } | null)?.code; + if (code === "ENOENT") { + return { entries: [] }; + } + if (error instanceof Error) { + throw error; + } + throw new Error(`Failed to read sandbox registry file: ${registryPath}`, { cause: error }); + } +} + +async function quarantineLegacyRegistry(registryPath: string): Promise { + const quarantinePath = `${registryPath}.invalid-${Date.now()}`; + await fs.rename(registryPath, quarantinePath).catch(async (error) => { + const code = (error as { code?: string } | null)?.code; + if (code !== "ENOENT") { + await fs.rm(registryPath, { force: true }); + } + }); + return quarantinePath; +} + +async function legacyShardPaths(dir: string): Promise { + try { + const names = await fs.readdir(dir); + return names + .filter((name) => name.endsWith(".json")) + .toSorted() + .map((name) => path.join(dir, name)); + } catch (error) { + const code = (error as { code?: string } | null)?.code; + if (code === "ENOENT") { + return []; + } + throw error; + } +} + +async function readLegacyShardFile(shardPath: string): Promise { + try { + const raw = await fs.readFile(shardPath, "utf-8"); + return safeParseJsonWithSchema(RegistryEntrySchema, raw) as RegistryEntryPayload | null; + } catch (error) { + const code = (error as { code?: string } | null)?.code; + if (code === "ENOENT") { + return null; + } + throw error; + } +} + +async function inspectMonolithicLegacyRegistry(target: LegacyRegistryTarget): Promise<{ + exists: boolean; + valid: boolean; + entries: RegistryEntryPayload[]; +}> { + try { + await fs.access(target.registryPath); + } catch (error) { + const code = (error as { code?: string } | null)?.code; + if (code === "ENOENT") { + return { exists: false, valid: true, entries: [] }; + } + throw error; + } + + const registry = await readLegacyRegistryFile(target.registryPath); + return { + exists: true, + valid: Boolean(registry), + entries: registry?.entries ?? [], + }; +} + +async function inspectShardedLegacyRegistry(target: LegacyRegistryTarget): Promise<{ + exists: boolean; + valid: boolean; + entries: RegistryEntryPayload[]; + invalidPath?: string; +}> { + const shardPaths = await legacyShardPaths(target.shardedDir); + const entries: RegistryEntryPayload[] = []; + for (const shardPath of shardPaths) { + const entry = await readLegacyShardFile(shardPath); + if (!entry) { + return { exists: true, valid: false, entries, invalidPath: shardPath }; + } + entries.push(entry); + } + return { exists: shardPaths.length > 0, valid: true, entries }; +} + +async function hasBrowserRegistryEntry(containerName: string): Promise { + const registry = await readBrowserRegistry(); + return registry.entries.some((entry) => entry.containerName === containerName); +} + +async function importLegacyRegistryEntry( + kind: LegacyRegistryKind, + entry: RegistryEntryPayload, +): Promise { + if (kind === "containers") { + if (await readRegistryEntry(entry.containerName)) { + return; + } + await updateRegistry(entry as SandboxRegistryEntry); + return; + } + if (await hasBrowserRegistryEntry(entry.containerName)) { + return; + } + await updateBrowserRegistry(entry as SandboxBrowserRegistryEntry); +} + +async function migrateTargetIfNeeded( + target: LegacyRegistryTarget, +): Promise { + const monolithic = await inspectMonolithicLegacyRegistry(target); + if (!monolithic.valid) { + const quarantinePath = await quarantineLegacyRegistry(target.registryPath); + return { ...target, status: "quarantined-invalid", entries: 0, quarantinePath }; + } + const sharded = await inspectShardedLegacyRegistry(target); + if (!sharded.valid) { + const quarantinePath = sharded.invalidPath + ? await quarantineLegacyRegistry(sharded.invalidPath) + : undefined; + return { ...target, status: "quarantined-invalid", entries: 0, quarantinePath }; + } + + if (!monolithic.exists && !sharded.exists) { + return { ...target, status: "missing", entries: 0 }; + } + + const entries = [...monolithic.entries, ...sharded.entries]; + if (entries.length === 0) { + await fs.rm(target.registryPath, { force: true }); + await fs.rm(`${target.registryPath}.lock`, { force: true }); + await fs.rm(target.shardedDir, { recursive: true, force: true }); + return { ...target, status: "removed-empty", entries: 0 }; + } + + for (const entry of entries) { + await importLegacyRegistryEntry(target.kind, entry); + } + + await fs.rm(target.registryPath, { force: true }); + await fs.rm(`${target.registryPath}.lock`, { force: true }); + await fs.rm(target.shardedDir, { recursive: true, force: true }); + return { ...target, status: "migrated", entries: entries.length }; +} + +function legacyRegistryTargets(): LegacyRegistryTarget[] { + return [ + { + kind: "containers", + registryPath: SANDBOX_REGISTRY_PATH, + shardedDir: SANDBOX_CONTAINERS_DIR, + }, + { + kind: "browsers", + registryPath: SANDBOX_BROWSER_REGISTRY_PATH, + shardedDir: SANDBOX_BROWSERS_DIR, + }, + ]; +} + +export async function inspectLegacySandboxRegistryFiles(): Promise< + LegacySandboxRegistryInspection[] +> { + const inspections: LegacySandboxRegistryInspection[] = []; + for (const target of legacyRegistryTargets()) { + const monolithic = await inspectMonolithicLegacyRegistry(target); + const sharded = monolithic.valid + ? await inspectShardedLegacyRegistry(target) + : { exists: false, valid: true, entries: [] }; + inspections.push({ + ...target, + exists: monolithic.exists || sharded.exists, + valid: monolithic.valid && sharded.valid, + entries: monolithic.entries.length + sharded.entries.length, + }); + } + return inspections; +} + +export async function migrateLegacySandboxRegistryFiles(): Promise< + LegacySandboxRegistryMigrationResult[] +> { + const results: LegacySandboxRegistryMigrationResult[] = []; + for (const target of legacyRegistryTargets()) { + results.push(await migrateTargetIfNeeded(target)); + } + return results; +} diff --git a/src/commands/doctor-sandbox.ts b/src/commands/doctor-sandbox.ts index ab0e88925de..ac890781f26 100644 --- a/src/commands/doctor-sandbox.ts +++ b/src/commands/doctor-sandbox.ts @@ -7,12 +7,6 @@ import { isDockerDaemonUnavailable, resolveSandboxScope, } from "../agents/sandbox.js"; -import { - inspectLegacySandboxRegistryFiles, - migrateLegacySandboxRegistryFiles, - type LegacySandboxRegistryInspection, - type LegacySandboxRegistryMigrationResult, -} from "../agents/sandbox/registry.js"; import { formatCliCommand } from "../cli/command-format.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { runCommandWithTimeout, runExec } from "../process/exec.js"; @@ -20,6 +14,12 @@ import type { RuntimeEnv } from "../runtime.js"; import { note } from "../terminal/note.js"; import { shortenHomePath } from "../utils.js"; import type { DoctorPrompter } from "./doctor-prompter.js"; +import { + inspectLegacySandboxRegistryFiles, + migrateLegacySandboxRegistryFiles, + type LegacySandboxRegistryInspection, + type LegacySandboxRegistryMigrationResult, +} from "./doctor-sandbox-registry-migration.js"; type SandboxScriptInfo = { scriptPath: string; diff --git a/src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts b/src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts index eeacd0e4fe2..7e51ceabd1c 100644 --- a/src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts +++ b/src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts @@ -21,7 +21,7 @@ vi.mock("../agents/sandbox.js", () => ({ resolveSandboxScope: vi.fn(() => "shared"), })); -vi.mock("../agents/sandbox/registry.js", () => ({ +vi.mock("./doctor-sandbox-registry-migration.js", () => ({ inspectLegacySandboxRegistryFiles, migrateLegacySandboxRegistryFiles, }));