feat(facade): session-aware in-process capability sources - #1454
Merged
Conversation
- sources module: CapabilitySource trait (optional SessionCtx per call, requires_session gating), SessionTokens registry (broker mints one opaque token per agent session, constant-time resolve, revoke on evict), per-request identity resolution from the Authorization header via the HTTP parts rmcp injects into request extensions. - facade: with_sources/serve_http_with; discovery merges source tools (downstream names win, shadowed source tools published as provider:tool); execution dispatches sources first with the same schema pre-flight the meta-tool applies, audit lines carry channel. - Session-bound sources are invisible AND unreachable to anonymous clients (no dangling catalog entries, nothing to probe). - Anonymous behavior with zero sources is byte-identical to before. Groundwork for routing #1447's per-session browser capability through the facade (one listener, one discovery surface).
…r session tokens Self-review findings: - BUG: execution dispatched sources before the downstream catalog while discovery publishes downstream-wins-bare-name — a source tool sharing a downstream tool's name would hijack calls addressed to the downstream tool. Execution now resolves the catalog first (mirroring the publish rule); shadowed source tools stay reachable via provider:tool. - GAP: the rmcp Parts-into-extensions mechanism (which token resolution rides on) had no end-to-end coverage. Added build_router + a tower oneshot test driving real HTTP initialize→tools/call with/without/ wrong Authorization, asserting source visibility flips accordingly. - constant_time_eq: black_box the accumulator so the claim is enforced, not asserted in a comment.
…ce calls Self-review round 2: - Discovery's collision set now grows as sources publish, so two sources sharing a tool name no longer produce duplicate catalog entries — the first registrant keeps the bare name, later ones publish as provider:tool (matching execution's registration-order resolution). - Source-call audit lines now carry args_sha256 like the meta_tool dispatcher's — one audit shape across both dispatch paths.
thepagent
approved these changes
Jul 25, 2026
pahud
pushed a commit
to brettchien/openab
that referenced
this pull request
Jul 25, 2026
…ade mode, default) Routes browser tools through the OAB MCP Facade as a session-aware in-process capability source (openab-mcp openabdev#1454), replacing per-session proxy servers as the default transport. Proxy and Option C bridge modes are unchanged and remain explicit opt-outs (OPENAB_BROWSER_MODE). - src/browser_source.rs: CapabilitySource over the existing AcpMcpTunnel (requires_session; D4 static-advertise; tunnel errors surface as MCP error results); FacadeRegistrar adapts the facade's SessionTokens to core's new SessionTokenRegistrar hook (core stays openab-mcp-free). - core mcp_proxy: BrowserMode::Facade (new default; runtime fallback to Proxy when no facade is serving), write_facade_mcp_config — a static, write-once 'openab' entry whose Authorization references ${OPENAB_SESSION_TOKEN}; the per-session secret rides the agent process env instead of config files, eliminating the shared-workdir clobber class entirely (incl. kiro --agent files + @OpenAB allowlist). - pool: with_facade_sessions wiring; mints/injects the token per spawn, revokes via the same DropGuard plumbing proxy mode uses. - main: facade constructed with the BrowserSource; one listener, one discovery surface (search_capabilities/execute_capability). - rmcp re-exported from openab-mcp for source implementors. - docs: facade-mode section in the browser setup guide.
8 tasks
brettchien
added a commit
to brettchien/openab
that referenced
this pull request
Jul 26, 2026
…tic-advertise, trust gate Rewrites reverse-MCP ADR §6.2-§6.6 now that the facade seam landed upstream (openabdev#1448/openabdev#1453 facade, openabdev#1454 session-aware CapabilitySource, openabdev#1446 ADR): - §6.2 expose every client-declared type:acp server through ONE in-process CapabilitySource (AcpTunnelSource) registered with the facade, rather than the bespoke per-(session,server) loopback proxies + N mcp.json entries. Sources are registered once at construction, so the source fans out internally and routes on the <server>.<tool> prefix to (channel_id, server_id). Session identity moves to the facade's SessionTokens. - §6.3 drop notifications/tools/list_changed outright — facade discovery is pull-based (search_capabilities re-reads per call), so nothing caches a tool list to invalidate. Keep the static-advertise posture, implemented as fetch-once-per-declared-server + per-(channel,server) cache; unavailability is a call error, not a vanishing catalog entry. - §6.4 new trust requirement: openabdev#1454 assumes operator-granted tool sets, but a tunnel source's tools are client-declared — require an operator allowlist (default: browser only) plus a per-declared-server tool_filter. - §6.5 records what this retires and leaves stdio bridge-mode removal as an explicit operator call; flags the meta-tool-vs-direct hop as open. - Browser ADR: correct D4 (list_changed dropped, static-advertise kept) and add a supersession notice over D2/D3/D5. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
brettchien
added a commit
to brettchien/openab
that referenced
this pull request
Jul 26, 2026
…vers divergence Follows the merge of main (adapter ADR openabdev#1446, gmail doc openabdev#1455) into this branch: - Link the OAB MCP Adapter ADR directly now that it is present, and note the whole facade series (openabdev#1446/openabdev#1448/openabdev#1449/openabdev#1450/openabdev#1453/openabdev#1454) is merged with no facade PR left open — §6 builds on a settled foundation. - Cite adapter ADR §6.2 / Alternative C ("no second generic inbound MCP server", browser and external capabilities share one delivery mechanism), which makes retiring the bespoke per-session proxy (F5) an upstream design requirement rather than optional cleanup. - Record an unresolved divergence: the adapter ADR says the facade is delivered via ACP `mcpServers` and explicitly not by editing CLI config files, while the as-built `write_facade_mcp_config` does write a static entry — deliberately, since browser D2 found Cursor ignores ACP-passed mcpServers. Flagged for the facade contract owner instead of unilaterally reconciling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
brettchien
added a commit
to brettchien/openab
that referenced
this pull request
Jul 26, 2026
270a2ff renamed the client-declared surface (`browser.*` -> `katashiro.*`) in code, config and the tunnel contract, but the two ADRs still described the old tool names — including the §6.4 pinned-tool list and the runtime sequence diagrams, which readers would otherwise copy verbatim into a policy that no longer matches. The one remaining `browser.click` is inside a verbatim quote of upstream openabdev#1454's sources.rs doc comment and is left as written. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thepagent
pushed a commit
that referenced
this pull request
Aug 3, 2026
…1447) * docs(acp): add MCP-over-ACP browser-control implementation blueprint (§7) Break the north-star (LLM operating the browser via MCP over /acp) into T0–T7 with sub-tasks, an OpenAB-side vs extension-side ownership split meeting at the MCP-over-ACP wire contract (T4), and the key findings that reshape the work: the agent→client request direction already exists on the downstream hop (request_permission is auto-replied in openab-core, so T1 is a relay not green-field), and mcpServers is currently [] (T5 injects a core proxy). Suggested order + which items are heavy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(adr): resolve MCP-over-ACP browser-control design (D1-D4) + flow diagrams Fold the resolved design decisions into the browser-control ADR §7: - D1: auto-approve all browser tool permissions (core keeps auto-replying request_permission); fine-grained control deferred. Drops the dedicated request_permission-relay task; T1's server->client machinery stays (needed by the upstream MCP tunnel). - D2: inject the proxy via each agent's native MCP config (Cursor -> .cursor/mcp.json), not ACP session/new mcpServers (Cursor ignores those; cf. zed-industries/zed#50924). Content (HTTP url+headers) is portable; no universal config location exists. - D3: downstream (agent<->core) is a normal in-process Streamable-HTTP MCP server on loopback (via rmcp), NOT an on-ACP-stream tunnel (the ACP maintainer backed off on-stream MCP; cf. discussion #58). Upstream (core/gateway<->extension) is the one legitimate tunnel and adopts the official MCP-over-ACP RFD framing (mcp/connect + mcp/message); the RFD's "type":"acp" downstream injection is unused (Cursor unsupported). - D4: core's HTTP MCP server is always-on and decoupled from the extension WS, so the WS can attach after session start; core static-advertises the browser toolset and emits notifications/tools/list_changed on attach/detach. Also adds a TL;DR flow, an as-designed execution flow, a detailed message-level runtime sequence, and the T0 spike checklist; updates Findings/Tasks/Ownership accordingly (T3 dropped, T4 = RFD framing, T5 = HTTP MCP server + per-adapter config injection). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(gateway/acp): server-initiated request direction (T1.2/1.3) Add the agent->client REQUEST direction to the ACP WebSocket server, the plumbing the MCP-over-ACP tunnel needs. The base only had client->server requests plus server->client notifications; this adds: - route_client_response(): the read loop now recognises an inbound client *response* (id present, no `method`, carries result/error) and routes it to the waiting request via a per-connection pending map, instead of answering it with -32600. Gated on !is_notification so notification/request handling is untouched. - pending_requests: Arc<Mutex<HashMap<u64, oneshot::Sender<Value>>>> per connection, drained on disconnect so in-flight awaiters unblock with "connection closed" rather than hanging until timeout. - send_request() + JsonRpcRequestOut: mint an id, register the oneshot, send the frame over the existing outbound channel, timeout-await the correlated response. Landed with #[allow(dead_code)] as ready infrastructure; its caller arrives with T1.4 (the core<->gateway bridge). Mirrors the existing client-side pattern in openab-core/src/acp/connection.rs. Adds an acp_requests test module (route + send_request round-trip, id minting, request/notification rejection, unmatched id). Gate green: clippy -D warnings + test --test-threads=1 + build, --features unified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(gateway/acp): construct trivial responses from generated types (T2.1) Migrate the three trivial outbound response payloads from hand-rolled `json!` to the generated `acp_schema` types, so the wire shape is type-checked: - session/new → NewSessionResponse { session_id: SessionId(..) } - session/resume → ResumeSessionResponse::default() (serializes to {}) - prompt final → PromptResponse { stop_reason }, with `stop_reason` now a typed StopReason enum (EndTurn / Cancelled) instead of a &str literal. rename + skip_serializing_if make the emitted wire byte-identical to the prior `json!`, so the existing conforms::<T> round-trip tests and handler behaviour tests are unchanged (292 pass). handle_initialize stays hand-rolled for now (nested agentCapabilities/agentInfo; low value, per base ADR §7 the trivial chat subset does not require typed construction). The remaining T2.2 (typing the mcp/connect + mcp/message bidirectional frames) lands with T4. Gate green: clippy -D warnings + test --test-threads=1 + build, --features unified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(gateway/acp): MCP-over-ACP tunnel frame API (T4.1) Add the gateway-side helpers for the official MCP-over-ACP RFD tunnel (agentclientprotocol.com/rfds/mcp-over-acp), built on the T1 send_request machinery (its first real callers): - mcp_connect(acpId) -> connectionId - mcp_message_request(connectionId, method, params) -> inner MCP result - mcp_disconnect(connectionId) - McpConnectParams / McpConnectResult / McpMessageParams / McpDisconnectParams (hand-rolled: these RFC methods are not in the generated acp_schema, which only has the session/new McpServer* declaration types + McpCapabilities), plus frame_result() to unwrap a response frame's result / surface its error. Per the RFD, mcp/message flattens the inner MCP method/params into the params object WITHOUT the inner MCP id; correlation is purely by the outer ACP id, and the response result is the inner MCP result payload. Adds a mock-tunnel round-trip test (mcp/connect -> connectionId, mcp/message tools/list -> result). Helpers carry #[allow(dead_code)] until T5 wires them to the core MCP proxy (their real caller). Remaining T4: session/new "type":"acp" parsing + advertise mcpCapabilities.acp, gateway<->core routing (with T5), contract doc. Gate green: clippy -D warnings + test --test-threads=1 + build, --features unified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(core): scaffold MCP proxy server deps + static browser toolset (T5.1a) Introduce the core-hosted MCP proxy for MCP-over-ACP browser control (D3/D4), starting with the dependency integration + the static tool set: - openab-core gains optional rmcp (server + transport-streamable-http-server), axum 0.8 (matches the gateway, one axum in the workspace), and tokio-util, gated behind a new `acp-mcp` feature so non-acp builds don't pull them. The root `acp` feature (included by `unified`) now enables `openab-core/acp-mcp`. This is the workspace's first rmcp server-side usage; it resolves + compiles cleanly alongside openab-agent's rmcp client features. - New `mcp_proxy` module (feature-gated) with `browser_tools()`: the fixed DOM-semantic tool set (click / read_dom / navigate / type / screenshot) that, per D4, core static-advertises regardless of whether an extension is attached. Built from rmcp `Tool::new` + typed input schemas; unit-tested. `browser_tools()` carries #[allow(dead_code)] until the ServerHandler wires it. Next (T5.1b): ServerHandler impl + spawn_mcp_server (loopback + bearer axum listener); then T5.2 config injection and T5.3 tunnel wiring. Gate green: clippy -D warnings + test --test-threads=1 + build, --features unified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(core): MCP proxy ServerHandler + loopback server (T5.1b) Stand up the core-hosted MCP server the colocated agent connects to (D3): - ProxyHandler impls rmcp ServerHandler: get_info advertises the tools capability; list_tools returns the static browser tool set (D4 static- advertise); call_tool returns "browser not connected" until the tunnel is wired (T5.3) — failing gracefully rather than hiding the tools (D4). - spawn_mcp_server binds an OS-assigned 127.0.0.1 port with its own axum listener (StreamableHttpService, stateless + JSON responses), graceful shutdown via a CancellationToken. The caller hands the port to the agent's native MCP config in T5.2. An HTTP integration test spawns the server, confirms it binds loopback, and that an MCP initialize returns a result advertising the tools capability. Bearer auth on the listener is added in T5.2 (the token is minted alongside the .cursor/mcp.json injection). Tunnel wiring (RemoteExtensionChannel -> mcp_connect /mcp_message) is T5.3. Gate green: clippy -D warnings + test --test-threads=1 + build, --features unified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(core): bearer gate on the MCP proxy server (T5.2 part 1) Even bound to loopback, the core MCP server now requires the token the agent's MCP config carries (D3), so another local process on the host can't reach the browser tools. spawn_mcp_server takes a `bearer` and layers an axum middleware that returns 401 when Authorization: Bearer <token> is absent or wrong; the caller mints the token and shares it with the agent config. Tests: authed initialize -> 200 + tools capability; missing / wrong token -> 401. Remaining T5.2: the per-agent adapter writes { url: 127.0.0.1:<port>, headers: Authorization Bearer } into the agent's native MCP config (Cursor -> .cursor/mcp.json) before boot, and wires spawn_mcp_server into openab startup. Gate green: clippy -D warnings + test --test-threads=1 + build, --features unified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(adr): correct runtime diagram — mcp/message flattens, no inner id The MCP-over-ACP RFD flattens the inner MCP method/params into the mcp/message params and does NOT carry an inner MCP id; correlation on the upstream tunnel is by the outer ACP id alone, and the response result IS the inner MCP result payload. Fix the detailed runtime sequence + the id-space note: mcp#7 lives only on the agent<->core HTTP hop; the core proxy maps its downstream mcp#7 <-> the upstream acp#55. (Was: "carried verbatim agent<->core<->extension".) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(gateway/acp): parse + record client-declared type:acp mcpServers (T4) The browser extension declares its MCP-over-ACP server in session/new via the RFD's mcpServers entry {"type":"acp","id":...,"name":...}. Parse those (raw, since the "acp" transport is an RFD proposal not in the generated schema) and record them per session (AcpSession.acp_mcp_servers), so the gateway can later mcp/connect to them (T5.3). session/resume re-records them since the client re-presents mcpServers. http/sse/stdio servers are ignored (the agent connects to those itself). D5-agnostic: needed regardless of the core MCP server topology. Field is #[allow(dead_code)] until the mcp/connect wiring consumes it. Tests: parse keeps only acp entries (+ empty cases); session/new records them. Not done here: advertising mcpCapabilities.acp in initialize — the generated McpCapabilities has only http/sse (the RFD acp flag isn't in stable v1), and since we own both ends the extension can declare type:acp unconditionally; left as a follow-up. Gate green: clippy -D warnings + test --test-threads=1 + build, --features unified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: MCP-over-ACP tunnel contract for the extension (T4.3) The spec the browser extension (katashiro, T6) implements: the gateway<->extension hop of MCP-over-ACP. Covers the type:acp session/new declaration, mcp/connect -> connectionId, mcp/message (inner method/params flattened, correlate by outer ACP id, result = inner MCP result), mcp/disconnect, the baseline browser tool set (click/read_dom/navigate/type/screenshot), and that permissions are auto-approved (D1) + the WS may attach after session start (D4). D5-agnostic (only the external hop; OpenAB-internal proxy/topology is out of scope), so it lets the extension side proceed in parallel. Linked from ADR §7 T4. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(gateway/acp): TunnelHandle — per-connection MCP tunnel handle (T5.3) The reusable abstraction the core MCP proxy needs to reach a specific browser: TunnelHandle bundles one /acp connection's outbound channel + pending-request map + id counter + the mcp/connect connectionId, and exposes async mcp_message() / disconnect() that tunnel an inner MCP request to that extension and await the result. Built on the T1/T4.1 send_request + mcp_message_request helpers. D5-agnostic: both the per-session and shared core-server designs route through this same handle. Round-trip test via a mock extension driver. Next: register a TunnelHandle per session's channel_id (after mcp/connect) in a shared registry (AppState), and consume it from the core ProxyHandler. Gate green: clippy -D warnings + test --test-threads=1 + build, --features unified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(gateway/acp): tunnel registry + establish_and_register_tunnel (T5.3) - AcpTunnelRegistry (channel_id -> TunnelHandle), mirroring AcpReplyRegistry, so the core MCP proxy can look up the tunnel for a given browser session. - establish_and_register_tunnel: mcp/connect to a session's declared "type":"acp" server, build a TunnelHandle from the returned connectionId, and register it under the session's channel_id. First real caller of mcp_connect/send_request. Documented as spawn-only (awaiting mcp_connect inline in the read loop would deadlock, since only that loop delivers the response). Test: a mock extension answers mcp/connect; the handle lands in the registry keyed by channel_id. #[allow(dead_code)] until the read loop spawns it and it's threaded through AppState (next). Then the core ProxyHandler consumes the registry to forward tools/list + tools/call. Gate green: clippy -D warnings + test --test-threads=1 + build, --features unified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(gateway): thread AcpTunnelRegistry through AppState (T5.3) Add acp_tunnel_registry: Option<AcpTunnelRegistry> to AppState alongside acp_reply_registry (same #[cfg(feature="acp")] gate), initialized wherever the reply registry is. This is the shared handle the connection read loop will populate (spawning establish_and_register_tunnel per declared type:acp server) and the core MCP proxy will consume to route a tool call to the right browser. No behaviour change yet — the field is constructed but not read until the read-loop spawn wiring lands next. Gate green: clippy -D warnings + test --test-threads=1 + build, --features unified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(gateway/acp): open MCP tunnels on session/new + cleanup (T5.3) Wire the tunnel producer into the connection read loop: - handle_acp_connection mints a per-connection next_req_id (Arc<AtomicU64>) for server-initiated requests. - handle_session_new returns the minted channel_id alongside the response. - On session/new, for each declared "type":"acp" server, tokio::spawn establish_and_register_tunnel (mcp/connect -> register a TunnelHandle under the channel_id). Spawned, never awaited inline: it awaits mcp/connect whose response only this same read loop delivers, so awaiting inline would deadlock. The task is tracked in prompt_tasks (aborted on disconnect). - Disconnect cleanup now removes the connection's channel_ids from BOTH the reply and tunnel registries (gathered once). Live behaviour needs a real extension (T7); this lands the plumbing + keeps the unit tests green. Next: the core ProxyHandler consumes acp_tunnel_registry to forward tools/list + tools/call to the right browser. Gate green: clippy -D warnings + test --test-threads=1 + build, --features unified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(core): BrowserTunnel trait + ProxyHandler forwards tool calls (T5.3, D6-a') Add the core-side tunnel interface (D6-a'): `trait BrowserTunnel { async fn call(channel_id, method, params) }`, implemented by the root (bridging to the gateway registry) so no core<->gateway crate dependency is introduced — matching the existing ChatAdapter pattern. - ProxyHandler now carries its session channel_id + an Option<Arc<dyn BrowserTunnel>> (D5-a: one server per session). call_tool forwards the tool as an MCP tools/call over the tunnel; list_tools stays static-advertised (D4); no tunnel / no browser attached -> "browser not connected" (D4). - spawn_mcp_server takes (channel_id, tunnel) and builds a per-session ProxyHandler in the service factory. Tests: forward via a mock BrowserTunnel; not-connected without one. Gate green: clippy -D warnings + test --test-threads=1 + build, --features unified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(core): start_session_server — per-session server + .cursor/mcp.json (T5.2/D5-a) start_session_server(channel_id, workdir, tunnel): mint a fresh bearer, start the loopback MCP proxy for that session, and MERGE an `openab-browser` HTTP entry (url + Authorization: Bearer) into <workdir>/.cursor/mcp.json without clobbering any servers already there (Cursor's native config; D2). Returns the bound addr + a CancellationToken the pool cancels to stop the server on session evict. Tests: writes the cursor config (url+bearer); merges into an existing mcp.json. Gate green: clippy -D warnings + test --test-threads=1 + build, --features unified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(core): start per-session MCP proxy on agent spawn (T5.2/D5-a) Wire the per-session MCP proxy into the pool's agent-launch path (feature acp-mcp): - For a browser (`acp:`) session, get_or_create starts a loopback MCP server + writes .cursor/mcp.json BEFORE spawning the agent, so the agent connects to it on boot. Non-acp sessions (Discord, etc.) are untouched. - Lifecycle: the server's CancellationToken drop_guard is stored INSIDE the AcpConnection (new mcp_server_guard field), so the server is cancelled whenever the connection is dropped — through any evict/suspend/hung-kill path — without touching each removal site. On a failed spawn/init the guard drops early and cancels too. - SessionPool gains a browser_tunnel field + with_browser_tunnel() builder (set by the root, D6-a'); passed into each per-session ProxyHandler. Gate green: clippy -D warnings + test --test-threads=1 + build, --features unified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(root): wire the browser tunnel bridge end-to-end (T5.3, D6-a') RootBrowserTunnel (src/browser_tunnel.rs) implements openab-core's BrowserTunnel trait by looking up a channel_id in the gateway's AcpTunnelRegistry and calling TunnelHandle.mcp_message — the root glue that connects the two sibling crates without either depending on the other (mirrors the ChatAdapter pattern). Wiring in `openab run` (feature acp): create ONE shared acp_tunnel_registry before the pool; give the pool a RootBrowserTunnel over it (with_browser_tunnel), and inject the SAME registry into the gateway AppState so acp_server populates the exact map the bridge reads. This closes the loop: agent tools/call -> core per-session MCP proxy -> RootBrowserTunnel -> gateway TunnelHandle -> mcp/message -> extension. Live path still needs a real extension + deploy (T7); everything compiles + unit tests green. Gate green: clippy -D warnings + test --test-threads=1 + build, --features unified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(adr): record the as-built OpenAB side (D5-a + D6-a', end-to-end) Add a §7 "As-built" section documenting the two decisions settled during implementation and the realised call path: D5 = per-session MCP server bound to the existing channel_id map (lifetime tied to the AcpConnection via a CancellationToken DropGuard); D6 = BrowserTunnel trait in core + impl in the root (RootBrowserTunnel), keeping core/gateway sibling-independent like the existing ChatAdapter glue. Notes remaining T5.4 / T6 / T7. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(e2e): MCP-over-ACP tunnel producer section in acp-ws-smoke (T7) Add a "MCP-over-ACP tunnel" section to the smoke suite: a mock extension declares a {type:acp} mcpServers entry in session/new, then asserts the gateway issues a server-initiated mcp/connect carrying the declared acpId, and answers it with a connectionId (registering the tunnel). This exercises the live read-loop spawn + server->client request path end-to-end — the concurrency unit tests can't reach. Runs against a live server (deploy T7). The tunnel path is inert for normal sessions (only triggers on a type:acp declaration), so it does not affect existing ACP traffic. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(e2e): complete the MCP-over-ACP tunnel suite (fan-out + filtering) Extend the tunnel section from a single-server check to full producer coverage via a collect_mcp_connects() helper: single type:acp → exactly one mcp/connect; fan-out (two type:acp servers → one distinct mcp/connect each, distinct request ids); mixed acp+http mcpServers → only the acp entry is tunnelled. All deterministic. Validated live against Falcon: 34/34 (tunnel 7/7). The agent→tool→browser leg is out of the WS suite's reach (needs a real extension, T6). Run: OPENAB_ACP_TOKEN=<key> uv run scripts/acp-ws-smoke.py ws://<host>/acp Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(acp-mcp): address Mira review nits — constant-time bearer + string-number id Two non-blocking hardening nits from the openab-side review: - mcp_proxy require_bearer: compare the loopback MCP bearer in constant time (subtle::ConstantTimeEq, matching the gateway's feishu/wecom signature checks) so a wrong token can't be recovered byte-by-byte via response timing. Adds `subtle` as an optional dep under the `acp-mcp` feature. - acp_server route_client_response: accept a stringified-number JSON-RPC id ("1") in addition to a numeric id, so a spec-loose client's responses still correlate to their pending request instead of being silently dropped. Gate (targeted, no repo-wide fmt — this container's rustfmt disagrees with the branch on pre-existing import ordering): clippy clean on both crates; openab-core mcp_proxy tests 8/8, openab-gateway acp_server tests 35/35 green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(acp-mcp): establish the browser tunnel on session/resume, not just session/new katashiro persists its ACP session and RECONNECTS via session/resume (not session/new), re-declaring its "type":"acp" browser MCP server each time. The session/new branch spawns establish_and_register_tunnel for each declared server, but session/resume only recorded them in the session state and never opened a tunnel — so a resumed browser session had no entry in the tunnel registry and the core MCP proxy returned "no browser attached to session acp_<uuid>" on every call. Mirror the session/new logic in the resume branch: derive the same deterministic channel_id from the sessionId and spawn establish_and_register_tunnel for each declared type:acp server. This is what makes the live loop work across katashiro's auto-reconnect (which always resumes). Gate: clippy clean, openab-gateway acp_server tests 35/35. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(acp-mcp): log browser tunnel open/register for live-session observability establish_and_register_tunnel is reached only when a client declared a "type":"acp" server, so an info line there answers "did the extension advertise itself?" from the gateway log alone (the raw upstream session frame isn't otherwise logged). Logs on open and on successful registry insert. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(acp-mcp): raise ACP frame cap 1→8 MiB for browser-tool results Browser tool results carried over the MCP-over-ACP tunnel (notably screenshots) routinely exceed the old 1 MiB inbound frame cap, which closed the WebSocket mid-response and wedged the extension in a reconnect loop. 8 MiB gives ample room for a compressed screenshot / large DOM snapshot while staying a sane DoS bound. Pairs with the katashiro-side switch to JPEG screenshots. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(acp-mcp): one browser tunnel per session — fix fan-out overwrite/orphan (M-B1) The tunnel registry is keyed by channel_id, and both the session/new and session/resume paths looped over every declared type:acp server calling establish_and_register_tunnel. With >1 server each insert overwrote the previous under the same channel_id, leaving the earlier tunnel opened-but-unreachable (orphaned). The core proxy only ever resolves a browser by channel_id, so one tunnel per session is the actual model. Factor both call sites into spawn_browser_tunnel(), which establishes only the first declared server and warns on extras. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(acp-mcp): 0600 mcp.json + strip stale bearer on evict (M-B2) start_session_server wrote <workdir>/.cursor/mcp.json with tokio::fs::write, leaving it at the umask default (typically 0644) — but the file embeds the live loopback bearer token, so any local user could read it. Write via write_private() which chmods it 0600. Also, on session evict (CancellationToken fires) strip the now-dead openab-browser entry so a stale credential doesn't linger; guarded to only remove the entry if it still points at our addr, so a concurrent/reconnected session that already replaced it isn't clobbered (the mcp.json path is shared across acp: sessions). Adds a 0600 assertion to the existing config-write test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(acp-mcp): lock subtle dependency Cargo.lock was missing the openab-core `subtle` entry added for the constant-time bearer compare, which would fail a `--locked` build. No code change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(mcp-proxy): write openab-browser into kiro-cli config too, not just Cursor start_session_server now merges the openab-browser entry into BOTH .cursor/mcp.json and .kiro/settings/mcp.json (each CLI ignores the other's), and cleans both on evict. kiro-cli parses the {url, headers} shape identically. Deployed as acpmcp-kirofix. Also add .dockerignore (exclude target/, .git/, data/) for the acp image builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: browser MCP agent setup — per-variant mcp.json how-to (Phase 2 #8) How the openab-browser tools reach each agent CLI: the per-session loopback proxy + where openab writes the {url, headers} entry per variant (Cursor/Kiro auto today; Claude/Codex/Gemini paths documented, not yet auto). Honest caveat: static manual config awaits the stable-endpoint redesign (#9). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(mcp-proxy): per-pod browser-bridge socket server (Option C, P1) serve_browser_socket: one unix socket multiplexes all sessions; the openab browser-bridge shim forwards {channel_id, inner MCP request} frames, routed via dispatch_browser_mcp -> the shared BrowserTunnel by channel. Reuses browser_tools() + tunnel.call (single source of truth vs the HTTP ProxyHandler). +8 tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(cli): openab browser-bridge subcommand — stdio MCP relay to the browser socket (Option C, P2) A thin per-session shim: reads OPENAB_BROWSER_CHANNEL, wraps each stdin MCP request as {channel_id, request}, forwards to the per-pod core socket, relays responses to stdout verbatim. All browser MCP logic stays in core; the agent's config line is static. Gated by feature acp. + wrap/relay tests over in-memory pipes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(acp): inject OPENAB_BROWSER_CHANNEL into the agent env (Option C, P3) AcpConnection::spawn gains a browser_channel param; for an acp: session the pool passes the channel_id so the agent (and the browser-bridge shim it later spawns) inherits it and routes browser tool calls to THIS session's tunnel. env_clear-safe (re-injected explicitly). + set_browser_channel unit tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(mcp-proxy): static write-once browser-bridge config (Option C, P4) write_bridge_mcp_config writes the SAME {command:openab, args:[browser-bridge]} entry to cursor + kiro mcp.json — no port/bearer, so it never goes stale and can't clobber across sessions (the root cause of multi-window browser flakiness). Merges without touching the user's servers; idempotent. Additive — P5 wires the proxy/bridge toggle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat: OPENAB_BROWSER_MODE proxy|bridge toggle wiring (Option C, P5) BrowserMode + browser_mode() (default proxy) + shared browser_socket_path(). Pool branches: proxy = per-session HTTP server (unchanged default); bridge = static write-once config, no per-session server. Broker starts the per-pod socket server once in bridge mode. browser-bridge shim uses the shared socket path. + parse tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(browser-bridge): resolve channel via process-ancestry, not env (Option C, b2 B1) The MCP client scrubs the child env (cursor gives the bridge only HOME/PATH/USER or the pod env, never the per-session OPENAB_BROWSER_CHANNEL), so env inheritance can't carry the channel. resolve_channel() now walks up the PPID chain and reads OPENAB_BROWSER_CHANNEL from the ancestor agent's /proc/<pid>/environ (openab injected it via the pool) — generic across all stdio-MCP vendors. Logs the resolved channel to stderr. + parse_ppid_from_stat / parse_channel_from_environ unit tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(mcp-proxy): revert bridge config to pure {command,args} (Option C, b2 B2) Drop the ${OPENAB_BROWSER_CHANNEL} config env — cursor doesn't expand it (spawns from pod/clean env). The bridge now resolves its channel via process-ancestry (B1), so the config is a byte-identical static entry again: idempotent, never stale, no clobber. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(acp): add acp_mcp_servers to test-only AcpSession initializers The 4 AcpSession constructors in `mod acp_review_fixes` tests missed the acp_mcp_servers field added in T4, breaking `cargo test -p openab-gateway --features acp` (E0063). build/clippy don't compile this crate's test target under `acp`, so only CI caught it. Also syncs Cargo.lock to the already-committed openab 0.10.0 version. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(acp): split into reverse-MCP mechanism ADR + browser-control ADR, embed diagrams Two ADRs instead of one: - acp-server-websocket-reverse-mcp.md — the generic reverse-MCP-over-ACP mechanism (roles, call route, protocol gap, §6 multi-server generalization: compound-key routing, dynamic tools/list + list_changed, per-server Option B). Embeds the architecture + MCP-usage sequence diagrams (mermaid), using browser control as the example. Flipped Proposed -> Accepted (as-built in #1447). - acp-server-websocket-mcp-browser.md — the browser-specific design + the contract the browser extension implements (D1-D6, detailed id-paired runtime sequence, tasks, as-built). Defers the mechanism to the reverse-MCP ADR. Update base ADR + tunnel-contract cross-links; mark base §6 browser critical-path done. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(adr): correct D4 list_changed overclaim in browser ADR list_changed is designed but not yet implemented (0 hits in gateway/core crates); it was described as shipped alongside static-advertise. Reword to mark it as P2b-tracked (reverse-MCP §6.2), not as-built. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(acp): compound-key (channel_id,server_id) tunnel registry + rename BrowserTunnel->AcpMcpTunnel (P1, Fork A) Behavior-preserving refactor toward generic multi-server MCP-over-ACP (reverse-MCP ADR §6). No functional change for the single browser server. - AcpTunnelRegistry: HashMap<String,_> -> HashMap<(String,String),_>; register under the client-declared srv.id; evict all (channel_id,*) on teardown. - Core trait BrowserTunnel -> AcpMcpTunnel; call() gains a server_id param. - Read side (Fork A): the single-browser proxy + bridge pass an empty server_id sentinel; RootBrowserTunnel resolves the sole tunnel on the channel (errors if ambiguous). Real per-server read-side routing (bridge-frame server_id) is deferred to P2. Gate: build --features acp, clippy --workspace -D warnings (+unified), test -p openab-gateway --features acp — all green (307 passed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: regenerate Cargo.lock for merged deps * fix(mcp-proxy): register browser server in kiro per-agent configs (--agent mode) When kiro-cli runs with --agent <name> — as every OAB bot deployment does — the MCP server list comes from .kiro/agents/<name>.json and tools are gated by that file's default-deny allowedTools, NOT from .kiro/settings/mcp.json (verified live on the b2 fleet deployment; see docs/gmail-native.md 'Kiro CLI gotcha'). Without this, browser tools are invisible to exactly the deployments this feature targets. - merge_kiro_agent_configs: merge the openab-browser entry into every .kiro/agents/*.json and add @openab-browser to allowedTools; agent files carry unrelated config, so unparseable files are skipped, never clobbered (unlike the settings writer); macOS ._* droppings ignored; idempotent; 0600 (proxy entries carry a live bearer). - cleanup_kiro_agent_configs on session evict: remove the entry and revoke the allowlist grant only when the URL is still ours, preserving a concurrent session's live entry (same rule as the settings cleanup). - Wired into both the per-session proxy writer and the static Option C bridge writer; 4 new tests. * feat(mcp): browser capabilities through the session-aware facade (Facade mode, default) Routes browser tools through the OAB MCP Facade as a session-aware in-process capability source (openab-mcp #1454), replacing per-session proxy servers as the default transport. Proxy and Option C bridge modes are unchanged and remain explicit opt-outs (OPENAB_BROWSER_MODE). - src/browser_source.rs: CapabilitySource over the existing AcpMcpTunnel (requires_session; D4 static-advertise; tunnel errors surface as MCP error results); FacadeRegistrar adapts the facade's SessionTokens to core's new SessionTokenRegistrar hook (core stays openab-mcp-free). - core mcp_proxy: BrowserMode::Facade (new default; runtime fallback to Proxy when no facade is serving), write_facade_mcp_config — a static, write-once 'openab' entry whose Authorization references ${OPENAB_SESSION_TOKEN}; the per-session secret rides the agent process env instead of config files, eliminating the shared-workdir clobber class entirely (incl. kiro --agent files + @openab allowlist). - pool: with_facade_sessions wiring; mints/injects the token per spawn, revokes via the same DropGuard plumbing proxy mode uses. - main: facade constructed with the BrowserSource; one listener, one discovery surface (search_capabilities/execute_capability). - rmcp re-exported from openab-mcp for source implementors. - docs: facade-mode section in the browser setup guide. * fix: facade_serving is acp-only — derive it, don't flag it (default-features -D warnings) * docs(adr): §6 builds on the OAB MCP Facade — one AcpTunnelSource, static-advertise, trust gate Rewrites reverse-MCP ADR §6.2-§6.6 now that the facade seam landed upstream (#1448/#1453 facade, #1454 session-aware CapabilitySource, #1446 ADR): - §6.2 expose every client-declared type:acp server through ONE in-process CapabilitySource (AcpTunnelSource) registered with the facade, rather than the bespoke per-(session,server) loopback proxies + N mcp.json entries. Sources are registered once at construction, so the source fans out internally and routes on the <server>.<tool> prefix to (channel_id, server_id). Session identity moves to the facade's SessionTokens. - §6.3 drop notifications/tools/list_changed outright — facade discovery is pull-based (search_capabilities re-reads per call), so nothing caches a tool list to invalidate. Keep the static-advertise posture, implemented as fetch-once-per-declared-server + per-(channel,server) cache; unavailability is a call error, not a vanishing catalog entry. - §6.4 new trust requirement: #1454 assumes operator-granted tool sets, but a tunnel source's tools are client-declared — require an operator allowlist (default: browser only) plus a per-declared-server tool_filter. - §6.5 records what this retires and leaves stdio bridge-mode removal as an explicit operator call; flags the meta-tool-vs-direct hop as open. - Browser ADR: correct D4 (list_changed dropped, static-advertise kept) and add a supersession notice over D2/D3/D5. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(adr): align §6 with the merged facade series + record the mcpServers divergence Follows the merge of main (adapter ADR #1446, gmail doc #1455) into this branch: - Link the OAB MCP Adapter ADR directly now that it is present, and note the whole facade series (#1446/#1448/#1449/#1450/#1453/#1454) is merged with no facade PR left open — §6 builds on a settled foundation. - Cite adapter ADR §6.2 / Alternative C ("no second generic inbound MCP server", browser and external capabilities share one delivery mechanism), which makes retiring the bespoke per-session proxy (F5) an upstream design requirement rather than optional cleanup. - Record an unresolved divergence: the adapter ADR says the facade is delivered via ACP `mcpServers` and explicitly not by editing CLI config files, while the as-built `write_facade_mcp_config` does write a static entry — deliberately, since browser D2 found Cursor ignores ACP-passed mcpServers. Flagged for the facade contract owner instead of unilaterally reconciling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(adr): tighten §6.4 trust gate — deny-all tool_filter + pinned browser tool set Falcon's review of §6 (F7): the operator allowlist of declared server names is not a trust boundary on its own. The name is chosen by the same remote client that declares the tools, so a client may declare a server named `browser` and publish an arbitrary tool set under it. Align the §6.6 F4 summary with the requirement §6.4 now states: the per-declared-server tool_filter is deny-all by default, and the `browser` entry ships pinned to its five known tools so a same-name declaration cannot inject others. Co-Authored-By: Claude <noreply@anthropic.com> * docs(adr): §6.1/§6.2 — declared id vs name, and last-attach-wins on same-name tunnels Found while scoping F1': the routing contract as written could not be implemented. A declaration is {type:"acp", id, name} and the reference client mints `id` as a fresh crypto.randomUUID() per connection while `name` ("browser") is stable. The registry is keyed by `id`, but the `<server>` segment of a tool name (`browser.click`) and the §6.4 allowlist are the `name` — so routing "on the prefix to the matching (channel_id, server_id) tunnel" can never match: the key is a UUID the tool name never contains. Record what review settled (Mira + Falcon, 2026-07-26): - registry stays keyed by (channel_id, id) — keying by name would let two same-name tunnels overwrite each other, the fan-out collapse §6 fixes — but must also record the declared name so a source can enumerate (name, id); - trust gating is keyed by name, since ids are per-connection UUIDs; - same-name collisions are last-attach-wins: the new tunnel replaces and evicts the older entry. Answering "ambiguous" there would wedge the client out of its own tools on every reconnect, because each reconnect mints a new id. Also clarify that the prefix selects the tunnel and is NOT stripped: the full published name is what goes over the tunnel, since that is what the server's own tools/call expects. Co-Authored-By: Claude <noreply@anthropic.com> * feat(acp): record declared server name, establish all declared tunnels, LWW on re-attach Tunnel-layer plumbing for §6 F1' (generalising the capability source to N client-declared servers). Behaviour-neutral for today's single-browser client: one declared server still yields exactly one tunnel. A declaration is {type:"acp", id, name} and the two fields have different lifetimes — the reference client mints `id` as a fresh crypto.randomUUID() per connection while `name` ("browser") is stable. The registry is keyed by `id`, but a tool name carries the `name` (browser.click) and the §6.4 trust gate is keyed by it too, so the name has to survive registration to be routable. - TunnelHandle records the declared `server_name` and exposes it. - establish_and_register_tunnel takes the declared name and resolves a re-declared name last-attach-wins: the new tunnel evicts stale same-name entries on the channel. Because a reconnect mints a new id, the dead tunnel would otherwise linger beside the live one, and answering "ambiguous, pass a server_id" there would wedge the client out of its own tools on every reconnect. The eviction is also what bounds registry growth. - spawn_browser_tunnel -> spawn_acp_tunnels now establishes EVERY declared server. The old first-only limit existed because the registry was keyed by channel_id alone, where a second server overwrote the first and orphaned its tunnel; the compound key removed that collision. - AcpMcpTunnel gains servers(channel_id) -> Vec<(name, id)>, implemented by RootBrowserTunnel over the registry. This is what lets a capability source resolve a tool prefix back to a tunnel; matching a prefix against the registry key alone can never work, since the key is a UUID the tool name never contains. Default impl is empty so test doubles are unaffected. The source-side consumer (AcpTunnelSource routing + the §6.4 trust gate) is the next step; nothing reads servers() yet. Co-Authored-By: Claude <noreply@anthropic.com> * feat(acp): route client-declared servers by name and gate their tool sets Second half of §6 F1' — the consumer of the tunnel plumbing added in 5dc45de0. BrowserSource becomes AcpTunnelSource: it fans out to every client-declared MCP server instead of assuming one implicit browser, and enforces the §6.4 trust gate. Behaviour for today's single-browser client is unchanged. Routing (§6.1/§6.2): the `<server>` prefix of a published tool name is the declared *name*, while the registry is keyed by the per-connection `id`, so `call` resolves name -> (channel_id, id) via the registry enumeration. The full published name is forwarded (`browser.click`), not the suffix — the prefix selects the tunnel, it is not stripped, because the server's own tools/call expects the name it published. Trust gate (§6.4), two independent checks: - the declared name must be in the operator allowlist (default: browser only); - the tool must be one that server is pinned to. The second is not redundant with the first. The name is chosen by the same remote client that declares the tools, so a client can re-declare the trusted name `browser` and publish `browser.exec`; the pin is what refuses it. Denied calls never reach the tunnel. Both cases are covered by tests. `browser` appears only as an entry in the default policy table — deliberately data, not a branch — so the routing code stays generic and admitting another client-side MCP service is a table entry (§6.2: no browser-specific branch). tools() serves the policy table statically and is deliberately NOT intersected with the tunnels currently attached: intersecting would make the catalog flap as a tab detaches, which §6.3 forbids, and would lose the pre-attach discovery D4 already provided. Availability is reported by call, never by a shrinking catalog. Session *scope* — restricting to the servers a given client declared — is a separate axis needing F3's declaration cache, so tools() ignores ctx for now; an allowlisted server's pinned tools are advertised to every session, which is the status quo for the browser. Co-Authored-By: Claude <noreply@anthropic.com> * docs(adr): §6.3 — policy entries seed the catalog; cache narrows, never grants Found while scoping the discovery cache: §6.3 said an un-cached declared server "contributes an empty set", which contradicts the static-advertise posture that §6.4's pinned sets and D4 both rely on, and which the source implemented in ba94efec (browser's pinned tools are advertised before the extension attaches — confirmed correct on review). Record the layering review settled instead: - a server's §6.4 policy entry is its pre-attach SEED as well as its filter, so a pinned server never drops to empty just because nothing has attached; - the per-(channel_id, server_id) cache holds fetched ∩ allowed and replaces the seed once a fetch succeeds, narrowing the catalog to what the server really publishes without ever widening past the policy; - a declared server with no policy entry contributes nothing because §6.4 is deny-all, not because it is un-cached. Caching is never itself a grant. Also record the ordering consequence: deny-all plus pinned entries that already carry full Tool schemas means fetching cannot surface anything the operator has not already permitted, so the discovery cache is invisible until the operator-facing config surface exists. The config surface lands first; the cache then supplies real schemas once operators may list tools by name alone. Co-Authored-By: Claude <noreply@anthropic.com> * feat(mcp): operator config surface for the client-declared server allowlist Completes the §6.4 gate: enforcement landed in ba94efec, but the allowlist and per-server tool filter were a hardcoded table with no way for an operator to change them. Adds [[mcp.acp_servers]] entries of {name, tools}. Keyed by the declared name, never the id — the reference client mints its id as a fresh UUID per connection, so an allowlist of ids could not match twice. ServerPolicy now separates the two jobs that were conflated in one field, which is the layering §6.3 settled: - `allowed` is the deny-all gate over tool NAMES; - `seed` is the pre-attach advertisement (full Tool values), always a subset of `allowed`, so narrowing the policy narrows the catalog and can never widen it. Operators may list tools by name alone. For a server with a built-in catalog the schemas are taken from it and narrowed to what was permitted, so restricting the browser to read_dom needs no restated JSON schema. A server admitted by name with no built-in catalog has no seed yet: it dispatches, but advertises nothing until discovery caching can fetch its real schemas — which is precisely the job that gives the cache a non-redundant purpose. Two deliberate behaviours, both tested: - an ABSENT/empty section keeps the built-in browser default, so omitting the config cannot silently break existing browser control; - writing ANY entry takes over the allowlist wholesale — browser is not retained alongside an operator's list, so the config never grants more than it states. Co-Authored-By: Claude <noreply@anthropic.com> * feat(acp): discovery cache — fetch each declared server's real tools/list Implements §6 F3'. A server an operator admitted by name alone had no schemas to advertise; it could dispatch but was invisible. Discovery fills that gap, which is the job that gives the cache a non-redundant purpose. Two deviations from §6.3 as written, both applied to the ADR in this commit: 1. The cache is keyed by (channel_id, NAME), not (channel_id, server_id). Ids are minted per connection, so an id-keyed entry would be orphaned by exactly the reconnect the cache exists to survive — it could never outlive the attach that populated it, which is the opposite of "serve regardless of current attach state". This is a corollary of the id-vs-name distinction §6.1 already records. Same-name collisions are impossible under last-attach-wins, so the name is a safe key. Covered by a test that reconnects under a fresh id. 2. Discovery is pull-triggered, not attach-triggered: a declared server with no cache entry has its fetch started from the next tools() call and its real set appears one discovery round later. The facade re-reads the catalog on every call, so one round of staleness is the whole cost, and it avoids threading an attach hook from the gateway (which owns attach) into the root (which owns the source). The cache stores what the server PUBLISHED, unfiltered, and the policy is applied on read. Filtering on read means tightening the policy takes effect immediately rather than waiting for an entry to be invalidated, and it keeps the invariant that caching is never itself a grant: a server that publishes a tool the operator never permitted stays both invisible and uncallable. Tested. A failed fetch leaves the seed in place — a seeded server never drops to empty — and clears its in-flight marker so the next round retries. Repeated discovery rounds do not pile up duplicate tools/list requests on one tunnel. Co-Authored-By: Claude <noreply@anthropic.com> * test(acp): two client-declared servers in one session Covers the multi-server claim §6.2 makes, through the real source: one session declares `browser` and a second, non-browser server; both are discovered and callable, tool names do not collide, and each server's policy is enforced independently. - both servers contribute to one catalog, each under its own prefix, with no duplicate names (the case a naive un-prefixed catalog would collapse); - each tool reaches the tunnel of the server that declared it, by name -> id; - a permission granted to one server does not leak to another: browser.click is permitted while notes.click is refused, proving the gate is per-server rather than a global tool-name allowlist — an easy thing to regress in a refactor and invisible with only one server configured; - one server detaching leaves its neighbour callable. SCOPE: this is the source-side half of F6, not a full end-to-end. F6 asks that "the agent discovers + calls tools from BOTH", and the agent-side leg runs through the facade's meta-tools — which cannot be exercised while facade mode is not live anywhere (no [mcp] configured; the browser deployment runs OPENAB_BROWSER_MODE=bridge). That is the same precondition F5 is blocked on. The gateway-side half — two type:acp servers each getting their own mcp/connect — is already covered by section_tunnel in scripts/acp-ws-smoke.py. What remains genuinely unproven is the facade <-> source seam. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(mcp): rename the client-declared browser surface to katashiro.* Counterpart to the katashiro-side rename. `browser` / `browser.*` collided with Playwright MCP's `browser_*` tools; the declared name and tool prefix become `katashiro` / `katashiro.*`. - mcp_proxy::browser_tools(): the five seed tools (D4 static-advertise) - browser_source::builtin_catalogs(): the catalog key, and every test that rides the default policy - config: the acp_servers doc comment naming the built-in default - docs: tunnel contract declaration + tool table, agent-setup tool list Both injection-regression tests (`unpinned_tool_on_an_allowlisted_server_is _refused`, `caching_is_never_itself_a_grant`) called `browser.exec`. Left unrenamed they still assert is_err, but for the wrong reason — an unknown server name rather than an unpinned tool on a trusted one — quietly gutting the check. They now call `katashiro.exec`. Names only; policy semantics, routing and schemas unchanged. Co-Authored-By: Claude <noreply@anthropic.com> * docs(adr): follow the katashiro.* rename through both ADRs 270a2ffd renamed the client-declared surface (`browser.*` -> `katashiro.*`) in code, config and the tunnel contract, but the two ADRs still described the old tool names — including the §6.4 pinned-tool list and the runtime sequence diagrams, which readers would otherwise copy verbatim into a policy that no longer matches. The one remaining `browser.click` is inside a verbatim quote of upstream #1454's sources.rs doc comment and is left as written. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: lead the browser-MCP setup guide with facade mode, demote proxy/bridge The guide still opened with the per-session loopback proxy as "how it reaches the agent" and only mentioned the facade in a trailing section, so a reader took the superseded design as current. Facade mode has been the default since bf37d25e. - Lead with a mode table (facade default; proxy/bridge as explicit opt-outs) and the `[mcp]` + `[[mcp.acp_servers]]` config needed to enable it. - Document what actually changes under the facade: one listener, a static write-once entry referencing `${OPENAB_SESSION_TOKEN}` (secret rides the process env, not a file), and discovery via search_capabilities rather than the agent's own tools/list. - Note the consequence the old text got backwards: because the entry is static, hand-configuring a variant openab doesn't auto-write is now viable. The old "gated on a stable browser-MCP endpoint" caveat is resolved, not pending. - Keep proxy's per-variant config table under an explicit "Legacy" heading. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(adr): fold the browser ADR into the reverse-MCP ADR as its worked example Consolidates the ACP ADR family from four documents to three (base, the original @pahud proposal, and this one). The browser design was split out earlier in this PR, but with the facade integration the two documents had grown overlapping diagrams and the browser ADR had accumulated content that no longer described anything shipping. Folded in as §7 "Worked example — browser control": the toolset (incl. why the declared name moved to `katashiro`), D1-D6 with their supersession notice, and the message-level round-trip with the two id spaces. Dropped rather than carried over: - "Execution flow (bootstrap)" — described the pre-facade per-session-proxy boot path and duplicated D2/D3/D5. - "Tasks (as executed)" — project-management history; the commits are the record. - The separate context/references sections, which duplicated §1 and §11. Net ~90 lines smaller than the two documents were. Referrers updated, including the two links in the merged OAB MCP Adapter ADR; no dangling references remain. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(mcp): note that mcp.audit must be named in RUST_LOG or auditing is silently off `mcp.audit` is a bare tracing target, not under the `openab` prefix, so the filter the deployment docs and our own fleet use — `RUST_LOG=openab=debug,openab_agent=debug` — matches none of the audit events and drops every audit line. Nothing indicates that auditing is disabled, so a deployment can believe it has a tool-call audit trail and have none. Found while verifying a live facade dispatch: the call demonstrably executed (the tool result reached the agent) with zero audit output. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: warn that a leftover mode entry silently bypasses the facade Each transport writer only adds its own mcp.json entry — facade writes `openab`, bridge writes `openab-browser` — and neither removes the other's. An agent that has run in both modes therefore loads both servers, exposing the same `katashiro.*` tools twice: once through the facade (policy + audit) and once straight through the old transport (neither). The model calls the direct one and the call leaves no audit trail at all, while appearing to work perfectly. Corrects the evidence in 1c1919ce: that commit attributed the missing audit lines to `RUST_LOG` alone, having assumed the observed call was a facade dispatch. It was not — a stale bridge entry was carrying it. The RUST_LOG note there is still correct and still required; it was simply not the reason auditing looked dead in that instance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci: run the acp-mcp core and acp root tests `cargo test --workspace` builds with default features, so two sets of tests this PR added never compiled in CI: openab-core's `acp-mcp`-gated mcp_proxy tests and the root package's `acp`-gated browser_source / browser_bridge tests. 67 tests between them — including the capability source's routing and trust-gate coverage — so the fixes they back could not gate a merge. Add two steps mirroring the existing acp-gateway one. The core step is filtered to `mcp_proxy::` on purpose. `acp-mcp` gates exactly one module, so the filter loses no coverage, and an unfiltered `-p openab-core` would pull in hooks::tests — the parallel flake the gateway step's comment already documents avoiding. Co-Authored-By: Claude <noreply@anthropic.com> * fix(acp): do not open tunnels after a rejected session/resume The read loop derived its own channel_id from the requested sessionId and used that as the condition for spawning tunnels. A well-formed `sess_<uuid>` derives successfully on all four of the handler's rejection paths — missing sessionId, malformed sessionId, per-connection cap, busy — so the guard was not checking what it appeared to check. Combined with last-write-wins same-name re-attach, a refused resume could evict the live tunnel it had just been refused in favour of: a client mid-prompt (busy) or over the cap would knock out the browser control of the session that legitimately held it. handle_session_resume now returns (JsonRpcResponse, Option<String>), handing back the channel only when the resume actually succeeded, and the loop spawns only on that Some. This mirrors handle_session_new's (resp, channel_id) and deletes the independent derivation rather than adding a second check beside it — leaving the derive in place would keep a misleading guard available for reuse. Regression coverage asserts a None channel on each of the four rejections. The over-cap and busy cases are the load-bearing ones: their sessionIds are well formed and their sessions really exist, so the old guard produced a channel and spawned. Co-Authored-By: Claude <noreply@anthropic.com> * fix(acp): do not mint a facade session token when its config write fails When write_facade_mcp_config failed, the code warned and carried on: it minted OPENAB_SESSION_TOKEN and spawned the agent anyway. The agent then had no `openab` entry, so it could not reach the facade at all, while a live credential stayed registered for that channel until eviction — and the only trace was a warning. Mint only when the write succeeded. The session still starts; it simply has no browser capabilities, which is the honest description of what happened. The failure is logged at ERROR, and with no token there is no revoke guard to arm. Two alternatives were considered and rejected. Aborting session setup lets a config-write failure kill an otherwise working agent, and browser control is one capability among many. Falling back to a direct transport is worse than it looks: proxy and bridge write into the same workdir, so a failure there is likely to repeat, and silently switching to a direct entry re-creates the facade-bypass this PR's other fix exists to remove. Extracted setup_facade_session so the invariant is testable — the pool's tests are pure-function units and driving the real path spawns an agent. A counting registrar proves mint is never called when the write fails, forced by making <workdir>/.cursor a file so create_dir_all errors. Co-Authored-By: Claude <noreply@anthropic.com> * fix(mcp): retire the direct browser transport when facade mode sets up The facade writer added its `openab` entry but left any previous `openab-browser` entry in place, so both loaded and the model could take the direct path — reaching the browser without passing through facade policy and audit. Observed live 2026-07-26 and until now only documented. Remove the stale entry from all three places the direct transports wrote: .cursor/mcp.json, .kiro/settings/mcp.json, and the kiro per-agent files. For the agent files the `@openab-browser` grant goes too — `allowedTools` is default deny, so a leftover grant is what keeps the bypass reachable even once the server entry is gone; removing one without the other is a half fix. Ownership is decided by exact shape, never by the key. `openab-browser` is not proof we wrote it, and an operator may have configured their own server there. Only the two shapes we ever wrote are removable: the bridge entry {command:"openab",args:["browser-bridge"]}, and the per-session proxy entry — a loopback http://127.0.0.1:<port>/mcp url carrying a bearer header. A remote url, a bearer-less loopback, a different command or an empty port are treated as operator-owned and preserved verbatim. The matcher deliberately errs toward under-removal: a leftover entry only preserves the bypass, while deleting an operator's configuration destroys work. Tests cover shape recognition against five foreign shapes, removal alongside untouched user servers and unrelated top-level keys, a foreign `openab-browser` preserved verbatim, and the agent-file case where @github survives while @openab-browser goes. Co-Authored-By: Claude <noreply@anthropic.com> * fix(acp): keep the 8 MiB frame allowance to tunnel results only Raising MAX_FRAME_BYTES to 8 MiB for browser tool results also raised it for every other inbound frame, so one connection could hold MAX_INFLIGHT_PROMPTS (32) x 8 MiB of prompt text — the ~256 MiB worst case the review flagged. Bound the raise to the traffic it was for. Browser results arrive as client RESPONSES to our server-initiated `mcp/message` requests — id present, no `method` — so responses keep the 8 MiB ceiling, while every method-bearing frame (session/prompt included) is held to MAX_NON_TUNNEL_FRAME_BYTES, the pre-existing 1 MiB. That is what removes the exposure: the worst case came from prompts, which are method-bearing. Note this is deliberately not a `method == "mcp/message"` test, even though that is the obvious reading. `mcp/message` is only ever sent outbound; there is no inbound frame carrying that method, so matching on it would cap the screenshot responses at 1 MiB and break the case the raise exists for. The 8 MiB check stays pre-parse and still closes the connection: an oversized frame cannot be parsed back to its id, so no response can be fabricated for it. The per-kind check runs after parsing, where the id is available — oversized requests get ACP_OVERLOADED with their id, and oversized notifications are dropped without a reply, since answering a notification is a protocol violation. The unbounded outbound channel is untouched and remains a documented follow-up inherited from #1418 F6; this change bounds only what this PR added. Co-Authored-By: Claude <noreply@anthropic.com> * fix(mcp): authenticate the bridge connection, not the frame The unix socket authenticated nothing. 0600 proves the peer shares our uid, but says nothing about which session it belongs to, and the channel_id in each frame is a value the caller picks — so any same-uid process could connect and drive another live session's browser. Derive the channel server-side instead. The shim already walked its own /proc ancestry for OPENAB_BROWSER_CHANNEL; that logic was right but ran on the wrong side, because a caller can always lie about its own answer. The server now takes the peer pid from SO_PEERCRED and runs the same walk itself, so the peer cannot choose. A connection whose channel cannot be established is refused outright rather than given a default session. Frames may still carry channel_id — the shim sends it — but it is only ever compared against the authenticated value, never used to select a session; a mismatch is dropped and logged. The ancestry helpers move from the shim into openab-core, next to the server that now authenticates with them, so there is one implementation rather than two that can drift. The shim delegates to it and its frame value is advisory. serve_browser_socket keeps its signature; serve_browser_socket_with_resolver takes an injectable peer->channel mapping because a test binary's ancestry carries no channel, so the real resolver would refuse every test connection. The regression test proves a frame naming another session gets no reply at all while the next legitimate frame is answered. Co-Authored-By: Claude <noreply@anthropic.com> * fix(mcp): revoke facade session tokens by token, not by channel Session lifetimes overlap. `mint` replaces whatever token a channel holds, so a replaced session's drop guard runs after its successor has already minted — and the guard revoked by channel, which removed the live token. The new agent lost facade access wit…
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Groundwork for routing #1447's per-session browser capability through the facade (one listener, one discovery surface), per tonight's design discussion. Self-contained: no behavior change for existing deployments.
What's added
sourcesmodule (openab-mcp):CapabilitySourcetrait — in-process providers registered at facade construction (no mcp.json entry, no extra listener, no subprocess). Every call receives an optionalSessionCtx;requires_session()marks session-bound sources.SessionTokens— the broker↔facade identity contract: broker mints one opaque token per agent session (delivered to that agent's MCP config asAuthorization: Bearer), revokes on evict; facade resolves it per request from the HTTP parts rmcp injects into request extensions (documented rmcp 1.7 mechanism — stateless, no session-binding map). Constant-time token comparison.with_sources/serve_http_with; discovery merges source tools into the catalog (downstream names win; shadowed source tools published asprovider:tool, mirroring the existing duplicate rule); execution dispatches sources first with the same JSON-Schema pre-flight the meta-tool applies; audit lines carry the channel id.Self-review round 2
Two more findings, fixed:
provider:tool(matching execution's registration-order resolution; regression test added).args_sha256like the meta_tool dispatcher — one audit shape across both dispatch paths.Noted-not-changed (deliberate):
SessionTokensusesstd::sync::RwLockwithexpect(no user code in the critical sections, poisoning is unreachable short of an allocator panic); execution resolving the downstream catalog before source dispatch adds a mostly-cache-hit lookup to source calls — consistency with the publish rule is worth that.Validation
openab-mcp: 214 tests pass (7 new: full-HTTP session-gating e2e; token mint/replace/revoke lifecycle, constant-time eq, ctx-from-http-parts resolution incl. no-parts/wrong-token; facade: session-bound invisibility+unreachability, ctx-routed execution, anonymous host-level source + schema pre-flight rejection)--workspace --all-features -D warningsclean; fmt cleanReview Contract
Goal
Give the facade a session-aware in-process capability-source interface and a broker-shared session-token registry, so session-bound capabilities (browser control, #1447) can later be served through the single facade listener and discovery surface.
Non-goals
Accepted Residual Risks
Acceptance Criteria
requires_sessionsource (tests).tower.rsdocumentedParts-in-extensions pattern).Follow-ups
BrowserSourcein the root binary (tunnel-backed), SessionPool mint/revoke + config-writer reuse, retire the per-session proxy default (Option C bridge untouched).requires_session=falsesource, retiring its separate port.