fix: #683 Failing to run MCP servers when deserializing run state data (#685)

Author: seratch

.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

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<TContext = UnknownContext>({
       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<MCPListToolsSpanData>,
+  ): Promise<FunctionTool<TContext, any, unknown>[]> => {
+    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<TContext, any, string>[] = mcpTools.map((t) =>
-        mcpToFunctionTool(t, server, convertSchemasToStrict),
-      );
-      if (server.cacheToolsList) {
-        _cachedTools[server.name] = mcpTools;
-      }
-      return tools;
-    },
-    { data: { server: server.name } },
-  );
+    }
+    const tools: FunctionTool<TContext, any, string>[] = 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 },
+  });
 }
 
 /**

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.

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] });