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.

[brettchien]

Correction to my previous reply first, because it matters more than the fixes. I wrote that
ownership of a stale openab-browser entry was decided by exact shape and that "anything else
under that name is preserved verbatim". That was true for the bridge entry and false for the
proxy entry. The check claimed to recognise "a bearer we minted" while comparing against nothing
we had kept, so any loopback url carrying any Bearer header matched — which is exactly the shape
of a local MCP server an operator runs themselves. Facade setup would have deleted their
configuration. That is fixed in e308e753 (below), but the wrong claim was in the reply, not just
in the code, and I would rather retract it explicitly than let it stand as something you had been
told and could reasonably rely on.

Round 3

R3-F2 — prove ownership before deleting (e308e753)

Fails closed now. Only the bridge entry qualifies: it is byte-identical every session and names our
own binary and subcommand, so matching it is itself the proof. The per-session proxy url and bearer
were never recorded anywhere, so ownership of that shape cannot be established and it is preserved.

Preserving costs little, which is what makes failing closed the right trade rather than a cop-out: a
leftover proxy entry names an ephemeral port belonging to a session that is gone —
start_session_server binds 127.0.0.1:0 and drops the listener with the session — so it is dead
configuration rather than a live bypass. The bridge entry is the one that would still resolve and
run, and it is still removed. The regression test proves an operator's own entry survives verbatim
in both .cursor/mcp.json and a kiro agent file, along with its @openab-browser grant; revoking
the grant alone would silently disable their server even with the entry intact.

R3-F1 — bound the declaration fan-out (399d892d)

accept_acp_servers dedupes by id and enforces a documented per-session maximum, in both
session/new and session/resume, before the session is inserted or any task is spawned — an
over-declaring request never reaches the work. Resume is guarded too because the client re-presents
its declarations every time, which makes it an equally good burst vector.

Over the cap the whole request is refused rather than truncated: truncating would silently honour
part of a request the client cannot know was clipped, leaving it to debug servers that never got
tunnels as a mysterious "not connected". Dedup runs before the cap, so repeats of one id are one
server rather than a cap violation. Tests cover the boundary, a five-thousand entry flood refused
rather than clipped, order-preserving duplicate collapse, and that exactly one task and one
mcp/connect result per unique declaration.

[brettchien]

Self-review findings on the current head — posting before you spend time on them

Separately from your three rounds, I ran an internal review pass over 399d892d — five independent
passes, one per surface (config rewriting, the ACP server, the capability source and bridge, token
lifecycle and CI wiring, docs-vs-code) — and re-read every candidate finding against the source
before writing it down. It turned up defects your rounds had not reached, including three that are
earlier fixes reappearing one room over. Posting them rather than quietly fixing them, since
some bear directly on claims I made in earlier replies.

Fixes are in flight; this is the list, not a request for a re-review.

The three repeats — these are the ones worth your attention

1. SessionTokens::mint still clobbers by channel (crates/openab-mcp/src/mcp/sources.rs:99).
Round 2's R1 made revocation token-specific, and I explained in that reply why the guard has to
carry the token rather than the channel. I never revisited the mint side, which still does
map.retain(|_, ctx| ctx.channel_id != channel_id). The connection pool explicitly supports racing
builders (pool.rs:611-620): A mints t1 and wins the insert; B mints t2, whose retain deletes
t1; B then sees A healthy, returns Ok(false), and drops the connection object holding its own
revoke guard, which revokes t2. Both tokens are gone while A's agent keeps presenting t1 — every
facade call fails auth and all session-gated tools silently disappear from its catalog. R1's fix was
correct and insufficient: I hardened the revocation path and left the same
one-entry-per-channel assumption living in mint.

2. session/resume stores the raw, uncapped declaration list
(crates/openab-gateway/src/adapters/acp_server.rs:1488). R3-F1 added accept_acp_servers and I
told you it runs "before the session is inserted or any task is spawned" on both paths. That is true
of the spawn path. The resume handler independently re-parses the raw params and stores those
on the session record, so the cap governs what gets spawned but not what gets retained. Since the
cap applies to the deduped count, thousands of duplicate declarations fit inside the 1 MiB frame,
pass the gate, and are held per session.

This is round-1 F1's shape exactly — a second, unchecked derivation living beside the checked one —
and F1's lesson was to delete the independent derivation rather than add a check next to it. I
applied that lesson to one path and recreated the pattern on the other in the same commit.

3. Two new tests are compiled but never executed (.github/workflows/ci.yml:66). The step added
for F5 filters on mcp_proxy::; the pool facade tests are acp::pool::tests::*, and the
--workspace step compiles them out under default features. F5 fixed "is the test compiled"; this
is "is it selected". So the token-lifecycle tests I cited as coverage in the round-2 reply have
never actually run in CI.

Config-rewriting defects (the code that edits an operator's mcp.json)

4. A malformed config file is silently replaced wholesale (crates/openab-core/src/mcp_proxy.rs:290,
also :474, :555): from_slice(&bytes).unwrap_or_else(|_| json!({})). Any file that fails to
parse — JSONC comments, a trailing comma, or a file truncated by finding 6 — is replaced by a fresh
object containing only our entry, destroying every other server the operator configured. The kiro
agent-file writer in the same file refuses to do exactly this (:381, "never clobber"), so the
correct policy is already present and applied inconsistently.

5. A valid-JSON non-object root panics session setup (:293-296): for a root of [],
cfg["mcpServers"] = json!({}) is IndexMut on an array.

6. Config writes are neither atomic nor serialized (:298, :334, :401, :483, :572):
truncate-then-write, no temp+rename, no lock. A crash mid-write leaves truncated JSON, which finding
4 then clobbers; two sessions sharing a workdir — the case the code documents at :308 — can
interleave read/modify/write and lose each other's entries.

7. The bearer-carrying file is briefly world-readable (write_private, :344-351): it writes
first and chmod 0600 after, leaving the token at umask permissions in between.

Trust surface

8. The ADR instructs operators to write an allowlist entry that disables browser control.
docs/adr/acp-server-websocket-reverse-mcp.md:290, :300, :355 say the allowlist defaults to
browser; the built-in default is katashiro, and any operator entry replaces the default
wholesale. Following the ADR yields deny-all for the real server, with nothing obviously wrong in
the config. §7.1 records the rename; §6.1, §6.4 and F4 were never updated.

9. Proxy and bridge modes forward any tool name to the tunnel (mcp_proxy.rs:148, :740);
browser_tools() gates only tools/list. The facade path is sound here — the call path re-checks
membership independently of the catalog, with tests that fail if either check is removed — so this
is confined to the two transports F5 retires, but the "static advertised set" property does not hold
for them today.

10. A typo'd config key silently widens access: no deny_unknown_fields, and an empty
acp_servers reverts to the built-in default, so [[mcp.acp_server]] (missing s) is ignored and
the operator who meant to narrow gets the full default set. Deny-all is also currently
inexpressible. A typo in a per-entry tools list fails safe, so the two directions disagree.

11. The bridge still has the unbounded-line bug R4 fixed server-side
(src/browser_bridge.rs:85, :99 use bare read_line, while read_frame_bounded exists in core
for precisely this reason). Same-uid peers, so low severity — but it is the same defect twice.

Test quality — the reason finding 2 survived

Nothing drives the WebSocket read loop. declaration_fan_out_is_capped_and_deduplicated and
only_accepted_declarations_spawn_tunnels exercise accept_acp_servers in isolation, so deleting
its call site entirely leaves the suite green — the "enforced before any allocation, on both
paths" property is asserted only by comments. Also: the frame-cap test feeds std::io::Cursor,
which yields everything in one fill_buf, so the multi-fill accumulation branch never executes and
an implementation bounding only per-chunk length passes; the tools/call routing fake ignores
params, so dropping arguments passes; channel isolation is untested because the fake tunnel
ignores channel_id, and browser_tunnel.rs has no tests at all.

Follow-ups, not being fixed in this PR

The discovery cache is insert-only and never evicted, and caches the extension's published tool list
unfiltered. Tunnel tasks share the in-flight prompt budget, so pending mcp/connects can block
prompts on the connection. start_session_server's error paths drop the cancellation token without
cancelling, leaving the loopback server running. Cross-connection teardown can evict a live tunnel —
the reply-registry twin of this is already an accepted F5 residual; the tunnel registry inherits it
un-annotated. Invalid declarations are dropped silently rather than rejected, and duplicate
[[mcp.acp_servers]] names last-wins without a warning.

Confirmed sound, for what it is worth

The facade trust gate is enforced on both the catalog and the call path, with tests that fail if
either check is removed; tool-name matching is exact and fail-closed, with no alias, case-fold or
dot-collapse escape; channel_id is never caller-supplied; tokens are 32-byte getrandom,
constant-time compared, never logged, and the on-disk config carries only a placeholder; the revoke
guard covers every exit path including panic unwind, and revoke_channel has no production call
sites left; no .await is held across a std::sync::Mutex; pending requests are drained on close
and timeouts remove their pending entry; notification-versus-request semantics are correct and
unsolicited responses are never answered; the wire contract matches the code field for field.

[brettchien]

Answering "Three Reasons We Might Not Need This PR" — we are removing both legacy transports

You raised this in all three rounds and we never answered it. Answering it now: you were right,
and the change is being made in this PR, not promised as a follow-up.

proxy and bridge are both removed. Facade becomes the only transport. Concretely that deletes
the stdio shim and the openab browser-bridge subcommand, the per-pod unix socket server and its
/proc ancestry channel resolver, the OPENAB_BROWSER_CHANNEL injection, the per-session loopback
HTTP server with its bearer and per-session config write/cleanup, and the bridge and proxy arms of
the mode switch.

The reason is stronger than "fewer paths". The design record shows bridge was chosen because a
fixed HTTP endpoint could not do per-session browser control — one shared mcp.json, one
endpoint, no way to tell which session a call came from. The facade is a fixed endpoint that solves
exactly that, with a per-session bearer in the agent's environment referenced from the shared file.
So the requirement that justified bridge no longer exists. The one open risk on that argument —
whether a vendor CLI expands ${VAR} inside a header value — has now been checked against a live
deployment rather than assumed: the config on disk carries the literal placeholder and the facade
path works end to end.

Breaking change, stated plainly: browser control now requires an [mcp] block. A deployment
without one previously fell through to the per-session proxy silently — no opt-in, and that
fallback path is the one with no tool allowlist. It will now simply have no browser control. We
chose that over auto-starting a listener, because opening a port and rewriting an agent workdir on
upgrade is the very coupling concern raised separately in round 3. Nothing starts implicitly any
more.

Four of the defects below leave by deletion rather than by fix: bridge authentication being
forgeable, tool names forwarded ungated on proxy and bridge, the orphan listener in
start_session_server, and the unbounded line reads in the shim.

On re-scoping mid-review: narrowing a PR after a review round can look like shrinking the
findings list instead of answering it. So, to be explicit — the full list stays below, every
remaining item is still being fixed, and the four above are named as deletions rather than quietly
dropped. Note also that the config-writer defect (item 7) is not rescued by this: the same
clobber-on-parse-failure and non-atomic write live in the facade's own writer, which is now the only
code that touches a user's mcp.json — so it matters more after this change, not less.

Your other two points are answered at the end of this comment.

Correction to an earlier security claim, and the current consolidated defect list

Two things in this comment: a retraction that should not wait, and the full list as it now stands so
this thread is the single source of truth rather than three partial ones.

Retraction — R2 (bridge socket authentication) was not closed

(The code discussed here is being deleted, but the retraction stands on its own: we told you something was fixed when it was not.)

In my round-2 reply I wrote that the bridge channel "is now derived server-side from the
SO_PEERCRED peer pid using the same /proc ancestry walk the shim already performed", and that a
connection whose channel cannot be established is refused. The first half is what makes the claim
wrong in a way that matters: the derivation runs server-side, but the data it reads is still
supplied by the peer. channel_from_process_ancestry (crates/openab-core/src/mcp_proxy.rs:787)
begins its walk at the connecting pid itself and reads /proc/<peer>/environ on the first
iteration, so a same-uid process that sets the variable in its own environment is believed before
any ancestor is consulted. The legitimate shim does not set it, which is why this was not visible in
testing.

So R2 moved the decision to a place that looks authoritative while leaving the input
attacker-chosen. That is the same defect the finding described, and my reply asserted it was fixed.
Treat that assertion as withdrawn.

The replacement mechanism is not settled, and I overstated it above. An earlier revision of
this comment called a per-session OPENAB_BRIDGE_SECRET — minted by the pool, injected into the
agent's environment, presented by the shim on connect — the "agreed fix". One reviewer does not
agree, and the objection is the sharper one: an environment secret lives in /proc/<pid>/environ,
which is exactly the surface this defect is about, so a same-uid process may be able to read the
credential rather than forge the claim — the same threat model, one step later. (Whether it can in
practice depends on the deployment's ptrace_scope, which is not something the fix should rest on.)
The shim's environment also gets scrubbed in some paths.

The alternatives on the table are server-side: a pidfd, or a pid + start-time generation map, or a
credential passed as an inherited file descriptor that the peer cannot read out of its own process
image. Ancestry verification is rejected either way — environ is self-injectable, a process can
fork() to shape its own ancestor chain, and pid reuse plus pid namespaces make "was this pid
spawned by us" unreliable on its own. The defect is confirmed by everyone; the mechanism is still
being argued, and I am not going to record agreement that does not exist.

Two smaller corrections to my previous comment while I am here:

• I wrote that deny-all is "currently inexpressible". That is wrong: an entry with an empty tools
  list yields an empty allowlist for that server, and because any explicit entry replaces the
  built-in default wholesale, unlisted servers are denied too. Only the empty section cannot
  express it. The real defect is the missing deny_unknown_fields, which lets a mistyped section be
  ignored and silently fall back to the full default set.
• I described roughly a dozen stale #[allow(dead_code)] attributes as disabling dead-code
  detection for the module. They are per-item attributes and do not do that; the count is 13, of
  which one is still doing work.

Current list — 11 defects, all independently confirmed

Every item below was verified against the source by at least two reviewers working independently,
and each of the six marked ✅ I also traced myself. Items 1–4 share one root cause: identity
inferred from data the other party controls, or from a key shared between parties.

#	Defect	Where
1 ✅	Bridge authentication is forgeable (above) — removed by deleting bridge	mcp_proxy.rs:787
2 ✅	Connection teardown removes tunnels by channel_id from a registry created once and shared across every WS connection, so an old connection's cleanup evicts a live tunnel — and the reply sink — belonging to a newer one	acp_server.rs:1302, main.rs:473
3 ✅	Token mint still evicts by channel (R1 fixed only the revoke side), and reset_session removes the creating gate that purge_session_entries documents must never be removed	sources.rs:99, pool.rs:825 vs :155
4 ✅	The tool allowlist exists only in the facade path; proxy and bridge forward tool names ungated — removed by deleting both transports	mcp_proxy.rs:148, :740
5 ✅	handle_initialize never advertises agentCapabilities.mcpCapabilities.acp, which the RFD requires	acp_server.rs
6 ✅	The tunnel never performs the inner MCP initialize/notifications/initialized lifecycle before tools/list or tools/call	acp_server.rs:769
7	The config writer clobbers a file it failed to parse, panics on a JSON array root, is non-atomic, and chmods after writing a file carrying a live bearer	mcp_proxy.rs:290, :294, :348
8	start_session_server spawns the listener before fallible steps; any later ? returns without cancelling, leaving an orphan listener with a live bearer — removed with proxy	mcp_proxy.rs:266
9	Resume is not reconciliation: it stores the raw uncapped declaration list, never removes tunnels for withdrawn declarations, and lets repeated resumes pile up connect tasks	acp_server.rs:1488
10	Tunnel establish tasks are pushed into the prompt task set, so pending mcp/connects consume the in-flight prompt budget and real prompts get a misleading overload error	acp_server.rs:854, :1169
11	The channel id is logged at info level; the session id is derivable from it, so an operator's logs hand out the credential needed to resume someone else's session	acp_server.rs

The one open disagreement is now closed — toward the stricter reading. Item 6's severity was
contested: one reviewer held it a compliance gap that no current deployment trips, since the
extension in use answers tools/list without a handshake; the other held that this PR's stated
deliverable is generic client-declared MCP servers, so a standards-compliant server being unusable
is an interoperability failure that should not be downgraded because today's single implementation
happens to be lenient. The first reviewer has since withdrawn the downgrade and both now agree:
standards compliance is compatibility for a generic reverse proxy. Item 6 is treated as blocking.

Item 11 was added after the list above was first published, and is worse than first described: the
acp_<uuid> emitted at info level does not merely reveal that a session exists — the sess_<uuid>
needed to resume it is derivable from it. Drop it to debug! or redact it.

Items 2 and 9 are not done when the teardown filter is fixed. A fix is only acceptable if it
also covers three cases a naive owner check misses: a late establish must not overwrite a
successor's handle; an aborted task must not insert a dead handle after cleanup has run; and a
same-key replacement must not silently drop the connection it replaced. These are the cases that
make an ownership generation necessary rather than a "only remove what I inserted" test.

Also being fixed here, though it is a performance defect rather than a correctness one: the
screenshot path deep-clones the payload twice. Both reviewers argued it belongs in this PR — it is
the steady-state hot path, it wastes megabytes per screenshot, and moving the value out is an
idiom rather than a refactor. I had it deferred; they were right.

Fixed before merge, though not blocking on their own — these are drift this PR itself
introduced, so they are not being deferred out of it: the missing #[serde(deny_unknown_fields)],
which lets a mistyped config section be ignored and silently fall back to the full default tool set
(described in the corrections above but not previously listed as work); the smoke test still
asserting the old 1 MiB framing semantics; and the documentation drift — browser vs katashiro
in the ADR, the reversed default-mode note, §6.6 still listing F1′/F3′/F4 as remaining when they
shipped in this PR, the setup guide's jq 'del(…)' file cleanup commands that print to stdout
instead of editing the file, and the tunnel contract having no limits section at all, so an
extension implementer cannot see the per-session server cap, the method-frame ceiling or the tunnel
timeout.

Known and deliberately deferred: the discovery cache having no eviction, consolidation of the
remaining config writers (a pure refactor, not appropriate mid-review), and kiro agent files being
chmod'd 0600 as a side effect of a path that carries no secret. The unbounded line reads that were
listed here previously are gone with the shim.

The PR stays in draft until items 1–11 land. I would rather post a list this long than have you find
these one round at a time.

What we are actually doing, in order

Deletions first, because they remove work rather than add it — three of the defects below stop
existing, and every later fix is then made against the final shape instead of against code that is
about to vanish.

1. Remove bridge — shim, subcommand, socket server, /proc ancestry resolver, channel injection,
   config writer, mode arm.
2. Remove proxy and make [mcp] required — per-session server, bearer, per-session config
   write/cleanup, and the silent Facade → Proxy demotion. With no [mcp], one INFO line saying
   browser control is unconfigured; nothing starts.
3. Add a WebSocket integration test — binds the real /acp route and drives a scripted client
   through initialize → session/new declaring a type:acp server → answering a real
   mcp/connect, then puts a genuine mcp/message across the socket and asserts both a result and
   an error, plus the mcp/disconnect on same-name replacement. No browser and no model, so it runs
   in CI. This is our answer to your second reason, and it gives the tunnel adapter its first tests.
4. Item 2 — teardown ownership, with three acceptance cases: a late establish must not overwrite a
   successor, an aborted task must not insert a dead handle after cleanup, and a same-key
   replacement must not silently drop the connection it replaced.
5. Item 3(b) — reset_session removing the creating gate that the neighbouring function documents
   must never be removed. 3(a) is already fixed.
6. Item 9 — resume as reconciliation, including retiring tunnels for withdrawn declarations. The
   test drives the read loop this time; testing the helper in isolation is what let this through.
7. Item 10 — separate task budgets for tunnel establishment and prompts.
8. Item 7 — the facade config writer: fail closed on parse failure, guard the non-object root,
   0600 temp file plus rename, serialised per path, with the destructive cases regression-tested.
9. Item 11 — stop logging a derivable resume credential.
10. Item 5 — advertise mcpCapabilities.acp.
11. Item 6 — the inner MCP initialize lifecycle.
12. The pre-merge cleanups listed above, plus removing every remaining mention of the two deleted
    transports from the docs and the ADR.

Then the PR comes out of draft. We would rather show you the plan than have you infer it from a
series of commits.