feat(cli): isolate ACP MCP per Session (PR5 γ)

[Sun-GLiang]

Summary

PR 5 γ from #3132, split out of #5222. This replaces the temporary rejection of client-provided stdio mcpServers with per-ACP-Session MCP configuration, process lifecycle, tool publication, grants, invocation, and cleanup. A Session-scoped Host capability binding prevents same-named servers on one ACP connection from leaking tools or grants across Sessions. The TUI publication path shares the extracted publication helper while Host remains the execution and authorization authority.

The Host protocol compatibility epoch advances 168 → 169 for the Session-scoped capability contract; compatible client/Host builds are required. A crashed MCP server withdraws its tools without blocking later prompts; Host registration slots are bounded per provider. This does not add HTTP/SSE/OAuth MCP management or session/load/session/resume support, which remains PR 6 scope.

Refs #3132
Refs #5222

Verification

• Rebased on main commit 64f5e83de after α/β merged.
• Full-workspace npm run build:test and npm run typecheck passed.
• npm run lint, npm run format:check, and the protocol epoch guard (168 → 169) passed. ASF header and CLI third-party notice checks passed on the earlier split head and were not rerun for this review update.
• 153 affected Runtime Host tests, 14 ACP Session MCP tests, and all five official-SDK child-process tests with a real Host and stdio MCP fixtures passed. These cover bounded registration, retirement ordering, crash withdrawal, permission/form continuation, parallel Session isolation, cancellation, full subscription capacity, and authoritative result reconciliation. The Core MCP grant test passed on the earlier split head.
• On the earlier split head, the full CLI dist suite reached 1148 passed and 3 skipped. Its two failures were the local managed-Host cold-start cases; both reproduced with the same connect_failed result on a clean 852a9748d control worktree. The full CLI suite was not rerun after this review update.
• Not rerun after this review update: full Runtime Host/Core/Desktop E2E suites or a new third-party Zed smoke test. Historical combined-PR evidence is documented separately and is not claimed as a γ-head rerun. The unrelated Side Chat E2E flake fix is not included.
• GitHub checks passed, including test, installed CLI validation on macOS/Linux/Windows, and the immutable source tarball build.

Review follow-up

• P1 epoch collision: rebased onto 64f5e83de and advanced the Host compatibility epoch to 169. The protocol epoch guard passes against that base.
• P2 unbounded Session registrations: limited each provider to 64 Session registration slots; replacing an existing slot remains allowed. Session retirement now runs in the capability mutation queue, closing the queued-replace race. Pre-creation publication means requiring an already-live Session ID would reject the legitimate ACP creation flow.
• P2 crashed MCP server: initial creation still requires every configured server to connect. Later prompts wait for the current tool snapshot to be published; a dead server's tools are withdrawn, and the Session can continue with remaining or built-in tools. Tests cover partial and complete withdrawal.
• Process cleanup: the MCP SDK 2.0.0 transport already escalates from SIGTERM to SIGKILL for its direct child. It does not guarantee cleanup of a wrapper's descendants. The ACP README now states that direct-child boundary explicitly; descendant cleanup remains a separate process-supervision concern.
• Interop notes: mcpServers is required by the ACP request type, and this PR intentionally accepts only stdio MCP servers. The ACP publication path normalizes server/tool IDs before Host registration; the general wire decoder still allows broader IDs that fail closed at grant storage.

Review focus

