diff --git a/.changeset/forty-dancers-pretend.md b/.changeset/forty-dancers-pretend.md new file mode 100644 index 00000000..44a0b7b8 --- /dev/null +++ b/.changeset/forty-dancers-pretend.md @@ -0,0 +1,5 @@ +--- +"@openai/agents-core": patch +--- + +Revert "feat(mcp): support structuredContent via useStructuredContent; return full CallToolResult" diff --git a/packages/agents-core/src/mcp.ts b/packages/agents-core/src/mcp.ts index e937338f..1aeda9d3 100644 --- a/packages/agents-core/src/mcp.ts +++ b/packages/agents-core/src/mcp.ts @@ -35,11 +35,6 @@ export const DEFAULT_SSE_MCP_CLIENT_LOGGER_NAME = export interface MCPServer { cacheToolsList: boolean; toolFilter?: MCPToolFilterCallable | MCPToolFilterStatic; - - /** - * Whether to include structuredContent in tool outputs when available. - */ - useStructuredContent?: boolean; connect(): Promise; readonly name: string; close(): Promise; @@ -47,7 +42,7 @@ export interface MCPServer { callTool( toolName: string, args: Record | null, - ): Promise; + ): Promise; invalidateToolsCache(): Promise; } @@ -55,7 +50,6 @@ export abstract class BaseMCPServerStdio implements MCPServer { public cacheToolsList: boolean; protected _cachedTools: any[] | undefined = undefined; public toolFilter?: MCPToolFilterCallable | MCPToolFilterStatic; - public useStructuredContent?: boolean; protected logger: Logger; constructor(options: MCPServerStdioOptions) { @@ -63,7 +57,6 @@ export abstract class BaseMCPServerStdio implements MCPServer { options.logger ?? getLogger(DEFAULT_STDIO_MCP_CLIENT_LOGGER_NAME); this.cacheToolsList = options.cacheToolsList ?? false; this.toolFilter = options.toolFilter; - this.useStructuredContent = options.useStructuredContent ?? false; } abstract get name(): string; @@ -73,7 +66,7 @@ export abstract class BaseMCPServerStdio implements MCPServer { abstract callTool( _toolName: string, _args: Record | null, - ): Promise; + ): Promise; abstract invalidateToolsCache(): Promise; /** @@ -92,7 +85,6 @@ export abstract class BaseMCPServerStreamableHttp implements MCPServer { public cacheToolsList: boolean; protected _cachedTools: any[] | undefined = undefined; public toolFilter?: MCPToolFilterCallable | MCPToolFilterStatic; - public useStructuredContent?: boolean; protected logger: Logger; constructor(options: MCPServerStreamableHttpOptions) { @@ -101,7 +93,6 @@ export abstract class BaseMCPServerStreamableHttp implements MCPServer { getLogger(DEFAULT_STREAMABLE_HTTP_MCP_CLIENT_LOGGER_NAME); this.cacheToolsList = options.cacheToolsList ?? false; this.toolFilter = options.toolFilter; - this.useStructuredContent = options.useStructuredContent ?? false; } abstract get name(): string; @@ -111,7 +102,7 @@ export abstract class BaseMCPServerStreamableHttp implements MCPServer { abstract callTool( _toolName: string, _args: Record | null, - ): Promise; + ): Promise; abstract invalidateToolsCache(): Promise; /** @@ -130,7 +121,6 @@ export abstract class BaseMCPServerSSE implements MCPServer { public cacheToolsList: boolean; protected _cachedTools: any[] | undefined = undefined; public toolFilter?: MCPToolFilterCallable | MCPToolFilterStatic; - public useStructuredContent?: boolean; protected logger: Logger; constructor(options: MCPServerSSEOptions) { @@ -138,7 +128,6 @@ export abstract class BaseMCPServerSSE implements MCPServer { options.logger ?? getLogger(DEFAULT_SSE_MCP_CLIENT_LOGGER_NAME); this.cacheToolsList = options.cacheToolsList ?? false; this.toolFilter = options.toolFilter; - this.useStructuredContent = options.useStructuredContent ?? false; } abstract get name(): string; @@ -148,7 +137,7 @@ export abstract class BaseMCPServerSSE implements MCPServer { abstract callTool( _toolName: string, _args: Record | null, - ): Promise; + ): Promise; abstract invalidateToolsCache(): Promise; /** @@ -212,7 +201,7 @@ export class MCPServerStdio extends BaseMCPServerStdio { callTool( toolName: string, args: Record | null, - ): Promise { + ): Promise { return this.underlying.callTool(toolName, args); } invalidateToolsCache(): Promise { @@ -248,7 +237,7 @@ export class MCPServerStreamableHttp extends BaseMCPServerStreamableHttp { callTool( toolName: string, args: Record | null, - ): Promise { + ): Promise { return this.underlying.callTool(toolName, args); } invalidateToolsCache(): Promise { @@ -284,7 +273,7 @@ export class MCPServerSSE extends BaseMCPServerSSE { callTool( toolName: string, args: Record | null, - ): Promise { + ): Promise { return this.underlying.callTool(toolName, args); } invalidateToolsCache(): Promise { @@ -457,7 +446,6 @@ export async function getAllMcpTools( /** * Converts an MCP tool definition to a function tool for the Agents SDK. - * When useStructuredContent is enabled, returns JSON strings for consistency with Python SDK. */ export function mcpToFunctionTool( mcpTool: MCPTool, @@ -475,36 +463,8 @@ export function mcpToFunctionTool( if (currentSpan) { currentSpan.spanData['mcp_data'] = { server: server.name }; } - const result = await server.callTool(mcpTool.name, args); - - if (result.content && result.content.length === 1) { - if ( - server.useStructuredContent && - 'structuredContent' in result && - result.structuredContent !== undefined - ) { - return JSON.stringify([result.content[0], result.structuredContent]); - } - return result.content[0]; - } else if (result.content && result.content.length > 1) { - if ( - server.useStructuredContent && - 'structuredContent' in result && - result.structuredContent !== undefined - ) { - const outputs = [...result.content, result.structuredContent]; - return JSON.stringify(outputs); - } - return result.content; - } else if ( - server.useStructuredContent && - 'structuredContent' in result && - result.structuredContent !== undefined - ) { - return JSON.stringify(result.structuredContent); - } - // Preserve backward compatibility: return empty array when no content - return result.content || []; + const content = await server.callTool(mcpTool.name, args); + return content.length === 1 ? content[0] : content; } const schema: JsonObjectSchema = { @@ -573,11 +533,6 @@ export interface BaseMCPServerStdioOptions { encodingErrorHandler?: 'strict' | 'ignore' | 'replace'; logger?: Logger; toolFilter?: MCPToolFilterCallable | MCPToolFilterStatic; - - /** - * Whether to include structuredContent in tool outputs when available. - */ - useStructuredContent?: boolean; timeout?: number; } export interface DefaultMCPServerStdioOptions @@ -600,11 +555,6 @@ export interface MCPServerStreamableHttpOptions { name?: string; logger?: Logger; toolFilter?: MCPToolFilterCallable | MCPToolFilterStatic; - - /** - * Whether to include structuredContent in tool outputs when available. - */ - useStructuredContent?: boolean; timeout?: number; // ---------------------------------------------------- @@ -629,11 +579,6 @@ export interface MCPServerSSEOptions { name?: string; logger?: Logger; toolFilter?: MCPToolFilterCallable | MCPToolFilterStatic; - - /** - * Whether to include structuredContent in tool outputs when available. - */ - useStructuredContent?: boolean; timeout?: number; // ---------------------------------------------------- @@ -676,22 +621,9 @@ export interface JsonRpcResponse { error?: any; } -/** - * Structured content that can be returned by MCP tools. - * Supports various data types including objects, arrays, primitives, and null. - */ -export type StructuredContent = - | Record - | unknown[] - | string - | number - | boolean - | null; - export interface CallToolResponse extends JsonRpcResponse { result: { content: { type: string; text: string }[]; - structuredContent?: StructuredContent; }; } export type CallToolResult = CallToolResponse['result']; diff --git a/packages/agents-core/src/shims/mcp-server/browser.ts b/packages/agents-core/src/shims/mcp-server/browser.ts index 7d809556..51b475b7 100644 --- a/packages/agents-core/src/shims/mcp-server/browser.ts +++ b/packages/agents-core/src/shims/mcp-server/browser.ts @@ -2,7 +2,7 @@ import { BaseMCPServerSSE, BaseMCPServerStdio, BaseMCPServerStreamableHttp, - CallToolResult, + CallToolResultContent, MCPServerSSEOptions, MCPServerStdioOptions, MCPServerStreamableHttpOptions, @@ -28,7 +28,7 @@ export class MCPServerStdio extends BaseMCPServerStdio { callTool( _toolName: string, _args: Record | null, - ): Promise { + ): Promise { throw new Error('Method not implemented.'); } invalidateToolsCache(): Promise { @@ -55,7 +55,7 @@ export class MCPServerStreamableHttp extends BaseMCPServerStreamableHttp { callTool( _toolName: string, _args: Record | null, - ): Promise { + ): Promise { throw new Error('Method not implemented.'); } invalidateToolsCache(): Promise { @@ -84,7 +84,7 @@ export class MCPServerSSE extends BaseMCPServerSSE { callTool( _toolName: string, _args: Record | null, - ): Promise { + ): Promise { throw new Error('Method not implemented.'); } diff --git a/packages/agents-core/src/shims/mcp-server/node.ts b/packages/agents-core/src/shims/mcp-server/node.ts index 73b4540d..00702690 100644 --- a/packages/agents-core/src/shims/mcp-server/node.ts +++ b/packages/agents-core/src/shims/mcp-server/node.ts @@ -5,7 +5,7 @@ import { BaseMCPServerStdio, BaseMCPServerStreamableHttp, BaseMCPServerSSE, - CallToolResult, + CallToolResultContent, DefaultMCPServerStdioOptions, InitializeResult, MCPServerStdioOptions, @@ -124,7 +124,7 @@ export class NodeMCPServerStdio extends BaseMCPServerStdio { async callTool( toolName: string, args: Record | null, - ): Promise { + ): Promise { const { CallToolResultSchema } = await import( '@modelcontextprotocol/sdk/types.js' ).catch(failedToImport); @@ -144,12 +144,12 @@ export class NodeMCPServerStdio extends BaseMCPServerStdio { }, ); const parsed = CallToolResultSchema.parse(response); - const result = parsed; + const result = parsed.content; this.debugLog( () => - `Called tool ${toolName} (args: ${JSON.stringify(args)}, result: ${JSON.stringify(result.content)})`, + `Called tool ${toolName} (args: ${JSON.stringify(args)}, result: ${JSON.stringify(result)})`, ); - return result as CallToolResult; + return result as CallToolResultContent; } get name() { @@ -245,7 +245,7 @@ export class NodeMCPServerSSE extends BaseMCPServerSSE { async callTool( toolName: string, args: Record | null, - ): Promise { + ): Promise { const { CallToolResultSchema } = await import( '@modelcontextprotocol/sdk/types.js' ).catch(failedToImport); @@ -265,12 +265,12 @@ export class NodeMCPServerSSE extends BaseMCPServerSSE { }, ); const parsed = CallToolResultSchema.parse(response); - const result = parsed; + const result = parsed.content; this.debugLog( () => - `Called tool ${toolName} (args: ${JSON.stringify(args)}, result: ${JSON.stringify(result.content)})`, + `Called tool ${toolName} (args: ${JSON.stringify(args)}, result: ${JSON.stringify(result)})`, ); - return result as CallToolResult; + return result as CallToolResultContent; } get name() { @@ -371,7 +371,7 @@ export class NodeMCPServerStreamableHttp extends BaseMCPServerStreamableHttp { async callTool( toolName: string, args: Record | null, - ): Promise { + ): Promise { const { CallToolResultSchema } = await import( '@modelcontextprotocol/sdk/types.js' ).catch(failedToImport); @@ -391,12 +391,12 @@ export class NodeMCPServerStreamableHttp extends BaseMCPServerStreamableHttp { }, ); const parsed = CallToolResultSchema.parse(response); - const result = parsed; + const result = parsed.content; this.debugLog( () => - `Called tool ${toolName} (args: ${JSON.stringify(args)}, result: ${JSON.stringify(result.content)})`, + `Called tool ${toolName} (args: ${JSON.stringify(args)}, result: ${JSON.stringify(result)})`, ); - return result as CallToolResult; + return result as CallToolResultContent; } get name() { diff --git a/packages/agents-core/test/mcpCache.test.ts b/packages/agents-core/test/mcpCache.test.ts index 4636aa3c..42c83eb5 100644 --- a/packages/agents-core/test/mcpCache.test.ts +++ b/packages/agents-core/test/mcpCache.test.ts @@ -3,7 +3,7 @@ import { getAllMcpTools } from '../src/mcp'; import type { FunctionTool } from '../src/tool'; import { withTrace } from '../src/tracing'; import { NodeMCPServerStdio } from '../src/shims/mcp-server/node'; -import type { CallToolResult } from '../src/mcp'; +import type { CallToolResultContent } from '../src/mcp'; import { RunContext } from '../src/runContext'; import { Agent } from '../src/agent'; @@ -27,8 +27,8 @@ class StubServer extends NodeMCPServerStdio { async callTool( _toolName: string, _args: Record | null, - ): Promise { - return { content: [] }; + ): Promise { + return []; } } diff --git a/packages/agents-core/test/mcpStructuredContent.test.ts b/packages/agents-core/test/mcpStructuredContent.test.ts deleted file mode 100644 index 35c61957..00000000 --- a/packages/agents-core/test/mcpStructuredContent.test.ts +++ /dev/null @@ -1,418 +0,0 @@ -import { describe, it, expect } from 'vitest'; -import { mcpToFunctionTool } from '../src/mcp'; -import { NodeMCPServerStdio } from '../src/shims/mcp-server/node'; -import type { CallToolResult } from '../src/mcp'; - -class StubServer extends NodeMCPServerStdio { - public toolList: any[]; - constructor(name: string, tools: any[], useStructuredContent?: boolean) { - super({ command: 'noop', name, useStructuredContent }); - this.toolList = tools; - this.cacheToolsList = false; - } - async connect(): Promise {} - async close(): Promise {} - async listTools(): Promise { - this._toolsList = this.toolList; - return this.toolList; - } - async callTool( - _toolName: string, - _args: Record | null, - ): Promise { - // default gets overridden in tests via monkey patching - return { content: [] } as CallToolResult; - } -} - -describe('MCP structuredContent handling', () => { - it('omits structuredContent by default and returns single item object', async () => { - const server = new StubServer( - 's', - [ - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - ], - false, - ); - // Patch callTool to return one content and structuredContent - (server as any).callTool = async () => ({ - content: [{ type: 'text', text: 'hello' }], - structuredContent: { foo: 1 }, - }); - - const tool = mcpToFunctionTool( - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - server, - false, - ); - - const out = await tool.invoke({} as any, '{}'); - // when not using structured content, return the single content object - expect(out).toEqual({ type: 'text', text: 'hello' }); - }); - - it('includes structuredContent when enabled: single content -> array with structuredContent appended', async () => { - const server = new StubServer( - 's', - [ - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - ], - true, - ); - (server as any).callTool = async () => ({ - content: [{ type: 'text', text: 'hello' }], - structuredContent: { foo: 1 }, - }); - - const tool = mcpToFunctionTool( - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - server, - false, - ); - - const out = await tool.invoke({} as any, '{}'); - expect(out).toEqual(JSON.stringify([{ type: 'text', text: 'hello' }, { foo: 1 }])); - }); - - it('includes structuredContent when enabled: no content -> structuredContent only', async () => { - const server = new StubServer( - 's', - [ - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - ], - true, - ); - (server as any).callTool = async () => ({ - content: [], - structuredContent: { foo: 1 }, - }); - - const tool = mcpToFunctionTool( - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - server, - false, - ); - - const out = await tool.invoke({} as any, '{}'); - expect(out).toEqual(JSON.stringify({ foo: 1 })); - }); - - it('includes structuredContent when enabled: multiple contents -> array with structuredContent appended', async () => { - const server = new StubServer( - 's', - [ - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - ], - true, - ); - (server as any).callTool = async () => ({ - content: [ - { type: 'text', text: 'a' }, - { type: 'text', text: 'b' }, - ], - structuredContent: { foo: 1 }, - }); - - const tool = mcpToFunctionTool( - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - server, - false, - ); - - const out = await tool.invoke({} as any, '{}'); - expect(out).toEqual(JSON.stringify([ - { type: 'text', text: 'a' }, - { type: 'text', text: 'b' }, - { foo: 1 }, - ])); - }); - - it('preserves falsy structuredContent values when enabled', async () => { - const server = new StubServer( - 's', - [ - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - ], - true, - ); - - // Test different falsy values - const falsyValues = [0, false, '', null]; - - for (const falsyValue of falsyValues) { - // Test with single content + falsy structuredContent - (server as any).callTool = async () => ({ - content: [{ type: 'text', text: 'hello' }], - structuredContent: falsyValue, - }); - - const tool = mcpToFunctionTool( - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - server, - false, - ); - - const out = await tool.invoke({} as any, '{}'); - expect(out).toEqual(JSON.stringify([{ type: 'text', text: 'hello' }, falsyValue])); - } - - // Test with no content + falsy structuredContent - for (const falsyValue of falsyValues) { - (server as any).callTool = async () => ({ - content: [], - structuredContent: falsyValue, - }); - - const tool = mcpToFunctionTool( - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - server, - false, - ); - - const out = await tool.invoke({} as any, '{}'); - expect(out).toEqual(JSON.stringify(falsyValue)); - } - }); - - it('handles undefined structuredContent gracefully', async () => { - const server = new StubServer( - 's', - [ - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - ], - true, // useStructuredContent enabled - ); - - // Test with structuredContent explicitly undefined - (server as any).callTool = async () => ({ - content: [{ type: 'text', text: 'hello' }], - structuredContent: undefined, - }); - - const tool = mcpToFunctionTool( - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - server, - false, - ); - - const out = await tool.invoke({} as any, '{}'); - // Should return just the content item since structuredContent is undefined - expect(out).toEqual({ type: 'text', text: 'hello' }); - }); - - it('handles mixed scenarios with some tools having structured content', async () => { - const server = new StubServer( - 's', - [ - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - ], - true, - ); - - // First call: has structured content - (server as any).callTool = async () => ({ - content: [{ type: 'text', text: 'with-structured' }], - structuredContent: { data: 'structured' }, - }); - - const tool = mcpToFunctionTool( - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - server, - false, - ); - - let out = await tool.invoke({} as any, '{}'); - expect(out).toEqual(JSON.stringify([ - { type: 'text', text: 'with-structured' }, - { data: 'structured' }, - ])); - - // Second call: no structured content - (server as any).callTool = async () => ({ - content: [{ type: 'text', text: 'without-structured' }], - }); - - out = await tool.invoke({} as any, '{}'); - // Should return just the content item when no structuredContent property - expect(out).toEqual({ type: 'text', text: 'without-structured' }); - }); - - it('handles empty content with useStructuredContent disabled', async () => { - const server = new StubServer( - 's', - [ - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - ], - false, // useStructuredContent disabled - ); - - // Tool returns empty content and structured content - (server as any).callTool = async () => ({ - content: [], - structuredContent: { important: 'data' }, - }); - - const tool = mcpToFunctionTool( - { - name: 't', - description: '', - inputSchema: { - type: 'object', - properties: {}, - required: [], - additionalProperties: false, - }, - }, - server, - false, - ); - - const out = await tool.invoke({} as any, '{}'); - // Should return empty array, ignoring structured content when disabled - expect(out).toEqual([]); - }); -}); diff --git a/packages/agents-core/test/mcpToolFilter.integration.test.ts b/packages/agents-core/test/mcpToolFilter.integration.test.ts index 04baf817..dfa37a2c 100644 --- a/packages/agents-core/test/mcpToolFilter.integration.test.ts +++ b/packages/agents-core/test/mcpToolFilter.integration.test.ts @@ -60,25 +60,23 @@ class StubFilesystemServer extends NodeMCPServerStdio { async callTool(name: string, args: any) { const blocked = (this.toolFilter as any)?.blockedToolNames ?? []; if (blocked.includes(name)) { - return { - content: [ - { type: 'text', text: `Tool "${name}" is blocked by MCP filter` }, - ], - }; + return [ + { type: 'text', text: `Tool "${name}" is blocked by MCP filter` }, + ]; } if (name === 'list_directory') { const files = fs.readdirSync(this.dir); - return { content: [{ type: 'text', text: files.join('\n') }] }; + return [{ type: 'text', text: files.join('\n') }]; } if (name === 'read_file') { const text = fs.readFileSync(path.join(this.dir, args.path), 'utf8'); - return { content: [{ type: 'text', text }] }; + return [{ type: 'text', text }]; } if (name === 'write_file') { fs.writeFileSync(path.join(this.dir, args.path), args.text, 'utf8'); - return { content: [{ type: 'text', text: 'ok' }] }; + return [{ type: 'text', text: 'ok' }]; } - return { content: [] }; + return []; } }