refactor(sandbox): simplify legacy registry migration (#120895)

This commit is contained in:
Peter Steinberger
2026-08-08 22:03:58 -07:00
committed by GitHub
parent 6192673da4
commit 3aec297be0
4 changed files with 124 additions and 207 deletions
@@ -36,6 +36,7 @@ vi.mock("../agents/sandbox/constants.js", () => ({
SANDBOX_BROWSERS_DIR,
}));
import { writeJsonFile } from "../../test/helpers/temp-repo.js";
import { hashTextSha256 } from "../agents/sandbox/hash.js";
import {
readBrowserRegistry,
@@ -98,37 +99,13 @@ function containerEntry(overrides: Partial<SandboxRegistryEntry> = {}): SandboxR
};
}
async function seedContainerRegistry(entries: SandboxRegistryEntry[]) {
await fs.writeFile(SANDBOX_REGISTRY_PATH, `${JSON.stringify({ entries }, null, 2)}\n`, "utf-8");
function seedRegistry(registryPath: string, entries: readonly unknown[]) {
writeJsonFile(registryPath, { entries });
}
async function seedBrowserRegistry(entries: SandboxBrowserRegistryEntry[]) {
await fs.writeFile(
SANDBOX_BROWSER_REGISTRY_PATH,
`${JSON.stringify({ entries }, null, 2)}\n`,
"utf-8",
);
}
async function seedShardedContainerRegistry(entries: SandboxRegistryEntry[]) {
await fs.mkdir(SANDBOX_CONTAINERS_DIR, { recursive: true });
function seedShardedRegistry(dir: string, entries: readonly { containerName: string }[]) {
for (const entry of entries) {
await fs.writeFile(
path.join(SANDBOX_CONTAINERS_DIR, `${hashTextSha256(entry.containerName)}.json`),
`${JSON.stringify(entry, null, 2)}\n`,
"utf-8",
);
}
}
async function seedShardedBrowserRegistry(entries: SandboxBrowserRegistryEntry[]) {
await fs.mkdir(SANDBOX_BROWSERS_DIR, { recursive: true });
for (const entry of entries) {
await fs.writeFile(
path.join(SANDBOX_BROWSERS_DIR, `${hashTextSha256(entry.containerName)}.json`),
`${JSON.stringify(entry, null, 2)}\n`,
"utf-8",
);
writeJsonFile(path.join(dir, `${hashTextSha256(entry.containerName)}.json`), entry);
}
}
@@ -162,29 +139,8 @@ function requireMigrationResult(
}
describe("legacy sandbox registry migration", () => {
it("normalizes legacy registry entries after explicit migration", async () => {
await seedContainerRegistry([
{
containerName: "legacy-container",
sessionKey: "agent:main",
createdAtMs: 1,
lastUsedAtMs: 1,
image: "openclaw-sandbox:test",
},
]);
await migrateLegacySandboxRegistryFiles();
const registry = await readRegistry();
expect(registry.entries).toHaveLength(1);
const [entry] = registry.entries;
expect(entry?.containerName).toBe("legacy-container");
expect(entry?.backendId).toBe("docker");
expect(entry?.runtimeLabel).toBe("legacy-container");
expect(entry?.configLabelKind).toBe("Image");
});
it("migrates legacy monolithic container and browser registry files after explicit repair", async () => {
await seedContainerRegistry([
seedRegistry(SANDBOX_REGISTRY_PATH, [
containerEntry({
containerName: "legacy-container",
sessionKey: "agent:legacy",
@@ -192,7 +148,7 @@ describe("legacy sandbox registry migration", () => {
configHash: "legacy-container-hash",
}),
]);
await seedBrowserRegistry([
seedRegistry(SANDBOX_BROWSER_REGISTRY_PATH, [
browserEntry({
containerName: "legacy-browser",
sessionKey: "agent:legacy",
@@ -205,12 +161,10 @@ describe("legacy sandbox registry migration", () => {
await seedStaleLock(`${SANDBOX_BROWSER_REGISTRY_PATH}.lock`);
const migrationResults = await migrateLegacySandboxRegistryFiles();
const containerMigration = requireMigrationResult(migrationResults, "containers");
const browserMigration = requireMigrationResult(migrationResults, "browsers");
expect(containerMigration.status).toBe("migrated");
expect(containerMigration.entries).toBe(1);
expect(browserMigration.status).toBe("migrated");
expect(browserMigration.entries).toBe(1);
expect(migrationResults).toEqual([
{ kind: "containers", status: "migrated", entries: 1 },
{ kind: "browsers", status: "migrated", entries: 1 },
]);
await expectPathMissing(SANDBOX_REGISTRY_PATH);
await expectPathMissing(SANDBOX_BROWSER_REGISTRY_PATH);
@@ -222,6 +176,7 @@ describe("legacy sandbox registry migration", () => {
expect(container?.containerName).toBe("legacy-container");
expect(container?.backendId).toBe("docker");
expect(container?.runtimeLabel).toBe("legacy-container");
expect(container?.configLabelKind).toBe("Image");
expect(container?.sessionKey).toBe("agent:legacy");
expect(container?.configHash).toBe("legacy-container-hash");
const browserRegistry = await readBrowserRegistry();
@@ -235,7 +190,7 @@ describe("legacy sandbox registry migration", () => {
});
it("migrates legacy sharded container and browser registry files after explicit repair", async () => {
await seedShardedContainerRegistry([
seedShardedRegistry(SANDBOX_CONTAINERS_DIR, [
containerEntry({
containerName: "legacy-container",
sessionKey: "agent:legacy",
@@ -243,7 +198,7 @@ describe("legacy sandbox registry migration", () => {
configHash: "legacy-container-hash",
}),
]);
await seedShardedBrowserRegistry([
seedShardedRegistry(SANDBOX_BROWSERS_DIR, [
browserEntry({
containerName: "legacy-browser",
sessionKey: "agent:legacy",
@@ -270,7 +225,7 @@ describe("legacy sandbox registry migration", () => {
lastUsedAtMs: 10,
}),
);
await seedContainerRegistry([
seedRegistry(SANDBOX_REGISTRY_PATH, [
containerEntry({
containerName: "container-a",
sessionKey: "legacy-session",
@@ -286,14 +241,14 @@ describe("legacy sandbox registry migration", () => {
});
it("prefers newer sharded entries over stale monolithic entries during legacy migration", async () => {
await seedContainerRegistry([
seedRegistry(SANDBOX_REGISTRY_PATH, [
containerEntry({
containerName: "container-a",
sessionKey: "legacy-session",
lastUsedAtMs: 1,
}),
]);
await seedShardedContainerRegistry([
seedShardedRegistry(SANDBOX_CONTAINERS_DIR, [
containerEntry({
containerName: "container-a",
sessionKey: "sharded-session",
@@ -333,12 +288,10 @@ describe("legacy sandbox registry migration", () => {
});
it("quarantines malformed sharded registry directories during migration", async () => {
await fs.mkdir(SANDBOX_CONTAINERS_DIR, { recursive: true });
await fs.mkdir(SANDBOX_BROWSERS_DIR, { recursive: true });
await seedShardedContainerRegistry([
seedShardedRegistry(SANDBOX_CONTAINERS_DIR, [
containerEntry({ containerName: "valid-container", sessionKey: "agent:valid" }),
]);
await seedShardedBrowserRegistry([
seedShardedRegistry(SANDBOX_BROWSERS_DIR, [
browserEntry({ containerName: "valid-browser", sessionKey: "agent:valid" }),
]);
await fs.writeFile(path.join(SANDBOX_CONTAINERS_DIR, "bad.json"), "{bad json", "utf-8");
+94 -118
View File
@@ -12,47 +12,9 @@ import {
insertSandboxBrowserRegistryEntryIfMissing,
insertSandboxRegistryEntryIfMissing,
} from "../agents/sandbox/registry.js";
import { acquireFileLock } from "../infra/file-lock.js";
import { withFileLock } from "../infra/file-lock.js";
import { safeParseJsonWithSchema } from "../utils/zod-parse.js";
type RegistryEntry = {
containerName: string;
};
type RegistryEntryPayload = RegistryEntry & Record<string, unknown>;
type RegistryFile = {
entries: RegistryEntryPayload[];
};
type ShardedRegistryRead<T extends RegistryEntry> = {
entries: T[];
validFiles: string[];
invalidFiles: string[];
};
type LegacyRegistryKind = "containers" | "browsers";
type LegacyRegistryTarget = {
kind: LegacyRegistryKind;
registryPath: string;
shardedDir: string;
};
export type LegacySandboxRegistryInspection = LegacyRegistryTarget & {
exists: boolean;
valid: boolean;
entries: number;
source: "monolithic" | "sharded";
};
export type LegacySandboxRegistryMigrationResult = LegacyRegistryTarget & {
status: "missing" | "migrated" | "removed-empty" | "quarantined-invalid";
entries: number;
source?: "monolithic" | "sharded";
quarantinePath?: string;
};
const RegistryEntrySchema = z
.object({
containerName: z.string(),
@@ -63,23 +25,46 @@ const RegistryFileSchema = z.object({
entries: z.array(RegistryEntrySchema),
});
async function withRegistryLock<T>(registryPath: string, fn: () => Promise<T>): Promise<T> {
const lock = await acquireFileLock(registryPath, {
stale: 30_000,
retries: { retries: 59, factor: 1, minTimeout: 1_000, maxTimeout: 1_000 },
});
try {
return await fn();
} finally {
await lock.release();
}
}
type RegistryEntryPayload = z.infer<typeof RegistryEntrySchema>;
type RegistryFile = z.infer<typeof RegistryFileSchema>;
type ShardedRegistryRead = {
entries: RegistryEntryPayload[];
invalidFiles: string[];
};
type LegacyRegistryKind = "containers" | "browsers";
type LegacyRegistryTarget = {
kind: LegacyRegistryKind;
registryPath: string;
shardedDir: string;
};
export type LegacySandboxRegistryInspection = {
kind: LegacyRegistryKind;
path: string;
exists: boolean;
valid: boolean;
entries: number;
source: "monolithic" | "sharded";
};
export type LegacySandboxRegistryMigrationResult = { kind: LegacyRegistryKind } & (
| { status: "missing" }
| { status: "migrated"; entries: number }
| { status: "removed-empty" }
| {
status: "quarantined-invalid";
path: string;
quarantinePath: string;
}
);
async function readLegacyRegistryFile(registryPath: string): Promise<RegistryFile | null> {
try {
const raw = await fs.readFile(registryPath, "utf-8");
const parsed = safeParseJsonWithSchema(RegistryFileSchema, raw) as RegistryFile | null;
return parsed;
return safeParseJsonWithSchema(RegistryFileSchema, raw);
} catch (error) {
const code = (error as { code?: string } | null)?.code;
if (code === "ENOENT") {
@@ -92,22 +77,19 @@ async function readLegacyRegistryFile(registryPath: string): Promise<RegistryFil
}
}
async function readShardedEntriesDetailed<T extends RegistryEntry>(
dir: string,
): Promise<ShardedRegistryRead<T>> {
async function readShardedEntriesDetailed(dir: string): Promise<ShardedRegistryRead> {
let files: string[];
try {
files = await fs.readdir(dir);
} catch (error) {
const code = (error as { code?: string } | null)?.code;
if (code === "ENOENT") {
return { entries: [], validFiles: [], invalidFiles: [] };
return { entries: [], invalidFiles: [] };
}
throw error;
}
const invalidFiles: string[] = [];
const validFiles: string[] = [];
const entries = await Promise.all(
files
.filter((name) => name.endsWith(".json"))
@@ -116,11 +98,9 @@ async function readShardedEntriesDetailed<T extends RegistryEntry>(
const filePath = path.join(dir, name);
try {
const raw = await fs.readFile(filePath, "utf-8");
const entry = safeParseJsonWithSchema(RegistryEntrySchema, raw) as T | null;
const entry = safeParseJsonWithSchema(RegistryEntrySchema, raw);
if (!entry) {
invalidFiles.push(filePath);
} else {
validFiles.push(filePath);
}
return entry;
} catch {
@@ -129,17 +109,10 @@ async function readShardedEntriesDetailed<T extends RegistryEntry>(
}
}),
);
const validEntries: T[] = [];
for (const entry of entries) {
if (entry) {
validEntries.push(entry);
}
}
return {
entries: validEntries.toSorted((left, right) =>
left.containerName.localeCompare(right.containerName),
),
validFiles: validFiles.toSorted(),
entries: entries
.filter((entry) => entry !== null)
.toSorted((left, right) => left.containerName.localeCompare(right.containerName)),
invalidFiles: invalidFiles.toSorted(),
};
}
@@ -174,10 +147,6 @@ async function quarantineInvalidShards(
return quarantineDir;
}
async function removeFiles(files: readonly string[]): Promise<void> {
await Promise.all(files.map((file) => fs.rm(file, { force: true })));
}
function writeLegacyEntryIfMissing(kind: LegacyRegistryKind, entry: RegistryEntryPayload): void {
if (kind === "containers") {
insertSandboxRegistryEntryIfMissing({
@@ -210,38 +179,43 @@ async function migrateMonolithicIfNeeded(
} catch (error) {
const code = (error as { code?: string } | null)?.code;
if (code === "ENOENT") {
return { ...target, source: "monolithic", status: "missing", entries: 0 };
return { kind: target.kind, status: "missing" };
}
throw error;
}
return await withRegistryLock(registryPath, async () => {
const registry = await readLegacyRegistryFile(registryPath);
if (!registry) {
const quarantinePath = await quarantineLegacyRegistry(registryPath);
return {
...target,
source: "monolithic",
status: "quarantined-invalid",
entries: 0,
quarantinePath,
};
}
if (registry.entries.length === 0) {
return await withFileLock(
registryPath,
{
stale: 30_000,
retries: { retries: 59, factor: 1, minTimeout: 1_000, maxTimeout: 1_000 },
},
async () => {
const registry = await readLegacyRegistryFile(registryPath);
if (!registry) {
const quarantinePath = await quarantineLegacyRegistry(registryPath);
return {
kind: target.kind,
status: "quarantined-invalid",
path: registryPath,
quarantinePath,
};
}
if (registry.entries.length === 0) {
await fs.rm(registryPath, { force: true });
return { kind: target.kind, status: "removed-empty" };
}
for (const entry of registry.entries) {
writeLegacyEntryIfMissing(target.kind, entry);
}
await fs.rm(registryPath, { force: true });
return { ...target, source: "monolithic", status: "removed-empty", entries: 0 };
}
for (const entry of registry.entries) {
writeLegacyEntryIfMissing(target.kind, entry);
}
await fs.rm(registryPath, { force: true });
return {
...target,
source: "monolithic",
status: "migrated",
entries: registry.entries.length,
};
});
return {
kind: target.kind,
status: "migrated",
entries: registry.entries.length,
};
},
);
}
async function migrateShardedIfNeeded(
@@ -258,34 +232,31 @@ async function migrateShardedIfNeeded(
}
}
if (!dirExists) {
return { ...target, source: "sharded", status: "missing", entries: 0 };
return { kind: target.kind, status: "missing" };
}
const { entries, validFiles, invalidFiles } =
await readShardedEntriesDetailed<RegistryEntryPayload>(target.shardedDir);
const { entries, invalidFiles } = await readShardedEntriesDetailed(target.shardedDir);
if (invalidFiles.length > 0) {
for (const entry of entries) {
writeLegacyEntryIfMissing(target.kind, entry);
}
await removeFiles(validFiles);
const quarantinePath = await quarantineInvalidShards(target.shardedDir, invalidFiles);
await fs.rm(target.shardedDir, { recursive: true, force: true });
return {
...target,
source: "sharded",
kind: target.kind,
status: "quarantined-invalid",
entries: entries.length,
path: target.shardedDir,
quarantinePath,
};
}
if (entries.length === 0) {
await fs.rm(target.shardedDir, { recursive: true, force: true });
return { ...target, source: "sharded", status: "removed-empty", entries: 0 };
return { kind: target.kind, status: "removed-empty" };
}
for (const entry of entries) {
writeLegacyEntryIfMissing(target.kind, entry);
}
await fs.rm(target.shardedDir, { recursive: true, force: true });
return { ...target, source: "sharded", status: "migrated", entries: entries.length };
return { kind: target.kind, status: "migrated", entries: entries.length };
}
function combineMigrationResults(
@@ -299,14 +270,16 @@ function combineMigrationResults(
if (sharded.status === "quarantined-invalid") {
return sharded;
}
const entries = monolithic.entries + sharded.entries;
const entries =
(monolithic.status === "migrated" ? monolithic.entries : 0) +
(sharded.status === "migrated" ? sharded.entries : 0);
if (entries > 0) {
return { ...target, status: "migrated", entries };
return { kind: target.kind, status: "migrated", entries };
}
if (monolithic.status === "removed-empty" || sharded.status === "removed-empty") {
return { ...target, status: "removed-empty", entries: 0 };
return { kind: target.kind, status: "removed-empty" };
}
return { ...target, status: "missing", entries: 0 };
return { kind: target.kind, status: "missing" };
}
function legacyRegistryTargets(): LegacyRegistryTarget[] {
@@ -336,7 +309,8 @@ export async function inspectLegacySandboxRegistryFiles(): Promise<
const code = (error as { code?: string } | null)?.code;
if (code === "ENOENT") {
inspections.push({
...target,
kind: target.kind,
path: target.registryPath,
source: "monolithic",
exists: false,
valid: true,
@@ -350,7 +324,8 @@ export async function inspectLegacySandboxRegistryFiles(): Promise<
if (!inspections.some((entry) => entry.kind === target.kind && entry.source === "monolithic")) {
const registry = await readLegacyRegistryFile(target.registryPath);
inspections.push({
...target,
kind: target.kind,
path: target.registryPath,
source: "monolithic",
exists: true,
valid: Boolean(registry),
@@ -358,7 +333,7 @@ export async function inspectLegacySandboxRegistryFiles(): Promise<
});
}
const sharded = await readShardedEntriesDetailed<RegistryEntryPayload>(target.shardedDir);
const sharded = await readShardedEntriesDetailed(target.shardedDir);
let shardedExists = false;
try {
shardedExists = (await fs.stat(target.shardedDir)).isDirectory();
@@ -369,7 +344,8 @@ export async function inspectLegacySandboxRegistryFiles(): Promise<
}
}
inspections.push({
...target,
kind: target.kind,
path: target.shardedDir,
source: "sharded",
exists: shardedExists,
valid: sharded.invalidFiles.length === 0,
+5 -11
View File
@@ -409,12 +409,7 @@ export async function maybeRepairSandboxImages(
function formatLegacyRegistryInspectionLine(file: LegacySandboxRegistryInspection): string {
const status = file.valid ? `${file.entries} entr${file.entries === 1 ? "y" : "ies"}` : "invalid";
const sourcePath = legacySandboxRegistryInspectionSourcePath(file);
return `- ${file.kind} ${file.source}: ${shortenHomePath(sourcePath)} (${status})`;
}
function legacySandboxRegistryInspectionSourcePath(file: LegacySandboxRegistryInspection): string {
return file.source === "sharded" ? file.shardedDir : file.registryPath;
return `- ${file.kind} ${file.source}: ${shortenHomePath(file.path)} (${status})`;
}
function formatLegacyRegistryMigrationLine(result: LegacySandboxRegistryMigrationResult): string {
@@ -425,9 +420,8 @@ function formatLegacyRegistryMigrationLine(result: LegacySandboxRegistryMigratio
return `- Removed empty legacy ${result.kind} registry files.`;
}
if (result.status === "quarantined-invalid") {
const sourcePath = result.source === "sharded" ? result.shardedDir : result.registryPath;
const file = shortenHomePath(sourcePath);
const quarantine = result.quarantinePath ? ` to ${shortenHomePath(result.quarantinePath)}` : "";
const file = shortenHomePath(result.path);
const quarantine = ` to ${shortenHomePath(result.quarantinePath)}`;
return `- Quarantined invalid legacy ${result.kind} registry ${file}${quarantine}.`;
}
return "";
@@ -447,7 +441,7 @@ export function legacySandboxRegistryInspectionToHealthFinding(
severity: "warning",
message: `Legacy sandbox registry file detected.
${formatLegacyRegistryInspectionLine(file)}`,
path: legacySandboxRegistryInspectionSourcePath(file),
path: file.path,
fixHint: `Run ${formatCliCommand("openclaw doctor --fix")} to migrate valid entries to SQLite.`,
};
}
@@ -463,7 +457,7 @@ export function legacySandboxRegistryInspectionToRepairEffect(
return {
kind: "state",
action,
target: legacySandboxRegistryInspectionSourcePath(file),
target: file.path,
dryRunSafe: false,
};
}
@@ -302,8 +302,7 @@ describe("maybeRepairSandboxRegistryFiles", () => {
inspectLegacySandboxRegistryFiles.mockResolvedValue([
{
kind: "containers",
registryPath: "/tmp/openclaw/sandbox/containers.json",
shardedDir: "/tmp/openclaw/sandbox/containers",
path: "/tmp/openclaw/sandbox/containers.json",
source: "monolithic",
exists: true,
valid: true,
@@ -328,8 +327,7 @@ describe("maybeRepairSandboxRegistryFiles", () => {
inspectLegacySandboxRegistryFiles.mockResolvedValue([
{
kind: "containers",
registryPath: "/tmp/openclaw/sandbox/containers.json",
shardedDir: "/tmp/openclaw/sandbox/containers",
path: "/tmp/openclaw/sandbox/containers.json",
source: "monolithic",
exists: true,
valid: true,
@@ -339,8 +337,6 @@ describe("maybeRepairSandboxRegistryFiles", () => {
migrateLegacySandboxRegistryFiles.mockResolvedValue([
{
kind: "containers",
registryPath: "/tmp/openclaw/sandbox/containers.json",
shardedDir: "/tmp/openclaw/sandbox/containers",
status: "migrated",
entries: 2,
},
@@ -361,8 +357,7 @@ describe("maybeRepairSandboxRegistryFiles", () => {
it("maps legacy registry files to structured findings and dry-run effects", () => {
const monolithicFile = {
kind: "containers",
registryPath: "/tmp/openclaw/sandbox/containers.json",
shardedDir: "/tmp/openclaw/sandbox/containers",
path: "/tmp/openclaw/sandbox/containers.json",
source: "monolithic",
exists: true,
valid: true,
@@ -370,6 +365,7 @@ describe("maybeRepairSandboxRegistryFiles", () => {
} as const;
const shardedFile = {
...monolithicFile,
path: "/tmp/openclaw/sandbox/containers",
source: "sharded",
} as const;
@@ -406,8 +402,7 @@ describe("maybeRepairSandboxRegistryFiles", () => {
expect(
legacySandboxRegistryInspectionToRepairEffect({
kind: "browsers",
registryPath: "/tmp/openclaw/sandbox/browsers.json",
shardedDir: "/tmp/openclaw/sandbox/browsers",
path: "/tmp/openclaw/sandbox/browsers.json",
source: "monolithic",
exists: true,
valid: false,
@@ -425,8 +420,7 @@ describe("maybeRepairSandboxRegistryFiles", () => {
expect(
legacySandboxRegistryInspectionToRepairEffect({
kind: "containers",
registryPath: "/tmp/openclaw/sandbox/containers.json",
shardedDir: "/tmp/openclaw/sandbox/containers",
path: "/tmp/openclaw/sandbox/containers.json",
source: "monolithic",
exists: true,
valid: true,