From 663379e3fe59d4e5eb2283d78749d0a6fa1a5b65 Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Fri, 21 Nov 2025 12:58:11 +0900 Subject: [PATCH] fix: #683 Failing to run MCP servers when deserializing run state data --- .changeset/loud-emus-rule.md | 5 + packages/agents-core/src/mcp.ts | 126 ++++++++++++--------- packages/agents-core/src/tracing/index.ts | 1 + packages/agents-core/test/runState.test.ts | 108 ++++++++++++++++++ 4 files changed, 185 insertions(+), 55 deletions(-) create mode 100644 .changeset/loud-emus-rule.md diff --git a/.changeset/loud-emus-rule.md b/.changeset/loud-emus-rule.md new file mode 100644 index 00000000..86297fc8 --- /dev/null +++ b/.changeset/loud-emus-rule.md @@ -0,0 +1,5 @@ +--- +'@openai/agents-core': patch +--- + +fix: #683 Failing to run MCP servers when deserializing run state data diff --git a/packages/agents-core/src/mcp.ts b/packages/agents-core/src/mcp.ts index 1aeda9d3..f2f3cd14 100644 --- a/packages/agents-core/src/mcp.ts +++ b/packages/agents-core/src/mcp.ts @@ -5,7 +5,13 @@ import { MCPServerStreamableHttp as UnderlyingMCPServerStreamableHttp, MCPServerSSE as UnderlyingMCPServerSSE, } from '@openai/agents-core/_shims'; -import { getCurrentSpan, withMCPListToolsSpan } from './tracing'; +import { + getCurrentSpan, + getCurrentTrace, + withMCPListToolsSpan, + type MCPListToolsSpanData, + type Span, +} from './tracing'; import { logger as globalLogger, getLogger, Logger } from './logger'; import debug from 'debug'; import { z } from 'zod'; @@ -314,68 +320,78 @@ async function getFunctionToolsFromServer({ mcpToFunctionTool(t, server, convertSchemasToStrict), ); } - return withMCPListToolsSpan( - async (span) => { - const fetchedMcpTools = await server.listTools(); - let mcpTools: MCPTool[] = fetchedMcpTools; - - if (runContext && agent) { - const context = { runContext, agent, serverName: server.name }; - const filteredTools: MCPTool[] = []; - for (const tool of fetchedMcpTools) { - const filter = server.toolFilter; - if (filter) { - if (typeof filter === 'function') { - const filtered = await filter(context, tool); - if (!filtered) { - globalLogger.debug( - `MCP Tool (server: ${server.name}, tool: ${tool.name}) is blocked by the callable filter.`, - ); - continue; - } - } else { - const allowedToolNames = filter.allowedToolNames ?? []; - const blockedToolNames = filter.blockedToolNames ?? []; - if (allowedToolNames.length > 0 || blockedToolNames.length > 0) { - const allowed = - allowedToolNames.length > 0 - ? allowedToolNames.includes(tool.name) - : true; - const blocked = - blockedToolNames.length > 0 - ? blockedToolNames.includes(tool.name) - : false; - if (!allowed || blocked) { - if (blocked) { - globalLogger.debug( - `MCP Tool (server: ${server.name}, tool: ${tool.name}) is blocked by the static filter.`, - ); - } else if (!allowed) { - globalLogger.debug( - `MCP Tool (server: ${server.name}, tool: ${tool.name}) is not allowed by the static filter.`, - ); - } - continue; + + const listToolsForServer = async ( + span?: Span, + ): Promise[]> => { + const fetchedMcpTools = await server.listTools(); + let mcpTools: MCPTool[] = fetchedMcpTools; + + if (runContext && agent) { + const context = { runContext, agent, serverName: server.name }; + const filteredTools: MCPTool[] = []; + for (const tool of fetchedMcpTools) { + const filter = server.toolFilter; + if (filter) { + if (typeof filter === 'function') { + const filtered = await filter(context, tool); + if (!filtered) { + globalLogger.debug( + `MCP Tool (server: ${server.name}, tool: ${tool.name}) is blocked by the callable filter.`, + ); + continue; + } + } else { + const allowedToolNames = filter.allowedToolNames ?? []; + const blockedToolNames = filter.blockedToolNames ?? []; + if (allowedToolNames.length > 0 || blockedToolNames.length > 0) { + const allowed = + allowedToolNames.length > 0 + ? allowedToolNames.includes(tool.name) + : true; + const blocked = + blockedToolNames.length > 0 + ? blockedToolNames.includes(tool.name) + : false; + if (!allowed || blocked) { + if (blocked) { + globalLogger.debug( + `MCP Tool (server: ${server.name}, tool: ${tool.name}) is blocked by the static filter.`, + ); + } else if (!allowed) { + globalLogger.debug( + `MCP Tool (server: ${server.name}, tool: ${tool.name}) is not allowed by the static filter.`, + ); } + continue; } } } - filteredTools.push(tool); } - mcpTools = filteredTools; + filteredTools.push(tool); } + mcpTools = filteredTools; + } + if (span) { span.spanData.result = mcpTools.map((t) => t.name); - const tools: FunctionTool[] = mcpTools.map((t) => - mcpToFunctionTool(t, server, convertSchemasToStrict), - ); - if (server.cacheToolsList) { - _cachedTools[server.name] = mcpTools; - } - return tools; - }, - { data: { server: server.name } }, - ); + } + const tools: FunctionTool[] = mcpTools.map((t) => + mcpToFunctionTool(t, server, convertSchemasToStrict), + ); + if (server.cacheToolsList) { + _cachedTools[server.name] = mcpTools; + } + return tools; + }; + + if (!getCurrentTrace()) { + return listToolsForServer(); + } + + return withMCPListToolsSpan(listToolsForServer, { + data: { server: server.name }, + }); } /** diff --git a/packages/agents-core/src/tracing/index.ts b/packages/agents-core/src/tracing/index.ts index 57dc48b3..18af1d88 100644 --- a/packages/agents-core/src/tracing/index.ts +++ b/packages/agents-core/src/tracing/index.ts @@ -19,6 +19,7 @@ export { export { NoopSpan, Span } from './spans'; export { NoopTrace, Trace } from './traces'; export { generateGroupId, generateSpanId, generateTraceId } from './utils'; +export type { MCPListToolsSpanData } from './spans'; /** * Add a processor to the list of processors. Each processor will receive all traces/spans. diff --git a/packages/agents-core/test/runState.test.ts b/packages/agents-core/test/runState.test.ts index f68276e8..d72bf14f 100644 --- a/packages/agents-core/test/runState.test.ts +++ b/packages/agents-core/test/runState.test.ts @@ -20,6 +20,9 @@ import { FakeShell, FakeEditor, } from './stubs'; +import { createAgentSpan } from '../src/tracing'; +import { getGlobalTraceProvider } from '../src/tracing/provider'; +import type { MCPServer, MCPTool } from '../src/mcp'; describe('RunState', () => { it('initializes with default values', () => { @@ -353,6 +356,111 @@ describe('deserialize helpers', () => { ).toBe(editorTool); }); + it('fromString tolerates agents gaining MCP servers after serialization', async () => { + const agentWithoutMcp = new Agent({ name: 'McpLite' }); + const state = new RunState(new RunContext(), 'input', agentWithoutMcp, 1); + state._lastProcessedResponse = { + newItems: [], + functions: [], + handoffs: [], + computerActions: [], + shellActions: [], + applyPatchActions: [], + mcpApprovalRequests: [], + toolsUsed: [], + hasToolsOrApprovalsToRun: () => false, + }; + + const serialized = state.toString(); + + const stubMcpServer: MCPServer = { + name: 'stub-server', + cacheToolsList: false, + toolFilter: undefined, + async connect() {}, + async close() {}, + async listTools() { + return []; + }, + async callTool() { + return []; + }, + async invalidateToolsCache() {}, + }; + + const agentWithMcp = new Agent({ + name: 'McpLite', + mcpServers: [stubMcpServer], + }); + + const restored = await RunState.fromString(agentWithMcp, serialized); + expect(restored._currentAgent.mcpServers).toHaveLength(1); + expect(restored._lastProcessedResponse?.hasToolsOrApprovalsToRun()).toBe( + false, + ); + }); + + it('fromString tolerates serialized traces with new MCP servers', async () => { + const traceProvider = getGlobalTraceProvider(); + const trace = traceProvider.createTrace({ name: 'restore-with-trace' }); + const agentWithoutMcp = new Agent({ name: 'McpTracey' }); + const state = new RunState(new RunContext(), 'input', agentWithoutMcp, 1); + state._trace = trace; + state._currentAgentSpan = createAgentSpan( + { data: { name: agentWithoutMcp.name } }, + trace, + ); + state._lastProcessedResponse = { + newItems: [], + functions: [], + handoffs: [], + computerActions: [], + shellActions: [], + applyPatchActions: [], + mcpApprovalRequests: [], + toolsUsed: [], + hasToolsOrApprovalsToRun: () => false, + }; + + const serialized = state.toString(); + + let listCalled = false; + const stubMcpTool: MCPTool = { + name: 'sample_tool', + description: '', + inputSchema: { + type: 'object', + properties: {}, + required: [], + additionalProperties: false, + }, + }; + const stubMcpServer: MCPServer = { + name: 'stub-traced-server', + cacheToolsList: false, + toolFilter: undefined, + async connect() {}, + async close() {}, + async listTools() { + listCalled = true; + return [stubMcpTool]; + }, + async callTool() { + return []; + }, + async invalidateToolsCache() {}, + }; + + const agentWithMcp = new Agent({ + name: 'McpTracey', + mcpServers: [stubMcpServer], + }); + + const restored = await RunState.fromString(agentWithMcp, serialized); + expect(restored._currentAgent.mcpServers).toHaveLength(1); + expect(listCalled).toBe(true); + }); + it('deserializeProcessedResponse restores currentStep', async () => { const tool = computerTool({ computer: new FakeComputer() }); const agent = new Agent({ name: 'Comp', tools: [tool] });