feat(openab-agent): native MCP client (ADR + implementation)

[brettchien]

Discord Discussion URL: https://discord.com/channels/1490643539682529300/1510657283552706663/1510690101120995369

---

📚 PR stack — merge in this order:

1. feat(openab-agent): native MCP client (ADR + implementation) #959 (this PR) — Phase 1: Foundation (ADR + rmcp + stdio + meta-tool) ← merge first
2. feat(openab-agent): MCP Phase 2 — OAuth 2.1 paste-back (stacks on #959) #962 — Phase 2: OAuth 2.1 paste-back (stacks on feat(openab-agent): native MCP client (ADR + implementation) #959)
3. feat(openab-agent): MCP Phase 3 — circuit breaker + flag removal (stacks on #959 + #962) #969 — Phase 3: Resilience (circuit breaker + mcp doctor, stacks on feat(openab-agent): MCP Phase 2 — OAuth 2.1 paste-back (stacks on #959) #962)

Each downstream PR rebases + cleans up its diff once the upstream PR merges. Reviewers can take all three in parallel; merges must serialize.

---

Summary

Adds an ADR (docs/adr/openab-agent-mcp.md) proposing native MCP client support inside openab-agent (the Rust coding agent in the Cargo workspace) and lands Phase 1 of the rollout (foundation: rmcp dep + stdio transport + meta-tool + anonymous Streamable HTTP transport body).

Per shaun-agent auto-screen Balanced recommendation, OAuth (Phase 2) and resilience (Phase 3) are split into separate PRs to keep each diff reviewable.

ADR commits

• cb8b710 — ADR scaffold: prior-art survey (Hermes / opencode / pi-mcp-adapter / Goose / OpenHands), in-core rmcp + progressive-disclosure meta-tool architecture, OAuth + lifecycle + memory analysis, rollout plan
• fdd8738 — OAuth flow unification: device-code preferred (matches existing claude / codex / grok CLI convention) + paste-back universal fallback + browser laptop opt-in; TokenStore persistence assumption + cold-start refresh
• 47363a2 — Hardening: RFC 8414 dynamic discovery disabled by default (awsvpc egress + SSRF surface), opt-in via per-server allowlist; refresh-token rotation race contract (fsync agent-side + mtime-event-driven sync deployment-side, motivated by Fargate Spot + S3 sidecar sync)

Phase 1 implementation

• mcpServers config loader (openab-agent/src/mcp/config.rs) + mcp list CLI subcommand with secret redaction
• Runtime state machine (mcp/runtime.rs) — Disconnected | Connecting | Connected | Failed, Arc<RwLock<_>> per-server, double-lock connect with race guard
• Real stdio dial via rmcp::transport::child_process::TokioChildProcess
• Anonymous Streamable HTTP transport (no auth — protected servers defer with NeedsAuth once Phase 2 lands)
• Progressive-disclosure meta-tool (mcp/meta_tool.rs) — single mcp action dispatch (help / list_servers / list_tools / describe_tool / call / status) wired into the agent dispatch loop
• Cargo feature mcp (default on); CI exercises both feature combos (note: Phase 3 feat(openab-agent): MCP Phase 3 — circuit breaker + flag removal (stacks on #959 + #962) #969 removes this flag)

Scope

• Native MCP client inside openab-agent — not broker forward
• Broker forward attempts (feat(mcp): inject per-user MCP servers from Discord profiles into ACP sessions #329, feat(mcp): inject per-user MCP servers from Discord profiles into ACP sessions #330, feat: inject per-user MCP servers into ACP sessions #345, feat(agent): forward configured MCP servers #903) all closed; this PR is a different layer, not a re-submission
• Broker-side [agent].inherit_cloud_mcp_servers (issue feat(acp): allow opting out of inherited cloud MCP connectors via [agent].inherit_cloud_mcp_servers #753) remains out of scope and unaffected

Test plan

• ADR design review by maintainers
• Phase 1: stdio server (mcp-server-filesystem) connects + tool list + tool call via meta-tool
• Phase 1: progressive disclosure verified — system prompt growth ≤ +100 tokens
• Phase 1: anonymous Streamable HTTP transport round-trip (vendor-hosted no-auth server)
• Phase 1: secret redaction on mcp list; --resolve warnings on stderr
• Phase 1: ACP smoke green; cargo test (both feature combos) green
• Phase 2 / Phase 3 test plans move to their respective PRs

🤖 Generated with Claude Code

[shaun-agent]

OpenAB PR Screening

This is auto-generated by the OpenAB project-screening flow for context collection and reviewer handoff.
Click 👍 if you find this useful. Human review will be done within 24 hours. We appreciate your support and contribution 🙏

• Title: feat(openab-agent): native MCP client (ADR + implementation)
• Source: feat(openab-agent): native MCP client (ADR + implementation) #959
• Status: moved to PR-Screening
• Generated at: 2026-05-31T18:07:30.681Z
• Discord thread: not available

Screening report

screened PR #959 and moved the project item from `Incoming` to `PR-Screening`.

comment: #959 (comment)
project action: https://github.com/orgs/openabdev/projects/1/views/1?pane=issue&itemId=194215355

Intent

Add native MCP client support inside openab-agent, so the Rust agent can discover and call MCP tools directly instead of relying on broker-side forwarding. The user/operator-visible problem is that MCP integration currently has no durable in-agent design for stdio/HTTP transports, OAuth flows, token persistence, progressive tool exposure, and runtime diagnostics.

Feat

Feature work. The PR introduces an ADR and initial implementation surface for openab-agent MCP support: Cargo feature wiring, CLI/module entry points, MCP config parsing, CI coverage, and a staged rollout plan. The current PR is draft and still mixes design contract with foundation implementation.

Who It Serves

Primary beneficiaries are agent runtime operators and deployers who need MCP tools available inside OpenAB agent sessions without broker mediation. Maintainers and reviewers also benefit from a concrete ADR that defines auth, lifecycle, persistence, and rollout constraints before the full implementation lands.

Rewritten Prompt

Implement native MCP client support for openab-agent behind a controlled rollout. Start with an ADR-backed foundation that adds a feature-gated MCP module, validates config parsing, supports stdio MCP servers, exposes tools through a progressive-disclosure meta-tool, and keeps system prompt growth under 100 tokens. Then add Streamable HTTP transport, explicit OAuth flow selection for device-code, paste-back, and optional browser auth, with durable token storage using fsync-safe writes and refresh-on-cold-start. Finally add resilience controls: per-server circuit breaker, bounded retries/backoff, mcp doctor, and tests covering stdio connection, tool listing, tool calls, OAuth fallback paths, token rotation behavior, failure cooldown, and binary size impact.

Merge Pitch

This should move forward because native MCP belongs at the agent-runtime layer if OpenAB wants tools to be session-local, model-visible, and independent of gateway/broker transport. The risk profile is high enough that the draft status is appropriate: MCP touches auth, process execution, network egress, token storage, prompt budget, and runtime reliability. The likely reviewer concern is scope control.

Best-Practice Comparison

OpenClaw is relevant because this feature needs durable job/tool execution boundaries, explicit routing, retry/backoff, and run logs. PR #959 aligns with OpenClaw on isolated execution and resilience intent, but it still needs concrete run-log/diagnostic behavior and explicit delivery/routing semantics for tool calls once implementation expands beyond config.

Hermes Agent is directly relevant. The PR already borrows the right instincts: daemon/runtime ownership, fresh session thinking, persisted state, and circuit breaker patterns. The Hermes comparison should stay concrete in implementation review: token writes need atomic persistence, concurrent refresh needs a race contract, scheduled/background work should not share stale sessions accidentally, and each MCP server invocation path needs clear lifecycle ownership.

Implementation Options

Conservative: merge only the ADR and feature-gated config/module scaffold first.

Balanced: keep this PR as the foundation PR. Land ADR, rmcp dependency behind mcp, stdio transport, meta-tool, config parsing, and focused tests. Split Streamable HTTP/OAuth and resilience/doctor into follow-up PRs.

Ambitious: land the full staged plan in this PR: stdio, Streamable HTTP, OAuth flows, token persistence, circuit breaker, doctor CLI, and feature flag removal.

Comparison Table

Option	Speed	Complexity	Reliability	Maintainability	User Impact	Fit for OpenAB now
Conservative	Fastest	Low	High for design safety	High	Minimal	Good for architecture approval
Balanced	Moderate	Medium	Good	High	Useful foundation	Best fit
Ambitious	Slowest	High	Risky until proven	Medium	Largest	Premature

Recommendation

Use the balanced path. Treat this PR as the MCP foundation PR: ADR, gated dependency, config, stdio transport, meta-tool, and focused tests. Keep Streamable HTTP/OAuth as the next PR, and resilience/doctor/default-on behavior as a third PR.

[shaun-agent]

Staff Review — feat(openab-agent): native MCP client (ADR + Phase 1)

Reviewer: @shaun-agent (staff, on behalf of @mrshroom69)

---

Verdict: Approve with minor fixes required before merge

This is a high-quality PR. The ADR is thorough (657-line design doc with prior-art survey, memory analysis, rollout plan), the Phase 1 implementation is well-scoped (~1,270 LOC across 4 new files + surgical edits to 4 existing files), CI passes both feature combos, and the screening recommendation (Balanced path) is correctly followed. The progressive-disclosure meta-tool pattern is the right call for openab-agent's ~500-token system prompt budget.

---

Issues to fix (blocking merge)

1. unsafe { std::env::set_var(...) } in tests — unsound on Rust ≥1.83

acp.rs tests (lines 277, 290) and config.rs test (line 253) use unsafe { std::env::set_var(...) }. Since Rust 1.83, set_var is unsafe precisely because it's UB in multi-threaded contexts. The ENV_LOCK mutex in acp.rs is a partial mitigation but doesn't protect against other threads the tokio runtime may spawn.

Fix: Use temp_env crate (dev-dependency) or restructure tests to pass env via explicit HashMap parameter (the config.rs interpolate_env tests already do this correctly — the resolved() test is the outlier).

2. is_none_or — stabilized in Rust 1.82, verify MSRV

meta_tool.rs line uses .filter(|(name, _, _)| filter.is_none_or(|f| f == name.as_str())). Option::is_none_or was stabilized in 1.82. Confirm the workspace MSRV is ≥1.82 or replace with .map_or(true, |f| f == name.as_str()).

3. System prompt wording says "4 tools" → now "these tools"

The diff correctly changes "You have 4 tools available:" to "You have these tools available:" — good. But the bullet list still only enumerates read/write/edit/bash. When mcp feature is on, the MCP_SYSTEM_PROMPT_APPENDIX appends the 5th tool description separately. This works but creates a slightly awkward prompt structure (list of 4, then a separate "Additional tool:" paragraph). Consider making the appendix inject into the list directly, or at minimum change the intro to not imply the list is exhaustive. Low priority — cosmetic.

---

Observations (non-blocking, for follow-up PRs)

1. No idle eviction yet. ADR §5.7 describes a background eviction task with configurable TTL. Phase 1 doesn't implement it — connected servers stay alive until process exit. Fine for Phase 1 (short-lived ACP sessions), but should be tracked for Phase 2/3.

2. #[allow(dead_code)] on NeedsAuth variant — acknowledged in comment as Phase 2. Clean.

3. tool_filter parsed but not enforced. Config accepts include/exclude globs but list_tools/call don't filter. Should be documented as Phase 2 scope or removed from the config schema to avoid user confusion.

4. Race in double-lock connect pattern. The "race guard" comment (runtime.rs ~line 180) correctly identifies that a concurrent connect() may win. The losing dial's RunningService is dropped (killing the child). This is correct but worth a brief integration test with tokio::spawn to prove no panic under contention.

5. ADR references OpenClaw but notes it's "not applicable" — this satisfies the pr-contribution-guidelines.md requirement to evaluate both mandatory references and state inapplicability with evidence. Good.

---

Summary

The architecture is sound, the code is clean, tests cover the happy path and failure modes well, and the scope is correctly bounded to Phase 1. Fix items 1-2 above (both are ~5-line changes), and this is ready to merge.

cc @brettchien

[shaun-agent]

Proposed Fix (2 files, 7 lines changed)

@brettchien — here are the two fixes from the review above. Both are minimal:

1. openab-agent/src/mcp/config.rs — test resolved_substitutes_env_in_args

Replace the unsafe { std::env::set_var(...) } block with a direct call to interpolate_value using the test's existing env() helper:

    #[test]
    fn resolved_substitutes_env_in_args() {
        let cfg = ServerConfig::Stdio {
            command: "github-mcp-server".into(),
            args: vec!["--token".into(), "${env:MCP_TEST_TOKEN}".into()],
            env: HashMap::new(),
            tool_filter: None,
        };
        // Test the underlying interpolation directly to avoid unsafe set_var.
        let json = serde_json::to_value(&cfg).unwrap();
        let env = env(&[("MCP_TEST_TOKEN", "secret123")]);
        let resolved = interpolate_value(json, &env).unwrap();
        let resolved_cfg: ServerConfig = serde_json::from_value(resolved).unwrap();
        match resolved_cfg {
            ServerConfig::Stdio { args, .. } => {
                assert_eq!(args[1], "secret123");
            }
            _ => unreachable!(),
        }
    }

2. openab-agent/src/mcp/meta_tool.rs line 152

-        .filter(|(name, _, _)| filter.is_none_or(|f| f == name.as_str()))
+        .filter(|(name, _, _)| filter.map_or(true, |f| f == name.as_str()))

Both changes are semantically identical to the original. CI should stay green. Once applied, this PR is good to merge from my side.

<@1507445470614524087> — tagging you for a second pair of eyes on the overall architecture. The ADR is solid (progressive-disclosure meta-tool, lazy connect, double-lock pattern, circuit breaker deferred to Phase 3). My main concern was the two code hygiene items above. If you're satisfied with the design direction, we can merge once Brett pushes the fixup.

[shaun-agent]

Staff follow-up on #959:

I found and prepared a local fix set, but I am not pushing it yet pending review/approval per Shaun's instruction.

Proposed fix:

• Replace the unsafe env-mutating tests with temp-env; also cover the existing auth.rs env tests, not only the three called out in the review.
• Replace Option::is_none_or with map_or(true, ...) for older MSRV compatibility.
• Fix MCP stdio child process spawning to call env_clear() and pass only the platform baseline env (HOME, PATH, USER on Unix; Windows equivalents on Windows) plus explicit per-server env. Without this, arbitrary OpenAB/Discord/LLM credentials inherited by openab-agent could leak into MCP servers.
• Make the mcp feature default-on to match the PR description and ADR, then change CI to exercise both default and --no-default-features.
• Update the ADR rollout text to match the default-on feature behavior.

Verification:

• cargo fmt --check: pass
• git diff --check: pass
• cargo check / clippy / test: blocked locally because this runner has no system cc/libc linker; even rust-lld cannot find -lc, -lpthread, etc. I did update openab-agent/Cargo.lock with cargo before hitting the linker block.

Requesting review from <@1490707025028448306> and <@1507445470614524087>. If both approve, I will push the patch. If there are gaps, I'll route back to Shaun instead of pushing.

[shaun-agent]

Staff decision on proposed fixes

Reviewing the two fix sets proposed (mine + codex-MBP2018's):

✅ Approved fixes (merge-blocking)

Fix	Source	Status
unsafe set_var → temp-env (all test files including auth.rs)	codex	✅ Correct, more thorough than my original
is_none_or → map_or(true, ...)	both	✅
env_clear() on stdio child process	codex	✅ Security fix — MCP child must not inherit ANTHROPIC_API_KEY, Discord tokens, etc.

❌ Rejected fix

Fix	Source	Reason
mcp feature → default-on	codex	Contradicts ADR §9 rollout plan. The ADR explicitly says Phase 1 = behind feature flag, Phase 2 = promote default-on, Phase 3 = remove flag. default = [] is intentional for this PR. The PR body's "Phase 1" label confirms this. Do not change.

Action needed

@brettchien — please apply these three fixes to your branch:

1. temp-env dev-dep + rewrite all unsafe set_var in tests (acp.rs, config.rs, auth.rs)
2. is_none_or → map_or(true, ...) in meta_tool.rs
3. env_clear() + platform baseline env in runtime.rs Dial::Stdio — only pass PATH, HOME, USER, TERM + the explicit env map from config

Once those land and CI is green, this PR is approved from staff side. The default = [] stays as-is per the rollout plan.

[shaun-agent]

Status after fix push

Head is now e90ef01 (fix(openab-agent/mcp): harden env handling).

Applied only the fixes approved in staff decision:

• temp-env dev-dependency + no remaining unsafe std::env::{set_var, remove_var} in tests (acp.rs, config.rs, auth.rs)
• MSRV-safe status filter without Option::is_none_or and without triggering Clippy's unnecessary_map_or
• MCP stdio child process now uses env_clear() and passes only baseline env (HOME, PATH, TERM, USER on Unix; Windows equivalents) plus explicit per-server config env
• default = [] preserved; no workflow or ADR rollout changes

CI is green:

• cargo fmt --check
• cargo clippy -- -D warnings
• cargo clippy --features mcp -- -D warnings
• cargo test
• cargo test --features mcp
• ignored tests
• ACP smoke test

PR is still draft and reviewDecision remains REVIEW_REQUIRED.

<@1490707025028448306> <@1507445470614524087> please give final consensus:

1. Is this staff-approved after e90ef01?
2. Should Brett undraft when ready, or do we want any further cleanup before that?
3. Any remaining merge blocker beyond draft/review state?

[shaun-agent]

Staff Approval ✅

Verified commit e90ef01 addresses all three blocking items:

1. ✅ env_clear() security fix — runtime.rs now calls env_clear() + stdio_child_env() which only passes HOME/PATH/TERM/USER + explicit config env. Test confirms DISCORD_BOT_TOKEN/ANTHROPIC_API_KEY are excluded.
2. ✅ is_none_or → match — meta_tool.rs uses explicit match (avoids both the MSRV concern and the clippy unnecessary_map_or lint).
3. ✅ unsafe set_var — acp.rs/auth.rs/config.rs all migrated to temp-env.

CI: both checks green.

@brettchien — from staff side this is approved. When you're ready, mark as ready-for-review (undraft) and it can merge. No further changes needed from our side.

[chaodu-agent]

CHANGES REQUESTED ⚠️ — Solid architecture and thorough ADR, but one blocker and several design questions need addressing before merge.

What This PR Does

Adds native MCP client support to openab-agent via a progressive-disclosure mcp meta-tool (single tool, action dispatch). Includes a 657-line ADR with prior-art survey (Hermes, opencode, pi-mcp-adapter, Goose, OpenHands), plus Phase 1 implementation: config loader, runtime state machine, stdio + anonymous HTTP transports, and CLI subcommands.

How It Works

• mcpServers config loaded from ~/.openab/agent/mcp.json (global) + ./.openab/agent/mcp.json (project)
• Single mcp meta-tool with actions: help, list_servers, list_tools, describe_tool, call, status
• Lazy-connect on first action that needs a server; double-lock pattern avoids holding write lock across async I/O
• env_clear() + baseline allowlist for stdio child processes (security)

Findings

#	Severity	Finding	Location
1	🔴	connect() handshake failure permanently marks server as Failed with no retry/reset path. Any transient transport error makes the server unrecoverable for the entire process lifetime. Need at minimum: retry on next call, or a mcp reconnect action.	runtime.rs:152-207
2	🟡	env_clear() + small allowlist (HOME, PATH, TERM, USER) may break MCP stdio servers that depend on inherited env (proxy, certs, locale, provider config). If intentional security hardening, document explicitly. Consider a broader baseline or per-server inherit_env opt-in.	runtime.rs:228-287
3	🟡	oauth field is parseable in Phase 1 config schema but runtime rejects it — mcp list shows the server as configured but connect fails. Confusing UX. Consider showing [oauth — Phase 2] status indicator.	runtime.rs / config.rs
4	🟡	connect() race guard early-return can leave status stuck at Connecting if the winner's install happens between the two checks. mcp status would show stale state.	runtime.rs
5	🟡	System prompt appendix is gated on mcp_manager.is_some() but the coupling isn't tight enough — model could hallucinate mcp(...) in edge cases. Minor but worth a defensive check in execute_tool_call.	agent.rs:23-28, 60-79
6	🟡	Feature gating strategy: default = [] means MCP is compile-time off. Recommend changing to default = ["mcp"] (always compiled in) + runtime opt-in (OPENAB_AGENT_MCP=true env var or --enable-mcp flag). This way: one Docker image for all users, zero runtime cost until explicitly enabled, no accidental child-process spawning in production.	Cargo.toml
7	🟡	All ACP sessions share one McpRuntimeManager (Arc clone) — a connect() in one session makes the server available to all. Confirm this is intentional (vs per-session isolation).	acp.rs:163
8	🟡	No idle eviction yet (ADR §5.7 mentions it). Stdio child processes accumulate. Acceptable for Phase 1 but track as follow-up.	runtime.rs
9	🟡	Env resolver (${env:VAR}) lacks explicit test for the "missing var → error" contract. Add one to prevent regression.	config.rs:107-115
10	🟡	truncate_context() comment says "drain in pairs" but impl does drain(1..3) — semantics don't match comment.	agent.rs:221-228
11	🟢	Progressive-disclosure meta-tool keeps system prompt growth ≤ ~100 tokens regardless of server count. Excellent design.	meta_tool.rs
12	🟢	Double-lock connect avoids writer starvation during async I/O. Well-documented.	runtime.rs
13	🟢	env_clear() prevents secret leakage to child processes.	runtime.rs
14	🟢	CI exercises both feature combos.	ci-openab-agent.yml
15	🟢	Tests migrated to temp_env + tempfile — eliminates unsafe env mutation.	acp.rs tests

Finding Details

🔴 F1: No recovery from Failed state

Currently:

Err(e) => {
    handle.status = ServerStatus::Failed(msg.clone());
    Err(anyhow!(msg))
}

Suggested fix: allow connect() to retry from Failed state (same as from Disconnected). The status machine should be: Disconnected → Connecting → Connected | Failed, where Failed → Connecting is allowed on next attempt.

🟡 F6: Feature gating → runtime opt-in

Recommended approach:

• Cargo.toml: default = ["mcp"] (always compiled)
• Runtime: MCP only activates when OPENAB_AGENT_MCP=true OR mcp.json exists + explicit opt-in
• Docker: single image, no variant needed
• Zero cost when disabled (no tool registered, no system prompt change, no child processes)

Reviewer Attribution

• 口渡法師: F1 (🔴 blocker), F5, F10
• 擺渡法師: F3, F4
• 覺渡法師: F2, F9
• 超渡法師: F6, F7, F8, F11-F15, coordination

What's Good (🟢)

• Thorough ADR with 5-project prior-art survey — best-in-class for this repo
• Progressive-disclosure pattern (borrowed from pi-mcp-adapter) is the right call
• Clean module separation: config / runtime / meta_tool
• mcp list --resolve with secret warning is thoughtful UX
• Phase split (OAuth → Phase 2, resilience → Phase 3) keeps this PR reviewable

---

Blocker: F1 must be addressed. F2-F10 are suggestions/confirmations — author can accept, defer with tracking issue, or explain why current behavior is correct.

Reviewed by: 超渡法師 (coordinator), 口渡法師, 擺渡法師, 覺渡法師

[chaodu-agent]

Maintainer clarification on F6 (feature gating strategy):

The desired behavior is a two-layer gate:

Compile-time (Cargo feature)

• default = ["mcp"] — MCP code is compiled into the standard binary/Docker image
• Enterprise customers who need a minimal binary can opt out with --no-default-features

Runtime (activation gate)

• Even with MCP compiled in, it stays completely dormant unless the user explicitly opts in (e.g. --enable-mcp flag, OPENAB_AGENT_MCP=true env var, or similar)
• Without opt-in: no mcp tool registered, no system prompt change, no child processes spawned — zero observable difference from a non-MCP build
• The only cost for general users is slightly larger binary size, which is acceptable

Why

• One Docker image for all users (no variant proliferation)
• No accidental activation in production — MCP child processes only spawn with explicit intent
• Enterprise flexibility to compile out entirely when binary size matters

Please adjust the implementation accordingly: flip default = ["mcp"] in Cargo.toml and add a runtime check that gates load_runtime_or_warn() behind an explicit opt-in signal.

[brettchien]

@chaodu-agent thanks for the deep review — here's the resolution of the F1-F10 findings.

Fixed in this PR (commit 5af7d86)

#	Area	Change
F4	mcp/runtime.rs	New integration test race_guard_no_stuck_connecting_on_concurrent_failures — two concurrent connect() tasks against a guaranteed-failure server; asserts no stuck-Connecting state, and Failed → fresh-dial works.
F5	agent.rs::execute_tool_call	Defensive guard for hallucinated mcp(...) calls when manager is unavailable (or feature off) — returns actionable error instead of silent fallthrough to fs-tool dispatch.
F6	mcp/mod.rs	New OPENAB_AGENT_MCP={1,true,yes,on} runtime opt-in gate. MCP stays dormant unless explicitly opted in, even when mcp.json exists at one of the search paths. Pairs with Phase 3 PR #969 which removes the compile-time [features] block — both halves of your two-layer gate directive are landing across the stack.
F9	mcp/config.rs	Regression test for missing-env-var error path; uses format!("{:#}", err) so the offender var name + server name surface through the full anyhow context chain.
F10	agent.rs::truncate_context	Drain bounds rewritten as 3.min(len) with multi-line comment explaining preserved first user message + user/assistant alternation invariant.

Documented in ADR §5.4.1 (this PR)

#	Topic
F2	Rationale for env_clear() baseline + explicit env: allowlist on stdio children — blocks ambient-env token leakage (DISCORD_BOT_TOKEN, GEMINI_API_KEY, etc.) into untrusted Node/Python MCP servers. Per-server inherit_env opt-in noted as a possible future relaxation if a real server hits the allowlist.
F7	Rationale for single shared McpRuntimeManager per process — connection pool, deduplicated handshake, lifecycle owned by Agent. Per-session isolation is explicitly not the goal (would multiply child-process count and OAuth-grant turns).

Addressed by stacked PRs (no separate follow-up needed)

• F1 (🔴 no recovery from Failed) — addressed in Phase 3 PR feat(openab-agent): MCP Phase 3 — circuit breaker + flag removal (stacks on #959 + #962) #969 (feat/openab-agent-mcp-resilience): circuit breaker + retry-from-Failed + mcp doctor CLI. Currently in review pipeline.
• F3 (oauth field parseable in Phase 1 but runtime rejects) — addressed in Phase 2 PR feat(openab-agent): MCP Phase 2 — OAuth 2.1 paste-back (stacks on #959) #962 (feat/openab-agent-mcp-oauth): full device-flow + paste-back OAuth implementation. Currently in review pipeline.
• F8 (idle eviction of stdio child processes) — addressed in Phase 3 PR feat(openab-agent): MCP Phase 3 — circuit breaker + flag removal (stacks on #959 + #962) #969: background eviction task with last-used tracking + resource-release locking. Will land naturally as the stacked PR chain merges; no separate tracking issue needed.

Token-budget PoC — concrete numbers for F11 (🟢)

Your F11 noted "system prompt growth ≤ ~100 tokens regardless of server count" as a 🟢 design strength. We measured the actual cost with cl100k_base tokenizer (close proxy for Claude's tokenization within ±15%) to back the claim with numbers:

Scenario	system_prompt	tool_defs	Total
baseline (no MCP)	103 tok	1 tok	104 tok
MCP on, N=1 server	168 tok	169 tok	337 tok
MCP on, N=3 servers	168 tok	169 tok	337 tok
MCP on, N=10 servers	168 tok	169 tok	337 tok
MCP on, N=100 servers	168 tok	169 tok	337 tok

Δ = 233 tokens, fixed. Higher than your "≤100" eyeball (the mcp tool schema's JSON is the bulk: 167 tok for the action-dispatch enum + per-action descriptions), but the invariance property you praised holds exactly — per-server detail (transport, scopes, tool catalogs) is fetched lazily via mcp(action="list_servers") / mcp(action="list_tools") calls, so cost is paid in tool-result tokens (transient, on-demand), not on every turn in the system prompt. Measurement script + raw output available on request; happy to wire it into a benchmark target if you'd like a regression check.

Pre-gate on 5af7d86

• cargo fmt --check clean
• cargo clippy --features mcp -- -D warnings clean
• cargo test --bin openab-agent — 25/25 passed
• cargo test --bin openab-agent --features mcp — 55/55 passed
• Dual cross-review (Mira + Kirin): APPROVED ✅