Skip to content

extract mcp-over-acp proxy, align with RFD#146

Open
nikomatsakis wants to merge 16 commits into
agentclientprotocol:mainfrom
nikomatsakis:mcp-over-acp-proxy
Open

extract mcp-over-acp proxy, align with RFD#146
nikomatsakis wants to merge 16 commits into
agentclientprotocol:mainfrom
nikomatsakis:mcp-over-acp-proxy

Conversation

@nikomatsakis
Copy link
Copy Markdown
Contributor

This PR makes two important changes

  • integrate the mcp-over-acp into the protocol properly, as defined in the RFD, "cargo feature"-gated
  • extract the "mcp bridging" from the conductor proper and into a polyfill proxy, creating a new crate that I expect to extend over time to help us deploy new features in a uniform way
  • rewrite tests to use this proxy

Also, removes some outdated text about html_panel etc.

@nikomatsakis nikomatsakis requested a review from benbrandt April 30, 2026 18:23
Copy link
Copy Markdown
Contributor Author

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Review for agent)

Comment thread src/agent-client-protocol-conductor/tests/trace_mcp_tool_call.rs Outdated
Comment thread src/agent-client-protocol-polyfill/src/mcp_over_acp/actor.rs
Comment thread src/agent-client-protocol-polyfill/src/mcp_over_acp/actor.rs
Comment thread src/agent-client-protocol-polyfill/src/mcp_over_acp/http.rs
nikomatsakis and others added 13 commits May 13, 2026 04:24
- Remove McpAcpTransport struct and MetaCapability impl from capabilities.rs
  (keep MetaCapability/MetaCapabilityExt traits for proxy capability)
- Rename McpConnectRequest.acp_url to acp_id across all source and tests
- Update snapshot tests for schema changes (mcpCapabilities.acp, auth removal)
- Switch agent-client-protocol-schema to path dependency
New crate src/agent-client-protocol-polyfill with mcp_over_acp module:
- McpOverAcpPolyfill proxy (ConnectTo<Conductor>)
- BridgeConnectionActor, BridgeListeners, BridgeConnection
- HTTP and stdio bridge transports
- Uses own BridgeMessage enum instead of ConductorMessage

Conductor bridge code still present (removed in Step 3).
- Remove McpBridgeMode enum and mcp_bridge_mode parameter
- Delete conductor/mcp_bridge module (actor, http, stdio)
- Remove bridge fields from ConductorResponder
- Remove bridge ConductorMessage variants and handling
- Remove NewSessionRequest MCP server transformation
- Update all 16 test files to remove McpBridgeMode
- Mark 13 bridge-dependent tests as #[ignore]
…Capabilities.acp)

Replace all _meta.symposium.mcp_acp_transport references with
mcpCapabilities.acp in:
- md/proxying-acp.md (8 references)
- md/protocol.md (3 references)
- md/conductor.md (2 references)
- md/mcp-bridge.md (1 reference)
- Add McpConnectionTo::acp_id(), deprecate acp_url()
- Remove orphaned 'conductor mcp $port' CLI subcommand and mcp_bridge.rs
- Fix all 13 previously-ignored tests to use McpOverAcpPolyfill::http()
- Fix BridgeResponder bug: route through BridgeMessage::ConnectionEstablished
  instead of creating new HashMap in on_receiving_result callback
- Auto-update 2 snapshot tests for new proxy in chain
- agent-client-protocol: removed McpAcpTransport, renamed acp_url→acp_id,
  added acp_id(), deprecated acp_url()
- agent-client-protocol-conductor: removed McpBridgeMode, removed
  conductor mcp CLI subcommand
Instead of unconditionally enabling unstable_mcp_over_acp on the schema
dependency, declare it as a feature on agent-client-protocol that
forwards to agent-client-protocol-schema/unstable_mcp_over_acp.

Conductor and polyfill crates opt in explicitly.
The conductor and polyfill don't reference McpServer::Acp or
McpCapabilities.acp in their source — only snapshot tests see the
field in serialized output. Move the feature to dev-dependencies.
- Restore doc comments in polyfill actor.rs and http.rs that were
  stripped during extraction from conductor
- Polyfill now intercepts InitializeProxyRequest and sets
  mcpCapabilities.acp = true in the response
- Re-enable unstable_mcp_over_acp feature for polyfill crate
  (it needs to set the acp field)
- Update snapshot tests to reflect acp: true

ProxiesAndAgent API redesign noted for separate PR.
- Fix two stale test references to the removed McpAcpTransport type
  (regression from "Step 1/1b") by switching to TestCapability
- Regenerate trace_* expect snapshots so they match the actual output
  under --all-features (adds auth capability fields exposed by
  unstable_auth_methods)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@nikomatsakis nikomatsakis marked this pull request as ready for review May 13, 2026 08:49
nikomatsakis and others added 3 commits May 13, 2026 07:25
CI runs clippy with `-D warnings` and surfaced three lints in the
new mcp_over_acp module:

- derivable_impls: replace the manual `impl Default for BridgeMode`
  with `#[derive(Default)]` and `#[default]` on the `Http` variant
- must_use_candidate: add `#[must_use]` to `McpOverAcpPolyfill::http`
  and `::stdio` constructors

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants