Skip to content

fix: create PR for chart bump instead of direct push - #8

Merged
thepagent merged 1 commit into
mainfrom
fix/ci-bump-pr-v2
Apr 3, 2026
Merged

fix: create PR for chart bump instead of direct push#8
thepagent merged 1 commit into
mainfrom
fix/ci-bump-pr-v2

Conversation

@thepagent

Copy link
Copy Markdown
Collaborator

After image build, bot creates a chart bump PR for human approval. Merging triggers release.yml.

@thepagent
thepagent merged commit 7d215de into main Apr 3, 2026
henrieopenclaw added a commit to henrieopenclaw/agent-broker that referenced this pull request Apr 6, 2026
Co-authored-by: thepagent <thepagent@users.noreply.github.com>
Reese-max pushed a commit to Reese-max/openab that referenced this pull request Apr 12, 2026
Co-authored-by: thepagent <thepagent@users.noreply.github.com>
chaodu-agent pushed a commit that referenced this pull request Jun 24, 2026
- Add child.wait() arm to URL-collection select! so a fast-failing auth
  command reports immediately instead of stalling the full 30s window (#1)
- Reject bot users in /auth, consistent with /remind (#8)
- Record invoking user_id in the auth start audit log (#7)
- Truncate output by UTF-16 code units to match Discord's 2000-char limit,
  preventing rejection on non-BMP-heavy output (#5)
- Handle std::sync::Mutex poison in drain/collect paths to avoid panic
  cascade and silent output loss (#9)
- Clarify the 'no output' error message with cause and remedy (#6)
- Fix docs intro contradicting /auth DM-only and mark it DM-only in the
  command table (#3)
brettchien added a commit to brettchien/openab that referenced this pull request Jun 24, 2026
The §9 Q4 tail referenced 'openab-agent-mcp.md open items openabdev#1 (reqwest
0.12/0.13 split) and openabdev#8 (doctor/runtime two-store split)' — but that ADR's
§10 Open Questions has only two items (mcp.json location; native-vs-broker
parity), neither matching, and no such numbered items / terms exist anywhere
in it. The phantom reference dated to the original draft. Replace with an
accurate statement: McpCredentialStore reuses the same TokenStore/auth.json
storage (openab-agent-mcp.md §6.1), so the lock lands once and serves both;
the reqwest version conflict is the rmcp-OAuth dependency issue surfaced on
the feat/openab-agent-mcp-resilience PR, not an mcp-ADR open item.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thepagent added a commit that referenced this pull request Jun 27, 2026
…g (codex + MCP) (#1190)

* docs(adr): openab-agent multi-vendor OAuth & credential storage

Proposed ADR for the openab-agent LLM-provider OAuth revamp: a two-axis
OAuthVendor adapter (auth flow vs inference transport), a cross-process
flock-guarded credential-store invariant for auth.json, the
CLAUDE_CODE_OAUTH_TOKEN env route, a 14-variant vendor feasibility matrix,
and the /auth (PR #1185) auth-trigger model. Surfaced while reviewing
PR #1187 (first OAuth vendor).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(adr): address review — prefer oauth2 crate, drop rollout, vendor names

- Build the OAuthVendor driver on the official `oauth2` crate (already in-tree
  via the MCP side) instead of a hand-rolled PKCE/exchange/refresh flow; the
  Anthropic JSON-token-body quirk is applied via the crate's custom http-client
  hook. Reframe §8 accordingly (hand-rolled flow is the rejected alternative).
- Remove the project Rollout-plan section (internal sequencing, not ADR
  material); keep the race-window mitigation in §5.4.
- Use vendor names only; drop internal fleet-agent references from the matrix.
- Replace unexplained "ToS-gray" with "ToS-risk" + a definition.
- Fix cross-references (crate-qualified paths, §-refs, line numbers) and move
  the settled model-default decision under "Decisions & open questions".

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(adr): per-tenant refresh lock + per-user PKCE keying (Mira review r1)

Fold in two release-blocker-class concurrency fixes from Mira's review:

- §5.4 refresh-token rotation: the prior "refresh outside lock, re-read on
  commit" claim was wrong — N processes each send a refresh with the same
  RT_old before committing, tripping OAuth 2.1 §10.4 reuse -> token-family
  revocation. Replace with a per-tenant exclusive lock: network refresh held
  under the tenant lock only (not the global lock), so exactly one refresh per
  tenant per expiry with no head-of-line blocking across tenants. Extends to
  mcp:<server> tenants.
- §7 /auth: key the pending PKCE verifier+state by the initiating Discord user
  id instead of a single global entry — prevents concurrent-user overwrite
  (PKCE mismatch) and session hijack.
- §5.1 OAuthVendor::redirect() -> Option, since device flows have no redirect.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(adr): bundled-secret storage + pending-entry GC (Mira review r2)

- §6/§9 Q2: correct the stale "git-safe" claim on the gemini GOCSPX-
  secret. The value is non-confidential by RFC 8252 / Google docs, but
  GitHub now push-protects Google secrets by default (changelog 2026-03)
  and partner-scans them for auto-revoke, so a raw literal is not safe in a
  public repo. Decide: encode-at-rest (scanner-evasion for a non-secret, NOT
  a security control) or env-inject at runtime — not raw text.
- §7: pending PKCE entries get created_at + a 15-min GC sweep in
  with_auth_locked so abandoned /auth attempts don't accumulate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(adr): close §9 Q2/Q3 — env-injection default, vendor go/no-go (Brett decisions r3)

- §9 Q2 DECIDED: env-injection (b) is the default for the gemini/agy bundled
  client_secret; encode-at-rest (a) is the fallback for bundled zero-config
  binaries only, framed as scanner-evasion (not security). Cite rclone
  rcloneEncryptedClientSecret + obscure.MustReveal() as the canonical
  precedent (§10).
- §9 Q3 DECIDED: GO gemini/grok (first wave) + agy (experimental, opt-in,
  ToS caveat — shares gemini's Code-Assist provider, residual risk is ToS not
  secret storage); No-Go cursor/kiro. Mirror as a build-decision line under §6.
- §10: add rclone + GitHub/Google secret-scanning references.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(adr): confirm agy secret + ecosystem evidence (GitHub survey r4)

GitHub survey 2026-06-24 grounds the agy GO decision:
- §6/§9 Q2: agy client_secret requirement CONFIRMED — it needs a GOCSPX-
  secret, a public constant >=20 antigravity-auth repos hardcode verbatim
  (NoeFabris/opencode-antigravity-auth, router-for-me/CLIProxyAPI, ...).
  Literal deliberately NOT pasted into the doc — would trip the very §9 Q2
  push-protection, so we dogfood the env/encode decision. Redirect confirmed
  localhost:51121/oauth-callback.
- §9 Q3: ecosystem evidence — agy OAuth is widely ported (opencode/pi/hermes/
  openclaw plugins + proxies), proving the integration, while the same
  ecosystem's anti-ban / quota-locking / multi-account-rotation tooling
  empirically confirms the ToS-ban + 429 risks, reinforcing the opt-in gate.
- §10: add the antigravity OAuth ecosystem reference set.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(adr): agy-vendor vs agy-CLI/ACP clarification + MCP store revamp in scope (r5)

Grounded by a codebase survey (2026-06-24):

- §4/§6: clarify "agy as a GO vendor" means a native OAuthVendor + Code-Assist
  inference provider — it does NOT run the agy CLI. agy speaks no ACP; the
  existing `antigravity` runtime variant (Mira/ECS) only works via a dedicated
  agy-acp adapter that shells out to the agy binary per prompt and polls its
  SQLite DB. The provider path sidesteps ACP entirely and supersedes the
  CLI-wrapper for native use, so agy's lack of ACP doesn't block the GO.
- §5.4/§9 Q4: make the MCP CredentialStore revamp explicitly in-scope. auth.json
  has NO lock today (only atomic rename); provider save_tokens (auth.rs:234) and
  McpCredentialStore::save/clear (auth.rs:284-328) are two independent unlocked
  RMW callers, so with_auth_locked must wrap BOTH or the race persists. Also
  correct the stale save_tokens_for name and note with_auth_locked is new.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(adr): fix stale §9 Q4 cross-reference

The §9 Q4 tail referenced 'openab-agent-mcp.md open items #1 (reqwest
0.12/0.13 split) and #8 (doctor/runtime two-store split)' — but that ADR's
§10 Open Questions has only two items (mcp.json location; native-vs-broker
parity), neither matching, and no such numbered items / terms exist anywhere
in it. The phantom reference dated to the original draft. Replace with an
accurate statement: McpCredentialStore reuses the same TokenStore/auth.json
storage (openab-agent-mcp.md §6.1), so the lock lands once and serves both;
the reqwest version conflict is the rmcp-OAuth dependency issue surfaced on
the feat/openab-agent-mcp-resilience PR, not an mcp-ADR open item.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(adr): §9 Q2 — encode-at-rest is the default, env is the alternative (Brett)

Brett's call given the 93%-plaintext ecosystem survey: keep the bundled
zero-config UX via encode-at-rest (obscure, rclone obscure.MustReveal style)
as the DEFAULT, with env-injection as the alternative for fleet/pod. Reverses
the prior (b)-default ordering. Framing unchanged: encode-at-rest is
scanner-evasion for a non-confidential value, not a security control. Record
the survey numbers (99/107 plaintext; auto-revoke largely unrealized) and that
the real risk mitigated is org-repo push-protection friction, not credential
loss.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(auth): cross-process locking for auth.json — codex + MCP (ADR §5.4)

Implements the ADR §5.4 invariant. auth.json had NO lock: provider
save_tokens and McpCredentialStore::save/clear each did an independent
unlocked read-modify-write, so a concurrent codex refresh and MCP save
last-writer-wins the whole map, and N processes (one openab-agent per Discord
thread) could each refresh the codex token with the same RT_old → OAuth 2.1
§10.4 token-family revocation (fleet-wide logout).

Two locks, flock(2) on sidecar files (kernel auto-releases on death), cfg(unix)
with a non-unix no-op:

- with_auth_locked: global exclusive lock across the re-read -> mutate ->
  atomic-write. ALL writers funnel through it — save_tokens (codex) and
  McpCredentialStore::save/clear (MCP) — so writers merge onto the latest
  on-disk state instead of lost-updating. Held only for the fast file RMW,
  never across network I/O.
- lock_tenant_refresh: per-tenant refresh serialisation. get_valid_token /
  force_refresh take the codex tenant lock (non-blocking try + async backoff +
  10s timeout, held across the network refresh), with a double-checked re-read
  so a process that waited adopts the token another already refreshed → exactly
  one real refresh per tenant per expiry, no RT_old reuse.

Uses libc::flock (already a cfg(unix) dep); rustix is NOT in-tree (ADR text was
optimistic). New test asserts the locked RMW merges codex + MCP tenants without
lost-update. fmt + clippy -D warnings + test (191 passed) green.

Known gap (for review): the MCP *network* refresh is owned by rmcp's
AuthorizationManager (it refreshes then calls CredentialStore::save), so MCP
writes get the file-integrity lock but MCP refreshes are not yet tenant-
serialised. Closing that needs an rmcp-level hook — proposed as follow-up.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(auth): address Mira code review — pending GC + portable WouldBlock

Round-2 code review (Mira):
- §7 pending-entry GC: add created_at to PendingPasteLogin and sweep
  AuthEntry::Pending older than 15 min inside with_auth_locked on every write,
  so abandoned /auth two-step attempts don't accumulate. PendingPasteLogin is
  currently a legacy tombstone with no live writer (PKCE state lives in rmcp's
  in-memory StateStore); the field + GC land now per ADR §7 and are
  forward-compatible with the forthcoming /auth two-step flow, and meanwhile
  sweep legacy stray entries (created_at default 0 reads as ancient). New test
  covers stale-swept / fresh-kept / real-tenant-untouched.
- flock_try_exclusive: match std::io::ErrorKind::WouldBlock instead of a single
  raw EWOULDBLOCK errno, covering EAGAIN/EWOULDBLOCK across libc/BSD.
- MCP network-refresh serialisation stays a follow-up (rmcp owns the refresh) —
  Mira concurs.

fmt + clippy -D warnings + test (192 passed) green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(auth): serialise MCP token refresh cross-process (close §5.4 (b) gap)

Closes the known gap from f370110: MCP refreshes now get the same per-tenant
serialisation as codex.

rmcp's CredentialStore exposes no pre-refresh hook, but openab drives the MCP
refresh explicitly in McpRuntimeManager::resolve_oauth_dial via
client.get_access_token(). Wrap that call (per-server) with the existing
auth::lock_tenant_refresh so only one process refreshes a given server at a
time. No explicit double-check needed: rmcp's get_access_token re-load()s
auth.json each call and returns the cached token without a network refresh when
remaining >= REFRESH_BUFFER (rmcp auth.rs:1238), so a process that loses the
race adopts the token the winner wrote to the shared file — no second RT_old
presentation, no OAuth 2.1 §10.4 family revocation. rmcp already single-flights
within one process via its AuthorizationManager Mutex; this closes the
cross-process gap.

- auth.rs: lock_tenant_refresh + AuthFileLock made pub(crate) for the mcp module.
- ADR §5.4: document the resolve_oauth_dial serialisation point.

fmt + clippy -D warnings + test (192 passed) green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(auth): /simplify cleanups — dedup lock acquire, reuse refresh fd

Quality-only cleanups from a 4-angle /simplify pass (no behaviour change):
- Extract lock_global(path) -> Result<Option<AuthFileLock>> and route both
  with_auth_locked and McpCredentialStore::clear through it, so the "global"
  sidecar name + the cfg(unix) acquire live in one place instead of two
  copy-pasted blocks. clear flattens (drop the inner run closure).
- lock_tenant_refresh opens the lock fd once and re-issues flock on it each
  retry, instead of re-opening (and re-create_dir_all-ing) the file every
  100ms under contention. Removes the now-unused flock_try_exclusive helper.
- Document lock_tenant_refresh's double-check contract (the lock only
  serialises; callers must re-check freshness after acquiring).

fmt + clippy -D warnings + test (192 passed) green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(auth): /code-review — harden GC + refresh-lock contract comments

Doc-only clarifications from a /code-review pass (no behaviour change):
- PendingPasteLogin.created_at: warn loudly that any writer of a fresh Pending
  entry MUST stamp created_at, else gc_stale_pending sweeps it on the next
  locked write (latent footgun for the forthcoming /auth two-step writer).
- lock_tenant_refresh: correct the contract — reuse-safety comes from loading
  the refresh token INSIDE the lock (so force_refresh, which always refreshes
  on a 401, is reuse-safe too); the post-lock expiry re-check is only an
  optimisation to skip a redundant refresh.

Review also surfaced a pre-existing, out-of-scope namespacing gap (an MCP
server literally named "codex" collides with the codex tenant's auth.json key
and refresh lock — committed MCP creds use the bare server name, not
mcp:<server> as ADR §5.4 describes) — flagged for a separate change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* build(openab-agent): declare standalone [workspace] to fix CI

ci-openab-agent.yml runs cargo fmt/clippy/test/build with
working-directory: openab-agent, but the crate is not a member of the parent
openab workspace (members = crates/openab-core, openab-gateway) and was not
excluded, so cargo errors 'current package believes it's in a workspace when
it's not' and every step fails before doing any work. openab-agent is
intentionally standalone (own version + dual reqwest 0.12/0.13 for rmcp), so
the correct fix is an empty [workspace] table making it its own root.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Revert "build(openab-agent): declare standalone [workspace] to fix CI"

This reverts commit b40693c.

* ci(openab-agent): append [workspace] in CI + harden ACP smoke test

ci-openab-agent.yml ran cargo from working-directory: openab-agent without the
[workspace] table the crate needs (it's standalone, not a parent-workspace
member), so every step failed with 'believes it's in a workspace when it's
not' — the workflow had been red independently of any PR. Dockerfile.unified
already works around this by appending the table at build time; replicate that
in the workflow rather than committing it to Cargo.toml (which would
double-append in the Dockerfile and break the image build).

Also harden the ACP smoke test: build the release binary in its own step and
bump the response timeout 5s -> 30s so a loaded runner doesn't flake the
agent's first-response window (the binary itself responds in <1s locally,
verified incl. a clean-HOME release build).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* ci(openab-agent): surface ACP smoke-test stderr + exit code for diagnosis

The smoke test produces empty stdout only in CI (the binary responds with
agentInfo in <1s locally across debug/release/clean-HOME/CI-env-var runs).
Capture stderr + exit code so the next CI run reveals why stdout is empty.

* openab-agent: address review findings on OAuth ADR + auth.json locking

Resolves the Changes-Requested review on the multi-vendor OAuth ADR +
cross-process auth.json locking PR. No behavioural change beyond log level
and a non-unix diagnostic.

ADR (docs/adr/openab-agent-oauth.md)
- Fix lock-file names to match code: auth.json.global.lock and
  auth.json.refresh.<tenant>.lock (were auth.json.lock / auth.json.<tenant>.lock).
- Add the path parameter to the with_auth_locked pseudocode signature.
- Correct the MCP refresh note: initialize_from_store() does the disk reload,
  not get_access_token.
- Correct the crate note: libc::flock directly (rustix is not in-tree).
- Document the deliberate fail-open trade-off on tenant-lock timeout.
- Mark the default-model removal (Decision 1) as a follow-up; this PR ships
  the ADR + locking only.

Code
- auth.rs: escalate the tenant-lock timeout log from warn! to error! and
  document the fail-open trade-off on lock_tenant_refresh.
- auth.rs: non-unix lock_global no-op now warns once instead of silently
  providing zero cross-process protection.
- mcp/runtime.rs: fix the rmcp reload comment and document the cross-module
  invariant that the refresh lock and credential entry share the server name.

CI (ci-openab-agent.yml)
- ACP smoke test now fails on a non-zero/timeout exit code, and the agentInfo
  assertions use { } so exit fails the step rather than just a subshell.

Gate: cargo fmt --check, clippy -D warnings, test (192 passed) all clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* openab-agent: fail closed on a contended refresh lock (F6)

Replaces the fail-open tenant-lock timeout with a bounded-refresh + fail-closed
design, so a contended lock can no longer reintroduce the double-refresh it
exists to prevent.

- Bound the refresh network call with an explicit 8s HTTP timeout on both the
  codex client (auth.rs) and the MCP AuthClient (mcp/runtime.rs), strictly
  shorter than the 10s lock-acquire deadline. With flock(2) auto-release on
  death, a live holder always frees the tenant lock before a waiter's deadline,
  so a lock-acquire timeout is genuinely abnormal.

- lock_tenant_refresh now returns RefreshLock { Held, Unavailable, TimedOut }.
  On TimedOut callers fail closed instead of refreshing unserialised:
  - codex get_valid_token / force_refresh return a retryable error;
  - MCP resolve_oauth_dial returns a new OauthDialError::Transient that leaves
    the server retryable WITHOUT forcing re-login (NeedsAuth) or tripping the
    circuit breaker (auth-level failures still map to NeedsAuth as before).
  A filesystem error opening the sidecar returns Unavailable and degrades to a
  best-effort unserialised refresh rather than blocking every refresh.

- Add a fail-closed timeout test (injectable deadline) and update ADR §5.4 to
  document the bounded-refresh + fail-closed invariant, superseding the earlier
  fail-open note.

Gate: cargo fmt --check, clippy -D warnings, test (193 passed) all clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* openab-agent: size lock timeout above worst-case multi-call refresh hold

Mira review (PR #1190) found the fail-closed invariant held only for the codex
path. The MCP path holds the tenant lock across TWO sequential bounded calls —
rmcp's initialize_from_store() (AS discovery) then get_access_token() (refresh)
— so the worst-case lock-hold is ~2 x REFRESH_HTTP_TIMEOUT (16s), which could
exceed the fixed 10s lock deadline and fail a waiter closed while the holder is
still legitimately progressing.

Derive REFRESH_LOCK_TIMEOUT from the bound instead of hardcoding it:
  REFRESH_LOCK_TIMEOUT = MAX_REFRESH_ROUND_TRIPS (2) * REFRESH_HTTP_TIMEOUT + 4s
                       = 20s
so the deadline is always above the worst-case hold and only a genuinely stuck
holder trips it. Normal-case latency is unchanged (the waiter polls every 100ms
and acquires as soon as the holder releases). Update the fn doc and ADR §5.4 to
state the multi-call hold explicitly.

Gate: cargo fmt --check, clippy -D warnings, test (193 passed) all clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: thepagent <hehsieh1010@gmail.com>
thepagent added a commit that referenced this pull request Jun 29, 2026
…-agent (#1187)

* feat(native-agent): add Anthropic OAuth (Claude Pro/Max) login + provider

Phase 1 of porting Pi's auth methods into openab-agent. Adds an
`anthropic-oauth` tenant alongside Codex: PKCE browser/paste login against
platform.claude.com, JSON token exchange + scope-less refresh, and an OAuth
mode on AnthropicProvider (Bearer + Claude Code identity headers/system block,
tool-name normalisation). Wires provider selection in acp.rs/llm.rs and a new
`auth anthropic-oauth` CLI subcommand.

Verified: cargo build clean (0 warnings), 194 tests pass incl. 4 new.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(native-agent): default Anthropic model to claude-opus-4-8

The fallback default was claude-sonnet-4-20250514 (Sonnet 4.0, ~13mo old),
which 404s on Claude Pro/Max OAuth subscriptions. Bump the three default-model
fallbacks to the current claude-opus-4-8 (verified live via OAuth). The model
catalog already listed it; only the fallback was stale.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(native-agent): address PR review (CI workspace, PKCE state, error UX)

- F1 (blocker): root Cargo.toml `exclude = ["openab-agent"]` so
  `cd openab-agent && cargo fmt/clippy/test` resolves standalone. The workspace
  restructure left openab-agent neither a member nor excluded; CI openab-agent
  only runs on openab-agent/** so it was dormant on main — this PR is the first
  change to trigger it. Also ran `cargo fmt`.
- F2: use an independent 32-byte random PKCE `state` instead of reusing the
  verifier, keeping the verifier back-channel-only (claude.ai rejects a short
  state as "Invalid request format"; 32 bytes matches the verifier length).
  Verified end-to-end with a real Pro/Max login + chat.
- F3: credential-error messages now name fully-qualified subcommands
  (`openab-agent auth anthropic-oauth` / `... codex-oauth`) and preserve the
  underlying read/parse error.
- F4: drop the `ponytail:` placeholder tag from a comment.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(native-agent): flush stdout drain on ACP server shutdown

The dispatch loop fed responses to a detached stdout-drain task; on stdin EOF
the loop ended and `#[tokio::main]` aborted the drain before it flushed the
last queued line, so a one-shot `initialize` could return nothing. This was a
latent race (main wins it by timing); this branch's slightly different startup
timing made the binary lose it ~85% locally, surfacing as the red `CI
openab-agent` ACP smoke test. Capture the drain handle and, after the loop,
drop the senders and bounded-await the drain so queued output is flushed before
return. Race test: 20/20 after (was 3/20).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: address review #6/#7/#11 (auth mode, 401 refresh, error UX)

Non-blocking polish from the PR re-review:

- #6 (acp.rs): on ACP model switch, an OAuth-forced session was rebuilt
  via `auto_with_model`, which prefers ANTHROPIC_API_KEY and silently
  dropped the forced anthropic-oauth provider when a key was also present.
  Rebuild now preserves the session's auth mode via a new
  LlmProvider::is_oauth() (Agent::provider_is_oauth()).

- #7 (llm.rs): the OAuth 401 branch swallowed force_refresh_for errors
  (`let _ = ...`) and retried with the stale token. Bubble the error.

- #11 (auth.rs): refresh_token failure message named bare
  `openab-agent auth`; now names the tenant subcommand via a shared
  auth_subcommand() helper (also dedupes load_tokens_for).

Deferred as follow-up (noted in PR): #8 --no-browser state validation,
#9 save_tokens_for keying, #10 non-Unix atomic write.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHjGw1uBHXoVPB3dXYEy7h

* fix: address review #8 — always verify CSRF state on bare-code paste

`--no-browser` bare-code paste defaulted the pasted state to the expected
value when no `#state` was present, so the `st != state` check passed
trivially and CSRF state was never verified. Require the `code#state`
form (or a full redirect URL) and reject a bare code with a clear message.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHjGw1uBHXoVPB3dXYEy7h

* fix: don't pin a hardcoded Anthropic default model (review F4 follow-up)

Per @brettchien: dateless 4.6+ model IDs are fixed canonical IDs, not
evergreen pointers, so a hardcoded default (claude-opus-4-8) is a
per-generation 404 timebomb — the same failure that retired the previous
claude-sonnet-4-20250514 default. It also silently bumped API-key users
onto pricier Opus (review #5).

Resolve the Anthropic model as: explicit override → OPENAB_AGENT_MODEL →
error ("no model configured; set OPENAB_AGENT_MODEL or select a model").

- llm.rs: `anthropic_model()` is now fallible (no default); constructors
  refactored (`build`/`api_key_from_env`/`ensure_oauth_token`) so a model
  override never requires OPENAB_AGENT_MODEL, and credential errors still
  precede the model error. `auto()` only falls through to OAuth when no
  API key is present.
- acp.rs: session new/load report the provider's resolved model instead
  of a hardcoded fallback. Removed the opus/gpt default sites.
- Kept the claude-opus-4-8 entry in the model catalog (offering ≠ default).
- docs/native-agent.md: document OPENAB_AGENT_MODEL is required for
  Anthropic (zero-config now fails loud).

Behavior change: no zero-config default model. Deployments set it via
env/values.yaml; local/zero-config users must export OPENAB_AGENT_MODEL.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHjGw1uBHXoVPB3dXYEy7h

* docs: fix stale PKCE state comment (review F5)

The doc comment on login_anthropic_browser_flow still said the verifier
doubles as `state` (Pi's old convention); since the PKCE fix the state is
an independent 32-byte random value. Correct the comment to match.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHjGw1uBHXoVPB3dXYEy7h

* feat(native-agent): accept canonical provider/model in OPENAB_AGENT_MODEL

ModelRef::parse + resolve_provider_choice let OPENAB_AGENT_MODEL carry
`provider/model_id` (e.g. anthropic/claude-sonnet-4-6) as a single source
of truth for both provider and model. Bare model ids and the existing
OPENAB_AGENT_PROVIDER var remain fully backward compatible.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: satisfy clippy::manual_is_multiple_of (Rust 1.96 stable)

Stable clippy 1.96 added manual_is_multiple_of; pre-existing modulo checks
in openab-core (format.rs, pre_seed.rs) and openab-gateway (wecom.rs) fail
`clippy --workspace -D warnings`. Mechanical fix; unblocks CI. Unrelated to
the OAuth change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: limit OAuth 401 force-refresh to one retry

The comment said "retry once" but `attempt < max_retries` allowed up to
3 consecutive force-refreshes on persistent 401s — wasting round-trips
and risking rotating a still-valid refresh token off the server's grace
window. Use an `oauth_refreshed` flag to match the documented intent.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat(native-agent): introduce OAuthVendor descriptor abstraction (ADR §5.1)

Collapse the hand-rolled per-vendor OAuth surface into a single OAuthVendor
trait + CodexVendor/AnthropicVendor descriptors. Adding a subscription-OAuth
vendor is now a descriptor, not a new hand-rolled flow:

- Unify the codex + anthropic PKCE login flows into one shared `login_pkce_flow`
  driven by the descriptor; fold the codex flow into the `accept_callback_code` /
  `code_from_redirect` helpers (the long-standing TODO) and unify the 127.0.0.1 bind.
- Drive `refresh_token` and the authorization-code exchange off `vendor.token_body()`
  instead of an `if provider == ANTHROPIC_NAMESPACE` branch.
- Shared pure builders `build_authorize_url` / `token_store_from_payload`, pinned by
  wire-format unit tests — the login authorize-URL/exchange hit live OAuth servers,
  so no integration test covers them.

The driver keeps the proven reqwest flows; swapping the engine onto
`oauth2::BasicClient` (as `mcp/runtime.rs` already does via a custom http hook) is a
follow-up internal change invisible to vendor authors — only the descriptor surface
lands here. `client_secret()`/`grant()` are the ADR §5.1 surface for later vendors
(gemini/agy bundled secret; copilot/kiro device-code).

205 tests pass (4 new wire-format locks).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(native-agent): credential precedence + CLAUDE_CODE_OAUTH_TOKEN (ADR §5.3)

Add `AnthropicAuth::OAuthEnv` for the pre-provisioned long-lived subscription
token route (`CLAUDE_CODE_OAUTH_TOKEN`) — the recommended fleet mode (ops mints
once, injects as a k8s secret; no interactive flow, no auth.json write, no
refresh race).

- `AnthropicProvider::auto[_with_model]()` now resolves in ADR §5.3 precedence:
  `ANTHROPIC_API_KEY` → `CLAUDE_CODE_OAUTH_TOKEN` → stored `anthropic-oauth` tenant.
  Each source surfaces its own errors rather than falling through to a
  lower-precedence credential error.
- `OAuthEnv` shares the OAuth `Bearer` + Claude Code identity path (extracted into
  `oauth_headers`); `is_oauth()` covers it so the system block + tool-name casing
  apply. The 401 force-refresh is gated to the stored tenant only — the env token
  has no tenant to refresh, so a 401 there surfaces (re-mint) instead of erroring
  on a missing tenant.

207 tests pass (+2 precedence tests).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(native-agent): ModelRef::parse only splits known provider prefixes (review F4)

`provider/model` previously split on the first `/`, mis-parsing HuggingFace-style
`org/model` ids (e.g. `meta-llama/Llama-3-8B`) for custom/OpenAI-compatible
endpoints — `org` became the "provider" and the real id was truncated. Now the
prefix is split off only when it's a `KNOWN_PROVIDERS` entry; otherwise the whole
string is the model id. Load-bearing for the planned single-string `provider/model`
config field. Known prefixes still split unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(native-agent): fail loud on present-but-misconfigured Anthropic creds (review F3)

`select_provider`'s auto-detect arm discarded the `AnthropicProvider::auto()`
error and fell through to Codex. A present Anthropic credential that failed for a
config reason (e.g. a key set but no model) was silently masked — the user got a
Codex provider, or a Codex-only error that hid the real cause.

Now: if any Anthropic credential source exists (API key / CLAUDE_CODE_OAUTH_TOKEN /
stored tenant), an `auto()` failure surfaces as a config error ("credential
present but unusable"). Codex fallthrough happens only when no Anthropic
credential exists at all, and that final error names both credential routes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(native-agent): centralized config.json for default model/params (ADR §5.5)

Add a small JSON config file next to auth.json so a deployment can declare the
default provider/model (and max_tokens) in a file instead of only via env vars —
the centralized-config gap the multi-vendor work surfaced.

- New `config` module: `{ model, max_tokens }` (single `provider/model` string,
  reusing ModelRef). Unknown keys tolerated (forward-compat for `providers`/etc.);
  malformed JSON fails loud; a missing file is an empty config.
- Resolution is env-over-config: `anthropic_model` / `anthropic_max_tokens` /
  `resolve_provider_choice` now fall `OPENAB_AGENT_*` env → config.json → built-in.
  A pod's injected env stays authoritative over a baked config.
- Secrets never live here — they stay in the locked `auth.json` store.

Per-provider `base_url` routing is a deliberate follow-up; this lands the default
provider/model/params surface. 213 tests pass (+5).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(native-agent): document OAuthVendor, credential precedence, config.json (review F5)

- Anthropic credentials section: the ADR §5.3 precedence (ANTHROPIC_API_KEY →
  CLAUDE_CODE_OAUTH_TOKEN fleet route → interactive anthropic-oauth) + the
  `openab-agent auth anthropic-oauth` login.
- New "Configuration file (config.json)" section: schema, env-over-config
  precedence, secrets-stay-in-auth.json.
- "Adding an OAuth vendor": the OAuthVendor descriptor model (ADR §5.1).
- Env table: CLAUDE_CODE_OAUTH_TOKEN, OPENAB_AGENT_ANTHROPIC_CLIENT_ID,
  OPENAB_CONFIG_PATH.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* style: cargo fmt (OAuthVendor + config.json)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(native-agent): prove §5.4 lock single-flight for the anthropic-oauth tenant

Brett asked to cover the new tenant the same way #1190's codex lock tests do, so
single-flight / fail-closed is proven for it rather than only structurally similar:

- `with_auth_locked_merges_anthropic_tenant_no_lost_update` — a concurrent codex
  write does not clobber a just-written `anthropic-oauth` token (the new tenant
  rides the same locked RMW funnel).
- `lock_tenant_refresh_fails_closed_for_anthropic_and_is_per_tenant` — a held
  anthropic refresh lock makes a second anthropic acquire fail closed (`TimedOut`),
  and does NOT block codex (per-tenant isolation — the reason §5.4 uses a
  per-tenant lock, not the global one).

215 tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(review): OAuth model-switch uses env token before auth.json

F1/F2: select_provider and ACP model-switch now check
CLAUDE_CODE_OAUTH_TOKEN before falling back to the stored
anthropic-oauth tenant in auth.json. Fleet pods that only set
the env token can switch models without an auth.json file.

Adds from_oauth_auto() / from_oauth_auto_with_model() that
implement the env-over-store precedence for OAuth rebuild paths.

F5: config.rs logs malformed config at error level (was warn)
so production monitoring catches a typo'd config.json.

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Can <can@zeabur.com>
Co-authored-by: thepagent <hehsieh1010@gmail.com>
Co-authored-by: chaodu-agent <chaodu-agent@openab.dev>
thepagent pushed a commit that referenced this pull request Jul 13, 2026
* docs(adr): revise identity-trust-none to three-layer architecture

Receiver → Trust Gate → Handler replaces the previous
'gate at handle_message()' design. Addresses all findings
from the PR #1263 mob review (howie + 3 LLM reviewers).

Key changes:
- §4.2: Trust Gate is a dedicated ingress layer upstream of Handler
- §5: New architecture diagram showing three-layer separation
- §7: Implementation plan starts with Receiver/Handler split
- Address #1: gate at actual convergence point (not handle_message)
- Address #2: trust lookup keys off per-event platform (not adapter)
- Address #3: slash commands gated (Handler is downstream of gate)
- Address #4: exhaustive scattered-checks inventory
- Address #5: explicit empty-vs-missing semantics
- Address #6: phased rollout (Phase 0-3)
- Address #7: echo rate-limit + bot exclusion + DM-preferred
- Address #8: gateway vs first-class section precedence
- Address #9: no static HashSet (runtime construction)
- Address #10: structured logging on allow + deny
- Address #11-#15: minor fixes (Teams ID, bot semantics, etc.)

* docs(adr): address team review findings

- Add type-level guarantee (GatedEvent vs InboundEvent) — compile-time
  enforcement, not just convention (#4)
- Clarify Gateway Receiver is one receiver that demuxes by platform (#11)
- Fix layer numbering inconsistency — use names, not numbers (#21)
- Add sender ID format table with per-platform gotchas (#22, #23, #24)
- Clarify is_bot bypass is caller-side, not inside decide() (擺渡-1)
- Change echo group fallback to silent drop (avoid UID leakage) (#6)

* docs(adr): add event loop binding design + fix is_bot L2 bypass

- Add §5 'Event loop binding' section: run_platform generic pipeline,
  EventReceiver/EventHandler traits, main.rs startup wiring
- Gateway platforms: one shared WS, demux by event.platform, fan-out
  to per-platform Handlers
- Fix is_bot bypass: bots skip L3 but STILL enforce L2 scope (擺渡-1 🔴)
- Add cross-crate boundary note for Gateway Receiver (擺渡-2 🟡)
- Include binding topology summary diagram

* docs(adr): address round 3 findings — tighten pseudocode precision

- GatedEvent: private field in narrow module (not pub(crate)), with
  read-only accessors and module layout diagram (諸葛村夫-1)
- gate_event: use configs.get().surface_allowed() to match real API (擺渡-3)
- Phase table: add Phase 0.5 for current partially-wired state on main,
  clarify Phase 2 means 'refuse to start' (諸葛村夫-2)

* docs(adr): fix seal() visibility — private fn, not pub(super)

seal() lives in the same module as gate_event(), so it should be a plain
private fn. pub(super) would unnecessarily expose it to the parent module.

* docs(adr): fix stale module layout comment — constructor is private, not pub(super)

* docs(adr): address LINE/Slack/Feishu review feedback

- Replace line-number refs with symbol+semantic descriptions (drift-proof)
- Rewrite echo section: platform-specific echo trait (LINE=Reply only,
  Slack=chat.postEphemeral, Discord=DM); leak-safe content by scope
- Add is_bot per-platform derivation table (pinned canonical rules)
- Document trusted_bot_ids as shared config (resolves Feishu circular dep)
- Clarify slash commands scope (Slack doesn't consume them)
- Update Slack sender ID: Enterprise Grid composite key (team_id, sender_id)
- Add non-message events section (assistant_thread_started must gate)
- Add Slack scope notes (Socket Mode only, MPIM=channel)
- Add LINE group policy: open/members dual-mode in decide()
- Add LINE @mention pre-filter as documented Receiver exception
- Feishu: gateway=L1 only, eliminate double-gating, empty list=deny-all

Addresses feedback from:
- @luffy-aiagent (LINE platform review)
- @antigenius0910 (Slack platform review)
- @wangyuyan-agent (Feishu platform review)

* docs(adr): address 9 review findings — API contract gaps + hardening

Fixes identified during group review:

F1+F6: Add workspace_id to InboundEvent; define Slack Enterprise Grid
       canonical sender_id format and config examples for Grid deployments
F2:    Replace HashMap<String, TrustConfig> with enum PlatformTrustConfig
       (Base/Line/Slack) — LINE group policy and Slack workspace-scoped
       trust now have proper type representations
F3:    Add cron bypass in gate_event() — system-initiated events skip
       L2/L3 (platform='cron' or sender_id='openab-cron')
F4:    Add #[cfg(test)] assume_trusted_for_test() constructor for
       GatedEvent — enables Handler unit testing without full pipeline
F5+F9: Change into_inner() to pub(crate); adjust safety claim wording
       from 'bypass impossible' to 'accidental bypass compile error'
F7:    Change gate_event() signature to take InboundEvent by value —
       zero-copy hot path (no .clone() on RawPlatformEvent)
F8:    Specify bounded LRU cache (max_capacity + TTL) for rate-limit
       state — prevents OOM from random sender_id flooding

* fix(adr): address review findings F1-F3 on identity-trust-none

F1 (critical): Remove sender_id spoofing hole in cron bypass — only check
    platform == "cron" since WeCom allows freeform UserIDs that could
    match any synthetic value. Update rationale accordingly.

F2: Add WeCom, Google Chat, MS Teams to pinned is_bot derivation and
    echo-delivery tables (all 8 platforms now covered).

F3: Fix pseudocode precision — unwrap_or no longer borrows a temporary;
    add PlatformTrustConfigs::get() and PlatformTrustConfig::surface_allowed()
    delegating method used by gate_event.

* fix(adr): address group review round 2 — 9 findings from B1/B4/B5/B8/B11/B12/B15

Fixes:
- WeCom is_bot: remove enter_agent (user-initiated, not bot); keep only
  trusted_bot_ids (B5 F1, B12 F1)
- SlackTrustConfig::decide(): workspace_users is now strict override
  (ignore allow_all_users); base fallback supports team_id:user_id composite
  key for Enterprise Grid (B4 F1, B5 F2, B12 F3)
- LINE group policy: fix prose vs code contradiction — unconfigured groups
  use default_group_policy, not DenyScope (B12 F2)
- Reserved platform validation: MUST-level requirement for all external
  Receivers to reject reserved platform names; cron bypass invariant
  documented; Phase 1 SHOULD for typed InboundSource enum (B1)
- decide() simplified to use self.get() — remove duplicate default logic (B5 F3)
- Echo rate-limit key updated to (platform, workspace_id, sender_id) (B12 F4, B15 F2)
- Module layout: InboundEvent in mod.rs (public), gate.rs narrow (B12 F5)
- into_inner() trust boundary doc: module-level vs crate-level explained,
  Phase 1 SHOULD for lint/annotation (B11, B15 F1)
- Slack Enterprise Grid gotcha: split dense table cell into footnote (B8 F2)

* fix(adr): add platform lowercase invariant to InboundEvent (B14 F2)

InboundEvent.platform MUST be lowercase — Receivers normalize before
constructing. This ensures consistency between gate_event's == "cron"
check and PlatformTrustConfigs::get()'s to_lowercase() lookup.

* docs(adr): clarify gw_event.platform source in run_gateway_platforms

Add comment noting platform field is assigned by gateway routing config,
not from webhook payload body — satisfies reserved platform invariant.

* docs(adr): WeCom corrections from external review (canyugs#18)

Three verified adjustments to the v2 trust architecture text:

- WeCom callback mode is DM-only: channel_type always "direct" with a
  per-user channel id, so "group routing" does not exist for WeCom and
  its L2 scope is effectively allow_dm only (verified wecom.rs:1059-71)
- Sender-id table: plain UserIDs are self-built-app only; external
  contact / ISV callbacks carry wm/wo-prefixed external_userid or
  encrypted OpenUserID (verified against official WeCom API docs)
- Bot-bypass wording: is_bot is hardcoded false in the WeCom Receiver
  today, so the L3 bot-bypass is a no-op for WeCom — the bypass is
  available uniformly but effective only where the Receiver can derive
  is_bot

Credit: canyugs.

* docs(adr): refresh Phase 0.5 snapshot to shipped state

Since the last revision, main gained: Slack L3 gate (#1363), per-platform
[section] trust for all 8 platforms (#1365/#1366/#1385), full
config-first parity with conformance guard (#1375/#1387), and the L1
unenforceable-auth startup warning (#1373). The Phase 0.5 row now lists
the actual inventory and names the one outstanding Phase 1 prerequisite
(standalone WS path still on should_skip_event, tracked on #1356).

---------

Co-authored-by: chaodu-agent <chaodu-agent@openab.dev>
Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
brettchien added a commit to brettchien/openab that referenced this pull request Jul 20, 2026
…penabdev#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 (openabdev#9).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
brettchien added a commit to brettchien/openab that referenced this pull request Jul 24, 2026
…penabdev#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 (openabdev#9).

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…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants