From c8b7058058fcdbe30dd1d0eabc55830a2f0be292 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 7 Apr 2026 07:50:16 +0100 Subject: [PATCH] perf(agents): remove slow browser and auth test paths --- ...uth-profiles.runtime-snapshot-save.test.ts | 72 ------- src/agents/auth-profiles.store.save.test.ts | 7 +- src/agents/auth-profiles/usage.test.ts | 54 +++++ ...w-tools.browser-plugin.integration.test.ts | 108 +++++----- src/agents/pi-tools.sandbox-policy.test.ts | 203 ------------------ src/agents/tools/pdf-tool.test.ts | 45 ++-- src/agents/tools/pdf-tool.ts | 34 +-- 7 files changed, 141 insertions(+), 382 deletions(-) delete mode 100644 src/agents/auth-profiles.runtime-snapshot-save.test.ts delete mode 100644 src/agents/pi-tools.sandbox-policy.test.ts diff --git a/src/agents/auth-profiles.runtime-snapshot-save.test.ts b/src/agents/auth-profiles.runtime-snapshot-save.test.ts deleted file mode 100644 index d9146a7b1ee..00000000000 --- a/src/agents/auth-profiles.runtime-snapshot-save.test.ts +++ /dev/null @@ -1,72 +0,0 @@ -import fs from "node:fs/promises"; -import os from "node:os"; -import path from "node:path"; -import { describe, expect, it } from "vitest"; -import { - activateSecretsRuntimeSnapshot, - clearSecretsRuntimeSnapshot, - prepareSecretsRuntimeSnapshot, -} from "../secrets/runtime.js"; -import { ensureAuthProfileStore, markAuthProfileUsed } from "./auth-profiles.js"; - -describe("auth profile runtime snapshot persistence", () => { - it("does not write resolved plaintext keys during usage updates", async () => { - const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-auth-runtime-save-")); - const agentDir = path.join(stateDir, "agents", "main", "agent"); - const authPath = path.join(agentDir, "auth-profiles.json"); - try { - await fs.mkdir(agentDir, { recursive: true }); - await fs.writeFile( - authPath, - `${JSON.stringify( - { - version: 1, - profiles: { - "openai:default": { - type: "api_key", - provider: "openai", - keyRef: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, - }, - }, - }, - null, - 2, - )}\n`, - "utf8", - ); - - const snapshot = await prepareSecretsRuntimeSnapshot({ - config: {}, - env: { OPENAI_API_KEY: "sk-runtime-openai" }, // pragma: allowlist secret - agentDirs: [agentDir], - }); - activateSecretsRuntimeSnapshot(snapshot); - - const runtimeStore = ensureAuthProfileStore(agentDir); - expect(runtimeStore.profiles["openai:default"]).toMatchObject({ - type: "api_key", - key: "sk-runtime-openai", - keyRef: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, - }); - - await markAuthProfileUsed({ - store: runtimeStore, - profileId: "openai:default", - agentDir, - }); - - const persisted = JSON.parse(await fs.readFile(authPath, "utf8")) as { - profiles: Record; - }; - expect(persisted.profiles["openai:default"]?.key).toBeUndefined(); - expect(persisted.profiles["openai:default"]?.keyRef).toEqual({ - source: "env", - provider: "default", - id: "OPENAI_API_KEY", - }); - } finally { - clearSecretsRuntimeSnapshot(); - await fs.rm(stateDir, { recursive: true, force: true }); - } - }); -}); diff --git a/src/agents/auth-profiles.store.save.test.ts b/src/agents/auth-profiles.store.save.test.ts index 85f6da5803e..3bd309986af 100644 --- a/src/agents/auth-profiles.store.save.test.ts +++ b/src/agents/auth-profiles.store.save.test.ts @@ -1,7 +1,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { resolveAuthStatePath, resolveAuthStorePath } from "./auth-profiles/paths.js"; import { clearRuntimeAuthProfileStoreSnapshots, @@ -11,6 +11,11 @@ import { } from "./auth-profiles/store.js"; import type { AuthProfileStore } from "./auth-profiles/types.js"; +vi.mock("./auth-profiles/external-auth.js", () => ({ + overlayExternalAuthProfiles: (store: T) => store, + shouldPersistExternalAuthProfile: () => true, +})); + describe("saveAuthProfileStore", () => { it("strips plaintext when keyRef/tokenRef are present", async () => { const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-auth-save-")); diff --git a/src/agents/auth-profiles/usage.test.ts b/src/agents/auth-profiles/usage.test.ts index 459e98ffca9..f83b4209761 100644 --- a/src/agents/auth-profiles/usage.test.ts +++ b/src/agents/auth-profiles/usage.test.ts @@ -6,6 +6,7 @@ import { clearExpiredCooldowns, isProfileInCooldown, markAuthProfileFailure, + markAuthProfileUsed, resolveProfilesUnavailableReason, resolveProfileUnusableUntil, resolveProfileUnusableUntilForDisplay, @@ -602,6 +603,59 @@ describe("clearAuthProfileCooldown", () => { }); }); +describe("markAuthProfileUsed", () => { + it("updates usage stats and persists through the fallback save path when lock update misses", async () => { + const store = makeStore({ + "anthropic:default": { + errorCount: 3, + cooldownUntil: Date.now() + 60_000, + }, + }); + + storeMocks.updateAuthProfileStoreWithLock.mockResolvedValue(null); + + await markAuthProfileUsed({ + store, + profileId: "anthropic:default", + agentDir: "/tmp/openclaw-auth-profiles-used", + }); + + expect(storeMocks.saveAuthProfileStore).toHaveBeenCalledWith( + store, + "/tmp/openclaw-auth-profiles-used", + ); + expect(store.usageStats?.["anthropic:default"]?.errorCount).toBe(0); + expect(store.usageStats?.["anthropic:default"]?.cooldownUntil).toBeUndefined(); + expect(store.usageStats?.["anthropic:default"]?.lastUsed).toEqual(expect.any(Number)); + }); + + it("adopts locked store usage stats without saving locally when lock update succeeds", async () => { + const store = makeStore({ + "anthropic:default": { + errorCount: 3, + cooldownUntil: Date.now() + 60_000, + }, + }); + const lockedStore = makeStore({ + "anthropic:default": { + lastUsed: 123_456, + errorCount: 0, + }, + }); + + storeMocks.updateAuthProfileStoreWithLock.mockResolvedValue(lockedStore); + + await markAuthProfileUsed({ + store, + profileId: "anthropic:default", + agentDir: "/tmp/openclaw-auth-profiles-used", + }); + + expect(storeMocks.saveAuthProfileStore).not.toHaveBeenCalled(); + expect(store.usageStats).toEqual(lockedStore.usageStats); + }); +}); + describe("markAuthProfileFailure — active windows do not extend on retry", () => { // Regression for https://github.com/openclaw/openclaw/issues/23516 // When all providers are at saturation backoff (60 min) and retries fire every 30 min, diff --git a/src/agents/openclaw-tools.browser-plugin.integration.test.ts b/src/agents/openclaw-tools.browser-plugin.integration.test.ts index f71f3656b37..d18e0f593d0 100644 --- a/src/agents/openclaw-tools.browser-plugin.integration.test.ts +++ b/src/agents/openclaw-tools.browser-plugin.integration.test.ts @@ -1,78 +1,68 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { createBundledBrowserPluginFixture } from "../../test/helpers/browser-bundled-plugin-fixture.js"; +import { afterEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; -import { clearPluginDiscoveryCache } from "../plugins/discovery.js"; -import { clearPluginLoaderCache } from "../plugins/loader.js"; -import { clearPluginManifestRegistryCache } from "../plugins/manifest-registry.js"; -import { resetPluginRuntimeStateForTest } from "../plugins/runtime.js"; import { resolveOpenClawPluginToolsForOptions } from "./openclaw-plugin-tools.js"; -function resetPluginState() { - clearPluginLoaderCache(); - clearPluginDiscoveryCache(); - clearPluginManifestRegistryCache(); - resetPluginRuntimeStateForTest(); -} +const hoisted = vi.hoisted(() => ({ + resolvePluginTools: vi.fn(), +})); + +vi.mock("../plugins/tools.js", () => ({ + resolvePluginTools: (...args: unknown[]) => hoisted.resolvePluginTools(...args), +})); describe("createOpenClawTools browser plugin integration", () => { - let bundledFixture: ReturnType | null = null; - - beforeEach(() => { - bundledFixture = createBundledBrowserPluginFixture(); - vi.stubEnv("OPENCLAW_BUNDLED_PLUGINS_DIR", bundledFixture.rootDir); - resetPluginState(); - }); - afterEach(() => { - resetPluginState(); - vi.unstubAllEnvs(); - bundledFixture?.cleanup(); - bundledFixture = null; + hoisted.resolvePluginTools.mockReset(); }); - it("loads the bundled browser plugin through normal plugin resolution", () => { - const tools = resolveOpenClawPluginToolsForOptions({ - options: { - config: { - plugins: { - allow: ["browser"], - }, - } as OpenClawConfig, - }, - resolvedConfig: { - plugins: { - allow: ["browser"], + it("keeps the browser tool returned by plugin resolution", () => { + hoisted.resolvePluginTools.mockReturnValue([ + { + name: "browser", + description: "browser fixture tool", + parameters: { + type: "object", + properties: {}, }, - } as OpenClawConfig, + async execute() { + return { + content: [{ type: "text", text: "ok" }], + }; + }, + }, + ]); + + const config = { + plugins: { + allow: ["browser"], + }, + } as OpenClawConfig; + + const tools = resolveOpenClawPluginToolsForOptions({ + options: { config }, + resolvedConfig: config, }); expect(tools.map((tool) => tool.name)).toContain("browser"); }); - it("omits the browser tool when the bundled browser plugin is disabled", () => { - const tools = resolveOpenClawPluginToolsForOptions({ - options: { - config: { - plugins: { - allow: ["browser"], - entries: { - browser: { - enabled: false, - }, - }, - }, - } as OpenClawConfig, - }, - resolvedConfig: { - plugins: { - allow: ["browser"], - entries: { - browser: { - enabled: false, - }, + it("omits the browser tool when plugin resolution returns no browser tool", () => { + hoisted.resolvePluginTools.mockReturnValue([]); + + const config = { + plugins: { + allow: ["browser"], + entries: { + browser: { + enabled: false, }, }, - } as OpenClawConfig, + }, + } as OpenClawConfig; + + const tools = resolveOpenClawPluginToolsForOptions({ + options: { config }, + resolvedConfig: config, }); expect(tools.map((tool) => tool.name)).not.toContain("browser"); diff --git a/src/agents/pi-tools.sandbox-policy.test.ts b/src/agents/pi-tools.sandbox-policy.test.ts deleted file mode 100644 index 4d8a7be65c6..00000000000 --- a/src/agents/pi-tools.sandbox-policy.test.ts +++ /dev/null @@ -1,203 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { createBundledBrowserPluginFixture } from "../../test/helpers/browser-bundled-plugin-fixture.js"; -import type { OpenClawConfig } from "../config/config.js"; -import { clearPluginDiscoveryCache } from "../plugins/discovery.js"; -import { clearPluginLoaderCache } from "../plugins/loader.js"; -import { clearPluginManifestRegistryCache } from "../plugins/manifest-registry.js"; -import { resetPluginRuntimeStateForTest } from "../plugins/runtime.js"; -import { createOpenClawCodingTools } from "./pi-tools.js"; -import { resolveSandboxConfigForAgent } from "./sandbox/config.js"; -import { createHostSandboxFsBridge } from "./test-helpers/host-sandbox-fs-bridge.js"; -import { createPiToolsSandboxContext } from "./test-helpers/pi-tools-sandbox-context.js"; - -function resetPluginState() { - clearPluginLoaderCache(); - clearPluginDiscoveryCache(); - clearPluginManifestRegistryCache(); - resetPluginRuntimeStateForTest(); -} - -function listToolNames(params: { - cfg: OpenClawConfig; - agentId?: string; - sessionKey?: string; - sandboxAgentId?: string; -}): string[] { - const workspaceDir = "/tmp/openclaw-sandbox-policy"; - const sessionKey = params.sessionKey ?? "agent:tavern:main"; - const sandboxAgentId = params.sandboxAgentId ?? params.agentId ?? "tavern"; - const sandbox = createPiToolsSandboxContext({ - workspaceDir, - fsBridge: createHostSandboxFsBridge(workspaceDir), - sessionKey, - tools: resolveSandboxConfigForAgent(params.cfg, sandboxAgentId).tools, - }); - return createOpenClawCodingTools({ - config: params.cfg, - agentId: params.agentId, - sessionKey, - sandbox, - workspaceDir, - }) - .map((tool) => tool.name) - .toSorted(); -} - -describe("pi-tools sandbox policy", () => { - let bundledFixture: ReturnType | null = null; - - beforeEach(() => { - bundledFixture = createBundledBrowserPluginFixture(); - vi.stubEnv("OPENCLAW_BUNDLED_PLUGINS_DIR", bundledFixture.rootDir); - resetPluginState(); - }); - - afterEach(() => { - resetPluginState(); - vi.unstubAllEnvs(); - bundledFixture?.cleanup(); - bundledFixture = null; - }); - - it("re-exposes omitted sandbox tools via sandbox alsoAllow", () => { - const names = listToolNames({ - cfg: { - agents: { - defaults: { - sandbox: { mode: "all", scope: "agent" }, - }, - list: [ - { - id: "tavern", - tools: { - sandbox: { - tools: { - alsoAllow: ["message", "tts"], - }, - }, - }, - }, - ], - }, - } as OpenClawConfig, - }); - - expect(names).toContain("message"); - expect(names).toContain("tts"); - }); - - it("re-enables default-denied sandbox tools when explicitly allowed", () => { - const names = listToolNames({ - cfg: { - agents: { - defaults: { - sandbox: { mode: "all", scope: "agent" }, - }, - list: [{ id: "tavern" }], - }, - tools: { - sandbox: { - tools: { - allow: ["browser"], - }, - }, - }, - plugins: { - allow: ["browser"], - }, - } as OpenClawConfig, - }); - - expect(names).toContain("browser"); - }); - - it("prefers the resolved sandbox context policy for legacy main session aliases", () => { - const cfg = { - agents: { - defaults: { - sandbox: { mode: "all", scope: "agent" }, - }, - list: [ - { - id: "tavern", - default: true, - tools: { - sandbox: { - tools: { - allow: ["browser"], - alsoAllow: ["message"], - }, - }, - }, - }, - ], - }, - plugins: { - allow: ["browser"], - }, - } as OpenClawConfig; - - const names = listToolNames({ - cfg, - sessionKey: "main", - sandboxAgentId: "tavern", - }); - - expect(names).toContain("browser"); - expect(names).toContain("message"); - }); - - it("preserves allow-all semantics for allow: [] plus alsoAllow", () => { - const names = listToolNames({ - cfg: { - agents: { - defaults: { - sandbox: { mode: "all", scope: "agent" }, - }, - list: [{ id: "tavern" }], - }, - tools: { - sandbox: { - tools: { - allow: [], - alsoAllow: ["browser"], - }, - }, - }, - plugins: { - allow: ["browser"], - }, - } as OpenClawConfig, - }); - - expect(names).toContain("browser"); - expect(names).toContain("read"); - }); - - it("keeps explicit sandbox deny precedence over explicit allow", () => { - const names = listToolNames({ - cfg: { - agents: { - defaults: { - sandbox: { mode: "all", scope: "agent" }, - }, - list: [{ id: "tavern" }], - }, - tools: { - sandbox: { - tools: { - allow: ["browser", "message"], - deny: ["browser"], - }, - }, - }, - plugins: { - allow: ["browser"], - }, - } as OpenClawConfig, - }); - - expect(names).not.toContain("browser"); - expect(names).toContain("message"); - }); -}); diff --git a/src/agents/tools/pdf-tool.test.ts b/src/agents/tools/pdf-tool.test.ts index bb0b728685f..2d8adc7e176 100644 --- a/src/agents/tools/pdf-tool.test.ts +++ b/src/agents/tools/pdf-tool.test.ts @@ -22,10 +22,11 @@ vi.mock("@mariozechner/pi-ai", async () => { type PdfToolModule = typeof import("./pdf-tool.js"); let createPdfTool: PdfToolModule["createPdfTool"]; +let PdfToolSchema: PdfToolModule["PdfToolSchema"]; async function loadCreatePdfTool() { - if (!createPdfTool) { - ({ createPdfTool } = await import("./pdf-tool.js")); + if (!createPdfTool || !PdfToolSchema) { + ({ createPdfTool, PdfToolSchema } = await import("./pdf-tool.js")); } return createPdfTool; } @@ -62,17 +63,6 @@ function requirePdfTool( type PdfToolInstance = ReturnType; -async function withAnthropicPdfTool( - run: (tool: PdfToolInstance, agentDir: string) => Promise, -) { - await withTempAgentDir(async (agentDir) => { - vi.stubEnv("ANTHROPIC_API_KEY", "anthropic-test"); - const cfg = withDefaultModel(ANTHROPIC_PDF_MODEL); - const tool = requirePdfTool((await loadCreatePdfTool())({ config: cfg, agentDir })); - await run(tool, agentDir); - }); -} - async function withConfiguredPdfTool( run: (tool: PdfToolInstance, agentDir: string) => Promise, ) { @@ -97,12 +87,6 @@ function resetAuthEnv() { vi.stubEnv("GITHUB_TOKEN", ""); } -function withDefaultModel(primary: string): OpenClawConfig { - return { - agents: { defaults: { model: { primary } } }, - } as OpenClawConfig; -} - function withPdfModel(primary: string): OpenClawConfig { return { agents: { defaults: { pdfModel: { primary } } }, @@ -299,17 +283,16 @@ describe("createPdfTool", () => { }); it("tool parameters have correct schema shape", async () => { - await withAnthropicPdfTool(async (tool) => { - const schema = tool.parameters; - expect(schema.type).toBe("object"); - expect(schema.properties).toBeDefined(); - const props = schema.properties as Record; - expect(props.prompt).toBeDefined(); - expect(props.pdf).toBeDefined(); - expect(props.pdfs).toBeDefined(); - expect(props.pages).toBeDefined(); - expect(props.model).toBeDefined(); - expect(props.maxBytesMb).toBeDefined(); - }); + await loadCreatePdfTool(); + const schema = PdfToolSchema; + expect(schema.type).toBe("object"); + expect(schema.properties).toBeDefined(); + const props = schema.properties as Record; + expect(props.prompt).toBeDefined(); + expect(props.pdf).toBeDefined(); + expect(props.pdfs).toBeDefined(); + expect(props.pages).toBeDefined(); + expect(props.model).toBeDefined(); + expect(props.maxBytesMb).toBeDefined(); }); }); diff --git a/src/agents/tools/pdf-tool.ts b/src/agents/tools/pdf-tool.ts index 45dbf6c2102..a73686069c1 100644 --- a/src/agents/tools/pdf-tool.ts +++ b/src/agents/tools/pdf-tool.ts @@ -45,6 +45,23 @@ const DEFAULT_MAX_PAGES = 20; const PDF_MIN_TEXT_CHARS = 200; const PDF_MAX_PIXELS = 4_000_000; +export const PdfToolSchema = Type.Object({ + prompt: Type.Optional(Type.String()), + pdf: Type.Optional(Type.String({ description: "Single PDF path or URL." })), + pdfs: Type.Optional( + Type.Array(Type.String(), { + description: "Multiple PDF paths or URLs (up to 10).", + }), + ), + pages: Type.Optional( + Type.String({ + description: 'Page range to process, e.g. "1-5", "1,3,5-7". Defaults to all pages.', + }), + ), + model: Type.Optional(Type.String()), + maxBytesMb: Type.Optional(Type.Number()), +}); + // --------------------------------------------------------------------------- // Model resolution (mirrors image tool pattern) // --------------------------------------------------------------------------- @@ -258,22 +275,7 @@ export function createPdfTool(options?: { label: "PDF", name: "pdf", description, - parameters: Type.Object({ - prompt: Type.Optional(Type.String()), - pdf: Type.Optional(Type.String({ description: "Single PDF path or URL." })), - pdfs: Type.Optional( - Type.Array(Type.String(), { - description: "Multiple PDF paths or URLs (up to 10).", - }), - ), - pages: Type.Optional( - Type.String({ - description: 'Page range to process, e.g. "1-5", "1,3,5-7". Defaults to all pages.', - }), - ), - model: Type.Optional(Type.String()), - maxBytesMb: Type.Optional(Type.Number()), - }), + parameters: PdfToolSchema, execute: async (_toolCallId, args) => { const record = args && typeof args === "object" ? (args as Record) : {};