α (#5384) and β (#5385) are merged. This branch was rebased onto main commit 64f5e83de and contains three γ commits. Please scrutinize Session scope, close/retire/reconnect races, provider fallback, and Desktop/TUI compatibility. The original #5222 is unchanged.

AI use

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Codex contributed the split implementation, tests, documentation, and this description. Retain the Generated-by: Codex trailer in the final squash commit. Independent human review is still required.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

Two-slice adversarial review (Host protocol + CLI/ACP client). Headline: the Session-isolation invariant holds with three layers of defense — binding selection requires providerId && sessionId double-match, assertRegistrationSession runs inside both prepareTool and callTool, and grants are constructed from the stored request's sessionId rather than trusting call-time input. Same-named servers across Sessions get distinct processes and disjoint grant slots; connection-wide and session-scoped registration overlap is rejected in both directions; unregister scope is server-side resolved so a client can't forge another scope. The authorization chain stays Host-owned end to end. Nice work.

Findings: 1 P1 + 3 P2 inline/below, plus P3s.

P2 — an MCP server crash permanently wedges the whole ACP Session (packages/cli/src/acp/session-mcp.ts:203-211). ready()/#assertConnected requires all configured servers connected; a server that dies mid-session flips to disconnected, and every subsequent session/prompt fails mcp_not_ready — including built-in tools that never touched MCP. ACP has no reconnect path (TUI has a manual reconnect; nothing in acp/ calls manager.reconnect()), and load/resume is PR-6 scope, so the client must close+recreate and loses session context. Fail-closed is the right security posture but the availability blast radius is too big for one disposable server. Options: bounded manager.reconnect() in ready(); or relax the prompt gate to "snapshot published" (the publication layer already withdraws a dead server's tools, so running without them doesn't break isolation); at minimum return a distinguishable error code so clients can prompt a session restart. ② path — a crash is a normal failure mode.

P2 — close/EOF doesn't reliably kill wrapper-spawned MCP servers (packages/mcp/src/index.ts transport close ~:3010; contract claimed in packages/cli/src/acp/README.md:133-134). StdioClientTransport.close() SIGTERMs only the direct child — no escalation, no process-group kill — but npx/uvx/python -m is the most common MCP launch shape, so grandchildren survive as orphans holding ports, files, and client-injected env. Not new machinery, but this PR writes "Session close terminates its processes" as an explicit contract and exposes it to the lower-trust ACP path. Fix centrally in @maka/mcp (POSIX detached + kill(-pid), or SIGTERM→timeout→SIGKILL; taskkill /T on Windows) — TUI benefits too.

P3s: retireSessions bypasses #activation.runMutation — a queued replace for a retired sessionId re-pins the slot until disconnect (memory only, client-capability-coordinator.ts:859-873); session/new hard-requires mcpServers array and rejects type outright — both defensible fail-closed choices, worth noting for interop; serverId wire charset is broader than the safeId grant-key charset — non-safe ids fail closed in grant storage, fine but worth a normalization note.

Note on the CLI dist suite: the two connect_failed cold-start failures reproducing on clean 852a9748d control are consistent with an unrelated flake — accepted.

[Astro-Han]

P1 — epoch collision, merge-blocker. main is already at 168 (167 was taken by #5468 removing turn.regenerate). This PR claims 167. Either merge resolution trips protocol-epoch-check.mjs: staying at 167 gives headEpoch < baseEpoch; bumping to 168 puts this PR's incompatible client-capability changes (sessionId, admission:'mcp') on an epoch base already saw → rejected. Rebase onto current main and take 169.

[Sun-GLiang]

Thanks, agreed. I rebased γ onto main at 64f5e83de and advanced the Host compatibility epoch to 169 in 2e37db52e. The protocol epoch guard reports 168 -> 169, and the GitHub test check now passes.

[Astro-Han]

P2 — sessionRegistrations has no bound, and sessionId is fully client-controlled. Every other dimension of this protocol has a hard cap (offers 32, tools 256, manifest 56KiB), but a connection holding canPublishClientCapabilities can loop client.capability.replace with fabricated sessionIds — each pins a ≤56KiB frozen manifest plus registration entry until disconnect, and retireSessions only cleans real session ids so forged ones never release. Host memory grows unboundedly within a connection. ③ attacker-controlled input. Fix: cap provider.sessionRegistrations.size (allowing replace on existing ids) or validate the sessionId is a live session.

[Sun-GLiang]

Agreed. client.capability.replace now caps each provider at 64 Session registration slots while still allowing replacement of an existing slot and reuse after unregister. Checking for an already-live Session would reject the intended ACP flow, which publishes capabilities before Host session.create. I also put retireSessions in the capability mutation queue, so a queued replacement cannot re-pin a retired slot. Both boundaries have regression tests in 2e37db52e.

[Astro-Han]

P2 — one dead server wedges the entire Session. After any configured server flips disconnected, every prompt fails here with mcp_not_ready — even for conversations that would only use built-in tools — and there is no reconnect path on the ACP side (manager.reconnect() exists but nothing in acp/ calls it). Suggest bounded reconnect here, or relax the gate to "current snapshot published" (a dead server's tools are already withdrawn by the publication layer), or at least a distinguishable error code.

[Sun-GLiang]

Agreed. Initial prepare() still requires every configured server to connect. For later prompts, ready() waits for the current capability publication to settle, so a crashed server's tools are withdrawn before the Session continues with its remaining or built-in tools. Regression tests cover both a partial withdrawal and the only server dying (complete withdrawal). The existing RequestError.data.code already carried mcp_not_ready; the availability fix removes the persistent failure path in 2e37db52e.

[Sun-GLiang]

@Astro-Han Thanks for the thorough review. I replied to the three inline findings above. Follow-up on the remaining points:

• Wrapper-spawned processes: I agree that descendants are not guaranteed to exit on Session close. One factual correction: the pinned @modelcontextprotocol/client 2.0.0 StdioClientTransport.close() already escalates from SIGTERM to SIGKILL for its direct child after a grace period. It does not kill a process group. I narrowed the ACP README's cleanup contract to direct-child transport cleanup and explicitly noted the launcher/descendant limitation. I have not added cross-platform process-tree supervision in this PR, so descendant cleanup remains an acknowledged limitation.
• P3 retirement race: retireSessions now runs in the capability mutation queue, and a regression test covers a queued replace followed by retirement.
• P3 interop/IDs: The current ACP NewSessionRequest requires an mcpServers array; this PR intentionally accepts only untagged stdio servers. The ACP publication path normalizes server/tool IDs before Host registration. The general wire decoder accepts a broader charset, which fails closed at grant decoding; I added that note to the PR description.

Commit 2e37db52e contains the changes. Local build, typecheck, lint, format, the protocol epoch guard, 153 affected Host tests, 14 ACP MCP tests, and five real Host/ACP child-process tests passed. All current GitHub checks are green.