fix(browser): keep upload cleanup out of startup

Co-authored-by: Dallin Romney <dallinromney@gmail.com>
This commit is contained in:
Vincent Koc
2026-08-19 00:51:20 -07:00
parent bd77ee1031
commit be0ed2fcd3
8 changed files with 230 additions and 23 deletions
@@ -0,0 +1,5 @@
/**
* Browser proxy upload cleanup runtime. Availability watches load only this
* narrow artifact instead of the full browser registration runtime.
*/
export { ensureBrowserProxyUploadCleanup } from "./src/browser-proxy-upload.js";
+4 -1
View File
@@ -45,6 +45,9 @@ const logger = createSubsystemLogger("browser");
const loadBrowserRegistrationRuntimeModule = createLazyRuntimeModule(
() => import("./register.runtime.js"),
);
const loadBrowserProxyUploadCleanupRuntimeModule = createLazyRuntimeModule(
() => import("./browser-proxy-upload-cleanup.runtime.js"),
);
function deriveChatTypeFromSessionKey(
sessionKey: string | undefined,
@@ -201,7 +204,7 @@ function createBrowserProxyNodeHostCommand(command: string): OpenClawPluginNodeH
...(command === BROWSER_PROXY_UPLOAD_COMMAND
? {
watchAvailability: () => {
void loadBrowserRegistrationRuntimeModule()
void loadBrowserProxyUploadCleanupRuntimeModule()
.then(({ ensureBrowserProxyUploadCleanup }) => ensureBrowserProxyUploadCleanup())
.catch((error: unknown) => {
logger.warn(`browser proxy upload cleanup startup failed: ${String(error)}`);
@@ -0,0 +1,36 @@
import { describe, expect, it, vi } from "vitest";
import { browserPluginNodeHostCommands } from "./plugin-registration.js";
const runtimeMocks = vi.hoisted(() => ({
ensureBrowserProxyUploadCleanup: vi.fn(async () => undefined),
cleanupRuntimeEvaluated: vi.fn(),
registrationRuntimeEvaluated: vi.fn(),
}));
vi.mock("./browser-proxy-upload-cleanup.runtime.js", () => {
runtimeMocks.cleanupRuntimeEvaluated();
return {
ensureBrowserProxyUploadCleanup: runtimeMocks.ensureBrowserProxyUploadCleanup,
};
});
vi.mock("./register.runtime.js", () => {
runtimeMocks.registrationRuntimeEvaluated();
return {};
});
describe("browser proxy upload cleanup registration", () => {
it("loads only the narrow cleanup runtime when availability watching starts", async () => {
const uploadCommand = browserPluginNodeHostCommands.find(
(command) => command.command === "browser.proxy.upload.v1",
);
uploadCommand?.watchAvailability?.();
await vi.waitFor(() => {
expect(runtimeMocks.ensureBrowserProxyUploadCleanup).toHaveBeenCalledOnce();
});
expect(runtimeMocks.cleanupRuntimeEvaluated).toHaveBeenCalledOnce();
expect(runtimeMocks.registrationRuntimeEvaluated).not.toHaveBeenCalled();
});
});
-1
View File
@@ -3,7 +3,6 @@
* registration lazy-load these exports when browser runtime behavior is needed.
*/
export { createBrowserTool } from "./src/browser-tool.js";
export { ensureBrowserProxyUploadCleanup } from "./src/browser-proxy-upload.js";
export { handleBrowserGatewayRequest } from "./src/gateway/browser-request.js";
export { runBrowserProxyCommand } from "./src/node-host/invoke-browser.js";
export { createBrowserPluginService } from "./src/plugin-service.js";
+2 -1
View File
@@ -6,7 +6,9 @@
*/
import os from "node:os";
import path from "node:path";
import { parseBrowserHttpUrl, redactCdpUrl } from "openclaw/plugin-sdk/browser-config";
import { mergeSsrFPolicies } from "openclaw/plugin-sdk/ssrf-policy";
import { isLoopbackHost } from "openclaw/plugin-sdk/ssrf-runtime";
import {
normalizeOptionalString,
normalizeOptionalTrimmedStringList,
@@ -21,7 +23,6 @@ import {
import type { SsrFPolicy } from "../infra/net/ssrf.js";
import { parseBooleanValue } from "../sdk-config.js";
import { resolveUserPath } from "../utils.js";
import { parseBrowserHttpUrl, redactCdpUrl, isLoopbackHost } from "./cdp.helpers.js";
import {
DEFAULT_AI_SNAPSHOT_MAX_CHARS,
DEFAULT_BROWSER_ACTION_TIMEOUT_MS,
@@ -19,7 +19,7 @@ type BuiltPluginControlPlaneModuleFailure = BuiltPluginControlPlaneModule & {
error: string;
};
type BuiltDoctorContractClosureViolation = BuiltPluginControlPlaneModule & {
type BuiltPluginControlPlaneClosureViolation = BuiltPluginControlPlaneModule & {
dependency: string;
importerPath: string;
};
@@ -31,6 +31,8 @@ type ProbeParams = {
const ROOT = resolveRepoRoot(import.meta.url);
const DIRECT_CONTRACT_FILES = ["contract-api.js", "doctor-contract-api.js"];
const BROWSER_PLUGIN_REGISTRATION_FILE = "index.js";
const BROWSER_PROXY_UPLOAD_CLEANUP_RUNTIME_FILE = "browser-proxy-upload-cleanup.runtime.js";
const LEGACY_SETUP_PROPERTIES = new Map<string, string>([
["legacyStateMigrations", "channel-legacy-state-migrations"],
["legacySessionSurface", "channel-legacy-session-surface"],
@@ -46,7 +48,11 @@ const DEFAULT_TIMEOUT_MS = 120_000;
// rejects them. `doctor-contract-closure-guard.test.ts` owns the same invariant over
// sources; bundling can merge runtime code into the artifact behind its back, so the
// built closure is checked here.
const FORBIDDEN_DOCTOR_CONTRACT_DEPENDENCIES = ["execa"];
const FORBIDDEN_STATIC_CLOSURE_DEPENDENCIES = new Map<string, string[]>([
["doctor-contract", ["execa"]],
["browser-plugin-registration", ["openclaw/plugin-sdk/gateway-runtime", "playwright-core"]],
["browser-proxy-upload-cleanup", ["openclaw/plugin-sdk/gateway-runtime", "playwright-core"]],
]);
const REQUIRE_PROBE_SOURCE = String.raw`
const { createRequire } = require("node:module");
const path = require("node:path");
@@ -122,6 +128,26 @@ export function listBuiltPluginControlPlaneModules(params: { rootDir?: string }
});
}
}
if (pluginId === "browser") {
const registrationPath = path.join(pluginDir, BROWSER_PLUGIN_REGISTRATION_FILE);
if (fs.existsSync(registrationPath)) {
const relativePath = path.relative(rootDir, registrationPath).split(path.sep).join("/");
modules.set(relativePath, {
pluginId,
kind: "browser-plugin-registration",
relativePath,
});
}
const modulePath = path.join(pluginDir, BROWSER_PROXY_UPLOAD_CLEANUP_RUNTIME_FILE);
if (fs.existsSync(modulePath)) {
const relativePath = path.relative(rootDir, modulePath).split(path.sep).join("/");
modules.set(relativePath, {
pluginId,
kind: "browser-proxy-upload-cleanup",
relativePath,
});
}
}
const setupEntryPath = path.join(pluginDir, "setup-entry.js");
if (!fs.existsSync(setupEntryPath)) {
continue;
@@ -243,20 +269,31 @@ function collectBuiltModuleStaticDependencies(entryPath: string): Map<string, st
return dependencies;
}
/** Fails when a built doctor artifact statically reaches a forbidden runtime dependency. */
export function collectBuiltDoctorContractClosureViolations(
function matchesForbiddenDependency(dependency: string, forbiddenDependency: string): boolean {
return dependency === forbiddenDependency || dependency.startsWith(`${forbiddenDependency}/`);
}
/** Fails when a built control-plane artifact statically reaches a forbidden runtime dependency. */
export function collectBuiltPluginControlPlaneClosureViolations(
modules: BuiltPluginControlPlaneModule[],
params: { rootDir?: string } = {},
): BuiltDoctorContractClosureViolation[] {
): BuiltPluginControlPlaneClosureViolation[] {
const rootDir = path.resolve(params.rootDir ?? ROOT);
const violations: BuiltDoctorContractClosureViolation[] = [];
for (const module of modules.filter((candidate) => candidate.kind === "doctor-contract")) {
const violations: BuiltPluginControlPlaneClosureViolation[] = [];
for (const module of modules) {
const forbiddenDependencies = FORBIDDEN_STATIC_CLOSURE_DEPENDENCIES.get(module.kind);
if (!forbiddenDependencies) {
continue;
}
const dependencies = collectBuiltModuleStaticDependencies(
path.join(rootDir, module.relativePath),
);
for (const dependency of FORBIDDEN_DOCTOR_CONTRACT_DEPENDENCIES) {
const importer = dependencies.get(dependency);
if (importer) {
for (const [dependency, importer] of dependencies) {
if (
forbiddenDependencies.some((forbiddenDependency) =>
matchesForbiddenDependency(dependency, forbiddenDependency),
)
) {
violations.push({
...module,
dependency,
@@ -279,18 +316,18 @@ export function verifyBuiltPluginControlPlaneModules(params: ProbeParams = {}) {
);
throw new Error(`built plugin control-plane module load failures:\n${details.join("\n")}`);
}
const closureViolations = collectBuiltDoctorContractClosureViolations(modules, params);
const closureViolations = collectBuiltPluginControlPlaneClosureViolations(modules, params);
if (closureViolations.length > 0) {
const details = closureViolations.map(
(violation) =>
`- ${violation.pluginId} ${violation.relativePath} statically reaches ${violation.dependency} through ${violation.importerPath}`,
);
throw new Error(
`built doctor contract closures reach forbidden runtime dependencies:\n${details.join("\n")}`,
`built plugin control-plane closures reach forbidden runtime dependencies:\n${details.join("\n")}`,
);
}
console.error(
`[plugin-control-plane-loads] verified ${modules.length} built modules with native require and checked doctor closures`,
`[plugin-control-plane-loads] verified ${modules.length} built modules with native require and checked guarded closures`,
);
}
@@ -77,6 +77,14 @@ describe("bundled plugin build entries", () => {
expect(artifacts).toContain("dist/extensions/codex/cli-metadata.js");
});
it("emits the browser proxy upload cleanup runtime as a bundled build entry", () => {
const entries = listBundledPluginBuildEntries();
expect(entries["extensions/browser/browser-proxy-upload-cleanup.runtime"]).toBe(
"extensions/browser/browser-proxy-upload-cleanup.runtime.ts",
);
});
it("builds narrow QA runner public surfaces", () => {
const entries = listBundledPluginBuildEntries();
@@ -4,7 +4,7 @@ import os from "node:os";
import path from "node:path";
import { afterEach, describe, expect, it, vi } from "vitest";
import {
collectBuiltDoctorContractClosureViolations,
collectBuiltPluginControlPlaneClosureViolations,
listBuiltPluginControlPlaneModules,
probeBuiltPluginControlPlaneModules,
verifyBuiltPluginControlPlaneModules,
@@ -123,9 +123,12 @@ describe("built doctor contract closures", () => {
write(rootDir, "dist/exec-chunk.js", 'import "execa";\nexport const rule = 1;\n');
expect(
collectBuiltDoctorContractClosureViolations(listBuiltPluginControlPlaneModules({ rootDir }), {
rootDir,
}),
collectBuiltPluginControlPlaneClosureViolations(
listBuiltPluginControlPlaneModules({ rootDir }),
{
rootDir,
},
),
).toEqual([
{
pluginId: "demo",
@@ -153,9 +156,124 @@ describe("built doctor contract closures", () => {
);
expect(
collectBuiltDoctorContractClosureViolations(listBuiltPluginControlPlaneModules({ rootDir }), {
rootDir,
}),
collectBuiltPluginControlPlaneClosureViolations(
listBuiltPluginControlPlaneModules({ rootDir }),
{
rootDir,
},
),
).toEqual([]);
});
});
describe("built browser startup closures", () => {
it("accepts narrow plugin registration and cleanup artifacts", () => {
const rootDir = makeRoot();
write(
rootDir,
"dist/extensions/browser/index.js",
'export { plugin } from "../../registration-chunk.js";\n',
);
write(
rootDir,
"dist/extensions/browser/browser-proxy-upload-cleanup.runtime.js",
'export { cleanup } from "../../cleanup-chunk.js";\n',
);
write(rootDir, "dist/registration-chunk.js", 'import "node:path";\nexport const plugin = 1;\n');
write(rootDir, "dist/cleanup-chunk.js", 'import "node:fs";\nexport const cleanup = 1;\n');
expect(
collectBuiltPluginControlPlaneClosureViolations(
listBuiltPluginControlPlaneModules({ rootDir }),
{ rootDir },
),
).toEqual([]);
});
it("roots the Playwright guard at the emitted Browser plugin startup entry", () => {
const rootDir = makeRoot();
write(
rootDir,
"dist/extensions/browser/index.js",
'export { plugin } from "../../registration-chunk.js";\n',
);
write(
rootDir,
"dist/registration-chunk.js",
'import "playwright-core/lib/server";\nexport const plugin = 1;\n',
);
expect(
collectBuiltPluginControlPlaneClosureViolations(
listBuiltPluginControlPlaneModules({ rootDir }),
{ rootDir },
),
).toEqual([
{
pluginId: "browser",
kind: "browser-plugin-registration",
relativePath: "dist/extensions/browser/index.js",
dependency: "playwright-core/lib/server",
importerPath: "dist/registration-chunk.js",
},
]);
});
it("also guards the cleanup artifact's own emitted closure", () => {
const rootDir = makeRoot();
write(
rootDir,
"dist/extensions/browser/browser-proxy-upload-cleanup.runtime.js",
'export { cleanup } from "../../cleanup-chunk.js";\n',
);
write(
rootDir,
"dist/cleanup-chunk.js",
'import "playwright-core";\nexport const cleanup = 1;\n',
);
expect(
collectBuiltPluginControlPlaneClosureViolations(
listBuiltPluginControlPlaneModules({ rootDir }),
{ rootDir },
),
).toEqual([
{
pluginId: "browser",
kind: "browser-proxy-upload-cleanup",
relativePath: "dist/extensions/browser/browser-proxy-upload-cleanup.runtime.js",
dependency: "playwright-core",
importerPath: "dist/cleanup-chunk.js",
},
]);
});
it("rejects the broad Gateway runtime from Browser startup closures", () => {
const rootDir = makeRoot();
write(
rootDir,
"dist/extensions/browser/index.js",
'export { plugin } from "../../registration-chunk.js";\n',
);
write(
rootDir,
"dist/registration-chunk.js",
'import "openclaw/plugin-sdk/gateway-runtime";\nexport const plugin = 1;\n',
);
expect(
collectBuiltPluginControlPlaneClosureViolations(
listBuiltPluginControlPlaneModules({ rootDir }),
{ rootDir },
),
).toEqual([
{
pluginId: "browser",
kind: "browser-plugin-registration",
relativePath: "dist/extensions/browser/index.js",
dependency: "openclaw/plugin-sdk/gateway-runtime",
importerPath: "dist/registration-chunk.js",
},
]);
});
});