feat(acp): browser control via MCP-over-ACP + stdio bridge (Phase 2)

[brettchien]

What problem does this solve?

The ACP-over-WebSocket base (#1418) ships a 1:1 streaming chat surface at GET /acp: a browser side-panel extension connects as an ACP client and drives an OpenAB agent. It deliberately stops at chat — no tool calls, no way for the agent's LLM to act on the page it is talking about.

This PR delivers the base ADR §6 north-star: let the agent's LLM autonomously operate the user's real, logged-in Chrome (read the DOM, screenshot, navigate, click, type) by exposing the browser as MCP tools and routing them MCP-over-ACP over the /acp WebSocket the extension already holds. No sandbox VM, no second connection, no per-frontend adapter.

It also generalizes that mechanism: any ACP WS client may declare one or more type:acp MCP servers, and they reach the agent through the OAB MCP Facade (#1448/#1453) as session-aware capability sources (#1454) — not through a bespoke per-session MCP server. Design: acp-server-websocket-reverse-mcp.md — the mechanism, the §6 multi-server generalization, and §7's worked example (browser control, incl. the contract the extension implements).

Closes # — None; tracked by the base ADR §6 "Critical path" roadmap, not a separate issue.

Discord Discussion URL: https://discord.com/channels/1491295327620169908/1527521703255605338

At a Glance

Architecture — the extension (a can't-listen MV3 client) is the MCP server, serving tools over its own outbound /acp WS; OpenAB is the MCP proxy; the agent LLM is the MCP client. Only the extension hop leaves the pod. Downstream, browser capabilities are delivered through the OAB MCP Facade (default); the pre-facade proxy and bridge transports remain as explicit OPENAB_BROWSER_MODE opt-outs.

flowchart LR
  EXT["<b>Side-panel MV3 extension</b> = MCP SERVER<br/>(cannot open a listening socket → serves MCP<br/>over the outbound /acp WS it already holds)<br/>declares {type:acp, id, name:'katashiro'}<br/>tools: katashiro.read_dom · screenshot · navigate · click · type"]
  subgraph POD["OPENAB POD — 'openab run', one process tree"]
    direction LR
    GW["<b>openab-gateway</b><br/>/acp WS server<br/>AcpTunnelRegistry<br/>keyed (channel_id, server_id)"]
    SRC["<b>AcpTunnelSource</b><br/>CapabilitySource<br/>requires_session<br/>allowlist + tool pin"]
    FAC["<b>OAB MCP Facade</b><br/>127.0.0.1:8848/mcp<br/>search_capabilities<br/>execute_capability"]
    AGENT["<b>agent CLI</b><br/>Cursor · Kiro · Claude · Codex<br/>LLM = MCP CLIENT"]
    GW <--> SRC
    SRC --> FAC
    FAC ==>|"Authorization: Bearer ${OPENAB_SESSION_TOKEN}<br/>(broker-minted per session, revoked on evict)"| AGENT
  end
  EXT <==>|"UPSTREAM — only remote hop<br/>MCP-over-ACP · mcp/message framing<br/>multiplexed with ACP chat on ONE /acp WSS<br/>8 MiB frame cap · JPEG screenshots"| GW
  classDef remote fill:#fde68a,stroke:#b45309,color:#111;
  classDef pod fill:#bfdbfe,stroke:#1e40af,color:#111;
  class EXT remote;
  class GW,SRC,FAC,AGENT pod;

Loading

MCP usage sequence (katashiro.click example) — Phase 1 connect + discovery, Phase 2 one autonomous action:

sequenceDiagram
    autonumber
    participant Tab as Chrome tab
    participant Ext as extension<br/>MCP SERVER
    participant GW as gateway /acp
    participant Src as AcpTunnelSource
    participant LLM as agent LLM<br/>MCP client
    Note over Ext,LLM: PHASE 1 — connect & discovery
    Ext->>GW: initialize · mcpServers=[{type:acp, id, name:"katashiro"}]
    Ext->>GW: session/new (or resume)
    GW->>GW: register TunnelHandle at (channel_id, server_id)
    GW->>Src: broker mints session token → agent MCP config
    LLM->>Src: search_capabilities (via facade)
    Src->>GW: tools/list (mcp/message) — once per declared server
    GW->>Ext: mcp/message → tools/list
    Ext-->>Src: real tool list → cached per (channel_id, server_id)
    Src-->>LLM: capabilities: katashiro.* (fetched ∩ allowed)
    Note over Tab,LLM: PHASE 2 — one autonomous action
    LLM->>Src: execute_capability katashiro.click(selector)
    Src->>GW: tools/call (mcp/message, same /acp WS)
    GW->>Ext: mcp/message → tools/call
    Ext->>Tab: chrome.scripting / tabs API
    Tab-->>Ext: DOM mutated / pixels
    Ext-->>LLM: tool result (JPEG screenshots, frame <= 8 MiB)
    Note over GW,Ext: only the gateway-to-extension hop leaves the pod

Loading

Prior Art & Industry Research

The problem: give the agent's LLM tools that drive the user's own remote, logged-in browser, where the tool provider is an MV3 extension that cannot open a listening socket.

OpenClaw — browser control is either an OpenClaw-managed, isolated Chrome profile driven by a local control service inside the Gateway (Control UI, managed browser), or the Chrome DevTools MCP server against a locally-reachable Chrome (MCP docs). openclaw mcp serve is a stdio bridge that keeps a stdio MCP session open and forwards to a local/remote Gateway over WebSocket — but it exposes routed channel conversations as MCP, not a remote browser. In every case the MCP server is colocated with the browser; OpenClaw does not route tools from a remote, can't-listen extension back to a colocated agent.

Hermes Agent — built-in browser tools work over accessibility-tree snapshots + stable refs + screenshots, with pluggable backends (Browserbase / Browser Use / Firecrawl cloud, or local Chromium / Camofox) (browser feature). hermes-computer-use is a pixel-level MCP server (screenshot + xdotool on an Xvfb display) that any MCP client connects to (repo). Here too the MCP server runs where the browser runs.

Takeaway — both projects colocate the browser-tool MCP server with the browser and let the agent be a normal MCP client. Neither tunnels MCP from a remote user's own can't-listen extension back to a colocated agent over the client's existing protocol connection. That inversion is the novel part. (ACP itself, and the general OpenClaw/Hermes ACP comparison, are covered in #1418.)

Proposed Solution

Three layers, behind the existing acp feature.

1. Protocol gap — agent→client REQUEST direction. The base only did client→agent prompts and agent→client notifications. Browser control needs the agent to ask the client and await a result, so acp_server's dispatch loop gains a server-initiated request path. Wire types are the generated serde-only v1 types from #1418.

2. Upstream hop — MCP-over-ACP tunnel (extension ↔ gateway). A tunnel frame API multiplexes MCP tools/list / tools/call / results over the same /acp WS using the official mcp/message framing; the client declares type:acp mcpServers on initialize and the gateway establishes a TunnelHandle per (channel_id, server_id) — a compound key, so one session can hold several declared servers (re-attach under the same name is last-write-wins). Opened on session/new and session/resume, torn down on cleanup. Wire contract: docs/mcp-over-acp-tunnel-contract.md.

3. Downstream hop — one CapabilitySource behind the OAB MCP Facade. Every client-declared server is exposed through a single in-process AcpTunnelSource (src/browser_source.rs) registered with the facade:

• requires_session() — anonymous facade clients neither discover nor can execute these tools. Identity is the facade's SessionTokens: the broker mints one opaque bearer per agent session, hands it to the agent as ${OPENAB_SESSION_TOKEN} (process env, not a config-file secret) and revokes it on evict.
• Multi-server fan-out — tools(ctx) returns the tools of every server that session's client declared; call routes on the <server>.<tool> prefix back to the right tunnel. No browser-specific branch in the source: admitting another client-side MCP service is config work, not a code change.
• Discovery — each declared server's real tools/list is fetched once on attach and cached per (channel_id, server_id); the catalog is served from cache regardless of current attach state, and backend unavailability surfaces as a call error, never a vanishing capability. notifications/tools/list_changed is deliberately not implemented: facade discovery is pull-based (search_capabilities re-reads per call), so nothing caches a tool list to invalidate.
• Trust gate — feat(facade): session-aware in-process capability sources #1454 assumes a source's tool set is the operator's grant, but a tunnel source's tools are client-declared. So an operator allowlist ([[mcp.acp_servers]], default: katashiro only) gates admitted server names, and each entry carries a deny-all tools pin. The name allowlist alone grants nothing — the catalog is fetched ∩ allowed, so a client declaring a trusted name still cannot publish extra tools.

proxy (per-session loopback HTTP MCP) and bridge (per-pod socket + openab browser-bridge stdio relay) are retained as explicit OPENAB_BROWSER_MODE opt-outs for CLIs or rollouts that need them.

Toolset — five DOM-semantic tools (katashiro.read_dom, katashiro.screenshot, katashiro.navigate, katashiro.click, katashiro.type). The ACP frame cap is raised 1→8 MiB to carry screenshot results (JPEG).

Why this approach?

• Extension-as-MCP-server over its own outbound WS — an MV3 extension cannot listen, and the user's browser is remote. Serving MCP over the /acp WS it already holds is the only way a can't-listen remote provider can be a full MCP server, and it reuses the one connection.
• MCP tools, not a custom ACP ExtRequest — only tools appear in the LLM's tool list, so only tools let the model autonomously act.
• DOM-semantic, not pixel computer tools — click(selector) / read_dom are cheaper, more reliable and model-agnostic.
• Behind the facade rather than a second inbound MCP server — the OAB MCP Adapter ADR (docs(adr): OAB MCP adapter #1446) §6.2 assigns the facade the same aggregator role and its Alternative C rejects "a second generic inbound MCP server". Reusing the facade also inherits its policy runtime (schema validation, timeouts, circuit breaking, redaction, audit) instead of reimplementing it.
• Static-advertise from a cache, not list_changed — keeps a stable catalog across reconnects without fabricating tools.

Known limitations — the browser tunnel binds to the ACP session, so the agent must be driven through the extension's ACP session (no Discord↔browser bridge). Under the facade the LLM reaches an action via search_capabilities → execute_capability, one hop more per turn than a direct tool; a per-provider "expose directly" option is deliberately deferred until interactive latency proves it necessary.

Alternatives Considered

• Custom ExtRequest per browser action — rejected: not surfaced to the LLM as a tool.
• Extension hosts a standalone HTTP/SSE MCP server — rejected: MV3 extensions cannot listen.
• Anthropic-style computer tool (screenshot + pixel coords) — subsumed; DOM-semantic tools are cheaper and model-agnostic.
• Colocated/sandbox browser (OpenClaw-managed / Hermes Xvfb) — rejected: the goal is the user's real, logged-in Chrome.
• Bespoke per-session MCP server + N mcp.json entries (this PR's own earlier design) — superseded by the facade integration; it would have reinvented the catalog, discovery and policy runtime the facade already provides, and violated the adapter ADR's "one aggregation point".
• One source per declared server — impossible by construction: facade sources are registered once at startup, so a single source fans out internally instead.

Validation

Rust changes, gated behind --features acp (openab-gateway/acp + openab-core/acp-mcp):

• cargo build --features acp — clean
• cargo clippy --workspace -- -D warnings and cargo clippy --workspace --features unified -- -D warnings — both clean
• cargo test -p openab-gateway --features acp — green (the acp-gated test target CI runs)
• Unit/integration coverage for the new behaviour: compound-key registry + last-attach-wins, multi-server routing by <server>.<tool>, allowlist + tool-pin refusal (unpinned tool denied, not "unknown server"), discovery cache narrowing (fetched ∩ allowed, cache can never widen past policy), and two client-declared servers in one session
• MCP-over-ACP tunnel e2e in scripts/acp-ws-smoke.py (producer section: tools/list / tools/call, fan-out + filtering)
• Live deployment — Facade mode running on a real agent (OrbStack, Cursor CLI): facade listener up on 127.0.0.1:8848, search_capabilities returns provider openab-browser with exactly the five pinned katashiro.* capabilities alongside host-level mcp.json providers, and anonymous probes of the facade correctly see only the two meta-tools (session-bound source hidden without a token)
• Live execute_capability round-trip driving the real tab — the browser extension, rebuilt on the renamed surface, declares {type:acp, name:"katashiro"}; the gateway registers the tunnel at (channel_id, server_id), the broker mints the session token, and the agent's execute_capability katashiro.read_dom reaches the user's actual tab and returns its DOM. The facade's audit trail brackets the dispatch:

  mcp.audit: facade source call      provider="openab-browser" tool=katashiro.read_dom channel="acp_27da364b…" args_sha256=44136fa3…
  mcp.audit: facade source call exit provider="openab-browser" tool=katashiro.read_dom channel="acp_27da364b…" args_sha256=44136fa3…
  

• Two-window session isolation (each window's agent reaching only its own browser) — covered by unit tests, not yet re-verified live on the renamed surface

Review Contract

Goal

Let a colocated agent's LLM autonomously operate the user's real remote browser via DOM-semantic MCP tools tunnelled MCP-over-ACP over the existing /acp WebSocket, and do it through the OAB MCP Facade as a session-aware CapabilitySource rather than a second agent-facing MCP server. Concretely: the agent→client request direction; the mcp/message tunnel keyed by (channel_id, server_id); AcpTunnelSource with multi-server fan-out, per-server discovery cache and an operator allowlist with deny-all tool pinning; broker-minted per-session tokens for identity.

Non-goals

• Retiring the pre-facade proxy / bridge transports (kept as OPENAB_BROWSER_MODE opt-outs; see Follow-ups).
• notifications/tools/list_changed — deliberately dropped, not deferred (pull-based facade discovery has no consumer for it).
• A per-provider "expose tools directly" bypass of the meta-tool path.
• Any Discord↔browser bridge: the tunnel is bound to the extension's ACP session by design.
• Fine-grained per-tool consent UX (core still auto-approves permissions, ADR D1).

Accepted Residual Risks

• Downstream MCP is loopback + bearer only. The facade binds 127.0.0.1 and trusts the host boundary; the per-session bearer rides the agent's process env. A hostile in-pod process shares the agent subprocess's trust boundary already.
• Client-declared tool sets are gated, not verified. The allowlist + deny-all pin stop a client publishing extra tools under a trusted name, but a compromised extension can still misuse the five pinned tools within the user's logged-in session — which is inherent to driving the user's real browser.
• 8 MiB frame cap — screenshots must be JPEG; a pathological result over the cap closes the frame rather than truncating.
• Static-advertise from cache can briefly show a capability whose backend has just detached; the call then returns an MCP error rather than the capability disappearing mid-turn. This is the deliberate trade (ADR §6.3).
• bridge mode resolves its channel by process ancestry — correct for the normal spawn tree; a relay started outside it fails closed.

Acceptance Criteria

• cargo build --features acp, cargo clippy --workspace [--features unified] -- -D warnings, and cargo test -p openab-gateway --features acp all green.
• Multi-server behaviour proven by test: two declared servers in one session are both discoverable and callable, routed by prefix, with no name collision.
• Trust gate proven by test: an unpinned tool from an allowlisted server is refused as a tool-pin refusal, and the cache can never widen the catalog past policy.
• Live: facade serving, search_capabilities lists exactly the pinned capabilities for a session-bound client, and anonymous clients see none of them.

Follow-ups

• Retire the bespoke per-session proxy and bridge mode once Facade mode has soaked. Beyond the adapter ADR's "one aggregation point", live testing surfaced a concrete hazard: each transport writer only adds its own mcp.json entry and never removes the other's, so an agent that has run in more than one mode loads both servers and exposes the same tools twice — once through the facade (policy + audit) and once straight through the old transport (neither). The model picks the direct one and the call leaves no audit trail at all while working perfectly; auditing looks dead with nothing indicating why. Documented in browser-mcp-agent-setup.md as an operational warning, but the durable fix is one transport (and, until then, cleanup-on-mode-switch).
• Reconcile the flagged divergence with the adapter ADR: it specifies delivery via ACP mcpServers and explicitly not by writing CLI config files, while the as-built writes a static openab entry because Cursor ignores ACP-passed mcpServers (browser ADR D2 / zed#50924). Owner of the facade contract to decide.
• Re-verify two-window session isolation live on the renamed surface (unit-tested; the live check predates the rename).
• Optional: carry a tool manifest in the client's initialize declaration so the catalog is known without a first tunnel round-trip.

[chaodu-obk]

Important

CHANGES REQUESTED ⚠️ - The MCP-over-ACP design is promising, but several failure and rollout paths can leave browser capability delivery inconsistent or bypass the facade's policy boundary.

What This PR Does

This PR lets an ACP client expose MCP servers over the existing /acp WebSocket, so a colocated agent can operate the user's browser through MCP-over-ACP. It adds server-initiated ACP requests, a compound-key tunnel registry, facade-backed session-aware capability discovery, operator allowlists, and legacy proxy/bridge transports.

How It Works

The gateway parses client-declared type:acp servers, opens an MCP connection for each server, and registers each tunnel by (channel_id, server_id). The root binary adapts that registry into the MCP facade's session-aware capability source. Facade sessions receive broker-minted bearer tokens; proxy and bridge remain explicit transport modes. Discovery is policy-filtered and cached, while tool calls resolve the declared server name to the current tunnel id.

Findings

#	Severity	Finding	Location
1	🟡	Rejected session/resume requests still spawn MCP tunnels	crates/openab-gateway/src/adapters/acp_server.rs:1022-1039
2	🟡	The 8 MiB frame cap increases per-connection memory exposure without a matching budget or backpressure limit	crates/openab-gateway/src/adapters/acp_server.rs:39
3	🟡	Facade config-write failures are logged but the session continues with a minted, unusable token	crates/openab-core/src/acp/pool.rs:416-422
4	🟡	Mode switching leaves the direct openab-browser entry, allowing calls to bypass facade policy and audit	crates/openab-core/src/mcp_proxy.rs:521-529
5	🟡	New acp-mcp and root browser tests are not exercised by the repository CI test jobs	.github/workflows/ci.yml:51-60
6	🟢	The compound-key registry and last-attach-wins behavior make reconnect routing deterministic	crates/openab-gateway/src/adapters/acp_server.rs:741-752
7	🟢	The facade source applies both server-name allowlisting and per-tool pinning, with deny-all behavior for unlisted tools	src/browser_source.rs:322-335
8	🟢	Session-token revocation and proxy cleanup are tied to connection/session lifetime guards	crates/openab-core/src/acp/pool.rs:423-433

Finding Details

🟡 F1: Do not open tunnels after a rejected resume

The loop sends the response from handle_session_resume, but then derives a channel id and calls spawn_acp_tunnels without checking resp.error. A resume rejected for a busy session or the per-connection session cap can therefore still emit mcp/connect requests and register handles for a session that was not accepted. This can replace a live same-name tunnel and sends protocol traffic after an error response.

Requested change: Only spawn tunnels when the resume response is successful, or have the handler return an explicit success result that gates tunnel creation. Add a regression test for busy and over-cap resume responses asserting that no tunnel-open side effect occurs.

🟡 F2: Bound the larger frame allowance

Raising the inbound text-frame limit from 1 MiB to 8 MiB is needed for screenshots, but the connection still has an unbounded outbound channel and permits up to 32 concurrent prompt tasks. Large prompt payloads can therefore be retained by multiple in-flight tasks, while multiple sessions can share a connection. The existing comment acknowledges that full backpressure and global limits are deferred.

Requested change: Add a per-connection byte/in-flight budget or apply the larger limit only to bounded MCP result traffic, and add a test covering the rejection boundary and the chosen resource bound. Document the resulting worst-case memory budget.

🟡 F3: Fail the session setup or surface an explicit degraded state

When write_facade_mcp_config fails, the pool only logs a warning, mints OPENAB_SESSION_TOKEN, and continues spawning the agent. The agent then has no openab config entry and cannot reach the facade, but the session appears to start normally and the token remains registered until eviction.

Requested change: Propagate the config-write error and abort session creation, or explicitly fall back to a working transport. At minimum, log an error that browser tools are unavailable and avoid minting a token that cannot be used.

🟡 F4: Remove stale direct entries during facade rollout

Facade mode writes the static openab entry but does not remove a previous openab-browser proxy/bridge entry. The setup guide documents that both entries can then be loaded and that the model may choose the direct path, bypassing facade policy and audit. Documentation is useful mitigation but does not prevent the security-relevant misconfiguration.

Requested change: On facade-mode setup, remove only entries that are demonstrably owned by the old transport, including the corresponding Kiro agent-file entry, or make mode switching mutually exclusive and fail closed when a stale entry is detected. Add a migration test preserving unrelated user servers.

🟡 F5: Exercise the new feature tests in CI

The PR adds substantial feature-gated test coverage in openab-core/src/mcp_proxy.rs, src/browser_source.rs, and src/browser_bridge.rs, but the existing CI runs default workspace tests plus gateway ACP tests. It does not run cargo test -p openab-core --features acp-mcp or the root package tests with --features acp. The new tests can pass locally while regressions remain invisible to the merge gate.

Requested change: Add explicit CI jobs or steps for the core acp-mcp tests and the root ACP browser tests, keeping the existing gateway ACP test job as-is.

Baseline Check

• PR opened: 2026-07-24
• Base branch: main
• Merge-base: 53061d696148106b2b7529f9d6c5dd802dff4545
• Reviewed head: 19f983127dacc6aa132f1bc400603ef8fda76a57
• Diff: 24 files, 4559 additions, 141 deletions
• Main already has: the base ACP WebSocket, MCP facade primitives, and session-pool lifecycle; it does not have this reverse-MCP tunnel, browser source, or transport wiring.
• Net-new value: server-initiated ACP requests, MCP-over-ACP fan-out, session-aware browser capability routing, policy pinning, and browser transport integration.

Validation and External Feedback

• Exact head and merge-base were verified from the local clone.
• git diff --check passed.
• GitHub reports the visible PR check runs completed without a failed conclusion; the CI gap above is about feature-specific tests not being scheduled.
• Local Rust validation could not run because cargo and rustc are not installed in this review environment.
• No existing GitHub review threads were present. Existing top-level notes were considered as context; no external inline concerns were omitted.

What's Good (🟢)

• The registry's (channel_id, server_id) key and same-name last-attach-wins rule avoid reconnect ambiguity and unbounded stale entries.
• The source's independent name allowlist and tool pin prevent a client-declared server from widening the operator grant.
• Constant-time bearer checks, loopback binding, private proxy config permissions, and session-lifetime revocation are strong defaults.
• The ADR and tunnel contract explain the protocol direction, capability boundaries, and accepted residual risks clearly.

5. Three Reasons We Might Not Need This PR

1. Facade-only may be enough - Keeping proxy and bridge as supported opt-outs multiplies config and security paths after the facade becomes the default.
2. The extension contract is still a deployment dependency - Without a continuously exercised client-side browser extension, server-side tunnel correctness can regress at the wire boundary.
3. The capability surface is broad for one consumer - A single browser provider now brings multi-server fan-out, discovery caching, three transports, and policy plumbing that could be reduced until a second MCP-over-ACP provider exists.

[chaodu-obk]

Important

CHANGES REQUESTED ⚠️ - Five important correctness, resource-boundary, rollout, and CI issues are detailed in the consolidated review.

Consolidated review: #1447 (comment)

[chaodu-obk]

🟡 F1 - Gate tunnel creation on resume success

This block runs after handle_session_resume sends its response, but it does not check resp.error. A busy or over-cap resume can therefore still emit mcp/connect and register a tunnel for a session that was rejected.

Requested change: Only call spawn_acp_tunnels when the resume response is successful, and add a regression test for rejected resume requests.

[chaodu-obk]

🟡 F2 - Bound the 8 MiB frame increase

The larger limit is needed for screenshots, but it applies to every inbound text frame while up to 32 prompt tasks may retain request payloads and the outbound channel is unbounded. Please add a per-connection byte or in-flight budget, or restrict the larger allowance to bounded MCP result traffic, and test the resulting memory boundary.

[chaodu-obk]

🟡 F3 - Do not continue with an unusable facade session

If write_facade_mcp_config fails, this path only warns, then mints OPENAB_SESSION_TOKEN and starts the agent. The agent has no openab entry and cannot reach the facade, but the session appears healthy and the token remains registered until eviction.

Requested change: Propagate the config error or explicitly fall back to a working transport; at minimum report a degraded session and avoid minting an unusable token.

[chaodu-obk]

🟡 F4 - Remove or reject stale direct transport entries

Facade mode adds the openab entry but leaves any old openab-browser entry in place. The setup guide documents that an agent may then select the direct path and bypass facade policy and audit.

Requested change: Remove only entries owned by the old transport, including Kiro agent-file entries, or fail closed when a stale entry is detected. Preserve unrelated user servers and add a migration test.

[chaodu-obk]

Important

CHANGES REQUESTED ⚠️ - The resume fix is correct, but the exact head still has session-isolation, resource-boundary, lifecycle, configuration, and CI blockers.

What This PR Does

This PR lets an ACP WebSocket client expose client-side MCP servers, including a browser extension, to a colocated agent. It adds server-initiated ACP requests, MCP-over-ACP tunnel registration, a session-aware facade source, operator tool pinning, and legacy proxy/bridge transports.

How It Works

The gateway multiplexes mcp/connect and mcp/message requests over the existing /acp WebSocket and indexes tunnels by (channel_id, server_id). The root adapts that registry to an AcpTunnelSource, which resolves declared server names, filters tools by operator policy, and routes calls through facade session tokens. Proxy and bridge remain explicit downstream modes.

Findings

#	Severity	Finding	Location
1	🔴	A stale facade guard can revoke a replacement session token for the same channel	crates/openab-core/src/acp/pool.rs:421-433
2	🔴	Bridge requests can impersonate any same-UID session by supplying its channel_id	crates/openab-core/src/mcp_proxy.rs:654-680
3	🟡	The 8 MiB frame cap still has no matching byte budget or bounded outbound queue	crates/openab-gateway/src/adapters/acp_server.rs:39,826
4	🟡	Unix bridge line framing has no maximum line length	crates/openab-core/src/mcp_proxy.rs:747
5	🟡	Facade config-write errors are logged while an unusable tokenized session continues	crates/openab-core/src/acp/pool.rs:416-422
6	🟡	Switching to facade mode leaves a stale direct openab-browser entry	crates/openab-core/src/mcp_proxy.rs:523-529
7	🟡	Last-attach-wins drops replaced tunnel handles without sending mcp/disconnect	crates/openab-gateway/src/adapters/acp_server.rs:749-751
8	🟡	The current unified Pi smoke check is failing on this head	.github/workflows/ci.yml / smoke-test-unified (pi, pi-acp)

Finding Details

🔴 F1: Do not revoke a newer facade token from an older guard

Facade setup mints a token for channel_id, then installs a drop guard whose cleanup calls revoke(channel_id). SessionTokens::mint intentionally replaces the prior token for that channel. If a connection is replaced while an older connection/task still holds its guard, the old guard can run after the replacement mint and revoke the new token, making the new agent lose facade access unexpectedly.

Requested change: Make revocation token-specific (or conditional on the token still being current), and add a replacement/eviction regression test that proves an old guard cannot revoke the newer token.

🔴 F2: Authenticate bridge channel selection

The Unix socket is mode 0600, but every same-UID client can submit a frame containing an arbitrary channel_id; the server trusts that field before dispatching tools/call. A compromised or mistaken agent process can therefore select another live session's channel and invoke its browser tunnel. Process ancestry is used by the shim to discover a channel, but the socket server does not verify that the submitted identity came from that shim.

Requested change: Bind authentication to the socket connection or a per-session credential, and derive/verify the channel server-side. Do not treat a caller-supplied channel_id as the session-authentication boundary.

🟡 F3: Bound frame memory and outbound backpressure

Raising MAX_FRAME_BYTES to 8 MiB is reasonable for screenshots, but the connection still uses an unbounded outbound channel and allows 32 concurrent prompt tasks. Multiple large results and queued frames can be retained without a byte budget, so the transport can amplify memory well beyond the nominal frame cap.

Requested change: Add a bounded byte/in-flight budget and bounded outbound queue, or restrict the larger limit to a bounded result path; add a test and document the worst-case memory bound.

🟡 F4: Bound Unix bridge frames

BufReader::lines() can grow its buffer until a newline arrives. A same-UID client that sends an unterminated line can retain an arbitrarily large allocation in the socket task.

Requested change: Read with a hard maximum (for example, reject frames over the selected MCP limit) and add a regression test for an overlong unterminated frame.

🟡 F5: Fail closed when facade config setup fails

If write_facade_mcp_config fails, the pool only warns, then mints OPENAB_SESSION_TOKEN and starts the agent. The agent has no usable facade entry, while the broker registers a token and reports a seemingly healthy session.

Requested change: Propagate the config error and abort setup, or explicitly choose a verified working fallback before minting the token. Do not create a tokenized session that cannot reach the facade.

🟡 F6: Remove or reject stale direct transport entries

Facade mode writes openab but does not remove a prior openab-browser proxy/bridge entry. The documented mixed configuration exposes the same browser tools both through the audited facade and directly through the legacy transport, allowing the model to bypass facade policy and audit.

Requested change: Remove only entries demonstrably owned by the legacy transport, including Kiro agent-file entries, or fail closed when a stale entry is detected; preserve unrelated user servers and test migration.

🟡 F7: Close replaced MCP connections

The LWW registry path removes older same-name handles with retain, but the only mcp/disconnect implementation is never called. Repeated reconnect/resume cycles can leave the extension holding old MCP connection state even though OpenAB no longer routes to it.

Requested change: Collect replaced handles while holding the registry lock, then issue mcp/disconnect after releasing the lock; also cover repeated same-name reattach in a lifecycle test.

🟡 F8: Resolve the failing unified smoke check

GitHub reports smoke-test-unified (pi, pi-acp) with conclusion failure for this exact head, while the other unified jobs and the main check are successful. The failure must be diagnosed and fixed or explicitly rerun and explained before merge.

Requested change: Make the exact-head unified Pi smoke job pass, or provide a reproducible justification and CI fix if this is a flaky/environmental failure.

Addressing External Reviewer Feedback

Reviewer A (Round 1)

Rejected session/resume requests still spawn MCP tunnels.

✅ Addressed in 08a60532db94e8919b1777204c467764603c00a8: handle_session_resume now returns a channel only on success, and tunnel spawning is gated on that result. Regression coverage covers malformed, over-cap, and busy rejection paths.

The new feature tests are not exercised by repository CI.

✅ Addressed in 08a60532db94e8919b1777204c467764603c00a8: CI now runs the openab-core acp-mcp tests and root ACP tests in addition to gateway ACP tests.

The frame budget, facade setup failure, and stale transport-entry concerns remain.

ℹ Still open: These are retained as F3, F5, and F6 above because the current head does not change their behavior.

Baseline Check

• PR opened: 2026-07-24
• Base branch: main
• Merge-base: 53061d696148106b2b7529f9d6c5dd802dff4545
• Reviewed head: 08a60532db94e8919b1777204c467764603c00a8
• Diff: 25 files, 4670 additions, 166 deletions
• Main already has: the base ACP WebSocket, MCP facade primitives, and session-pool lifecycle.
• Net-new value: reverse MCP-over-ACP request routing, multi-server tunnel registration, session-aware browser capability routing, policy pinning, and browser transport integration.

Validation

• Exact-head clone verified at 08a60532db94e8919b1777204c467764603c00a8.
• Declared base and merge-base were verified locally; git diff --check passed.
• GitHub check and validate jobs passed, and most smoke jobs passed.
• GitHub smoke-test-unified (pi, pi-acp) failed on the reviewed head; see F8.
• Local Rust and Python runtimes are unavailable, so local Rust tests and Python syntax validation could not run.

What's Good (🟢)

• The rejected-resume fix gates all tunnel side effects on successful resume and includes focused regression tests.
• The compound (channel_id, server_id) registry and same-name last-attach-wins rule make reconnect routing deterministic.
• Constant-time bearer checks, loopback binding, private proxy config permissions, and session-lifetime token handling are strong defaults.
• The facade source applies independent server-name and tool-pin gates, with deny-all behavior for unlisted tools and cache filtering that cannot widen policy.
• The added feature-gated CI steps and broad unit coverage materially improve regression detection.

5. Three Reasons We Might Not Need This PR

1. Facade-only may be sufficient - Retaining proxy and bridge as opt-outs multiplies configuration and security paths after facade mode becomes the default.
2. The extension contract remains a deployment dependency - Without a continuously exercised client-side browser extension, server-side tunnel behavior can regress at the wire boundary.
3. The capability surface is broad for one consumer - Multi-server fan-out, discovery caching, three transports, and policy plumbing may be more scope than needed until a second MCP-over-ACP provider exists.

[chaodu-obk]

Important

CHANGES REQUESTED ⚠️ - The resume fix is correct, but the exact head still has session-isolation, resource-boundary, lifecycle, configuration, and CI blockers.

What This PR Does

This PR lets an ACP WebSocket client expose client-side MCP servers, including a browser extension, to a colocated agent. It adds server-initiated ACP requests, MCP-over-ACP tunnel registration, a session-aware facade source, operator tool pinning, and legacy proxy/bridge transports.

How It Works

The gateway multiplexes mcp/connect and mcp/message requests over the existing /acp WebSocket and indexes tunnels by (channel_id, server_id). The root adapts that registry to an AcpTunnelSource, which resolves declared server names, filters tools by operator policy, and routes calls through facade session tokens. Proxy and bridge remain explicit downstream modes.

Findings

#	Severity	Finding	Location
1	🔴	A stale facade guard can revoke a replacement session token for the same channel	crates/openab-core/src/acp/pool.rs:421-433
2	🔴	Bridge requests can impersonate any same-UID session by supplying its channel_id	crates/openab-core/src/mcp_proxy.rs:654-680
3	🟡	The 8 MiB frame cap still has no matching byte budget or bounded outbound queue	crates/openab-gateway/src/adapters/acp_server.rs:39,826
4	🟡	Unix bridge line framing has no maximum line length	crates/openab-core/src/mcp_proxy.rs:747
5	🟡	Facade config-write errors are logged while an unusable tokenized session continues	crates/openab-core/src/acp/pool.rs:416-422
6	🟡	Switching to facade mode leaves a stale direct openab-browser entry	crates/openab-core/src/mcp_proxy.rs:523-529
7	🟡	Last-attach-wins drops replaced tunnel handles without sending mcp/disconnect	crates/openab-gateway/src/adapters/acp_server.rs:749-751
8	🟡	The current unified Pi smoke check is failing on this head	.github/workflows/ci.yml / smoke-test-unified (pi, pi-acp)

Finding Details

🔴 F1: Do not revoke a newer facade token from an older guard

Facade setup mints a token for channel_id, then installs a drop guard whose cleanup calls revoke(channel_id). SessionTokens::mint intentionally replaces the prior token for that channel. If a connection is replaced while an older connection/task still holds its guard, the old guard can run after the replacement mint and revoke the new token, making the new agent lose facade access unexpectedly.

Requested change: Make revocation token-specific (or conditional on the token still being current), and add a replacement/eviction regression test that proves an old guard cannot revoke the newer token.

🔴 F2: Authenticate bridge channel selection

The Unix socket is mode 0600, but every same-UID client can submit a frame containing an arbitrary channel_id; the server trusts that field before dispatching tools/call. A compromised or mistaken agent process can therefore select another live session's channel and invoke its browser tunnel. Process ancestry is used by the shim to discover a channel, but the socket server does not verify that the submitted identity came from that shim.

Requested change: Bind authentication to the socket connection or a per-session credential, and derive/verify the channel server-side. Do not treat a caller-supplied channel_id as the session-authentication boundary.

🟡 F3: Bound frame memory and outbound backpressure

Raising MAX_FRAME_BYTES to 8 MiB is reasonable for screenshots, but the connection still uses an unbounded outbound channel and allows 32 concurrent prompt tasks. Multiple large results and queued frames can be retained without a byte budget, so the transport can amplify memory well beyond the nominal frame cap.

Requested change: Add a bounded byte/in-flight budget and bounded outbound queue, or restrict the larger limit to a bounded result path; add a test and document the worst-case memory bound.

🟡 F4: Bound Unix bridge frames

BufReader::lines() can grow its buffer until a newline arrives. A same-UID client that sends an unterminated line can retain an arbitrarily large allocation in the socket task.

Requested change: Read with a hard maximum (for example, reject frames over the selected MCP limit) and add a regression test for an overlong unterminated frame.

🟡 F5: Fail closed when facade config setup fails

If write_facade_mcp_config fails, the pool only warns, then mints OPENAB_SESSION_TOKEN and starts the agent. The agent has no usable facade entry, while the broker registers a token and reports a seemingly healthy session.

Requested change: Propagate the config error and abort setup, or explicitly choose a verified working fallback before minting the token. Do not create a tokenized session that cannot reach the facade.

🟡 F6: Remove or reject stale direct transport entries

Facade mode writes openab but does not remove a prior openab-browser proxy/bridge entry. The documented mixed configuration exposes the same browser tools both through the audited facade and directly through the legacy transport, allowing the model to bypass facade policy and audit.

Requested change: Remove only entries demonstrably owned by the legacy transport, including Kiro agent-file entries, or fail closed when a stale entry is detected; preserve unrelated user servers and test migration.

🟡 F7: Close replaced MCP connections

The LWW registry path removes older same-name handles with retain, but the only mcp/disconnect implementation is never called. Repeated reconnect/resume cycles can leave the extension holding old MCP connection state even though OpenAB no longer routes to it.

Requested change: Collect replaced handles while holding the registry lock, then issue mcp/disconnect after releasing the lock; also cover repeated same-name reattach in a lifecycle test.

🟡 F8: Resolve the failing unified smoke check

GitHub reports smoke-test-unified (pi, pi-acp) with conclusion failure for this exact head, while the other unified jobs and the main check are successful. The failure must be diagnosed and fixed or explicitly rerun and explained before merge.

Requested change: Make the exact-head unified Pi smoke job pass, or provide a reproducible justification and CI fix if this is a flaky/environmental failure.

Addressing External Reviewer Feedback

Reviewer A (Round 1)

Rejected session/resume requests still spawn MCP tunnels.

✅ Addressed in 08a60532db94e8919b1777204c467764603c00a8: handle_session_resume now returns a channel only on success, and tunnel spawning is gated on that result. Regression coverage covers malformed, over-cap, and busy rejection paths.

The new feature tests are not exercised by repository CI.

✅ Addressed in 08a60532db94e8919b1777204c467764603c00a8: CI now runs the openab-core acp-mcp tests and root ACP tests in addition to gateway ACP tests.

The frame budget, facade setup failure, and stale transport-entry concerns remain.

ℹ Still open: These are retained as F3, F5, and F6 above because the current head does not change their behavior.

Baseline Check

• PR opened: 2026-07-24
• Base branch: main
• Merge-base: 53061d696148106b2b7529f9d6c5dd802dff4545
• Reviewed head: 08a60532db94e8919b1777204c467764603c00a8
• Diff: 25 files, 4670 additions, 166 deletions
• Main already has: the base ACP WebSocket, MCP facade primitives, and session-pool lifecycle.
• Net-new value: reverse MCP-over-ACP request routing, multi-server tunnel registration, session-aware browser capability routing, policy pinning, and browser transport integration.

Validation

• Exact-head clone verified at 08a60532db94e8919b1777204c467764603c00a8.
• Declared base and merge-base were verified locally; git diff --check passed.
• GitHub check and validate jobs passed, and most smoke jobs passed.
• GitHub smoke-test-unified (pi, pi-acp) failed on the reviewed head; see F8.
• Local Rust and Python runtimes are unavailable, so local Rust tests and Python syntax validation could not run.

What's Good (🟢)

• The rejected-resume fix gates all tunnel side effects on successful resume and includes focused regression tests.
• The compound (channel_id, server_id) registry and same-name last-attach-wins rule make reconnect routing deterministic.
• Constant-time bearer checks, loopback binding, private proxy config permissions, and session-lifetime token handling are strong defaults.
• The facade source applies independent server-name and tool-pin gates, with deny-all behavior for unlisted tools and cache filtering that cannot widen policy.
• The added feature-gated CI steps and broad unit coverage materially improve regression detection.

5. Three Reasons We Might Not Need This PR

1. Facade-only may be sufficient - Retaining proxy and bridge as opt-outs multiplies configuration and security paths after facade mode becomes the default.
2. The extension contract remains a deployment dependency - Without a continuously exercised client-side browser extension, server-side tunnel behavior can regress at the wire boundary.
3. The capability surface is broad for one consumer - Multi-server fan-out, discovery caching, three transports, and policy plumbing may be more scope than needed until a second MCP-over-ACP provider exists.

[brettchien]

Thanks for both passes. Every finding from both rounds was reproducible against the code, and all
nine are addressed — each in its own commit with regression coverage. Two judgement calls, one
correction to a fix of my own, and one push-back are called out below.

Round 2

R1 — an older guard could revoke a newer facade token (1970bae5)

Revocation is now token-specific end to end: the registrar's revoke takes the token,
SessionTokens gains revoke_token beside the existing revoke_channel, and the pool's drop
guard carries the token it minted rather than the channel it minted for. Session lifetimes overlap
— the successor mints while the predecessor's guard is still pending — so a late teardown is now a
no-op instead of a silent outage. revoke_channel stays for deliberate channel-wide eviction, and
both methods document which case they serve.

R2 — bridge channel selection was unauthenticated (63fb63a7)

The socket authenticated nothing: 0600 proves the peer shares our uid, not which session it
belongs to, and the channel_id in each frame is a value the caller picks. The channel is now
derived server-side from the SO_PEERCRED peer pid using the same /proc ancestry walk the shim
already performed — the logic was right but ran on the side that can lie about its own answer. A
connection whose channel cannot be established is refused outright rather than given a default
session. Frames may still carry channel_id, but it is only compared against the authenticated
value; a mismatch is dropped and logged, so a misconfigured shim is visible rather than silently
honoured. The ancestry helpers moved into core beside the server that authenticates with them, so
there is one implementation rather than two that can drift.

R1 and R2 are the same shape: authorising on an identifier too coarse for the decision. One had
"same uid" standing in for "same session"; the other had "same channel" standing in for "same
session instance".

R7 — replaced tunnels were dropped without mcp/disconnect (3aa8f6a1)

Stale handles are now taken out of the registry rather than dropped by retain, and disconnected
after the lock is released — forced rather than preferred, since disconnect is async and the
registry is a std::sync::Mutex. The disconnects run in a spawned task: a replaced connection is
often already dead, which is usually why it is being replaced, so waiting on its response would
stall the tunnel that just came up. The test pins the connectionId of the replaced connection;
without that, an implementation that disconnected the freshly registered tunnel would pass equally.

R4 — unbounded bridge line framing (d9a2679a)

BufReader::lines() is replaced by a bounded reader that errors once a pending frame would exceed
the cap, with the connection dropped because a stream cannot be resynchronised mid-frame. The cap
matches the ACP tunnel's own frame ceiling so the bridge is never the tighter bottleneck for
legitimate traffic. Worth noting the trigger is broader than the finding states: no malice is
required, since a shim that dies mid-line pins the same allocation — which is why the bound still
matters even though R2 narrowed who can reach the socket. Framing is now byte-oriented, so a
non-UTF8 frame is skipped like any other malformed one instead of ending the read loop.

F8 — the failing unified Pi smoke check

This one is infrastructure, and I would ask for a rerun rather than a code change. In run
30324392489, job smoke-test-unified (pi, pi-acp) failed inside docker/setup-buildx-action@v3
while booting buildkit: pulling moby/buildkit:buildx-stable-1 timed out against
registry-1.docker.io (context deadline exceeded while awaiting headers). That is before any
repository code is fetched, built or executed, so no change on this head could have caused it or
can fix it. The same workflow succeeded on an earlier head of this branch and has been skipped on
subsequent ones. Happy to be told otherwise if you are seeing a different failure mode.

Round 1

F5 — feature-gated tests never ran in CI (b6fdeb4a, corrected in 34b5f2a2)

cargo test --workspace builds with default features, so the acp-mcp tests in
openab-core/src/mcp_proxy.rs and the acp tests in the root package were never compiled. Two
steps were added mirroring the existing acp-gateway one. The core step is filtered to mcp_proxy::
— acp-mcp gates exactly one module, so the filter loses no coverage, and an unfiltered
-p openab-core would pull in the hooks::tests flake the gateway step already documents avoiding.

I got this wrong on the first attempt and it is worth flagging rather than quietly fixing. The
filter ends in mcp_proxy::, and a trailing colon in a YAML plain scalar reads as a mapping
indicator, so ci.yml stopped parsing and GitHub failed the entire workflow with a workflow-file
error. CI was red on this branch from that commit until 34b5f2a2 quoted the command. The local
gates stayed green throughout because they run cargo directly and never parse the workflow, and the
broken runs show up under the file path rather than the workflow's name — when a workflow cannot be
parsed, its name: cannot be read either. On 34b5f2a2 the added steps run and pass: 30 tests for
the core filter and 42 for the root package.

F1 — tunnels opened after a rejected session/resume (08a60532)

The loop derived its own channel_id from the requested sessionId and used that as the spawn
condition, but a well-formed sess_<uuid> derives successfully on all four rejection paths, so the
guard was not checking what it appeared to. With last-write-wins re-attach, a refused resume could
evict the live tunnel it had just been refused in favour of. handle_session_resume now returns
(JsonRpcResponse, Option<String>), handing back the channel only on success, and the independent
derivation is deleted rather than left beside a second check. Coverage asserts a None channel on
each rejection; over-cap and busy are the load-bearing cases, since their ids are well formed and
their sessions really exist.

F3 — continuing with an unusable facade session (a3068b3a)

Mints only when the config write succeeded; the session still starts, without browser capabilities,
which is the honest description of what happened. Aborting session setup would let a config-write
failure kill an otherwise working agent, and falling back to a direct transport would write into the
same workdir — likely failing too — while silently re-creating the bypass F4 removes.

F4 — stale direct entry bypassing facade policy (16df6de2)

Facade setup retires the entry it replaces in all three locations, including the kiro agent files
and their @openab-browser grant — allowedTools is default-deny, so a leftover grant is what
keeps the bypass reachable once the server entry is gone. Ownership is decided by exact shape, never
by the key: only the bridge entry and the loopback proxy entry we wrote are removable, and anything
else under that name is preserved verbatim. The matcher errs toward under-removal, because a
leftover entry only preserves the bypass while deleting an operator's configuration destroys work.

F2 — the 8 MiB frame allowance (28cad689)

Took the suggested direction. One note, because the obvious reading does not work: mcp/message is
only ever sent outbound, so no inbound frame carries that method — browser results arrive as client
responses, id present and no method. Matching on method == "mcp/message" would have capped
the screenshot responses at 1 MiB and broken the case the raise exists for. Responses therefore keep
the 8 MiB ceiling and every method-bearing frame, session/prompt included, is held to the
pre-existing 1 MiB — which is what removes the exposure, since the worst case came from in-flight
prompts. The 8 MiB check stays pre-parse and still closes the connection, as an oversized frame
cannot be parsed back to its id; the per-kind check runs after parsing, where oversized requests get
ACP_OVERLOADED with their id and oversized notifications are dropped without a reply.

Deliberately not changed

The unbounded outbound channel. It predates this PR — the comment above the caps already defers a
bounded outbound channel, idle eviction and global connection limits to the F6 follow-up — and the
F2 change bounds only the exposure this PR introduced. Happy to pull it forward if you would rather
not carry it.