feat(mcp): wire ACP MCP servers through to pi (stdio/http/sse + acp transport)

[hancengiz]

Summary

pi-acp previously advertised mcpCapabilities { http: false, sse: false } and silently dropped every MCP server passed in session/new (the only handling was // Pi doesn't support mcpServers, but we accept and store.). This PR implements real MCP wiring through to pi without claiming MCP support for plain pi by default.

pi has no built-in MCP support, but the community pi-mcp-adapter extension reads .pi/mcp.json and spawns/connects MCP servers. This change routes ACP-provided MCP servers through that mechanism when MCP is enabled/detected.

Capability advertisement

pi-acp now advertises MCP capabilities conditionally:

• Default for plain pi / no MCP adapter detected:

  { "http": false, "sse": false, "acp": false }

• If pi-mcp-adapter is detected globally (package dir or settings package entry), or PI_ACP_ENABLE_MCP=true is set:

  { "http": true, "sse": true, "acp": true }

• PI_ACP_ENABLE_MCP=false forces MCP capability advertisement off.

This avoids telling ACP clients that MCP is supported when plain pi cannot actually consume MCP config.

What it does when MCP is enabled

• stdio / http / sse servers are translated into pi-mcp-adapter config entries and merged into the project-local .pi/mcp.json for the session. The original file is backed up (.pi/mcp.json.pi-acp.bak) and restored on session close.
• acp transport (MCP-over-ACP RFD) is bridged: pi-acp spawns a tiny dependency-free stdio shim (dist/mcp-shim.js) that pi launches via pi-mcp-adapter. The shim relays newline-delimited JSON-RPC to a local socket owned by AcpMcpBridge, which routes messages over the ACP channel using mcp/connect, mcp/message, and mcp/disconnect.
• Inbound (server-originated) mcp/message notifications are forwarded to the owning session's bridge to the shim to pi. Server-originated requests (e.g. sampling/createMessage) are declined, since handling them would require pi's MCP client to also act as an MCP server.
• Restores .pi/mcp.json and disposes bridges/sockets on session close and on spawn failure.

Robustness hardening (second commit)

Follow-up commit addressing review feedback and hardening the failure paths:

• Backup safety (thanks @ChristianLuciani for catching this): if the backup of the user's .pi/mcp.json cannot be created, MCP wiring is skipped for the session instead of overwriting the file — previously restore() could delete the user's config in that case.
• Per-server failure isolation: a broken acp-transport server (e.g. its socket cannot be created) is skipped instead of silently dropping all MCP servers for the session.
• Timeouts on all ACP round-trips: mcp/connect 15s, mcp/message 5min (MCP tool calls can legitimately run long), mcp/disconnect 5s. An unresponsive client now yields a JSON-RPC error to pi instead of a request that hangs forever.
• Windows: the shim socket uses a \\.\pipe\ named pipe on win32 (unix socket paths don't work with net.Server.listen there).
• Spec correctness: mcp/disconnect on shim socket close is now sent as a request (the SDK schema defines it with a response), matching the dispose path.

Architecture

ACP client --(ACP)--> pi-acp --(RPC)--> pi --(stdio MCP)--> pi-mcp-adapter
                          |                      |
                          |   stdio/http/sse:    |
                          +-> writes .pi/mcp.json<+
                          |
                          |   acp transport:
                          +-> AcpMcpBridge --(socket)--> mcp-shim (pi launches as stdio MCP server)

Requires

The pi-mcp-adapter extension must be installed in pi (pi install npm:pi-mcp-adapter). Without it, pi has no MCP support; the capability defaults to false unless explicitly forced with PI_ACP_ENABLE_MCP=true.

Limitations / notes

• initialize has no cwd, so project-local-only pi-mcp-adapter installs cannot always be detected at capability-advertisement time. Use PI_ACP_ENABLE_MCP=true if you rely on a project-local MCP adapter.
• The merged .pi/mcp.json is restored on clean session close. If pi-acp is killed abruptly, the managed servers remain in .pi/mcp.json (a .pi-acp.bak is left for manual recovery).
• Server-originated MCP requests are declined (see above); notifications are supported.
• The acp capability is currently marked UNSTABLE/experimental by the ACP schema, matching the RFD status.

Testing

• npm run typecheck, npm run lint, npm run build all clean.
• Full suite green: 118 tests (rebased on latest main, includes the new upstream tests).
• New unit tests: conditional MCP capability advertisement (default false, env true/false, package-dir detection, settings detection), config translation for stdio/http/sse/acp, .pi/mcp.json merge + restore, and a regression test for the backup-failure data-loss case (fails against the pre-fix code).
• New component tests: AcpMcpBridge connect round-trip (mcp/connect + mcp/message request/response), notification forwarding (no spurious response), inbound server-originated notification forwarding to the shim, connection tracking, mcp/message rejection surfacing as a JSON-RPC error, message and connect timeout behavior, mcp/disconnect sent as a request on socket close, and per-server failure isolation in setupMcpServers (real socket-failure injection).
• Manual smoke test: built mcp-shim.js spawned as a subprocess relays a tools/call request through a real socket and returns the response. PASS

Files

• src/mcp-shim.ts — standalone stdio to socket relay shim (built to dist/mcp-shim.js).
• src/acp/mcp/config.ts — ACP to pi-mcp-adapter config translation + managed .pi/mcp.json.
• src/acp/mcp/bridge.ts — AcpMcpBridge: socket server, mcp/connect/message/disconnect relay.
• src/acp/mcp/index.ts — setupMcpServers orchestrator.
• src/acp/pi-settings.ts — MCP capability detection and env override.
• src/acp/session.ts / src/acp/agent.ts — lifecycle wiring, conditional capabilities, inbound routing.
• tsup.config.ts — build the shim as a second entry.
• README + tests.

Closes the "MCP servers are accepted in ACP params and stored in session state, but not wired through to pi" limitation while keeping default capability advertising honest for plain pi.

[ChristianLuciani]

@hancengiz this is really impressive work — clean architecture, thorough tests, and the .pi/mcp.json backup/restore mechanism with crash recovery is exactly the right level of care for touching user config files. The mcp-shim at 79 lines with zero dependencies is a model of good module design. I ran the suite locally (typecheck + lint + 105 tests) and everything is green.

I was about to open an issue proposing exactly this integration when I found your PR — you have already built it, and built it well. 😄

A couple of things I wanted to raise for discussion:

1. Backup failure edge case in writeManagedMcpConfig (config.ts)

I think there is a subtle bug here: if copyFileSync fails during backup (disk full, permissions), backedUp stays false, the code proceeds to overwrite the user original .pi/mcp.json with the managed version, and then restore() — seeing backedUp === false and no backup file — deletes it. The user loses their config.

The fix is small — if the backup fails, skip the write entirely:

let backupFailed = false
try {
  if (existsSync(path) && !existsSync(backupPath)) {
    copyFileSync(path, backupPath)
  }
} catch {
  backupFailed = true
}

if (backupFailed) {
  return { path, restore: () => {} }  // do not touch the file
}

Edge case for sure (backup of a small JSON file failing is rare), but since it is user config, being defensive feels warranted.

2. Should acp transport have its own gate?

getMcpCapabilities() gates http, sse, and acp on a single boolean. The PR description notes that acp transport is UNSTABLE/experimental per the ACP schema. If that transport needs to be disabled independently (spec change, stability), there is currently no knob — you have to turn off everything.

Not a blocker at all, but wanted to flag it in case you or @svkozak want a PI_ACP_ENABLE_MCP_ACP or similar before this lands. Happy either way.

One observation on size: at ~1300 lines / 12 files, this is a substantial review. The MCP-over-ACP transport (bridge + shim, ~420 lines + tests) is self-contained enough that it could ship as a follow-up. But that is a strategic call for the maintainer — the code is solid either way.

---

I would love to help move this forward — testing against a real pi-mcp-adapter setup, reviewing follow-up changes, whatever is useful. Thanks for building this!

[hancengiz]

@svkozak Sergii any comments on this?

[svkozak]

@hancengiz appreciate the effort, but after reviewing I don't think this is a good fit for pi-acp:

• It solves the wrong primary problem. pi-acp users run Pi locally. Pi already has its own MCP configuration path (which is not native, but itself requires an extension). Terminal users need that path for terminal Pi anyway.

• It creates two sources of truth. Zed MCP settings and Pi MCP settings can differ. The adapter must then merge them, choose precedence, handle name conflicts, and restore state.

• It makes pi-acp a configuration manager. The adapter should start Pi and translate ACP events. It should not edit .pi/mcp.json, preserve backups, recover crashes, or coordinate concurrent sessions.

• It has a real data-loss bug. The current session replacement flow can delete an existing user .pi/mcp.json. Fixing this needs locks or shared ownership across processes. That is a large cost for a convenience feature.

Other than that, there are a few other issues that may further complicate things.

[ChristianLuciani]

@svkozak thanks for the thoughtful response — this is really useful clarity. The distinction between "ACP translator" and "configuration manager" makes complete sense: the adapter should start Pi and translate events, not own MCP config, merge precedence rules, or coordinate state across processes. That is a clean scope boundary and I am aligned with it.

Appreciate you taking the time to explain the reasoning rather than just closing. It helps everyone who comes to this issue understand why, not just what.