From a49a5bfa74101f2ebd8438fe3d092f3dd8f8a61d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 28 Jul 2026 19:24:39 -0400 Subject: [PATCH] perf(codex): serve catalog pages stale-while-revalidate and skip warm-client start resolution (#115403) * perf(codex): serve catalog pages stale-while-revalidate and skip warm-client start resolution * fix(codex): skip empty-tool runtime discovery --- .../src/app-server/dynamic-tool-build.ts | 66 ++++++----- .../src/app-server/shared-client.test.ts | 49 ++++++++ .../codex/src/app-server/shared-client.ts | 109 +++++++++++++++++- extensions/codex/src/session-catalog.test.ts | 91 ++++++++++++++- extensions/codex/src/session-catalog.ts | 77 ++++++++++--- .../agent-bundle-mcp-runtime-shared.test.ts | 51 ++++++++ src/agents/agent-bundle-mcp-runtime-shared.ts | 20 +++- src/agents/runtime-plan/tools.test.ts | 14 +++ src/agents/runtime-plan/tools.ts | 31 ++--- src/plugin-sdk/agent-harness-runtime.ts | 5 + 10 files changed, 446 insertions(+), 67 deletions(-) create mode 100644 src/agents/agent-bundle-mcp-runtime-shared.test.ts diff --git a/extensions/codex/src/app-server/dynamic-tool-build.ts b/extensions/codex/src/app-server/dynamic-tool-build.ts index ceceb398efed..2d7152619326 100644 --- a/extensions/codex/src/app-server/dynamic-tool-build.ts +++ b/extensions/codex/src/app-server/dynamic-tool-build.ts @@ -226,12 +226,13 @@ export async function buildDynamicTools(input: DynamicToolBuildParams) { }); const modelHasVision = params.model.input?.includes("image") ?? false; const agentDir = params.agentDir ?? resolveAgentDir(params.config ?? {}, input.sessionAgentId); - const { - createOpenClawCodingTools: defaultCreateOpenClawCodingTools, - resolveWebSearchToolPolicy, - } = await import("openclaw/plugin-sdk/agent-harness"); + const injectedOpenClawCodingToolsFactory = dynamicToolBuildState.openClawCodingToolsFactory; + let agentHarnessModule: typeof import("openclaw/plugin-sdk/agent-harness") | undefined; + const loadAgentHarnessModule = async () => + (agentHarnessModule ??= await import("openclaw/plugin-sdk/agent-harness")); const createOpenClawCodingTools = - dynamicToolBuildState.openClawCodingToolsFactory ?? defaultCreateOpenClawCodingTools; + injectedOpenClawCodingToolsFactory ?? + (await loadAgentHarnessModule()).createOpenClawCodingTools; toolBuildStages.mark("load-agent-harness-tools"); const sessionKeys = resolveOpenClawCodingToolsSessionKeys(params, input.sandboxSessionKey); const nativeExecutionPolicy = resolveCodexNativeExecutionPolicyForDynamicTools(input); @@ -378,36 +379,41 @@ export async function buildDynamicTools(input: DynamicToolBuildParams) { }); toolBuildStages.mark("vision-filtering"); const webSearchPresent = visionFilteredTools.some((tool) => tool.name === "web_search"); - const webSearchPolicy = resolveWebSearchToolPolicy({ - config: params.config, - modelProvider: params.model.provider, - modelId: params.modelId, - agentId: input.sessionAgentId, - sessionKey: input.sandboxSessionKey, - sandboxToolPolicy: input.sandbox?.tools, - messageProvider: resolveCodexMessageToolProvider(params), - agentAccountId: params.agentAccountId, - groupId: params.groupId, - groupChannel: params.groupChannel, - groupSpace: params.groupSpace, - spawnedBy: params.spawnedBy, - senderId: params.senderId, - senderName: params.senderName, - senderUsername: params.senderUsername, - senderE164: params.senderE164, - inputProvenance: params.inputProvenance, - trustedInternalHandoff: params.trustedInternalHandoff, - scheduledToolPolicy: params.scheduledToolPolicy, - }); - const senderScopedWebSearchRestriction = - !webSearchPolicy.allowed && webSearchPolicy.persistentAllowed; - const transientWebSearchRestriction = - senderScopedWebSearchRestriction || isCodexMemoryFlushRun(params); const persistentCodexWebSearchSurface = params.config?.tools?.web?.search?.enabled !== false && !(input.pluginConfig.codexDynamicToolsExclude ?? []).some( (name) => normalizeCodexDynamicToolName(name) === "web_search", ); + // An injected tool factory can prove that no managed or native search surface exists. + // Avoid loading the heavyweight default factory graph when no search policy can affect output. + const webSearchPolicy = + webSearchPresent || persistentCodexWebSearchSurface + ? (await loadAgentHarnessModule()).resolveWebSearchToolPolicy({ + config: params.config, + modelProvider: params.model.provider, + modelId: params.modelId, + agentId: input.sessionAgentId, + sessionKey: input.sandboxSessionKey, + sandboxToolPolicy: input.sandbox?.tools, + messageProvider: resolveCodexMessageToolProvider(params), + agentAccountId: params.agentAccountId, + groupId: params.groupId, + groupChannel: params.groupChannel, + groupSpace: params.groupSpace, + spawnedBy: params.spawnedBy, + senderId: params.senderId, + senderName: params.senderName, + senderUsername: params.senderUsername, + senderE164: params.senderE164, + inputProvenance: params.inputProvenance, + trustedInternalHandoff: params.trustedInternalHandoff, + scheduledToolPolicy: params.scheduledToolPolicy, + }) + : { allowed: false, persistentAllowed: false }; + const senderScopedWebSearchRestriction = + !webSearchPolicy.allowed && webSearchPolicy.persistentAllowed; + const transientWebSearchRestriction = + senderScopedWebSearchRestriction || isCodexMemoryFlushRun(params); input.onPersistentWebSearchPolicyResolved?.( webSearchPresent || (persistentCodexWebSearchSurface && diff --git a/extensions/codex/src/app-server/shared-client.test.ts b/extensions/codex/src/app-server/shared-client.test.ts index 8651d6ac31fc..735984d233f3 100644 --- a/extensions/codex/src/app-server/shared-client.test.ts +++ b/extensions/codex/src/app-server/shared-client.test.ts @@ -301,6 +301,55 @@ describe("shared Codex app-server client", () => { expect(startSpy).not.toHaveBeenCalled(); }); + it("skips auth-store resolution only while the same config-owned client stays warm", async () => { + const first = createClientHarness(); + const replacement = createClientHarness(); + const startSpy = vi + .spyOn(CodexAppServerClient, "start") + .mockReturnValueOnce(first.client) + .mockReturnValueOnce(replacement.client); + mocks.resolveCodexAppServerAuthProfileIdForAgent.mockImplementation(() => { + mocks.resolveCodexAppServerAuthProfileStore(); + return "openai:work"; + }); + const startOptions: CodexAppServerStartOptions = { + transport: "stdio", + homeScope: "agent", + command: "codex", + args: ["app-server"], + headers: {}, + }; + const config = { auth: { order: { openai: ["openai:work"] } } }; + const options = { config, startOptions, timeoutMs: 1_000 }; + + const firstAcquire = getLeasedSharedCodexAppServerClient(options); + await sendInitializeResult(first, "openclaw/0.143.0 (Linux; test)"); + await expect(firstAcquire).resolves.toBe(first.client); + expect(releaseLeasedSharedCodexAppServerClient(first.client)).toBe(true); + expect(mocks.resolveCodexAppServerAuthProfileStore).toHaveBeenCalledOnce(); + + await expect(getLeasedSharedCodexAppServerClient(options)).resolves.toBe(first.client); + expect(releaseLeasedSharedCodexAppServerClient(first.client)).toBe(true); + expect(mocks.resolveCodexAppServerAuthProfileStore).toHaveBeenCalledOnce(); + + await expect( + getLeasedSharedCodexAppServerClient({ + ...options, + config: { auth: { order: { openai: ["openai:work"] } } }, + }), + ).resolves.toBe(first.client); + expect(releaseLeasedSharedCodexAppServerClient(first.client)).toBe(true); + expect(mocks.resolveCodexAppServerAuthProfileStore).toHaveBeenCalledTimes(2); + + expect(clearSharedCodexAppServerClientIfCurrent(first.client)).toBe(true); + const replacementAcquire = getLeasedSharedCodexAppServerClient(options); + await sendInitializeResult(replacement, "openclaw/0.143.0 (Linux; test)"); + await expect(replacementAcquire).resolves.toBe(replacement.client); + expect(releaseLeasedSharedCodexAppServerClient(replacement.client)).toBe(true); + expect(mocks.resolveCodexAppServerAuthProfileStore).toHaveBeenCalledTimes(3); + expect(startSpy).toHaveBeenCalledTimes(2); + }); + it("does not spawn after startup context exceeds its total deadline", async () => { vi.useFakeTimers(); let resolveManaged: ((value: CodexAppServerStartOptions) => void) | undefined; diff --git a/extensions/codex/src/app-server/shared-client.ts b/extensions/codex/src/app-server/shared-client.ts index f8afb125911e..35144e3ed852 100644 --- a/extensions/codex/src/app-server/shared-client.ts +++ b/extensions/codex/src/app-server/shared-client.ts @@ -60,6 +60,22 @@ type SharedCodexAppServerClientStartup = { type SharedCodexAppServerClientState = { clients: Map; leasedReleases: WeakMap void>>; + warmClientsByConfig?: WeakMap< + object, + WeakMap> + >; +}; + +type WarmSharedCodexAppServerClient = { + key: string; + entry: SharedCodexAppServerClientEntry; + client: CodexAppServerClient; +}; + +type WarmSharedCodexAppServerClientIdentity = { + config: object; + startOptions: CodexAppServerStartOptions; + selectorKey: string; }; type CodexAppServerClientStartMetadata = { @@ -393,6 +409,82 @@ async function resolveCodexAppServerClientStartContext( }; } +function resolveWarmSharedCodexAppServerClientIdentity( + options?: CodexAppServerClientOptions, +): WarmSharedCodexAppServerClientIdentity | undefined { + if ( + !options?.config || + typeof options.config !== "object" || + !options.startOptions || + options.pluginConfig !== undefined || + options.authProfileStore !== undefined || + options.preparedAuth !== undefined || + options.runtimeArtifactMode !== undefined || + options.expectedRuntimeArtifact !== undefined + ) { + return undefined; + } + const authProfileSelector = + options.authProfileId === null + ? ["native"] + : options.authProfileId === undefined + ? ["implicit"] + : ["profile", options.authProfileId]; + return { + config: options.config, + startOptions: options.startOptions, + selectorKey: JSON.stringify([ + options.agentDir ?? resolveDefaultAgentDir(options.config), + authProfileSelector, + options.authBindingFingerprint ?? null, + options.authRequirement ?? null, + ]), + }; +} + +function readWarmSharedCodexAppServerClient( + state: SharedCodexAppServerClientState, + identity: WarmSharedCodexAppServerClientIdentity, +): WarmSharedCodexAppServerClient | undefined { + const aliases = state.warmClientsByConfig?.get(identity.config)?.get(identity.startOptions); + const warm = aliases?.get(identity.selectorKey); + if (!warm) { + return undefined; + } + // Config/start options are immutable runtime snapshots. The alias is valid only while the + // canonical keyed entry still owns this physical client; close, retirement, and auth-profile + // invalidation remove that entry, forcing the next acquire through full start-context resolution. + if ( + state.clients.get(warm.key) !== warm.entry || + warm.entry.client !== warm.client || + warm.entry.closeWhenIdle || + warm.entry.closeError + ) { + aliases?.delete(identity.selectorKey); + return undefined; + } + return warm; +} + +function rememberWarmSharedCodexAppServerClient( + state: SharedCodexAppServerClientState, + identity: WarmSharedCodexAppServerClientIdentity, + warm: WarmSharedCodexAppServerClient, +): void { + state.warmClientsByConfig ??= new WeakMap(); + let byStartOptions = state.warmClientsByConfig.get(identity.config); + if (!byStartOptions) { + byStartOptions = new WeakMap(); + state.warmClientsByConfig.set(identity.config, byStartOptions); + } + let aliases = byStartOptions.get(identity.startOptions); + if (!aliases) { + aliases = new Map(); + byStartOptions.set(identity.startOptions, aliases); + } + aliases.set(identity.selectorKey, warm); +} + /** Gets or starts a shared Codex app-server client without retaining a lease. */ export async function getSharedCodexAppServerClient( options?: CodexAppServerClientOptions, @@ -518,6 +610,16 @@ async function acquireSharedCodexAppServerClient( if (options?.abandonSignal?.aborted) { throw new CodexAppServerStartupError("aborted", "codex app-server initialize aborted"); } + const state = getSharedCodexAppServerClientState(); + const warmIdentity = resolveWarmSharedCodexAppServerClientIdentity(options); + const warmClient = warmIdentity + ? readWarmSharedCodexAppServerClient(state, warmIdentity) + : undefined; + if (warmClient) { + options?.onStartedClient?.(warmClient.client); + const release = leaseOptions?.leased ? retainSharedClientEntry(warmClient.entry) : undefined; + return release ? { client: warmClient.client, release } : { client: warmClient.client }; + } const acquireStartedAt = Date.now(); const timeoutMs = options?.timeoutMs ?? 0; const context = await withCodexAppServerAcquireDeadline( @@ -563,7 +665,6 @@ async function acquireSharedCodexAppServerClient( const key = runtimeArtifactMode ? `${baseKey}\0runtime-artifact:capture-v1:${expectedRuntimeArtifactKey}` : baseKey; - const state = getSharedCodexAppServerClientState(); const entry = getOrCreateSharedClientEntry(state, key); if (runtimeArtifactMode) { entry.runtimeArtifactStartupAbort ??= new AbortController(); @@ -643,6 +744,9 @@ async function acquireSharedCodexAppServerClient( config: options?.config, }); const release = leaseOptions?.leased ? retainSharedClientEntry(entry) : undefined; + if (warmIdentity) { + rememberWarmSharedCodexAppServerClient(state, warmIdentity, { key, entry, client }); + } return release ? { client, release } : { client }; } catch (error) { // This deadline belongs to one waiter, not the shared physical client. @@ -1002,6 +1106,7 @@ export function resetSharedCodexAppServerClientForTests(): void { const clients = collectSharedClients(state); state.clients.clear(); state.leasedReleases = new WeakMap(); + state.warmClientsByConfig = new WeakMap(); for (const client of clients) { client.close(); } @@ -1012,6 +1117,7 @@ export function clearSharedCodexAppServerClient(): void { const state = getSharedCodexAppServerClientState(); const clients = collectSharedClients(state); state.clients.clear(); + state.warmClientsByConfig = new WeakMap(); for (const client of clients) { client.close(); } @@ -1146,6 +1252,7 @@ export async function clearSharedCodexAppServerClientAndWait(options?: { const state = getSharedCodexAppServerClientState(); const clients = collectSharedClients(state); state.clients.clear(); + state.warmClientsByConfig = new WeakMap(); await Promise.all(clients.map((client) => client.closeAndWait(options))); } diff --git a/extensions/codex/src/session-catalog.test.ts b/extensions/codex/src/session-catalog.test.ts index 32354a14cdc6..7fbe4b406117 100644 --- a/extensions/codex/src/session-catalog.test.ts +++ b/extensions/codex/src/session-catalog.test.ts @@ -555,11 +555,11 @@ describe("Codex supervision catalog", () => { expect(cloneSpy).toHaveBeenCalledTimes(4); }); - it("memoizes thread lists across the stable poll cadence and invalidates by config", async () => { + it("serves an expired page while one background refresh updates the next poll", async () => { let now = 1_000; let runtimeConfig = {} as OpenClawConfig; commandRpcMocks.codexControlRequest.mockResolvedValue({ - data: [idleThread({ source: "cli" })], + data: [idleThread({ id: "thread-stale", source: "cli" })], }); const control = createCodexSessionCatalogControl({ getPluginConfig: () => ({ supervision: { enabled: true } }), @@ -576,7 +576,35 @@ describe("Codex supervision catalog", () => { expect(commandRpcMocks.codexControlRequest).toHaveBeenCalledOnce(); now += 2; - await control.listPage({ limit: 25 }); + let resolveRefresh!: (value: unknown) => void; + let refreshSettled = false; + const refresh = new Promise((resolve) => { + resolveRefresh = resolve; + }).then((value) => { + refreshSettled = true; + return value; + }); + commandRpcMocks.codexControlRequest.mockReturnValueOnce(refresh); + const firstExpiredPoll = control.listPage({ limit: 25 }); + const overlappingExpiredPoll = control.listPage({ limit: 25 }); + + await expect(Promise.all([firstExpiredPoll, overlappingExpiredPoll])).resolves.toEqual([ + expect.objectContaining({ + sessions: [expect.objectContaining({ threadId: "thread-stale" })], + }), + expect.objectContaining({ + sessions: [expect.objectContaining({ threadId: "thread-stale" })], + }), + ]); + expect(refreshSettled).toBe(false); + expect(commandRpcMocks.codexControlRequest).toHaveBeenCalledTimes(2); + + resolveRefresh({ data: [idleThread({ id: "thread-refreshed", source: "cli" })] }); + await vi.waitFor(async () => { + await expect(control.listPage({ limit: 25 })).resolves.toMatchObject({ + sessions: [expect.objectContaining({ threadId: "thread-refreshed" })], + }); + }); expect(commandRpcMocks.codexControlRequest).toHaveBeenCalledTimes(2); runtimeConfig = { agents: {} } as OpenClawConfig; @@ -604,8 +632,11 @@ describe("Codex supervision catalog", () => { commandRpcMocks.codexControlRequest.mockResolvedValue({ data: [idleThread({ id: "thread-recovered", source: "cli" })], }); - await expect(control.listPage({ limit: 25 })).resolves.toMatchObject({ - sessions: [expect.objectContaining({ threadId: "thread-recovered" })], + await expect(control.listPage({ limit: 25 })).resolves.toEqual(first); + await vi.waitFor(async () => { + await expect(control.listPage({ limit: 25 })).resolves.toMatchObject({ + sessions: [expect.objectContaining({ threadId: "thread-recovered" })], + }); }); expect(commandRpcMocks.codexControlRequest).toHaveBeenCalledTimes(3); }); @@ -649,7 +680,12 @@ describe("Codex supervision catalog", () => { }); now += 1; await expect(control.listPage({ limit: 25 })).resolves.toMatchObject({ - sessions: [expect.objectContaining({ threadId: "thread-recovered" })], + sessions: [expect.objectContaining({ threadId: "thread-stale" })], + }); + await vi.waitFor(async () => { + await expect(control.listPage({ limit: 25 })).resolves.toMatchObject({ + sessions: [expect.objectContaining({ threadId: "thread-recovered" })], + }); }); expect(commandRpcMocks.codexControlRequest).toHaveBeenCalledTimes(3); }); @@ -681,6 +717,49 @@ describe("Codex supervision catalog", () => { await expect(passive).resolves.toEqual(stale); }); + it("does not expose a pending cold fill as stale after a forced refresh overtakes it", async () => { + let resolveCold!: (value: unknown) => void; + let resolveForced!: (value: unknown) => void; + commandRpcMocks.codexControlRequest + .mockReturnValueOnce( + new Promise((resolve) => { + resolveCold = resolve; + }), + ) + .mockReturnValueOnce( + new Promise((resolve) => { + resolveForced = resolve; + }), + ); + const control = createCodexSessionCatalogControl({ + getPluginConfig: () => ({ supervision: { enabled: true } }), + getRuntimeConfig: () => config, + }); + + const cold = control.listPage({ limit: 25 }); + const forced = control.listPage({ limit: 25, forceRefresh: true }); + const passive = control.listPage({ limit: 25 }); + resolveForced({ data: [idleThread({ id: "thread-forced", source: "cli" })] }); + + await expect(Promise.all([forced, passive])).resolves.toEqual([ + expect.objectContaining({ + sessions: [expect.objectContaining({ threadId: "thread-forced" })], + }), + expect.objectContaining({ + sessions: [expect.objectContaining({ threadId: "thread-forced" })], + }), + ]); + + resolveCold({ data: [idleThread({ id: "thread-cold", source: "cli" })] }); + await expect(cold).resolves.toMatchObject({ + sessions: [expect.objectContaining({ threadId: "thread-cold" })], + }); + await expect(control.listPage({ limit: 25 })).resolves.toMatchObject({ + sessions: [expect.objectContaining({ threadId: "thread-forced" })], + }); + expect(commandRpcMocks.codexControlRequest).toHaveBeenCalledTimes(2); + }); + it("force-refreshes after a cached specific-thread miss", async () => { let includeThread = false; commandRpcMocks.codexControlRequest.mockImplementation(async () => ({ diff --git a/extensions/codex/src/session-catalog.ts b/extensions/codex/src/session-catalog.ts index 51d17b7e8162..4d554a67bbd1 100644 --- a/extensions/codex/src/session-catalog.ts +++ b/extensions/codex/src/session-catalog.ts @@ -135,6 +135,7 @@ type CodexCatalogRequestOptions = { type CodexCatalogPageCacheEntry = { expiresAt: number; page: Promise; + settledPage?: Promise; stalePage?: Promise; }; @@ -429,31 +430,70 @@ export function createCodexSessionCatalogControl(params: { } const key = codexCatalogPageCacheKey(pageParams); const cached = cache.get(key); - if (pageParams.forceRefresh !== true && cached && cached.expiresAt > now()) { - // thread/list traverses Codex's thread store and rollout metadata. A config-scoped 32s page - // survives the 30s poll cadence; TTL and forceRefresh bound staleness for new native rows. - // Without this memo every sidebar poll repeats the app-server roundtrip and metadata scan. + if (pageParams.forceRefresh !== true && cached) { + // A settled page always serves immediately. Expiry only starts one background refresh; + // its result becomes visible on the next poll (one polling cycle, about 30s, for a native + // session created outside OpenClaw). The TTL is a refresh trigger, never a serve gate. cache.delete(key); cache.set(key, cached); - try { - return await cached.page; - } catch (error) { - if (cached.stalePage) { - return await cached.stalePage; - } - throw error; + if (cached.stalePage) { + return await cached.stalePage; } + if (cached.expiresAt > now()) { + return await cached.page; + } + + const stalePage = cached.settledPage; + if (!stalePage) { + return await cached.page; + } + const page = control.listPage(pageParams); + const entry: CodexCatalogPageCacheEntry = { + expiresAt: Number.POSITIVE_INFINITY, + page, + settledPage: stalePage, + stalePage, + }; + cache.set(key, entry); + void page.then( + () => { + if (cache.get(key) === entry) { + delete entry.stalePage; + entry.settledPage = page; + entry.expiresAt = now() + CODEX_SESSION_CATALOG_LIST_TTL_MS; + } + }, + async () => { + let stale: CodexSessionCatalogPage | undefined; + try { + stale = await stalePage; + } catch { + // A still-pending cold fill is not a real stale page. + } + if (cache.get(key) !== entry) { + return; + } + if (stale) { + cache.delete(key); + const settledPage = Promise.resolve(stale); + cache.set(key, { expiresAt: now(), page: settledPage, settledPage }); + } else { + cache.delete(key); + } + }, + ); + return await stalePage; } if (cached) { cache.delete(key); } const serveStaleOnError = pageParams.forceRefresh !== true; const page = control.listPage(pageParams); - const stalePage = cached?.stalePage ?? cached?.page; + const stalePage = cached?.settledPage; const entry: CodexCatalogPageCacheEntry = { - expiresAt: now() + CODEX_SESSION_CATALOG_LIST_TTL_MS, + expiresAt: Number.POSITIVE_INFINITY, page, - ...(stalePage ? { stalePage } : {}), + ...(stalePage ? { stalePage, settledPage: stalePage } : {}), }; cache.set(key, entry); while (cache.size > CODEX_SESSION_CATALOG_LIST_CACHE_MAX_ENTRIES) { @@ -465,7 +505,11 @@ export function createCodexSessionCatalogControl(params: { } try { const result = await page; - delete entry.stalePage; + if (cache.get(key) === entry) { + delete entry.stalePage; + entry.settledPage = page; + entry.expiresAt = now() + CODEX_SESSION_CATALOG_LIST_TTL_MS; + } return result; } catch (error) { if (stalePage) { @@ -478,7 +522,8 @@ export function createCodexSessionCatalogControl(params: { if (stale) { if (cache.get(key) === entry) { cache.delete(key); - cache.set(key, { expiresAt: now(), page: Promise.resolve(stale) }); + const settledPage = Promise.resolve(stale); + cache.set(key, { expiresAt: now(), page: settledPage, settledPage }); } if (serveStaleOnError) { return stale; diff --git a/src/agents/agent-bundle-mcp-runtime-shared.test.ts b/src/agents/agent-bundle-mcp-runtime-shared.test.ts new file mode 100644 index 000000000000..efc037186c49 --- /dev/null +++ b/src/agents/agent-bundle-mcp-runtime-shared.test.ts @@ -0,0 +1,51 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { + SESSION_MCP_RUNTIME_MANAGER_KEY, + shouldLoadRequesterScopedMcpHarnessRuntime, +} from "./agent-bundle-mcp-runtime-shared.js"; + +const globalStore = globalThis as Record; +const originalManager = globalStore[SESSION_MCP_RUNTIME_MANAGER_KEY]; + +afterEach(() => { + if (originalManager === undefined) { + delete globalStore[SESSION_MCP_RUNTIME_MANAGER_KEY]; + } else { + globalStore[SESSION_MCP_RUNTIME_MANAGER_KEY] = originalManager; + } +}); + +describe("requester-scoped MCP harness preflight", () => { + it("skips the runtime graph without a requester or advertised catalog", () => { + delete globalStore[SESSION_MCP_RUNTIME_MANAGER_KEY]; + + expect( + shouldLoadRequesterScopedMcpHarnessRuntime({ + sessionId: "session-1", + requesterSenderId: " ", + }), + ).toBe(false); + }); + + it("loads for an authenticated requester", () => { + expect( + shouldLoadRequesterScopedMcpHarnessRuntime({ + sessionId: "session-1", + requesterSenderId: "sender-1", + }), + ).toBe(true); + }); + + it("loads advertised stubs for an unauthenticated requester", () => { + globalStore[SESSION_MCP_RUNTIME_MANAGER_KEY] = { + getAdvertisedScopedCatalog: (sessionId: string) => + sessionId === "session-1" ? { servers: {}, tools: [{}] } : null, + }; + + expect( + shouldLoadRequesterScopedMcpHarnessRuntime({ + sessionId: "session-1", + }), + ).toBe(true); + }); +}); diff --git a/src/agents/agent-bundle-mcp-runtime-shared.ts b/src/agents/agent-bundle-mcp-runtime-shared.ts index 3f117403c851..acbe70d2576c 100644 --- a/src/agents/agent-bundle-mcp-runtime-shared.ts +++ b/src/agents/agent-bundle-mcp-runtime-shared.ts @@ -1,7 +1,11 @@ /** Shared session MCP runtime constants and create-runtime factory type. */ import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { PluginManifestRegistry } from "../plugins/manifest-registry.js"; -import type { SessionMcpRequesterScope, SessionMcpRuntime } from "./agent-bundle-mcp-types.js"; +import type { + SessionMcpRequesterScope, + SessionMcpRuntime, + SessionMcpRuntimeManager, +} from "./agent-bundle-mcp-types.js"; import type { McpServerConnectionResolved } from "./mcp-connection-resolver.js"; export const SESSION_MCP_RUNTIME_MANAGER_KEY = Symbol.for("openclaw.sessionMcpRuntimeManager"); @@ -11,6 +15,20 @@ export const SESSION_MCP_RUNTIME_SWEEP_INTERVAL_MS = 60 * 1000; // far above concurrent-run parallelism, so active requesters never evict. export const SESSION_MCP_MAX_IDLE_REQUESTER_RUNTIMES = 64; +/** Checks whether harness-scoped MCP can affect a turn without loading its runtime graph. */ +export function shouldLoadRequesterScopedMcpHarnessRuntime(params: { + sessionId: string; + requesterSenderId?: string | null; +}): boolean { + if (params.requesterSenderId?.trim()) { + return true; + } + const manager = (globalThis as Record)[SESSION_MCP_RUNTIME_MANAGER_KEY] as + | SessionMcpRuntimeManager + | undefined; + return (manager?.getAdvertisedScopedCatalog(params.sessionId)?.tools.length ?? 0) > 0; +} + export type CreateSessionMcpRuntime = (params: { sessionId: string; sessionKey?: string; diff --git a/src/agents/runtime-plan/tools.test.ts b/src/agents/runtime-plan/tools.test.ts index ff92e0ed9bc0..cf17145bd3d1 100644 --- a/src/agents/runtime-plan/tools.test.ts +++ b/src/agents/runtime-plan/tools.test.ts @@ -205,6 +205,20 @@ describe("AgentRuntimePlan tool policy helpers", () => { }); }); + it("does not load a provider runtime to normalize an empty tool set", () => { + const onPreNormalizationSchemaDiagnostics = vi.fn(); + + expect( + normalizeAgentRuntimeTools({ + tools: [], + provider: "openai", + onPreNormalizationSchemaDiagnostics, + }), + ).toEqual([]); + expect(onPreNormalizationSchemaDiagnostics).toHaveBeenCalledWith([], []); + expect(mocks.normalizeProviderToolSchemas).not.toHaveBeenCalled(); + }); + it("preserves plugin metadata when provider schema normalization clones tools", () => { // Provider normalization may clone tool objects; plugin metadata has to move // with the clone so later dispatch still knows the owning plugin/MCP server. diff --git a/src/agents/runtime-plan/tools.ts b/src/agents/runtime-plan/tools.ts index f9c5f5756cad..89c64c40d176 100644 --- a/src/agents/runtime-plan/tools.ts +++ b/src/agents/runtime-plan/tools.ts @@ -122,20 +122,25 @@ export function normalizeAgentRuntimeTools< TSchemaType, TResult >[]; + const planNormalized = params.runtimePlan?.tools.normalize(normalizableTools, planContext); + // Empty fallback input cannot gain provider-specific schema changes. Avoid loading a provider + // runtime just to return the same empty list; runtime plans still receive their normal callback. const normalized = - params.runtimePlan?.tools.normalize(normalizableTools, planContext) ?? - normalizeProviderToolSchemas({ - tools: normalizableTools, - provider: params.provider, - config: params.config, - workspaceDir: params.workspaceDir, - env: params.env ?? process.env, - modelId: params.modelId, - modelApi: params.modelApi, - model: params.model, - runtimeHandle: params.runtimeHandle, - allowRuntimePluginLoad: params.allowProviderRuntimePluginLoad, - }); + planNormalized ?? + (normalizableTools.length === 0 + ? normalizableTools + : normalizeProviderToolSchemas({ + tools: normalizableTools, + provider: params.provider, + config: params.config, + workspaceDir: params.workspaceDir, + env: params.env ?? process.env, + modelId: params.modelId, + modelApi: params.modelApi, + model: params.model, + runtimeHandle: params.runtimeHandle, + allowRuntimePluginLoad: params.allowProviderRuntimePluginLoad, + })); const normalizedTools = Array.isArray(normalized) ? normalized : normalizableTools; return preserveRuntimeToolMetadata(normalizableTools, normalizedTools); } diff --git a/src/plugin-sdk/agent-harness-runtime.ts b/src/plugin-sdk/agent-harness-runtime.ts index dc5ccbcc5f58..6381bbdf69c6 100644 --- a/src/plugin-sdk/agent-harness-runtime.ts +++ b/src/plugin-sdk/agent-harness-runtime.ts @@ -2,6 +2,7 @@ // Keep heavyweight tool construction out of this module so harness imports can // register quickly inside gateway startup and Docker e2e runs. +import { shouldLoadRequesterScopedMcpHarnessRuntime } from "../agents/agent-bundle-mcp-runtime-shared.js"; import { mergeAgentRunAttemptTerminal, normalizeAgentRunAttemptTerminal, @@ -393,6 +394,10 @@ export async function materializeRequesterScopedMcpToolsForHarnessRun( > > > { + const shouldLoad = shouldLoadRequesterScopedMcpHarnessRuntime(params); + if (!shouldLoad) { + return undefined; + } const { materializeRequesterScopedMcpToolsForHarnessRun: materialize } = await import("../agents/agent-bundle-mcp-harness.js"); return materialize(params);