From 5c66a838629687285145c0679f263471c7818c36 Mon Sep 17 00:00:00 2001 From: Seth Date: Wed, 2 Sep 2026 14:01:47 -0400 Subject: [PATCH 1/7] fix: register ACP MCP tools as native callable tools via cpython proxy --- .gitignore | 1 + .../coding-agent/src/core/agent-session.ts | 14 +++++ .../coding-agent/src/core/mcp/mcp-manager.ts | 4 ++ .../coding-agent/src/core/tools/acp-mcp.ts | 57 +++++++++++++++++++ packages/coding-agent/src/core/tools/index.ts | 1 + 5 files changed, 77 insertions(+) create mode 100644 packages/coding-agent/src/core/tools/acp-mcp.ts diff --git a/.gitignore b/.gitignore index d8b22e91e6..3874875319 100644 --- a/.gitignore +++ b/.gitignore @@ -43,3 +43,4 @@ collect.sh __pycache__/ *.pyc +*.tgz diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 39dab65732..3f357c9025 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -271,6 +271,7 @@ import { createSyntheticSourceInfo, type SourceInfo } from "./source-info.js"; import { type BuildSystemPromptOptions, buildSystemPrompt } from "./system-prompt.js"; import { THINKING_LEVELS } from "./thinking-levels.js"; import { type BashOperations, createLocalBashOperations } from "./tools/bash.js"; +import { createAcpMcpToolDefinitions } from "./tools/acp-mcp.js"; import { createAllToolDefinitions } from "./tools/index.js"; import { IpythonKernelProvisioner } from "./tools/ipython.js"; import { createToolDefinitionFromAgentTool } from "./tools/tool-definition-wrapper.js"; @@ -1392,6 +1393,19 @@ export class AgentSession { activeToolNames: this.getActiveToolNames(), includeAllExtensionTools: true, }); + const acpServers = this._mcpManager?.getAcpServers() ?? []; + if (acpServers.length > 0 && this._ipythonKernelProvisioner) { + const mcpTools = createAcpMcpToolDefinitions(acpServers, this._ipythonKernelProvisioner); + for (const tool of mcpTools) { + const existing = this._customTools.findIndex((t) => t.name === tool.name); + if (existing >= 0) this._customTools[existing] = tool; + else this._customTools.push(tool); + } + this._refreshToolRegistry({ + activeToolNames: this.getActiveToolNames(), + includeAllExtensionTools: true, + }); + } this._baseSystemPrompt = this._rebuildSystemPrompt(this.getActiveToolNames()); this.agent.state.systemPrompt = this._baseSystemPrompt; } diff --git a/packages/coding-agent/src/core/mcp/mcp-manager.ts b/packages/coding-agent/src/core/mcp/mcp-manager.ts index 6031651078..885aac4811 100644 --- a/packages/coding-agent/src/core/mcp/mcp-manager.ts +++ b/packages/coding-agent/src/core/mcp/mcp-manager.ts @@ -219,6 +219,10 @@ export class McpManager { } /** Enabled persistent and session-scoped servers available through the generic kernel API. */ + getAcpServers(): AcpMcpServerConfig[] { + return [...this.acpServers.values()]; + } + getEnabledGenericServers(): string[] { const servers = Array.from(this.integrations.values()) .filter( diff --git a/packages/coding-agent/src/core/tools/acp-mcp.ts b/packages/coding-agent/src/core/tools/acp-mcp.ts new file mode 100644 index 0000000000..2c128d3cda --- /dev/null +++ b/packages/coding-agent/src/core/tools/acp-mcp.ts @@ -0,0 +1,57 @@ +import type { ToolDefinition } from "../extensions/types.js"; +import type { IpythonKernelProvisioner } from "./ipython.js"; +import type { AcpMcpServerConfig } from "../mcp/acp-mcp-types.js"; + +export function createAcpMcpToolDefinitions( + servers: readonly AcpMcpServerConfig[], + provisioner: IpythonKernelProvisioner, +): ToolDefinition[] { + const definitions: ToolDefinition[] = []; + for (const server of servers) { + const safeName = server.name.replace(/[^a-zA-Z0-9_-]/g, "_"); + const serverName = JSON.stringify(server.name); + + definitions.push({ + name: `mcp_list_tools_${safeName}`, + label: `list tools from ${server.name}`, + description: `List every tool the "${server.name}" MCP server exposes. ` + + `Call this first, then use mcp_call_${safeName} to invoke a specific tool.`, + parameters: { type: "object", properties: {}, required: [], additionalProperties: false } as any, + execute: async (_toolCallId, _params, signal, _onUpdate, _ctx) => { + const m = await provisioner.ensure(() => {}, signal); + const code = `import json; tools = await mcp.list_tools(${serverName}); print(json.dumps(tools, default=str))`; + const result = await m.execute(code, { signal }); + return { content: [{ type: "text" as const, text: result.stdout || result.stderr || "(empty)" }] }; + }, + }); + + definitions.push({ + name: `mcp_call_${safeName}`, + label: `call tool on ${server.name}`, + description: `Call a tool on the "${server.name}" MCP server. ` + + `Use mcp_list_tools_${safeName} first to discover available tool names and argument schemas.`, + parameters: { + type: "object", + properties: { + tool: { type: "string", description: `Tool name on "${server.name}".` }, + arguments: { type: "object", description: "JSON arguments for the tool.", additionalProperties: true }, + }, + required: ["tool", "arguments"], + additionalProperties: false, + } as any, + execute: async (_toolCallId, params, signal, _onUpdate, _ctx) => { + const { tool, arguments: args } = params as { tool: string; arguments: Record }; + const argsJson = JSON.stringify(args ?? {}); + const m = await provisioner.ensure(() => {}, signal); + const code = `import json; result = await mcp.call_tool(${serverName}, ${JSON.stringify(tool)}, ${argsJson}); print(json.dumps(result, default=str))`; + try { + const result = await m.execute(code, { signal }); + return { content: [{ type: "text" as const, text: result.stdout || result.stderr || "(empty)" }] }; + } catch (error) { + return { content: [{ type: "text" as const, text: `Error: ${String(error)}` }] }; + } + }, + }); + } + return definitions; +} diff --git a/packages/coding-agent/src/core/tools/index.ts b/packages/coding-agent/src/core/tools/index.ts index f846d0bf68..5f3bc8f3f1 100644 --- a/packages/coding-agent/src/core/tools/index.ts +++ b/packages/coding-agent/src/core/tools/index.ts @@ -37,6 +37,7 @@ export { truncateTail, } from "./truncate.js"; +export { createAcpMcpToolDefinitions } from "./acp-mcp.js"; import type { AgentTool } from "@earendil-works/pi-agent-core"; import type { ToolDefinition } from "../extensions/types.js"; import { createIpythonToolDefinition, type IpythonToolOptions } from "./ipython.js"; From 52104103d083e7ab02bdfc2e57484597631d7b12 Mon Sep 17 00:00:00 2001 From: Seth Date: Wed, 2 Sep 2026 14:11:51 -0400 Subject: [PATCH 2/7] fix: add MCP proxy tool names to allowlist so model can see them --- packages/coding-agent/src/core/agent-session.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 3f357c9025..b40091cc62 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -1389,23 +1389,22 @@ export class AgentSession { } private _rebuildRuntimeForAcpMcpServers(): void { - this._buildRuntime({ - activeToolNames: this.getActiveToolNames(), - includeAllExtensionTools: true, - }); const acpServers = this._mcpManager?.getAcpServers() ?? []; + const activeToolNames = this.getActiveToolNames(); if (acpServers.length > 0 && this._ipythonKernelProvisioner) { const mcpTools = createAcpMcpToolDefinitions(acpServers, this._ipythonKernelProvisioner); for (const tool of mcpTools) { const existing = this._customTools.findIndex((t) => t.name === tool.name); if (existing >= 0) this._customTools[existing] = tool; else this._customTools.push(tool); + if (!activeToolNames.includes(tool.name)) activeToolNames.push(tool.name); + this._allowedToolNames?.add(tool.name); } - this._refreshToolRegistry({ - activeToolNames: this.getActiveToolNames(), - includeAllExtensionTools: true, - }); } + this._buildRuntime({ + activeToolNames, + includeAllExtensionTools: true, + }); this._baseSystemPrompt = this._rebuildSystemPrompt(this.getActiveToolNames()); this.agent.state.systemPrompt = this._baseSystemPrompt; } From 4c8872196dcb2138d56e35b8b3956c24be2db20e Mon Sep 17 00:00:00 2001 From: Seth Date: Wed, 2 Sep 2026 15:07:13 -0400 Subject: [PATCH 3/7] fix: add missing details field to ACP MCP tool execute results --- .../coding-agent/src/core/tools/acp-mcp.ts | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/coding-agent/src/core/tools/acp-mcp.ts b/packages/coding-agent/src/core/tools/acp-mcp.ts index 2c128d3cda..180b3cac68 100644 --- a/packages/coding-agent/src/core/tools/acp-mcp.ts +++ b/packages/coding-agent/src/core/tools/acp-mcp.ts @@ -1,6 +1,6 @@ import type { ToolDefinition } from "../extensions/types.js"; -import type { IpythonKernelProvisioner } from "./ipython.js"; import type { AcpMcpServerConfig } from "../mcp/acp-mcp-types.js"; +import type { IpythonKernelProvisioner } from "./ipython.js"; export function createAcpMcpToolDefinitions( servers: readonly AcpMcpServerConfig[], @@ -14,21 +14,26 @@ export function createAcpMcpToolDefinitions( definitions.push({ name: `mcp_list_tools_${safeName}`, label: `list tools from ${server.name}`, - description: `List every tool the "${server.name}" MCP server exposes. ` + + description: + `List every tool the "${server.name}" MCP server exposes. ` + `Call this first, then use mcp_call_${safeName} to invoke a specific tool.`, parameters: { type: "object", properties: {}, required: [], additionalProperties: false } as any, execute: async (_toolCallId, _params, signal, _onUpdate, _ctx) => { const m = await provisioner.ensure(() => {}, signal); const code = `import json; tools = await mcp.list_tools(${serverName}); print(json.dumps(tools, default=str))`; const result = await m.execute(code, { signal }); - return { content: [{ type: "text" as const, text: result.stdout || result.stderr || "(empty)" }] }; + return { + content: [{ type: "text" as const, text: result.stdout || result.stderr || "(empty)" }], + details: {}, + }; }, }); definitions.push({ name: `mcp_call_${safeName}`, label: `call tool on ${server.name}`, - description: `Call a tool on the "${server.name}" MCP server. ` + + description: + `Call a tool on the "${server.name}" MCP server. ` + `Use mcp_list_tools_${safeName} first to discover available tool names and argument schemas.`, parameters: { type: "object", @@ -46,9 +51,12 @@ export function createAcpMcpToolDefinitions( const code = `import json; result = await mcp.call_tool(${serverName}, ${JSON.stringify(tool)}, ${argsJson}); print(json.dumps(result, default=str))`; try { const result = await m.execute(code, { signal }); - return { content: [{ type: "text" as const, text: result.stdout || result.stderr || "(empty)" }] }; + return { + content: [{ type: "text" as const, text: result.stdout || result.stderr || "(empty)" }], + details: {}, + }; } catch (error) { - return { content: [{ type: "text" as const, text: `Error: ${String(error)}` }] }; + return { content: [{ type: "text" as const, text: `Error: ${String(error)}` }], details: {} }; } }, }); From 41985b989f65730b02754dd9fa8da1ce8b129dc3 Mon Sep 17 00:00:00 2001 From: Seth Date: Wed, 2 Sep 2026 16:44:17 -0400 Subject: [PATCH 4/7] fix(coding-agent): harden ACP MCP proxy lifecycle Fixes #2002 --- .gitignore | 1 - .../.changes/acp-mcp-native-tools.md | 1 + .../coding-agent/src/core/agent-session.ts | 65 +++++++--- .../coding-agent/src/core/mcp/mcp-manager.ts | 15 ++- .../coding-agent/src/core/tools/acp-mcp.ts | 87 +++++++++---- packages/coding-agent/src/core/tools/index.ts | 2 +- .../test/agent-session-services.test.ts | 6 +- .../2002-acp-mcp-native-tools.test.ts | 121 ++++++++++++++++++ 8 files changed, 242 insertions(+), 56 deletions(-) create mode 100644 packages/coding-agent/.changes/acp-mcp-native-tools.md create mode 100644 packages/coding-agent/test/suite/regressions/2002-acp-mcp-native-tools.test.ts diff --git a/.gitignore b/.gitignore index 3874875319..d8b22e91e6 100644 --- a/.gitignore +++ b/.gitignore @@ -43,4 +43,3 @@ collect.sh __pycache__/ *.pyc -*.tgz diff --git a/packages/coding-agent/.changes/acp-mcp-native-tools.md b/packages/coding-agent/.changes/acp-mcp-native-tools.md new file mode 100644 index 0000000000..a119e226f4 --- /dev/null +++ b/packages/coding-agent/.changes/acp-mcp-native-tools.md @@ -0,0 +1 @@ +- Added native callable tools for MCP servers supplied by ACP clients. ([#2002](https://github.com/PrimeIntellect-ai/prime-agent/pull/2002)) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index b40091cc62..cadee36722 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -270,8 +270,8 @@ import { import { createSyntheticSourceInfo, type SourceInfo } from "./source-info.js"; import { type BuildSystemPromptOptions, buildSystemPrompt } from "./system-prompt.js"; import { THINKING_LEVELS } from "./thinking-levels.js"; +import { acpMcpToolNames, createAcpMcpToolDefinitions } from "./tools/acp-mcp.js"; import { type BashOperations, createLocalBashOperations } from "./tools/bash.js"; -import { createAcpMcpToolDefinitions } from "./tools/acp-mcp.js"; import { createAllToolDefinitions } from "./tools/index.js"; import { IpythonKernelProvisioner } from "./tools/ipython.js"; import { createToolDefinitionFromAgentTool } from "./tools/tool-definition-wrapper.js"; @@ -1130,6 +1130,7 @@ export class AgentSession { private _resourceLoader: ResourceLoader; private _customTools: ToolDefinition[]; + private _acpMcpTools: ToolDefinition[] = []; private _baseToolDefinitions: Map = new Map(); private _cwd: string; private _agentDir?: string; @@ -1342,6 +1343,7 @@ export class AgentSession { if (servers.length > 0) throw new Error("MCP is unavailable in this session"); return; } + this._assertAcpMcpToolNamesAvailable(acpMcpToolNames(servers)); if (!this._mcpManager.replaceAcpServers(servers, ownerId)) return; this._rebuildRuntimeForAcpMcpServers(); } @@ -1349,8 +1351,11 @@ export class AgentSession { async releaseAcpMcpServers(ownerId: string, serverNames: readonly string[]): Promise { if (!this._mcpManager?.canReleaseAcpServers(ownerId)) return; if (this._mcpManager.replaceAcpServers([], ownerId)) { - // Host MCP handlers read this manager dynamically, so credentials disappear - // before the kernel-side transport is closed. + const removedToolNames = new Set(this._acpMcpTools.map((tool) => tool.name)); + const activeToolNames = this.getActiveToolNames().filter((name) => !removedToolNames.has(name)); + for (const name of removedToolNames) this._allowedToolNames?.delete(name); + this._acpMcpTools = []; + this._refreshToolRegistry({ activeToolNames, includeAllExtensionTools: true }); this._baseSystemPrompt = this._rebuildSystemPrompt(this.getActiveToolNames()); this.agent.state.systemPrompt = this._baseSystemPrompt; } @@ -1388,19 +1393,25 @@ export class AgentSession { } } - private _rebuildRuntimeForAcpMcpServers(): void { - const acpServers = this._mcpManager?.getAcpServers() ?? []; - const activeToolNames = this.getActiveToolNames(); - if (acpServers.length > 0 && this._ipythonKernelProvisioner) { - const mcpTools = createAcpMcpToolDefinitions(acpServers, this._ipythonKernelProvisioner); - for (const tool of mcpTools) { - const existing = this._customTools.findIndex((t) => t.name === tool.name); - if (existing >= 0) this._customTools[existing] = tool; - else this._customTools.push(tool); - if (!activeToolNames.includes(tool.name)) activeToolNames.push(tool.name); - this._allowedToolNames?.add(tool.name); + private _assertAcpMcpToolNamesAvailable(names: readonly string[]): void { + const occupiedNames = new Set([ + ...this._baseToolDefinitions.keys(), + ...this._customTools.map((tool) => tool.name), + ...this._extensionRunner.getAllRegisteredTools().map((tool) => tool.definition.name), + ]); + for (const name of names) { + if (occupiedNames.has(name)) { + throw new Error(`ACP MCP tool name conflicts with an existing tool: ${name}`); } } + } + + private _rebuildRuntimeForAcpMcpServers(): void { + const previousToolNames = new Set(this._acpMcpTools.map((tool) => tool.name)); + const nextToolNames = acpMcpToolNames(this._mcpManager?.getAcpServers() ?? []); + this._assertAcpMcpToolNamesAvailable(nextToolNames); + const activeToolNames = this.getActiveToolNames().filter((name) => !previousToolNames.has(name)); + activeToolNames.push(...nextToolNames); this._buildRuntime({ activeToolNames, includeAllExtensionTools: true, @@ -4404,7 +4415,7 @@ export class AgentSession { rlmDepth: this._rlmDepth, rlmParentAgent: this._rlmParentAgent, harnessState: this._loadMergedHarnessState(), - genericMcpServers: this._mcpManager?.getEnabledGenericServers(), + genericMcpServers: this._mcpManager?.getEnabledPersistentGenericServers(), }; return buildSystemPrompt(this._baseSystemPromptOptions); } @@ -8998,14 +9009,16 @@ export class AgentSession { const previousActiveToolNames = this.getActiveToolNames(); const allowedToolNames = this._allowedToolNames; const registeredTools = this._extensionRunner.getAllRegisteredTools(); + const sdkToolEntry = (definition: ToolDefinition) => ({ + definition, + sourceInfo: createSyntheticSourceInfo(``, { + source: "sdk" as const, + }), + }); const allCustomTools = [ ...registeredTools, - ...this._customTools.map((definition) => ({ - definition, - sourceInfo: createSyntheticSourceInfo(``, { - source: "sdk", - }), - })), + ...this._customTools.map(sdkToolEntry), + ...this._acpMcpTools.map(sdkToolEntry), ]; const isAllowedTool = (name: string): boolean => !allowedToolNames || allowedToolNames.has(name); const allowedCustomTools = allCustomTools.filter((tool) => isAllowedTool(tool.definition.name)); @@ -9167,6 +9180,16 @@ export class AgentSession { this._bindExtensionCore(this._extensionRunner); this._applyExtensionBindings(this._extensionRunner); + const previousAcpMcpToolNames = new Set(this._acpMcpTools.map((tool) => tool.name)); + const acpServers = this._mcpManager?.getAcpServers() ?? []; + const acpMcpTools = this._ipythonKernelProvisioner + ? createAcpMcpToolDefinitions(acpServers, this._ipythonKernelProvisioner) + : []; + this._assertAcpMcpToolNamesAvailable(acpMcpTools.map((tool) => tool.name)); + for (const name of previousAcpMcpToolNames) this._allowedToolNames?.delete(name); + for (const tool of acpMcpTools) this._allowedToolNames?.add(tool.name); + this._acpMcpTools = acpMcpTools; + const defaultActiveToolNames = this._baseToolsOverride ? Object.keys(this._baseToolsOverride) : ["ipython"]; const baseActiveToolNames = [...(options.activeToolNames ?? defaultActiveToolNames)]; if (this._goalState.status === "active" && this._includeGoals) { diff --git a/packages/coding-agent/src/core/mcp/mcp-manager.ts b/packages/coding-agent/src/core/mcp/mcp-manager.ts index 885aac4811..eac915c915 100644 --- a/packages/coding-agent/src/core/mcp/mcp-manager.ts +++ b/packages/coding-agent/src/core/mcp/mcp-manager.ts @@ -223,8 +223,8 @@ export class McpManager { return [...this.acpServers.values()]; } - getEnabledGenericServers(): string[] { - const servers = Array.from(this.integrations.values()) + getEnabledPersistentGenericServers(): string[] { + return Array.from(this.integrations.values()) .filter( (integration) => integration.userDeclared && @@ -232,9 +232,14 @@ export class McpManager { !getCatalogEntry(integration.server) && this.isAuthed(integration), ) - .map((integration) => integration.server); - for (const server of this.acpServers.keys()) servers.push(server); - return [...new Set(servers)].sort((left, right) => left.localeCompare(right)); + .map((integration) => integration.server) + .sort((left, right) => left.localeCompare(right)); + } + + getEnabledGenericServers(): string[] { + return [...new Set([...this.getEnabledPersistentGenericServers(), ...this.acpServers.keys()])].sort( + (left, right) => left.localeCompare(right), + ); } /** Status for the /mcp list command. */ diff --git a/packages/coding-agent/src/core/tools/acp-mcp.ts b/packages/coding-agent/src/core/tools/acp-mcp.ts index 180b3cac68..8298b050a7 100644 --- a/packages/coding-agent/src/core/tools/acp-mcp.ts +++ b/packages/coding-agent/src/core/tools/acp-mcp.ts @@ -1,40 +1,81 @@ import type { ToolDefinition } from "../extensions/types.js"; +import type { ExecuteResult } from "../kernel/index.js"; import type { AcpMcpServerConfig } from "../mcp/acp-mcp-types.js"; import type { IpythonKernelProvisioner } from "./ipython.js"; +const ACP_MCP_SERVER_NAME_PATTERN = /^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$/; + +export function acpMcpToolNames(servers: readonly AcpMcpServerConfig[]): string[] { + const names: string[] = []; + const seenServers = new Set(); + for (const server of servers) { + if (!ACP_MCP_SERVER_NAME_PATTERN.test(server.name)) { + throw new Error(`Invalid ACP MCP server name: ${server.name}`); + } + if (seenServers.has(server.name)) { + throw new Error(`Duplicate ACP MCP server: ${server.name}`); + } + seenServers.add(server.name); + names.push(`mcp_list_tools_${server.name}`, `mcp_call_${server.name}`); + } + return names; +} + +function executionResult(result: ExecuteResult) { + let text = result.stdout; + if (result.stderr) text += `${text ? "\n" : ""}${result.stderr}`; + if (result.result) text += `${text ? "\n" : ""}${result.result}`; + if (result.error) text += `${text ? "\n" : ""}${result.error.traceback.join("\n")}`; + if (result.status !== "ok") { + throw new Error(text || `MCP kernel execution ${result.status}`); + } + return { + content: [{ type: "text" as const, text: text || "(empty)" }], + details: { + durationMs: result.durationMs, + status: result.status, + stdout: result.stdout, + stderr: result.stderr, + result: result.result, + }, + }; +} + +async function executeMcpCode(provisioner: IpythonKernelProvisioner, code: string, signal: AbortSignal | undefined) { + const manager = await provisioner.ensure(() => {}, signal); + return executionResult(await manager.execute(code, { signal })); +} + export function createAcpMcpToolDefinitions( servers: readonly AcpMcpServerConfig[], provisioner: IpythonKernelProvisioner, ): ToolDefinition[] { + const names = acpMcpToolNames(servers); const definitions: ToolDefinition[] = []; - for (const server of servers) { - const safeName = server.name.replace(/[^a-zA-Z0-9_-]/g, "_"); + for (const [index, server] of servers.entries()) { + const listToolName = names[index * 2]!; + const callToolName = names[index * 2 + 1]!; const serverName = JSON.stringify(server.name); definitions.push({ - name: `mcp_list_tools_${safeName}`, + name: listToolName, label: `list tools from ${server.name}`, description: `List every tool the "${server.name}" MCP server exposes. ` + - `Call this first, then use mcp_call_${safeName} to invoke a specific tool.`, - parameters: { type: "object", properties: {}, required: [], additionalProperties: false } as any, + `Call this first, then use ${callToolName} to invoke a specific tool.`, + parameters: { type: "object", properties: {}, required: [], additionalProperties: false }, execute: async (_toolCallId, _params, signal, _onUpdate, _ctx) => { - const m = await provisioner.ensure(() => {}, signal); - const code = `import json; tools = await mcp.list_tools(${serverName}); print(json.dumps(tools, default=str))`; - const result = await m.execute(code, { signal }); - return { - content: [{ type: "text" as const, text: result.stdout || result.stderr || "(empty)" }], - details: {}, - }; + const code = `print(__import__("json").dumps(await mcp.list_tools(${serverName}), default=str))`; + return executeMcpCode(provisioner, code, signal); }, }); definitions.push({ - name: `mcp_call_${safeName}`, + name: callToolName, label: `call tool on ${server.name}`, description: `Call a tool on the "${server.name}" MCP server. ` + - `Use mcp_list_tools_${safeName} first to discover available tool names and argument schemas.`, + `Use ${listToolName} first to discover available tool names and argument schemas.`, parameters: { type: "object", properties: { @@ -43,21 +84,13 @@ export function createAcpMcpToolDefinitions( }, required: ["tool", "arguments"], additionalProperties: false, - } as any, + }, execute: async (_toolCallId, params, signal, _onUpdate, _ctx) => { const { tool, arguments: args } = params as { tool: string; arguments: Record }; - const argsJson = JSON.stringify(args ?? {}); - const m = await provisioner.ensure(() => {}, signal); - const code = `import json; result = await mcp.call_tool(${serverName}, ${JSON.stringify(tool)}, ${argsJson}); print(json.dumps(result, default=str))`; - try { - const result = await m.execute(code, { signal }); - return { - content: [{ type: "text" as const, text: result.stdout || result.stderr || "(empty)" }], - details: {}, - }; - } catch (error) { - return { content: [{ type: "text" as const, text: `Error: ${String(error)}` }], details: {} }; - } + const code = + `print(__import__("json").dumps(await mcp.call_tool(${serverName}, ${JSON.stringify(tool)}, ` + + `__import__("json").loads(${JSON.stringify(JSON.stringify(args ?? {}))})), default=str))`; + return executeMcpCode(provisioner, code, signal); }, }); } diff --git a/packages/coding-agent/src/core/tools/index.ts b/packages/coding-agent/src/core/tools/index.ts index 5f3bc8f3f1..7465c8edc3 100644 --- a/packages/coding-agent/src/core/tools/index.ts +++ b/packages/coding-agent/src/core/tools/index.ts @@ -1,3 +1,4 @@ +export { acpMcpToolNames, createAcpMcpToolDefinitions } from "./acp-mcp.js"; export { type BashOperations, type BashSpawnContext, @@ -37,7 +38,6 @@ export { truncateTail, } from "./truncate.js"; -export { createAcpMcpToolDefinitions } from "./acp-mcp.js"; import type { AgentTool } from "@earendil-works/pi-agent-core"; import type { ToolDefinition } from "../extensions/types.js"; import { createIpythonToolDefinition, type IpythonToolOptions } from "./ipython.js"; diff --git a/packages/coding-agent/test/agent-session-services.test.ts b/packages/coding-agent/test/agent-session-services.test.ts index 7c1eec6c7c..ca25424d6d 100644 --- a/packages/coding-agent/test/agent-session-services.test.ts +++ b/packages/coding-agent/test/agent-session-services.test.ts @@ -186,7 +186,9 @@ describe("createAgentSessionFromServices", () => { ], "owner-a", ); - expect(session.systemPrompt).toContain("Enabled generic MCP servers: `filesystem`, `task`, `zebra`."); + expect(session.systemPrompt).toContain("Enabled generic MCP servers: `filesystem`, `zebra`."); + expect(session.systemPrompt).not.toContain('await mcp.list_tools("task")'); + expect(session.getActiveToolNames()).toEqual(expect.arrayContaining(["mcp_list_tools_task", "mcp_call_task"])); expect(session.systemPrompt).not.toContain("task-secret"); rebuildRuntime.mockClear(); const waitForIdle = vi.spyOn(session.agent, "waitForIdle"); @@ -202,6 +204,8 @@ describe("createAgentSessionFromServices", () => { expect(execute.mock.calls[0]?.[0]).toContain("await _prime_mcp.reload(_prime_mcp_name)"); expect(execute.mock.calls[0]?.[0]).toContain('["task"]'); expect(session.systemPrompt).toContain("Enabled generic MCP servers: `filesystem`, `zebra`."); + expect(session.getAllTools().map((tool) => tool.name)).not.toContain("mcp_call_task"); + expect(session.getActiveToolNames()).not.toContain("mcp_call_task"); settingsManager.setGlobalMcpServer("added", { type: "stdio", command: "new-secret" }); settingsManager.removeGlobalMcpServer("filesystem"); diff --git a/packages/coding-agent/test/suite/regressions/2002-acp-mcp-native-tools.test.ts b/packages/coding-agent/test/suite/regressions/2002-acp-mcp-native-tools.test.ts new file mode 100644 index 0000000000..151f03f33c --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/2002-acp-mcp-native-tools.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it, vi } from "vitest"; +import type { ExtensionContext, ToolDefinition } from "../../../src/core/extensions/types.js"; +import type { ExecuteResult } from "../../../src/core/kernel/index.js"; +import { McpManager } from "../../../src/core/mcp/mcp-manager.js"; +import { acpMcpToolNames, createAcpMcpToolDefinitions } from "../../../src/core/tools/acp-mcp.js"; +import type { IpythonKernelProvisioner } from "../../../src/core/tools/ipython.js"; +import { createHarness } from "../harness.js"; + +function provisionerWith(result: ExecuteResult) { + const execute = vi.fn(async (_code: string) => result); + const ensure = vi.fn(async () => ({ execute })); + return { + execute, + provisioner: { ensure } as unknown as IpythonKernelProvisioner, + }; +} + +function httpServer(name: string) { + return { name, type: "http" as const, url: `https://${name}.example/mcp`, headers: {} }; +} + +function requireTool(tool: ToolDefinition | undefined): ToolDefinition { + if (!tool) throw new Error("Expected ACP MCP tool definition"); + return tool; +} + +const context = {} as ExtensionContext; + +describe("PR 2002 ACP MCP native tools", () => { + it("decodes JSON arguments in Python without polluting the persistent namespace", async () => { + const { execute, provisioner } = provisionerWith({ + stdout: '{"ok": true}\n', + stderr: "", + status: "ok", + durationMs: 1, + }); + const [, callTool] = createAcpMcpToolDefinitions([httpServer("task")], provisioner); + + const result = await requireTool(callTool).execute( + "call-1", + { tool: "configure", arguments: { enabled: true, value: null } }, + undefined, + undefined, + context, + ); + + expect(result.details).toMatchObject({ status: "ok" }); + expect(result.content).toEqual([{ type: "text", text: '{"ok": true}\n' }]); + const code = execute.mock.calls[0]?.[0] ?? ""; + expect(code).toContain('__import__("json").loads("{\\"enabled\\":true,\\"value\\":null}")'); + expect(code).not.toMatch(/(?:tools|result)\s*=/); + }); + + it("reports failed and thrown kernel executions as tool errors", async () => { + const failed = provisionerWith({ + stdout: "", + stderr: "cell failed", + status: "error", + error: { ename: "RuntimeError", evalue: "boom", traceback: ["traceback line"] }, + durationMs: 2, + }); + const [listTool] = createAcpMcpToolDefinitions([httpServer("task")], failed.provisioner); + await expect(requireTool(listTool).execute("call-2", {}, undefined, undefined, context)).rejects.toThrow( + "traceback line", + ); + + const ensure = vi.fn(async () => { + throw new Error("kernel unavailable"); + }); + const [throwingTool] = createAcpMcpToolDefinitions([httpServer("task")], { + ensure, + } as unknown as IpythonKernelProvisioner); + await expect(requireTool(throwingTool).execute("call-3", {}, undefined, undefined, context)).rejects.toThrow( + "kernel unavailable", + ); + }); + + it("rejects invalid or duplicate server names before generating ambiguous tools", () => { + expect(() => acpMcpToolNames([httpServer("foo.bar")])).toThrow("Invalid ACP MCP server name"); + expect(() => acpMcpToolNames([httpServer("task"), httpServer("task")])).toThrow("Duplicate ACP MCP server"); + }); + + it("rebinds proxies on reload, removes them on release, and rejects custom-tool collisions", async () => { + const harness = await createHarness(); + const manager = new McpManager({ authStorage: harness.authStorage }); + Reflect.set(harness.session, "_mcpManager", manager); + try { + harness.session.replaceAcpMcpServers([httpServer("task")], "owner-a"); + const toolNames = harness.session.getAllTools().map((tool) => tool.name); + expect(toolNames).toContain("mcp_list_tools_task"); + expect(toolNames).toContain("mcp_call_task"); + expect(harness.session.getActiveToolNames()).toContain("mcp_call_task"); + expect(harness.session.systemPrompt).not.toContain('await mcp.list_tools("task")'); + + const beforeReload = Reflect.get(harness.session, "_toolDefinitions") as Map; + const originalCallTool = beforeReload.get("mcp_call_task"); + await harness.session.reload(); + const afterReload = Reflect.get(harness.session, "_toolDefinitions") as Map; + expect(afterReload.get("mcp_call_task")).not.toBe(originalCallTool); + + await harness.session.releaseAcpMcpServers("owner-a", ["task"]); + expect(harness.session.getAllTools().map((tool) => tool.name)).not.toContain("mcp_call_task"); + expect(harness.session.getActiveToolNames()).not.toContain("mcp_call_task"); + + const customTools = Reflect.get(harness.session, "_customTools") as ToolDefinition[]; + customTools.push({ + name: "mcp_call_task", + label: "existing", + description: "existing custom tool", + parameters: { type: "object", properties: {} }, + execute: async () => ({ content: [{ type: "text", text: "existing" }], details: {} }), + }); + expect(() => harness.session.replaceAcpMcpServers([httpServer("task")], "owner-b")).toThrow( + "conflicts with an existing tool", + ); + expect(manager.getAcpServers()).toEqual([]); + } finally { + harness.cleanup(); + } + }); +}); From 2a4574827e7f2a9b8cff7a8d6130b6f2d9eea923 Mon Sep 17 00:00:00 2001 From: Seth Date: Wed, 2 Sep 2026 16:51:06 -0400 Subject: [PATCH 5/7] fix(coding-agent): reject MCP without cpython Fixes #2002 --- packages/coding-agent/src/core/agent-session.ts | 6 ++++++ .../regressions/2002-acp-mcp-native-tools.test.ts | 14 ++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index cadee36722..179d5423b8 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -1343,6 +1343,9 @@ export class AgentSession { if (servers.length > 0) throw new Error("MCP is unavailable in this session"); return; } + if (servers.length > 0 && !this._ipythonKernelProvisioner) { + throw new Error("ACP MCP servers require the built-in cpython tool"); + } this._assertAcpMcpToolNamesAvailable(acpMcpToolNames(servers)); if (!this._mcpManager.replaceAcpServers(servers, ownerId)) return; this._rebuildRuntimeForAcpMcpServers(); @@ -9182,6 +9185,9 @@ export class AgentSession { const previousAcpMcpToolNames = new Set(this._acpMcpTools.map((tool) => tool.name)); const acpServers = this._mcpManager?.getAcpServers() ?? []; + if (acpServers.length > 0 && !this._ipythonKernelProvisioner) { + throw new Error("ACP MCP servers require the built-in cpython tool"); + } const acpMcpTools = this._ipythonKernelProvisioner ? createAcpMcpToolDefinitions(acpServers, this._ipythonKernelProvisioner) : []; diff --git a/packages/coding-agent/test/suite/regressions/2002-acp-mcp-native-tools.test.ts b/packages/coding-agent/test/suite/regressions/2002-acp-mcp-native-tools.test.ts index 151f03f33c..862231173d 100644 --- a/packages/coding-agent/test/suite/regressions/2002-acp-mcp-native-tools.test.ts +++ b/packages/coding-agent/test/suite/regressions/2002-acp-mcp-native-tools.test.ts @@ -80,6 +80,20 @@ describe("PR 2002 ACP MCP native tools", () => { expect(() => acpMcpToolNames([httpServer("task"), httpServer("task")])).toThrow("Duplicate ACP MCP server"); }); + it("rejects ACP MCP servers when the built-in cpython runtime is unavailable", async () => { + const harness = await createHarness({ tools: [] }); + const manager = new McpManager({ authStorage: harness.authStorage }); + Reflect.set(harness.session, "_mcpManager", manager); + try { + expect(() => harness.session.replaceAcpMcpServers([httpServer("task")], "owner-a")).toThrow( + "require the built-in cpython tool", + ); + expect(manager.getAcpServers()).toEqual([]); + } finally { + harness.cleanup(); + } + }); + it("rebinds proxies on reload, removes them on release, and rejects custom-tool collisions", async () => { const harness = await createHarness(); const manager = new McpManager({ authStorage: harness.authStorage }); From 36de03eea9ae839bc455e481f8afad049ea07123 Mon Sep 17 00:00:00 2001 From: Seth Date: Wed, 2 Sep 2026 16:53:57 -0400 Subject: [PATCH 6/7] refactor(coding-agent): narrow generic MCP accessor Fixes #2002 --- packages/coding-agent/src/core/mcp/mcp-manager.ts | 9 ++------- packages/coding-agent/test/mcp-manager.test.ts | 6 +++--- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/coding-agent/src/core/mcp/mcp-manager.ts b/packages/coding-agent/src/core/mcp/mcp-manager.ts index eac915c915..b924b3c35e 100644 --- a/packages/coding-agent/src/core/mcp/mcp-manager.ts +++ b/packages/coding-agent/src/core/mcp/mcp-manager.ts @@ -218,11 +218,12 @@ export class McpManager { return handlers; } - /** Enabled persistent and session-scoped servers available through the generic kernel API. */ + /** Session-scoped servers supplied by the active ACP client. */ getAcpServers(): AcpMcpServerConfig[] { return [...this.acpServers.values()]; } + /** Enabled user-declared servers available through the generic kernel API. */ getEnabledPersistentGenericServers(): string[] { return Array.from(this.integrations.values()) .filter( @@ -236,12 +237,6 @@ export class McpManager { .sort((left, right) => left.localeCompare(right)); } - getEnabledGenericServers(): string[] { - return [...new Set([...this.getEnabledPersistentGenericServers(), ...this.acpServers.keys()])].sort( - (left, right) => left.localeCompare(right), - ); - } - /** Status for the /mcp list command. */ listStatus(): Array<{ server: string; label: string; enabled: boolean; usesOAuth: boolean }> { return Array.from(this.integrations.values()).map((integration) => ({ diff --git a/packages/coding-agent/test/mcp-manager.test.ts b/packages/coding-agent/test/mcp-manager.test.ts index fc3589f4fa..fdc83c64b9 100644 --- a/packages/coding-agent/test/mcp-manager.test.ts +++ b/packages/coding-agent/test/mcp-manager.test.ts @@ -145,7 +145,7 @@ describe("McpManager", () => { }); expect(manager.listStatus().find((s) => s.server === "remote")?.enabled).toBe(false); expect(manager.listStatus().find((s) => s.server === "unbound")?.enabled).toBe(false); - expect(manager.getEnabledGenericServers()).toEqual([]); + expect(manager.getEnabledPersistentGenericServers()).toEqual([]); }); it("honors a bearer-token env var for user-declared servers", () => { @@ -175,7 +175,7 @@ describe("McpManager", () => { }), }); - expect(manager.getEnabledGenericServers()).toEqual(["alpha", "zebra"]); + expect(manager.getEnabledPersistentGenericServers()).toEqual(["alpha", "zebra"]); }); it("picks up mcpServers added after construction on refresh()", () => { @@ -267,7 +267,7 @@ describe("McpManager", () => { credentialSource: "acp", }); await expect(handlers["mcp.refresh"]({ server: "task" })).rejects.toThrow("does not use host OAuth"); - expect(manager.getEnabledGenericServers()).toContain("task"); + expect(manager.getAcpServers().map((server) => server.name)).toContain("task"); expect(manager.replaceAcpServers([], "owner-b")).toBe(false); expect(() => From 2c006d475cb5907950751b99a04809af52529534 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Thu, 3 Sep 2026 11:26:54 +0200 Subject: [PATCH 7/7] fix(coding-agent): cap ACP MCP server names so composed tool names fit provider limits --- packages/coding-agent/src/core/tools/acp-mcp.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/coding-agent/src/core/tools/acp-mcp.ts b/packages/coding-agent/src/core/tools/acp-mcp.ts index 8298b050a7..d8932a7ad3 100644 --- a/packages/coding-agent/src/core/tools/acp-mcp.ts +++ b/packages/coding-agent/src/core/tools/acp-mcp.ts @@ -3,7 +3,8 @@ import type { ExecuteResult } from "../kernel/index.js"; import type { AcpMcpServerConfig } from "../mcp/acp-mcp-types.js"; import type { IpythonKernelProvisioner } from "./ipython.js"; -const ACP_MCP_SERVER_NAME_PATTERN = /^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$/; +// 48 keeps `mcp_list_tools_` within providers' 64-char tool-name limits. +const ACP_MCP_SERVER_NAME_PATTERN = /^[A-Za-z0-9][A-Za-z0-9_-]{0,48}$/; export function acpMcpToolNames(servers: readonly AcpMcpServerConfig[]): string[] { const names: string[] = [];