feat(coding-agent): support ACP MCP programs - #1378
Conversation
e278ab6 to
30b44fa
Compare
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| })); | ||
| if (this._resourceLoader.replaceSkillResources) { | ||
| this._resourceLoader.replaceSkillResources(resources, source); | ||
| } else if (resources.length > 0) { |
There was a problem hiding this comment.
🟡 Medium core/agent-session.ts:1401
The fallback extendResources call preserves previously registered skill paths, so repeated ACP MCP updates leave removed tools loaded and an empty update cannot clear them; renamed tools can also collide with stale generated skills. Use a true replacement path for ResourceLoader implementations that lack replaceSkillResources (or update the interface so replacement is always available).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/core/agent-session.ts around line 1401:
The fallback `extendResources` call preserves previously registered skill paths, so repeated ACP MCP updates leave removed tools loaded and an empty update cannot clear them; renamed tools can also collide with stale generated skills. Use a true replacement path for `ResourceLoader` implementations that lack `replaceSkillResources` (or update the interface so replacement is always available).
Evidence trail:
packages/coding-agent/src/core/agent-session.ts:1386-1409 @ e278ab6f8e045e12bcdf248276ebad6343789619
packages/coding-agent/src/core/resource-loader.ts:29-40, 297-352 @ e278ab6f8e045e12bcdf248276ebad6343789619
packages/coding-agent/src/core/sdk.ts:60-61, 155-184 @ e278ab6f8e045e12bcdf248276ebad6343789619
packages/coding-agent/src/modes/acp/acp-mcp.ts:212-247 @ e278ab6f8e045e12bcdf248276ebad6343789619
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 30b44fa. Configure here.
| throw acp.RequestError.invalidParams({ reason: "MCP servers are unavailable in this ACP host" }); | ||
| } | ||
| try { | ||
| await configureMcpServers?.(params.mcpServers, actualCwd ?? requestedCwd); |
There was a problem hiding this comment.
Empty MCP setup still replaces skills
High Severity
session/new always runs MCP configure, and configure always calls replaceTemporarySkills, even when mcpServers is empty. Daemon connections expose that method regardless of the temporary_skills capability, so an older daemon rejects every new ACP session. The same path also rebuilds the IPython kernel on sessions that never used MCP, and initialize still advertises HTTP MCP support.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 30b44fa. Configure here.
| function skillImportName(server: string, tool: string): string { | ||
| const normalized = `${server}_${tool}`.replace(/\W/g, "_"); | ||
| return /^\d/.test(normalized) ? `_${normalized}` : normalized; | ||
| } |
There was a problem hiding this comment.
MCP names keep uppercase letters
Medium Severity
skillImportName only rewrites non-word characters and never lowercases. MCP server and tool names may include uppercase letters, so the later lowercase-only skill-name check rejects those tools and fails the whole session/new instead of installing a valid Python skill.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 30b44fa. Configure here.
| parameters.sort( | ||
| key=lambda parameter: parameter.default is not inspect.Parameter.empty | ||
| ) | ||
| return inspect.Signature(parameters) |
There was a problem hiding this comment.
Schema keywords break generated skills
Medium Severity
_mcp_tool_signature keeps JSON Schema properties that pass str.isidentifier() but are Python keywords such as from or class. inspect.Parameter then raises ValueError while the generated module is imported, so the skill is installed as unavailable and the MCP tool cannot be called.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 30b44fa. Configure here.
| ) | ||
| streams = stdio_client( | ||
| StdioServerParameters(command=command, args=args, env=env) | ||
| ) |
There was a problem hiding this comment.
Stdio MCP calls drop session cwd
Medium Severity
Tool discovery starts stdio MCP servers with the ACP session cwd, but the generated Python skill serializes only command, args, and env. Runtime StdioServerParameters therefore omit cwd, so later tool calls can spawn in a different working directory than discovery used.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 30b44fa. Configure here.
| if (options.ownStdout !== false && !options.stream) { | ||
| takeOverStdout(); | ||
| } | ||
| const mcp = connection.replaceTemporarySkills |
There was a problem hiding this comment.
🟠 High acp/acp-mode.ts:248
Daemon-backed ACP sessions fail during session/new because mcp is created whenever replaceTemporarySkills exists, even when the daemon lacks the temporary_skills capability; the unconditional configureMcpServers call then invokes that unsupported method (including for an empty mcpServers list). Check the advertised capability before constructing/using AcpMcpSkillInstaller, or skip configuration when no MCP servers were supplied.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/modes/acp/acp-mode.ts around line 248:
Daemon-backed ACP sessions fail during `session/new` because `mcp` is created whenever `replaceTemporarySkills` exists, even when the daemon lacks the `temporary_skills` capability; the unconditional `configureMcpServers` call then invokes that unsupported method (including for an empty `mcpServers` list). Check the advertised capability before constructing/using `AcpMcpSkillInstaller`, or skip configuration when no MCP servers were supplied.
Evidence trail:
packages/coding-agent/src/modes/acp/acp-mode.ts:248-255,303-312 @ 30b44fae; packages/coding-agent/src/modes/agent-connection/daemon-agent-connection.ts:459-468 @ 30b44fae; packages/coding-agent/src/modes/acp/acp-mcp.ts:212-246 @ 30b44fae; git diff MERGE_BASE REVIEWED_COMMIT -- packages/coding-agent/src/modes/acp/acp-mode.ts
2 participants
Summary
session/newWhy
Verifiers passes environment tools to ACP agents as MCP servers, but Prime Agent previously ignored
mcpServers. That made tool-bearing Prime Agent rollouts impossible and required downstream integrations to report MCP as unsupported.The agent should own the translation from external MCP tools to its native model-facing interface. This change keeps IPython as the model-facing tool and exposes each ACP MCP tool as a pre-imported Python program, matching Prime Agent's RLM architecture.
Validation
npm run checknpx vitest --run packages/coding-agent/test/acp-mcp.test.ts packages/coding-agent/test/agent-connection-daemon.test.ts packages/coding-agent/test/daemon-protocol.test.ts— 86 passeduv run --project . --with pytest --with 'mcp>=1.28,<2' --with 'httpx>=0.27,<1' pytest test/test_mcp_base.py— 21 passedA separate, small Verifiers PR will add the Prime Agent ACP harness after this behavior is available in a release.
Note
Medium Risk
Introduces session-bound external MCP connectivity and a new daemon capability, but keeps it isolated from persisted MCP settings and validates tool/skill naming collisions before install.
Overview
ACP mode now honors HTTP and stdio MCP servers passed in
session/new.mcpServers, which were previously ignored. On session creation the agent discovers each server's tools and materializes one temporary Python skill per tool in the RLM kernel (e.g.task-tools/lookup→task_tools_lookup), using JSON Schema–derived signatures and the client's transport/credentials only—not persisted user MCP settings.replaceTemporarySkillsreplaces all skills for a given source (acp:mcp) across in-process sessions, the resource loader, and daemon clients (schema revision 17, capabilitytemporary_skills). Reconfiguring or disposing an ACP session swaps or removes generated skill packages and rejects name collisions with existing skills.The runtime adds
AcpMcpIntegrationandmake_acp_mcp_skillfor stdio/HTTP calls from generated modules. ACPinitializeadvertises HTTP MCP support only when the active connection can install temporary skills. Docs and TypeScript/Python tests cover discovery, wiring, and daemon capability gating.Reviewed by Cursor Bugbot for commit 30b44fa. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add MCP server support to ACP mode, exposing tools as session-scoped Python programs
session/new; the coding agent discovers their tools and generates one temporary Python skill package per tool in the RLM kernel.@modelcontextprotocol/sdk, and Python skill generation usingrlm.make_acp_mcp_skill.AcpMcpIntegrationandmake_acp_mcp_skill, which generate async Python functions with signatures derived from JSON Schema and bind them to the specified MCP tool.replaceTemporarySkillsmethod is added toAgentSession,AgentConnection, and the daemon protocol (schema revision 17, capabilitytemporary_skills), allowing skills to be atomically replaced between runs.Macroscope summarized 30b44fa.