From 0fb5cb8e6f3bf790046589fc09374f08edc5314a Mon Sep 17 00:00:00 2001 From: Pranit Date: Tue, 27 Oct 2026 11:12:53 -0400 Subject: [PATCH] Run MCP tools the model calls without loading them first A tool the model loads with capability_search or mcp_select_tool stays loaded only for that turn. When the model called it again in a later turn, or after a resume, the terminal app and fx ask failed the call with "Dynamic MCP tool not selected for this model step", and the model had to load the tool again before it could use it. ACP already loads such a call's current definition from the live catalog and sends the call through normal validation and permission review. The terminal app and fx ask now do the same. A name that no live server has still fails without reaching any server. --- src/core/app/app_agent_runtime.zig | 18 +++++ src/core/app/app_callbacks.zig | 6 ++ src/core/cli/cli_ask.zig | 6 ++ src/main.zig | 4 + tests/e2e/mcp-stdio.test.ts | 114 +++++++++++++++++++++++++++++ 5 files changed, 148 insertions(+) diff --git a/src/core/app/app_agent_runtime.zig b/src/core/app/app_agent_runtime.zig index 16778f8db..ff144dd84 100644 --- a/src/core/app/app_agent_runtime.zig +++ b/src/core/app/app_agent_runtime.zig @@ -708,6 +708,24 @@ pub fn Runtime(comptime App: type) type { return tool_runtime.validateToolCall(toolContext(app, ignored_list_entries, max_list_entries, max_read_file_bytes, max_read_file_lines, max_read_file_line_len, max_command_output_bytes, gateway_retry_count, gateway_chat_url), arena, call); } + /// The current definition of a live MCP tool the model called without + /// loading it this turn, or null when no permitted tool has that name. + pub fn resolveUnselectedMcpTool( + app: *App, + arena: Allocator, + name: []const u8, + ignored_list_entries: []const []const u8, + max_list_entries: usize, + max_read_file_bytes: usize, + max_read_file_lines: usize, + max_read_file_line_len: usize, + max_command_output_bytes: usize, + gateway_retry_count: usize, + gateway_chat_url: []const u8, + ) !?tool_mcp_runtime.SelectedTool { + return tool_runtime.resolveUnselectedMcpTool(toolContext(app, ignored_list_entries, max_list_entries, max_read_file_bytes, max_read_file_lines, max_read_file_line_len, max_command_output_bytes, gateway_retry_count, gateway_chat_url), arena, name); + } + pub fn checkToolAvailability( app: *App, arena: Allocator, diff --git a/src/core/app/app_callbacks.zig b/src/core/app/app_callbacks.zig index b745e49b9..f821a2f16 100644 --- a/src/core/app/app_callbacks.zig +++ b/src/core/app/app_callbacks.zig @@ -310,6 +310,7 @@ pub fn Bindings(comptime App: type) type { .append_static_context = agentAppendStaticContext, .validate_tool_call = agentValidateToolCall, .snapshot_mcp_definition = agentSnapshotMcpDefinition, + .resolve_unselected_mcp_tool = if (comptime @hasDecl(App, "resolveUnselectedMcpTool")) agentResolveUnselectedMcpTool else null, .prepare_skill_call = if (comptime @hasField(App, "skills") and @hasDecl(App, "toolRegistry")) agentPrepareSkillCall else null, .check_tool_availability = agentCheckToolAvailability, .request_tool_permission = agentRequestToolPermission, @@ -963,6 +964,11 @@ pub fn Bindings(comptime App: type) type { return .unavailable; } + fn agentResolveUnselectedMcpTool(ctx: *anyopaque, arena: Allocator, name: []const u8) !?tool_mcp_runtime.SelectedTool { + const app: *App = @ptrCast(@alignCast(ctx)); + return app.resolveUnselectedMcpTool(arena, name); + } + fn agentValidateToolCall(ctx: *anyopaque, arena: Allocator, call: ToolCall) !agent_runtime.ToolCallValidationResult { const app: *App = @ptrCast(@alignCast(ctx)); return app.validateToolCall(arena, call); diff --git a/src/core/cli/cli_ask.zig b/src/core/cli/cli_ask.zig index 78e530a84..64e57ffdb 100644 --- a/src/core/cli/cli_ask.zig +++ b/src/core/cli/cli_ask.zig @@ -1970,6 +1970,7 @@ fn agentRuntimeDeps(ctx: *AskContext) agent_runtime.AgentRuntimeDeps { .append_static_context = appendStaticContext, .validate_tool_call = validateToolCall, .snapshot_mcp_definition = snapshotMcpDefinition, + .resolve_unselected_mcp_tool = resolveUnselectedMcpTool, .prepare_skill_call = prepareSkillCall, .check_tool_availability = checkToolAvailability, .request_tool_permission = requestToolPermissionOutcomeWithRequest, @@ -2173,6 +2174,11 @@ fn snapshotMcpDefinition(raw_ctx: *anyopaque, arena: Allocator, name: []const u8 return tool_runtime.snapshotMcpDefinition(ctx.toolContext(), arena, name, known); } +fn resolveUnselectedMcpTool(raw_ctx: *anyopaque, arena: Allocator, name: []const u8) !?tool_mcp_runtime.SelectedTool { + const ctx: *AskContext = @ptrCast(@alignCast(raw_ctx)); + return tool_runtime.resolveUnselectedMcpTool(ctx.toolContext(), arena, name); +} + fn validateToolCall(raw_ctx: *anyopaque, arena: Allocator, call: ToolCall) !agent_runtime.ToolCallValidationResult { const ctx: *AskContext = @ptrCast(@alignCast(raw_ctx)); if (try ctx.cfg.mode_registry.toolPolicyDeniedJson(arena, ctx.deps.tool_set, ctx.mode_id, call.name)) |reason| { diff --git a/src/main.zig b/src/main.zig index 72947b9c1..96fe93cdf 100644 --- a/src/main.zig +++ b/src/main.zig @@ -1941,6 +1941,10 @@ const App = struct { return AgentAppRuntime.validateToolCall(self, arena, call, &ignored_list_entries, max_list_entries, max_read_file_bytes, max_read_file_lines, max_read_file_line_len, max_command_output_bytes, builtin_gateway.retry_count, builtin_gateway.defaultChatUrl()); } + pub fn resolveUnselectedMcpTool(self: *App, arena: Allocator, name: []const u8) !?tool_mcp_runtime.SelectedTool { + return AgentAppRuntime.resolveUnselectedMcpTool(self, arena, name, &ignored_list_entries, max_list_entries, max_read_file_bytes, max_read_file_lines, max_read_file_line_len, max_command_output_bytes, builtin_gateway.retry_count, builtin_gateway.defaultChatUrl()); + } + pub fn checkToolAvailability(self: *App, arena: Allocator, call: ToolCall) !?[]const u8 { return AgentAppRuntime.checkToolAvailability(self, arena, call, &ignored_list_entries, max_list_entries, max_read_file_bytes, max_read_file_lines, max_read_file_line_len, max_command_output_bytes, builtin_gateway.retry_count, builtin_gateway.defaultChatUrl()); } diff --git a/tests/e2e/mcp-stdio.test.ts b/tests/e2e/mcp-stdio.test.ts index 839dab2be..f98b028dd 100644 --- a/tests/e2e/mcp-stdio.test.ts +++ b/tests/e2e/mcp-stdio.test.ts @@ -1058,6 +1058,120 @@ describe("modern MCP stdio compatibility", () => { await expectFixtureProcessesExited(wire); }, 45_000); + test.skipIf(!tmuxAvailable())( + "the TUI runs a later turn's direct call to an MCP tool an earlier turn loaded", + async () => { + // The shell can ask, so it says so in every request's capabilities. + const root = createRoot("tui-later-turn-direct-call", MODERN_FIXTURE, { + expectedElicitation: "both", + }); + const stderrPath = join(root.root, "stderr.log"); + const activeGateway = startFakeGateway([ + fakeGatewayToolCall("select_mcp", "mcp_select_tool", { name: TOOL_NAME }), + fakeGatewayToolCall("first_call", TOOL_NAME, { text: "first" }), + fakeGatewayFinalText("FIRST_TURN_DONE"), + fakeGatewayToolCall("later_call", TOOL_NAME, { text: "later" }), + fakeGatewayFinalText("LATER_TURN_DONE"), + ], { + models: [{ id: MODEL, type: "language", tags: ["tool-use"] }], + }); + gateway = activeGateway; + tui = await TmuxSession.create({ + isolated: true, + cwd: root.workspace, + width: 140, + height: 40, + stderrPath, + env: fixtureEnv(root, activeGateway), + }); + + await tui.waitForComposer(15_000); + await tui.sendText("Load the fixture echo tool and call it."); + await tui.waitForText("FIRST_TURN_DONE", 20_000); + await tui.waitForComposer(10_000); + expect(toolResultText(activeGateway.requests[2]!.body, "first_call")).toContain(`${MODERN_RESULT}:first`); + await tui.sendText("Call the echo tool again."); + await tui.waitForText("LATER_TURN_DONE", 20_000); + await tui.waitForComposer(10_000); + + expect(activeGateway.requests).toHaveLength(5); + // The later turn starts without the tool: the model calls it directly. + const laterTools = (JSON.parse(activeGateway.requests[3]!.body) as { + tools?: Array<{ name: string }>; + }).tools?.map((tool) => tool.name) ?? []; + expect(laterTools).not.toContain(TOOL_NAME); + const laterResult = toolResultText(activeGateway.requests[4]!.body, "later_call"); + expect(laterResult).toContain(`${MODERN_RESULT}:later`); + expect(laterResult).not.toContain("not selected"); + const calls = readWire(root.wireLogPath).filter( + (entry) => entry.message.method === "tools/call", + ); + expect(calls.map((entry) => entry.message.params?.arguments)).toEqual([ + { text: "first" }, + { text: "later" }, + ]); + + // Each turn's tool group counts its MCP call as one, never as a command. + const scrollback = await tui.captureFullScrollback(); + expect(scrollback).toContain("1 MCP call"); + expect(scrollback).not.toMatch(/\b\d+ commands?\b/); + expect(scrollback).not.toContain("not selected"); + expect(readFileSync(stderrPath, "utf8")).toBe(""); + + await tui.kill(); + tui = null; + await expectFixtureProcessesExited(readWire(root.wireLogPath)); + }, + 60_000, + ); + + test("a resumed fx ask runs a direct call to an MCP tool an earlier turn loaded", async () => { + const root = createRoot("ask-later-turn-direct-call", MODERN_FIXTURE); + const firstGateway = startToolGateway("FIRST_ASK_DONE"); + gateway = firstGateway; + const first = await runFx( + ["ask", "--json", "--auto", "Load the fixture echo tool and call it."], + { cwd: root.workspace, env: fixtureEnv(root, firstGateway), timeoutMs: 20_000 }, + ); + expect(first.code).toBe(0); + const sessionId = JSON.parse(first.stdout).session_id; + expect(sessionId).toBeTruthy(); + firstGateway.stop(); + gateway = null; + + const missingTool = "mcp_fixture_missing"; + const laterGateway = startFakeGateway([ + fakeGatewayToolCall("missing_call", missingTool, { text: "nothing" }), + fakeGatewayToolCall("later_call", TOOL_NAME, { text: "later" }), + fakeGatewayFinalText("LATER_ASK_DONE"), + ], { + models: [{ id: MODEL, type: "language", tags: ["tool-use"] }], + }); + gateway = laterGateway; + const later = await runFx( + ["ask", "--json", "--auto", "--resume", sessionId, "Call the echo tool again."], + { cwd: root.workspace, env: fixtureEnv(root, laterGateway), timeoutMs: 20_000 }, + ); + expect(later.code).toBe(0); + expect(JSON.parse(later.stdout)).toMatchObject({ + session_id: sessionId, + output: expect.stringContaining("LATER_ASK_DONE"), + }); + expect(laterGateway.requests).toHaveLength(3); + // A name no live server has still fails without reaching any server. + expect(toolResultText(laterGateway.requests[1]!.body, "missing_call")).toContain(missingTool); + const laterResult = toolResultText(laterGateway.requests[2]!.body, "later_call"); + expect(laterResult).toContain(`${MODERN_RESULT}:later`); + expect(laterResult).not.toContain("not selected"); + const wire = readWire(root.wireLogPath); + const calls = wire.filter((entry) => entry.message.method === "tools/call"); + expect(calls.map((entry) => entry.message.params?.arguments)).toEqual([ + { text: "hello" }, + { text: "later" }, + ]); + await expectFixtureProcessesExited(wire); + }, 45_000); + for (const childMode of ["persistent"] as const) { const label = "persistent"; test(`${label} child's search never offers a tool a permission rule denies`, async () => {