feat(mcp): OAB MCP Facade MVP — shared openab-mcp crate + broker loopback HTTP server#1448
Conversation
…er enforcement Implements the OAB MCP Adapter ADR (#1446) MVP slice in openab-agent: - New 'openab-agent mcp-facade' subcommand: inbound stdio MCP server exposing exactly search_capabilities / execute_capability (ADR §6.1, §6.4), backed by the existing McpRuntimeManager + meta_tool dispatcher (one capability dispatcher, two frontends). - search_capabilities: lazy discovery across all configured servers with per-provider failure isolation, name-collision qualification (server:tool), risk labels from MCP tool annotations, and redacted provider errors in an 'unavailable' list. - execute_capability: exact-name resolution against current discovery, then delegation to the shared call path (jsonschema argument validation, timeouts, circuit breaker, redaction). - tool_filter enforcement (accepted MCP ADR §5.6 contract, previously parsed but unenforced): glob include/exclude applied at discovery (fetch_tools, pre-cache) and execution (call_tool, pre-connect) for both the meta-tool and the facade. - rmcp features: + server, transport-io. Broker-side ACP session/new mcpServers advertisement is PR-2.
|
LGTM ✅ - The OAB MCP Facade MVP is correctly scoped and enforces the documented least-privilege tool surface. Note No Critical or Important findings were identified for the reviewed commit. What This PR DoesThis PR adds the agent-facing OAB MCP Facade MVP as an How It WorksThe facade reuses the existing Findings
Finding Details🟢 F1: Shared dispatcher and runtimeThe facade delegates execution through 🟢 F2: Defense-in-depth filter enforcementFiltered tools are removed before the shared tools cache is populated, and guessed filtered names are rejected before 🟢 F3: Failure isolation and redactionDiscovery continues across configured providers when one fails and returns a concise redacted error under 🟢 F4: Narrow protocol surfaceThe stdio server advertises exactly 🟢 F5: Focused regression coverageThe patch covers empty catalogs, unavailable providers, unknown execution names, duplicate-name qualification, glob semantics, include/exclude precedence, and the Gmail least-privilege profile shape. Addressing External Reviewer FeedbackNo external GitHub reviewer comments or submitted reviews were present for this round, so there is no unresolved external feedback to address. Baseline Check
5. Three Reasons We Might Not Need This PR
These are documented scope and rollout trade-offs, not blockers for this MVP. Validation
|
![chaodu-obk[bot]](https://avatars.githubusercontent.com/u/262003297?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM ✅ - No Critical or Important findings were identified.
Consolidated review: #1448 (comment)
GitHub event: APPROVE
…back HTTP server Implements the OAB MCP Adapter ADR (#1446) MVP: - New workspace crate crates/openab-mcp: the MCP runtime extracted from openab-agent (client runtime, OAuth/PKCE + auth.json store, layered mcp.json config, tool cache, circuit breaker, jsonschema validation, redaction) plus the new OAB MCP Facade — an inbound MCP server exposing exactly search_capabilities / execute_capability. - Facade transport: loopback-only Streamable HTTP (http://127.0.0.1: <port>/mcp). Non-loopback binds are refused. Any MCP-capable coding CLI on the host can connect. - Broker activation: presence of [mcp] in config.toml starts the listener in-process (absent = no listener, backward compatible). openab-agent re-exports the crate (crate::{mcp,auth} shims + llm type layer + HostBridge) — one runtime, two hosts, zero duplication. - tool_filter enforcement (accepted MCP ADR §5.6, previously parsed but unenforced): glob include/exclude applied at discovery (pre-cache) and execution (pre-connect), for the meta-tool and facade alike. - openab-agent mcp-facade --listen: standalone facade server. - acp.rs env-mutating tests moved to temp_env (single global lock domain; a private ENV_LOCK raced with temp_env-based tests once the moved tests changed binary scheduling); session_new_missing_key now sandboxes HOME instead of deleting the developer's real auth.json. - Dockerfile.unified + ci-openab-agent.yml updated for the new member.
4c12c65 to
26a8395
Compare
chaodu-agent
changed the title
The workspace gained crates/openab-mcp but only Dockerfile.unified's dummy stage was updated; the base and per-CLI Dockerfiles failed at 'cargo build' with 'failed to load manifest for workspace member'. Apply the same COPY/mkdir/stub/rm/touch pattern everywhere (incl. Dockerfile.gateway/builder/native/agentcore, which share the pattern but are outside this PR's smoke matrix).
Gmail live validation report (ADR rollout step 3, run early per maintainer request)Ran the Gmail hosted-MCP profile end-to-end against What passed ✅
Blocked upstream ⛔ — Google preview gating (not a PR bug)Every
Cause: the Gmail MCP server requires Google Workspace Developer Preview Program enrollment — per-account and per-GCP-project (the program registers your project; approval is manual, "about a week"). Suggest adding to the ADR §6.5 Gmail profile / §9 risks: execution (not discovery) is gated on DPP enrollment of both the authorizing account and the OAuth client's GCP project; REST access is not a valid preflight signal for MCP access. Bug found 🐛 — no refresh token from Google (headless blocker)The paste-back authorize URL contains no Impact: for any Google provider the session dies after ~1h and recovery requires an interactive re-login — unusable for the headless bot deployments (ECS) this is destined for. Suggested fix: when resolving a custom provider whose authorize endpoint is Recommendation — today's evidence tips rollout step 5 toward the native adapterThe ADR already stages this decision (§10 step 5: hosted MCP vs native Gmail adapter) and documents OpenWorker's native Gmail REST connector as prior art (§5.3). Today's run adds three concrete data points in favor of the Capability Plugin / native Gmail adapter path (§6.1), at least until Gmail MCP reaches GA:
Hosted-MCP validation continues in parallel (DPP application submitted) — it remains the right long-term default per the ADR; this only re-sequences what ships first. State
|
Follow-up: DPP application rejected — hosted Gmail MCP is Workspace-account-only during previewClosing the loop on the validation report above. The Developer Preview Program application was rejected: "We could not verify your Google Workspace account" — the program requires a Google Workspace account; consumer Implications, stronger than the original finding:
Meanwhile #1449 has been validated end-to-end in production topology: b2 (ECS) → OAB MCP Facade ( |
…MCP server (#1449) * feat(mcp): native Gmail adapter — six-tool §6.5 profile over Gmail REST as stdio MCP server Capability Plugin per ADR §5.3/§6.1 and rollout step 5, motivated by live validation (PR #1448): hosted Gmail MCP gates tools/call behind Workspace Developer Preview enrollment and its terms bar pre-GA shipping, while the same OAuth token works against GA Gmail REST. - crates/openab-mcp/src/native/gmail.rs: search_threads / get_thread / get_message / list_labels / list_drafts / create_draft with names and argument shapes mirroring the hosted server (GA cut-over = config-only); drafts-only write surface; header-injection-hardened MIME builder; Gmail-id validation before path use. - Own OAuth login (PKCE S256 paste-back) sending access_type=offline + prompt=consent — stores a refresh token (the rmcp-driven flow cannot), single-flight refresh with rotation-tolerant persistence via the shared auth.json McpCredentialStore under the gmail-native key. - Served over stdio (openab-agent gmail-native serve|login): registered in mcp.json as a normal stdio server, so tool_filter, schema validation, breaker, timeouts, and redaction apply unchanged. * style: cargo fmt + fix injection-test assertion (address guard trips on control char first) * style: final rustfmt * refactor(gmail-native): serve over loopback Streamable HTTP instead of stdio Same transport and trust model as the OAB MCP Facade (require_loopback reused; non-loopback binds refused; no auth layer — host boundary is the trust boundary). mcp.json registration becomes a "type": "http" entry at http://127.0.0.1:8850/mcp. Drops the rmcp transport-io feature. * docs: gmail-native operator guide (setup, registration, security notes) * refactor: move gmail-native serve|login to the broker binary openab-agent is the coding agent (ACP client + MCP client toolchain); long-running serving surfaces belong to the broker binary, which every bot image already ships — the b2 validation required smuggling the agent binary into the container precisely because the adapter lived on the wrong binary. openab-agent keeps the client-side mcp CLI only. * style: fmt + doc reference fix * refactor(cli): nest under `openab mcp <addon> <action>` namespace Maintainer direction: MCP add-ons get one namespace (openab mcp gmail-native serve|login); long-running serving remains config-driven ([mcp] + openab run). Standalone/dev serving and interactive logins are the namespace's job. * style: fmt --------- Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
* docs: OAB MCP Facade operator guide Documents the #1448 feature: enablement (broker [mcp] / standalone), the two-tool surface, audit lines, client registration (incl. the kiro --agent config gotcha), trust model, and the pod-vs-fleet positioning relative to octobroker (ADR §4.1). * docs: add mermaid architecture diagram to the facade guide * docs: describe facade-only run mode (replaces removed openab-agent mcp-facade) --------- Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
…browser tunnel wiring alongside the [mcp] facade block; Cargo.lock regenerated
2 participants
0. Discord Discussion URL
Design discussed in the OpenAB Discord; the governing design is ADR PR #1446 (OAB MCP Adapter, §6.2 as amended for the broker-hosted loopback HTTP transport). No public Discord URL is included in the repository context.
1. What problem does this solve?
Implements the MVP of the OAB MCP Adapter ADR (#1446): the OAB MCP Facade — a loopback-only Streamable HTTP MCP server exposing exactly two agent-facing tools,
search_capabilitiesandexecute_capability, so any MCP-capable coding CLI on the host (Kiro, Claude Code, Codex, …) connects tohttp://127.0.0.1:<port>/mcp. The broker serves it in-process, enabled by a[mcp]section inconfig.toml.It also closes a real gap the ADR's least-privilege profiles depend on: the accepted MCP ADR (§5.6) documents
tool_filterglob include/exclude lists and the config schema parses them, but nothing enforced them until now.2. How it works
crates/openab-mcp: the entire MCP runtime extracted fromopenab-agent— outbound client (Streamable HTTP + stdio), OAuth/PKCE +auth.jsoncredential store (moved wholesale: atomic writes, corruption quarantine, refresh locks), layeredmcp.jsonconfig, tool cache, circuit breaker, jsonschema argument validation, secret redaction — plus the new facade. The broker links it;openab-agentre-exports it (crate::{mcp,auth}shims, shared LLM type layer,HostBridge). One runtime, two hosts, zero duplication.meta_toolcall path the native agent uses.tool_filterenforcement at both boundaries: discovery drops filtered tools before caching; execution rejects filtered names before any connect.[mcp]section → no listener, no behavior change.mcp.jsonremains the only provider source of truth ([mcp]carries listener settings only — ADR Alternative E).openab-agent mcp-facade --listen 127.0.0.1:8848serves the same facade standalone.from_configno longer implicitly binds an LLM provider (the crate has no concrete providers); the agent injects it viaset_provideralongside the host bridge — sampling behavior unchanged for the agent, correctly absent for the broker.3. Scope
Root workspace (new crate + broker wiring + Dockerfile.unified dummy-stage + CI paths) and
openab-agent(re-export shims). ACPsession/newmcpServersadvertisement is a documented follow-up — the loopback URL is already reachable by any CLI the operator points at it.4. Validation
On macOS arm64:
openab-mcp: 198 tests pass (moved runtime/meta-tool/auth tests + facade tests + loopback-bind validation)openab-agent: fmt clean, clippy-D warnings= 0 findings, 62 tests pass--workspace --all-features -D warnings= 0 errors;openab-coretests pass incl. 3 new[mcp]parse tests (only failure is the documented pre-existingsecrets::resolve_exec_nonzero_exitmacOS-env issue, identical on main)initializehandshake returnsoab-mcp-facade;tools/listreturns exactly the two tools;--listen 0.0.0.0:…refused[mcp]in config.toml binds the facade at startup and answersinitializewithoab-mcp-facadeon127.0.0.1:18850/mcpacp.rsenv-mutating tests moved from a privateENV_LOCKtotemp_env(single global lock domain); full agent suite green in parallel runsReview Contract
Goal
Ship the OAB MCP Facade MVP per ADR #1446 §6.2 (as amended): a loopback-only Streamable HTTP MCP server exposing exactly
search_capabilities/execute_capability, hosted in-process by the broker when[mcp]is present inconfig.toml, backed by the MCP runtime extracted into the sharedcrates/openab-mcpcrate — with the ADR'stool_filterleast-privilege contract actually enforced.Non-goals
session/newmcpServersadvertisement of the facade URL (follow-up; not required for CLIs configured directly).auth.jsonormcp.jsonlocations; provider config stays inmcp.json(no second source of truth).Accepted Residual Risks
[mcp]is enabled. Mitigations: loopback-only bind enforced at startup, explicit warnings in ADR/config example/startup log, opt-in activation; deployments colocating untrusted processes must not enable it.auth.jsoncode wholesale; behavior is regression-tested (198 tests) but subtle path/permission differences on exotic platforms would surface only in the follow-up provider validation.search_capabilitieswith no query lists the full filtered catalog (token-heavy on very large downstream catalogs); mitigated by enforced include filters and the query parameter.Acceptance Criteria
origin/mainon the same toolchain.[mcp]absent → no listener (config parse test); present → listener on the configured loopback address (live smoke), default127.0.0.1:8848.listenrefused at startup (unit test + live smoke).tools/listreturns exactly the two documented tools;initializeidentifiesoab-mcp-facade.tool_filter-excluded tool is invisible to discovery and rejected pre-connect at execution (regression tests, incl. the Gmail default-profile send/delete block).unavailablewith a redacted error (regression test).openab-agentbehavior unchanged through the re-export shims (62 agent tests green, incl. ACP session, model-switch, sampling paths).Follow-ups
session/newmcpServersadvertisement of the facade URL (currently hardcoded[]in openab-core, unchanged here).tools/list_changed-driven cache invalidation surfaced through the facade; optional result cap/pagination for large catalogs.openab-agent's now-redundant direct deps further (kept minimal already).