mirror of
https://github.com/vercel-labs/fx.git
synced 2026-10-28 10:15:17 +08:00
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.
This commit is contained in:
5 files changed
+148
No files matched your search
@@ -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,
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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| {
|
||||
|
||||
@@ -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());
|
||||
}
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
Reference in new issue
Block a user