From 0fd003eea9e519672d77804908a667402725e7e7 Mon Sep 17 00:00:00 2001 From: Sushant Chaudhary Date: Thu, 21 May 2026 20:57:04 +0200 Subject: [PATCH 1/3] fix: hide MCP list tools from client responses --- src/routes/responses/innerStream.ts | 15 ++---- src/routes/responses/mcpStream.test.ts | 22 +++++---- src/routes/responses/mcpStream.ts | 48 +++++------------- tests/responses.test.js | 67 ++++++-------------------- 4 files changed, 43 insertions(+), 109 deletions(-) diff --git a/src/routes/responses/innerStream.ts b/src/routes/responses/innerStream.ts index bcadef9..4035064 100644 --- a/src/routes/responses/innerStream.ts +++ b/src/routes/responses/innerStream.ts @@ -12,7 +12,7 @@ import { config } from "../../lib/config.js"; import { formatInputToMessages } from "./messageFormatting.js"; import { buildLLMPayload } from "./payloadBuilder.js"; import { handleOneTurnStream } from "./handleOneTurn.js"; -import { listMcpToolsStream, callApprovedMCPToolStream } from "./mcpStream.js"; +import { listMcpTools, callApprovedMCPToolStream } from "./mcpStream.js"; export async function* innerRunStream( req: ValidatedRequest, @@ -99,18 +99,9 @@ export async function* innerRunStream( } } } - // Otherwise, list tools from MCP server + // Otherwise, list tools from MCP server for internal orchestration only. if (!mcpListTools) { - for await (const event of listMcpToolsStream(tool, responseObject, traceContext, log)) { - yield event; - } - const lastOutput = responseObject.output.at(-1); - if (!lastOutput || lastOutput.type !== "mcp_list_tools") { - throw new Error( - `Expected mcp_list_tools output after listMcpToolsStream, got ${lastOutput?.type ?? "undefined"}` - ); - } - mcpListTools = lastOutput; + mcpListTools = await listMcpTools(tool, traceContext, log); } // Only allowed tools are forwarded to the LLM diff --git a/src/routes/responses/mcpStream.test.ts b/src/routes/responses/mcpStream.test.ts index 6c82ea6..a643c5f 100644 --- a/src/routes/responses/mcpStream.test.ts +++ b/src/routes/responses/mcpStream.test.ts @@ -32,7 +32,7 @@ vi.mock("../../mcp.js", () => ({ connectMcpServer: vi.fn(), })); -import { listMcpToolsStream, callApprovedMCPToolStream } from "./mcpStream.js"; +import { listMcpTools, listMcpToolsStream, callApprovedMCPToolStream } from "./mcpStream.js"; import { connectMcpServer, callMcpTool } from "../../mcp.js"; import { createMockResponseObject, createMockLogger, collectEvents } from "./__test_helpers__/mocks.js"; import type { McpServerParams } from "../../schemas.js"; @@ -57,7 +57,7 @@ describe("listMcpToolsStream", () => { vi.clearAllMocks(); }); - it("yields correct event sequence on success", async () => { + it("fetches tools internally without adding public response output or events", async () => { const mockClient = { listTools: vi.fn().mockResolvedValue({ tools: [ @@ -74,19 +74,21 @@ describe("listMcpToolsStream", () => { (connectMcpServer as ReturnType).mockResolvedValue(mockClient); const responseObject = createMockResponseObject(); + const result = await listMcpTools(mcpTool, traceContext, log); const events = await collectEvents(listMcpToolsStream(mcpTool, responseObject, traceContext, log)); const types = events.map((e) => e.type); - expect(types).toEqual([ - "response.output_item.added", - "response.mcp_list_tools.in_progress", - "response.mcp_list_tools.completed", - "response.output_item.done", - ]); - expect(types.filter((t) => t === "response.output_item.done")).toHaveLength(1); + expect(result).toMatchObject({ + type: "mcp_list_tools", + server_label: "test-server", + tools: [{ name: "search", input_schema: { type: "object" }, description: "Search tool" }], + }); + expect(types).toEqual([]); + expect(responseObject.output).toEqual([]); + expect(types.some((type) => type.includes("mcp_list_tools"))).toBe(false); }); - it("yields failed event and throws on connection error", async () => { + it("throws on connection error without yielding public failure events", async () => { (connectMcpServer as ReturnType).mockRejectedValue(new Error("Connection refused")); const responseObject = createMockResponseObject(); diff --git a/src/routes/responses/mcpStream.ts b/src/routes/responses/mcpStream.ts index 6281b96..713fe0e 100644 --- a/src/routes/responses/mcpStream.ts +++ b/src/routes/responses/mcpStream.ts @@ -14,12 +14,11 @@ import { } from "./types.js"; import { buildJsonAttribute, recordError } from "./utils.js"; -export async function* listMcpToolsStream( +export async function listMcpTools( tool: McpServerParams, - responseObject: IncompleteResponse, traceContext: Context, log: Logger -): AsyncGenerator { +): Promise { const span = tracer.startSpan( "gen_ai.execute_tool", { @@ -38,32 +37,11 @@ export async function* listMcpToolsStream( server_label: tool.server_label, tools: [], }; - responseObject.output.push(outputObject); - - yield { - type: "response.output_item.added", - output_index: responseObject.output.length - 1, - item: outputObject, - sequence_number: SEQUENCE_NUMBER_PLACEHOLDER, - }; - - yield { - type: "response.mcp_list_tools.in_progress", - item_id: outputObject.id, - output_index: responseObject.output.length - 1, - sequence_number: SEQUENCE_NUMBER_PLACEHOLDER, - }; let mcp: Awaited> | undefined; try { mcp = await connectMcpServer(tool, log); const mcpTools = await mcp.listTools(); - yield { - type: "response.mcp_list_tools.completed", - item_id: outputObject.id, - output_index: responseObject.output.length - 1, - sequence_number: SEQUENCE_NUMBER_PLACEHOLDER, - }; outputObject.tools = mcpTools.tools.map((mcpTool) => ({ input_schema: mcpTool.inputSchema, name: mcpTool.name, @@ -71,22 +49,11 @@ export async function* listMcpToolsStream( description: mcpTool.description, })); span.setAttribute("mcp.tools.count", outputObject.tools.length); - yield { - type: "response.output_item.done", - output_index: responseObject.output.length - 1, - item: outputObject, - sequence_number: SEQUENCE_NUMBER_PLACEHOLDER, - }; + return outputObject; } catch (error) { const errorMessage = `Failed to list tools from MCP server '${tool.server_label}': ${error instanceof Error ? error.message : "Unknown error"}`; log.error({ err: error, server_label: tool.server_label }, "Failed to list MCP tools"); recordError(span, error); - yield { - type: "response.mcp_list_tools.failed", - item_id: outputObject.id, - output_index: responseObject.output.length - 1, - sequence_number: SEQUENCE_NUMBER_PLACEHOLDER, - }; throw new Error(errorMessage); } finally { if (mcp) { @@ -96,6 +63,15 @@ export async function* listMcpToolsStream( } } +export async function* listMcpToolsStream( + tool: McpServerParams, + _responseObject: IncompleteResponse, + traceContext: Context, + log: Logger +): AsyncGenerator { + await listMcpTools(tool, traceContext, log); +} + /* * Perform an approved MCP tool call and stream the response. */ diff --git a/tests/responses.test.js b/tests/responses.test.js index 5088852..3a2a205 100644 --- a/tests/responses.test.js +++ b/tests/responses.test.js @@ -497,23 +497,11 @@ describe("responses.js", function () { }); assert.ok(Array.isArray(response.output)); - assert.ok(response.output.length >= 2); - - // Check first output item (mcp_list_tools) - const listToolsOutput = response.output[0]; - assert.equal(listToolsOutput.type, "mcp_list_tools"); - assert.equal(listToolsOutput.server_label, "gitmcp"); - assert.ok(listToolsOutput.id); - assert.ok(Array.isArray(listToolsOutput.tools)); - assert.ok(listToolsOutput.tools.length > 0); - - // Check that tools array contains expected tools - const toolNames = listToolsOutput.tools.map((tool) => tool.name); - assert.ok(toolNames.includes("fetch_tiktoken_documentation")); - assert.ok(toolNames.includes("search_tiktoken_documentation")); - - // Check second output item (mcp_call) - const mcpCallOutput = response.output[1]; + assert.ok(response.output.length >= 1); + assert.ok(!response.output.some((item) => item.type === "mcp_list_tools")); + + // Check first output item (mcp_call) + const mcpCallOutput = response.output[0]; assert.equal(mcpCallOutput.type, "mcp_call"); assert.equal(mcpCallOutput.name, "fetch_tiktoken_documentation"); assert.equal(mcpCallOutput.server_label, "gitmcp"); @@ -571,23 +559,11 @@ describe("responses.js", function () { }); assert.ok(Array.isArray(response.output)); - assert.ok(response.output.length === 2); - - // Check first output item (mcp_list_tools) - const listToolsOutput = response.output[0]; - assert.equal(listToolsOutput.type, "mcp_list_tools"); - assert.equal(listToolsOutput.server_label, "gitmcp"); - assert.ok(listToolsOutput.id); - assert.ok(Array.isArray(listToolsOutput.tools)); - assert.ok(listToolsOutput.tools.length > 0); - - // Check that tools array contains expected tools - const toolNames = listToolsOutput.tools.map((tool) => tool.name); - assert.ok(toolNames.includes("fetch_tiktoken_documentation")); - assert.ok(toolNames.includes("search_tiktoken_documentation")); - - // Check second output item (mcp_approval_request) - const approvalRequestOutput = response.output[1]; + assert.ok(response.output.length === 1); + assert.ok(!response.output.some((item) => item.type === "mcp_list_tools")); + + // Check first output item (mcp_approval_request) + const approvalRequestOutput = response.output[0]; assert.equal(approvalRequestOutput.type, "mcp_approval_request"); assert.equal(approvalRequestOutput.name, "fetch_tiktoken_documentation"); assert.equal(approvalRequestOutput.server_label, "gitmcp"); @@ -616,23 +592,11 @@ describe("responses.js", function () { }); assert.ok(Array.isArray(response.output)); - assert.ok(response.output.length >= 2); - - // Check first output item (mcp_list_tools) - const listToolsOutput = response.output[0]; - assert.equal(listToolsOutput.type, "mcp_list_tools"); - assert.equal(listToolsOutput.server_label, "gitmcp"); - assert.ok(listToolsOutput.id); - assert.ok(Array.isArray(listToolsOutput.tools)); - assert.ok(listToolsOutput.tools.length > 0); - - // Check that tools array contains expected tools - const toolNames = listToolsOutput.tools.map((tool) => tool.name); - assert.ok(toolNames.includes("fetch_tiktoken_documentation")); - assert.ok(toolNames.includes("search_tiktoken_documentation")); - - // Check second output item (mcp_call) - const mcpCallOutput = response.output[1]; + assert.ok(response.output.length >= 1); + assert.ok(!response.output.some((item) => item.type === "mcp_list_tools")); + + // Check first output item (mcp_call) + const mcpCallOutput = response.output[0]; assert.equal(mcpCallOutput.type, "mcp_call"); assert.equal(mcpCallOutput.name, "fetch_tiktoken_documentation"); assert.equal(mcpCallOutput.server_label, "gitmcp"); @@ -703,6 +667,7 @@ describe("responses.js", function () { assert.ok(Array.isArray(response.output)); assert.ok(response.output.length === 1); + assert.ok(!response.output.some((item) => item.type === "mcp_list_tools")); // Check that the first output item is an approval request (not a list_tools call) const approvalRequestOutput = response.output[0]; From d116ff151ae92c24f2117c9015a06872b43a4ce1 Mon Sep 17 00:00:00 2001 From: Sushant Chaudhary Date: Thu, 21 May 2026 20:57:09 +0200 Subject: [PATCH 2/3] chore(demo): remove MCP list tools rendering --- demo/components/chat.tsx | 3 --- demo/lib/assistant.ts | 22 +--------------------- 2 files changed, 1 insertion(+), 24 deletions(-) diff --git a/demo/components/chat.tsx b/demo/components/chat.tsx index 31a7653..b66f341 100644 --- a/demo/components/chat.tsx +++ b/demo/components/chat.tsx @@ -4,7 +4,6 @@ import React, { useCallback, useEffect, useRef, useState } from "react"; import ToolCall from "./tool-call"; import Message from "./message"; import Annotations from "./annotations"; -import McpToolsList from "./mcp-tools-list"; import McpApproval from "./mcp-approval"; import { Item, McpApprovalRequestItem } from "@/lib/assistant"; import LoadingMessage from "./loading-message"; @@ -58,8 +57,6 @@ const Chat: React.FC = ({ items, onSendMessage, onApprovalResponse }) )} - ) : item.type === "mcp_list_tools" ? ( - ) : item.type === "mcp_approval_request" ? ( ) : null} diff --git a/demo/lib/assistant.ts b/demo/lib/assistant.ts index 7b64b38..80a2a83 100644 --- a/demo/lib/assistant.ts +++ b/demo/lib/assistant.ts @@ -46,13 +46,6 @@ export interface ToolCallItem { }[]; } -export interface McpListToolsItem { - type: "mcp_list_tools"; - id: string; - server_label: string; - tools: { name: string; description?: string }[]; -} - export interface McpApprovalRequestItem { type: "mcp_approval_request"; id: string; @@ -61,7 +54,7 @@ export interface McpApprovalRequestItem { arguments?: string; } -export type Item = MessageItem | ToolCallItem | McpListToolsItem | McpApprovalRequestItem; +export type Item = MessageItem | ToolCallItem | McpApprovalRequestItem; export const handleTurn = async (messages: any[], tools: any[], onMessage: (data: any) => void) => { try { @@ -477,19 +470,6 @@ export const processMessages = async () => { console.log("response completed", data); const { response } = data; - // Handle MCP tools list - const mcpListToolsMessage = response.output.find((m: Item) => m.type === "mcp_list_tools"); - - if (mcpListToolsMessage) { - chatMessages.push({ - type: "mcp_list_tools", - id: mcpListToolsMessage.id, - server_label: mcpListToolsMessage.server_label, - tools: mcpListToolsMessage.tools || [], - }); - setChatMessages([...chatMessages]); - } - // Handle MCP approval request const mcpApprovalRequestMessage = response.output.find((m: Item) => m.type === "mcp_approval_request"); From b95c79f67f93ae2aeeb656c641b752b2b69509a0 Mon Sep 17 00:00:00 2001 From: Sushant Chaudhary Date: Thu, 21 May 2026 21:02:01 +0200 Subject: [PATCH 3/3] fix: remove unused MCP list stream wrapper --- src/routes/responses/closeOutputItem.ts | 2 +- src/routes/responses/innerStream.test.ts | 2 +- src/routes/responses/mcpStream.test.ts | 12 +++--------- src/routes/responses/mcpStream.ts | 9 --------- 4 files changed, 5 insertions(+), 20 deletions(-) diff --git a/src/routes/responses/closeOutputItem.ts b/src/routes/responses/closeOutputItem.ts index 80d2e6c..dfbb9b8 100644 --- a/src/routes/responses/closeOutputItem.ts +++ b/src/routes/responses/closeOutputItem.ts @@ -259,7 +259,7 @@ export async function* closeLastOutputItem( sequence_number: SEQUENCE_NUMBER_PLACEHOLDER, }; } else if (lastOutputItem?.type === "mcp_list_tools") { - // Already finalized by `listMcpToolsStream`; do not re-emit done. + // Internal MCP tool-list metadata is not client-visible; do not re-emit done. } else { throw new StreamingError( `Not implemented: expected message, function_call, or mcp_call, got ${(lastOutputItem as ResponseOutputItem)?.type}` diff --git a/src/routes/responses/innerStream.test.ts b/src/routes/responses/innerStream.test.ts index e9d3c6e..261755e 100644 --- a/src/routes/responses/innerStream.test.ts +++ b/src/routes/responses/innerStream.test.ts @@ -40,7 +40,7 @@ vi.mock("./handleOneTurn.js", () => ({ // Mock mcpStream vi.mock("./mcpStream.js", () => ({ - listMcpToolsStream: vi.fn(), + listMcpTools: vi.fn(), callApprovedMCPToolStream: vi.fn(), })); diff --git a/src/routes/responses/mcpStream.test.ts b/src/routes/responses/mcpStream.test.ts index a643c5f..07b68d9 100644 --- a/src/routes/responses/mcpStream.test.ts +++ b/src/routes/responses/mcpStream.test.ts @@ -32,7 +32,7 @@ vi.mock("../../mcp.js", () => ({ connectMcpServer: vi.fn(), })); -import { listMcpTools, listMcpToolsStream, callApprovedMCPToolStream } from "./mcpStream.js"; +import { listMcpTools, callApprovedMCPToolStream } from "./mcpStream.js"; import { connectMcpServer, callMcpTool } from "../../mcp.js"; import { createMockResponseObject, createMockLogger, collectEvents } from "./__test_helpers__/mocks.js"; import type { McpServerParams } from "../../schemas.js"; @@ -42,7 +42,7 @@ import type { Logger } from "pino"; const log = createMockLogger() as unknown as Logger; -describe("listMcpToolsStream", () => { +describe("listMcpTools", () => { const traceContext = {} as Context; const mcpTool: McpServerParams = { server_label: "test-server", @@ -75,25 +75,19 @@ describe("listMcpToolsStream", () => { const responseObject = createMockResponseObject(); const result = await listMcpTools(mcpTool, traceContext, log); - const events = await collectEvents(listMcpToolsStream(mcpTool, responseObject, traceContext, log)); - const types = events.map((e) => e.type); expect(result).toMatchObject({ type: "mcp_list_tools", server_label: "test-server", tools: [{ name: "search", input_schema: { type: "object" }, description: "Search tool" }], }); - expect(types).toEqual([]); expect(responseObject.output).toEqual([]); - expect(types.some((type) => type.includes("mcp_list_tools"))).toBe(false); }); it("throws on connection error without yielding public failure events", async () => { (connectMcpServer as ReturnType).mockRejectedValue(new Error("Connection refused")); - const responseObject = createMockResponseObject(); - - await expect(collectEvents(listMcpToolsStream(mcpTool, responseObject, traceContext, log))).rejects.toThrow( + await expect(listMcpTools(mcpTool, traceContext, log)).rejects.toThrow( "Failed to list tools from MCP server 'test-server'" ); }); diff --git a/src/routes/responses/mcpStream.ts b/src/routes/responses/mcpStream.ts index 713fe0e..2b6d34d 100644 --- a/src/routes/responses/mcpStream.ts +++ b/src/routes/responses/mcpStream.ts @@ -63,15 +63,6 @@ export async function listMcpTools( } } -export async function* listMcpToolsStream( - tool: McpServerParams, - _responseObject: IncompleteResponse, - traceContext: Context, - log: Logger -): AsyncGenerator { - await listMcpTools(tool, traceContext, log); -} - /* * Perform an approved MCP tool call and stream the response. */