feat(core): compile-time mcp feature gate (#4799)

AGENTS.md

@@ -242,6 +242,7 @@ GGML_NATIVE=OFF cargo check --manifest-path Cargo.toml \
 | `meet` | ON | `openhuman::meet` (join-URL validation) + `openhuman::meet_agent` (live STT/LLM/TTS loop) + `openhuman::agent_meetings` (backend-delegated Meet bot over Socket.IO) | none — see note |
 | `skills` | ON | `openhuman::skills` + `openhuman::skill_runtime` + `openhuman::skill_registry` domains — SKILL.md discovery/parse/install, workflow execution + run logs, remote catalogs, the `skill_setup` / `skill_executor` builtin agents, and the 16 skill agent tools | none (see below) |
 | `flows` | ON | `openhuman::flows` (saved automation graphs — create/run/schedule, the `workflow_builder` + `flow_discovery` agents), `openhuman::tinyflows` (engine seam), `openhuman::rhai_workflows` (`.ragsh` language-workflow tool) | `tinyflows`, `jaq-core`, `jaq-std`, `jaq-json`, `rhai` |
+| `mcp` | ON | `openhuman::mcp_server` (the `openhuman mcp` stdio/HTTP server), `openhuman::mcp_registry` (dynamic Smithery installs — `mcp_clients` RPC namespace, SQLite, boot spawn, supervisor, OAuth), `openhuman::mcp_audit` (write-audit log), and the static config-declared server set in `openhuman::mcp_client`. ~19 agent tools, ~20k LOC | **none** (see scope note) |
 
 **Facade pattern (pathfinder for the other gates).** `pub mod voice;` is **always compiled** as a facade: the real submodules are `#[cfg(feature = "voice")]`, and a `#[cfg(not(feature = "voice"))] mod stub;` (`src/openhuman/voice/stub.rs`) re-exposes the same public surface that always-on / other-gated callers use (`server`, `dictation_listener`, `streaming`, `reply_speech`, `cloud_transcribe`, `cli`, `create_stt_provider`, `effective_stt_provider`, `publish_ptt_transcript_committed`) with no-op / `None` / disabled-error bodies. Callers therefore do **not** need per-call `#[cfg]`. When voice is off: the voice/audio controllers are unregistered (unknown-method over `/rpc`, absent from `/schema`), the `audio_generate_podcast` agent tools are absent, and `openhuman voice` returns a "voice disabled" error. Stub signatures must match the real ones exactly — the disabled build (`--no-default-features --features tokenjuice-treesitter`) is the **only** thing that catches drift, so run it before pushing any change to the voice surface.
 
@@ -283,6 +284,28 @@ When skills are off: the `skills` / `skill_runtime` / `skill_registry` controlle
 
 **Testing gotcha (applies to every gate).** The CI smoke lane runs `cargo check` only — it never runs `cargo test --no-default-features`, so CI stays green while the disabled-build **test** suite is broken. Tests that hard-assert a gated family (`.expect("a flows.* method exists")`, `assert!(full_ns.contains("flows"))`, `group_for_namespace("flows")`, built-in-agent id lists) must be `#[cfg]`-gated in lockstep with the feature. Run `GGML_NATIVE=OFF cargo test --lib --no-default-features --features tokenjuice-treesitter core::` locally before pushing any gate change.
 
+#### The `mcp` gate
+
+Follows the voice facade+stub pattern for `mcp_server` / `mcp_registry` / `mcp_audit` (`stub.rs` in each), with two refinements worth copying:
+
+- **Type carve-out.** Inert, dependency-free type modules stay **ungated**: `mcp_registry::types`, `mcp_audit::types`, `mcp_server::tools::types` (`McpToolSpec`). They are `serde`/`serde_json`-only data consumed by always-compiled callers (the orchestrator prompt builder, `tool_registry`). Both builds therefore share the **one real type definition** — the stubs carry behaviour only, so struct fields can never drift between the enabled and disabled builds. `ConnectedServerOverview` was moved from `connections.rs` into `types.rs` for exactly this reason and is re-exported from `connections` so existing paths still resolve.
+- **Split facade (`mcp_client`).** `mcp_client` is **not** gated wholesale, because the directory does not match the real dependency graph. `mcp_client::sanitize` and `mcp_client::client` (`McpHttpClient`, `redact_endpoint`, `McpUnauthorizedError`) stay **always compiled**: they are mis-housed shared utilities. The `gitbooks` docs tool dials `McpHttpClient` directly (GitBook is modelled as a legacy MCP server), and the orchestrator prompt sanitizes **skill** descriptions through `sanitize::sanitize_for_llm` — neither has anything to do with MCP, and stubbing them would silently break a docs tool and corrupt the orchestrator prompt in slim builds. Only `registry`, `stdio`, `spawn_env`, `setup_agent` are gated. **The gate follows the real dependency graph, not the directory name.** (Relocating `sanitize` + `client` out of `mcp_client` is worthwhile follow-up.) A bonus of keeping `client` compiled: the `McpServerNeedsAuth` classifier coupling test in `core::observability` stays always-compiled — no `#[cfg]`, no wording-drift leak.
+
+**Scope note — the `mcp` gate drops ZERO dependencies.** There is no MCP SDK in this crate: the dependency declarations contain no MCP-specific SDK or transport dependency; `test-mcp-stub` is the only MCP-named bin target. The entire protocol stack is hand-rolled over tokio process stdio + `reqwest` + `axum`, all of which are load-bearing for non-MCP domains. The gate is worth having for the ~20k LOC / ~19 agent tools / RPC surface it removes, but the issue-level DoD line claiming it "sheds the MCP SDK / transport stack" is superseded by this correction. The `mcp = []` feature list in `Cargo.toml` is intentionally empty — do not "fix" it by adding `dep:` entries.
+
+**Static vs dynamic — the naming is INVERTED from intuition.** Both halves must be gated or the gate is only half-applied:
+
+| Module | Despite the name, it is… | Backed by | Agent tools |
+| ------ | ------------------------ | --------- | ----------- |
+| `mcp_client` | the **STATIC**, config-declared server set (`[[mcp_client.servers]]` in TOML → `McpServerRegistry::from_config`) | TOML config | `mcp_list_servers`, `mcp_list_tools`, `mcp_call_tool` |
+| `mcp_registry` | the **DYNAMIC**, user-installed Smithery servers (live connection map, boot spawn, supervisor, OAuth) | SQLite `mcp_clients.db` | 11 × `mcp_registry_*` |
+
+**CLI when compiled out.** `src/core/cli.rs` is deliberately **untouched**: the `"mcp" | "mcp-server"` arm resolves to the stub's `run_stdio_from_cli`, which returns a "mcp feature disabled at compile time … rebuild with `--features mcp`" error. Deleting the arm would let `mcp` fall through to generic namespace resolution and fail with `unknown namespace: mcp` — which reads like a user typo rather than a build fact, and would leave an MCP host (Claude Desktop / Cursor) hanging on stdout that never speaks JSON-RPC. Pinned by `mcp_subcommand_reports_disabled_build_when_gate_off` in `src/core/cli_tests.rs`.
+
+**Dangling `mcp_agent` in the orchestrator TOML is expected and safe.** `agent.toml` is data and cannot be `#[cfg]`'d, so the orchestrator keeps listing `mcp_agent` in `subagents` even when the agent is compiled out. Both resolution sites already tolerate unknown ids — `collect_orchestrator_tools` warns and skips, `validate_tier_hierarchy` `continue`s — so the core still boots. `orchestrator_tolerates_unresolvable_subagent_id` / `orchestrator_tolerates_absent_mcp_agent` in `loader.rs` pin that contract; do not "tighten" unknown-subagent handling into a hard error without re-checking them. `src/core/legacy_aliases.rs`'s frontend-catalog drift tests ignore gated namespaces for the same data-vs-code reason.
+
+`src/core/all.rs` needs **no** `#[cfg]` for this gate: the stub aggregators return empty vecs, so the registration sites keep compiling unchanged.
+
 ### Event bus (`src/core/event_bus/`)
 
 Typed pub/sub + native request/response. Both singletons — use module-level functions.

Cargo.toml

@@ -347,7 +347,7 @@ tokio = { version = "1", features = ["test-util"] }
 proptest = "1"
 
 [features]
-default = ["tokenjuice-treesitter", "voice", "web3", "media", "meet", "skills", "flows"]
+default = ["tokenjuice-treesitter", "voice", "web3", "media", "meet", "skills", "flows", "mcp"]
 # AST-aware code compression (tree-sitter Rust/TS/Python grammars; C build).
 # On by default; disable to fall back to the brace-depth heuristic.
 tokenjuice-treesitter = [
@@ -437,6 +437,27 @@ meet = []
 # types are a type carve-out, not part of the gated behaviour. See
 # `src/openhuman/skills/stub.rs`.
 skills = []
+# MCP domains: `mcp_server` (the `openhuman mcp` stdio/HTTP server exposing our
+# tool surface), `mcp_registry` (dynamic Smithery installs — SQLite, lifecycle,
+# `mcp_clients` RPC namespace), `mcp_audit` (write-audit log), and the
+# static/config-declared server set in `mcp_client`. Default-ON — the desktop
+# app always ships with MCP. Composes with the runtime `DomainSet::mcp` flag
+# (#4796): this feature narrows the compile-time surface, `DomainSet` gates it
+# at runtime.
+#
+# INTENTIONALLY EMPTY — do NOT "fix" this by adding `dep:` entries. There is no
+# MCP SDK in this crate: the whole protocol stack is hand-rolled over tokio
+# process stdio + reqwest + axum, every one of which is load-bearing for
+# non-MCP domains. So this gate drops ~20k LOC and ~19 agent tools but ZERO
+# dependencies. The issue-level DoD line claiming it "sheds the MCP SDK /
+# transport stack" is superseded by this correction (see AGENTS.md).
+#
+# NOTE: `mcp_client::{sanitize, client}` stay ALWAYS compiled — they are
+# mis-housed shared utilities (the gitbooks docs tool dials `McpHttpClient`;
+# the orchestrator prompt sanitises *skill* descriptions through
+# `sanitize::sanitize_for_llm`). The gate follows the real dependency graph,
+# not the directory name.
+mcp = []
 sandbox-landlock = ["dep:landlock"]
 sandbox-bubblewrap = []
 peripheral-rpi = ["dep:rppal"]

app/src-tauri/Cargo.toml

@@ -164,11 +164,11 @@ openhuman_core = { path = "../..", package = "openhuman", default-features = fal
     "media",
     "tokenjuice-treesitter",
     "voice",
-    "tokenjuice-treesitter",
     "web3",
     "flows",
     "meet",
     "skills",
+    "mcp",
 ] }
 tinyjuice = { version = "0.2.1", default-features = false }
 

src/core/all_tests.rs

@@ -335,6 +335,7 @@ fn schema_for_rpc_method_finds_security_policy_info() {
 }
 
 #[test]
+#[cfg(feature = "mcp")]
 fn schema_for_rpc_method_finds_internal_mcp_audit_list() {
     let schema = schema_for_rpc_method("openhuman.mcp_audit_list");
     assert!(
@@ -965,9 +966,63 @@ fn group_mapping_smoke() {
     #[cfg(feature = "meet")]
     assert_eq!(group_for_namespace("meet"), Some(DomainGroup::Meet));
     // Internal-only registry is grouped too (mcp_audit → Mcp).
+    // Compiled out with the `mcp` feature: `group_for_namespace` reads the LIVE
+    // registry, and the gate unregisters the mcp_audit controller entirely.
+    #[cfg(feature = "mcp")]
     assert_eq!(group_for_namespace("mcp_audit"), Some(DomainGroup::Mcp));
 }
 
+// --- `mcp` compile-time gate (#4799) ------------------------------------
+
+/// With the `mcp` feature ON (the default / shipped desktop build), both MCP
+/// namespaces are registered: `mcp_clients` (the dynamic Smithery registry,
+/// agent-facing) and `mcp_audit` (the write-audit log, internal-only).
+///
+/// Paired with `mcp_namespaces_absent_when_gate_off` below so the gate is
+/// pinned in BOTH directions — an assert that only ever runs in one build
+/// configuration cannot prove a gate works.
+#[test]
+#[cfg(feature = "mcp")]
+fn mcp_namespaces_registered_when_gate_on() {
+    assert_eq!(
+        group_for_namespace("mcp_clients"),
+        Some(DomainGroup::Mcp),
+        "with `mcp` compiled in, the dynamic registry's `mcp_clients` \
+         namespace must be registered"
+    );
+    assert_eq!(
+        group_for_namespace("mcp_audit"),
+        Some(DomainGroup::Mcp),
+        "with `mcp` compiled in, the internal `mcp_audit` namespace must be \
+         registered"
+    );
+}
+
+/// With the `mcp` feature OFF, both MCP namespaces are gone from the live
+/// registry — every `openhuman.mcp_clients_*` / `openhuman.mcp_audit_*` method
+/// is an unknown method over `/rpc` and absent from `/schema`.
+///
+/// This is the compile-time analogue of the runtime `DomainSet::mcp` filter:
+/// `DomainSet` can hide these namespaces at runtime, this feature removes the
+/// code that backs them altogether. Note the stubs make this work with NO
+/// `#[cfg]` in `src/core/all.rs` — the aggregators simply return empty vecs.
+#[test]
+#[cfg(not(feature = "mcp"))]
+fn mcp_namespaces_absent_when_gate_off() {
+    assert_eq!(
+        group_for_namespace("mcp_clients"),
+        None,
+        "with `mcp` compiled out, the `mcp_clients` namespace must not be \
+         registered — the stub aggregator returns an empty vec"
+    );
+    assert_eq!(
+        group_for_namespace("mcp_audit"),
+        None,
+        "with `mcp` compiled out, the internal `mcp_audit` namespace must not \
+         be registered — the stub aggregator returns an empty vec"
+    );
+}
+
 // --- #4797: `flows` compile-time gate (directional proof) -------------------
 //
 // One namespace, not three: `tinyflows` registers no controllers, and

src/core/cli_tests.rs

@@ -140,3 +140,58 @@ fn load_dotenv_for_cli_reads_cwd_dotenv_without_overwriting_existing_env() {
     );
     assert_eq!(loaded_app_env.as_deref(), Some("production"));
 }
+
+// --- `mcp` compile-time gate (#4799) ------------------------------------
+
+/// With the `mcp` feature compiled out, `openhuman mcp` must fail with a
+/// diagnostic that names the BUILD as the cause — not a generic
+/// "unknown namespace" error.
+///
+/// Why this matters enough to test: the naive way to gate the CLI is to delete
+/// the `"mcp" | "mcp-server"` match arm. That is WRONG — `mcp` would fall
+/// through to generic namespace resolution and die with `unknown namespace:
+/// mcp`, which reads like the user typo'd a command rather than like a
+/// property of this build. Instead `cli.rs` is untouched and the arm resolves
+/// to `mcp_server::stub::run_stdio_from_cli`, which bails with the message
+/// asserted below. An MCP host (Claude Desktop, Cursor, …) spawning
+/// `openhuman mcp` therefore gets a non-zero exit + a one-line reason on
+/// stderr instead of hanging on stdout that never speaks JSON-RPC.
+#[test]
+#[cfg(not(feature = "mcp"))]
+fn mcp_subcommand_reports_disabled_build_when_gate_off() {
+    let _guard = env_lock();
+
+    let err = crate::core::cli::run_from_cli_args(&["mcp".to_string()])
+        .expect_err("`openhuman mcp` must fail when the `mcp` feature is compiled out");
+    let msg = err.to_string();
+
+    assert!(
+        msg.contains("mcp feature disabled"),
+        "error must name the compile-time gate as the cause; got: {msg}"
+    );
+    assert!(
+        msg.contains("--features mcp"),
+        "error must tell the user how to get a working build; got: {msg}"
+    );
+    assert!(
+        !msg.contains("unknown namespace"),
+        "must NOT degrade into generic namespace resolution — that reads like a typo, \
+         not a build fact; got: {msg}"
+    );
+}
+
+/// The `mcp-server` alias must behave identically to `mcp` — both arms route
+/// to the same stub, so neither can silently regress into the fall-through.
+#[test]
+#[cfg(not(feature = "mcp"))]
+fn mcp_server_alias_reports_disabled_build_when_gate_off() {
+    let _guard = env_lock();
+
+    let err = crate::core::cli::run_from_cli_args(&["mcp-server".to_string()])
+        .expect_err("`openhuman mcp-server` must fail when the `mcp` feature is compiled out");
+
+    assert!(
+        err.to_string().contains("mcp feature disabled"),
+        "the `mcp-server` alias must give the same build-fact diagnostic as `mcp`"
+    );
+}

src/core/legacy_aliases.rs

@@ -349,6 +349,33 @@ mod tests {
             .collect()
     }
 
+    /// Whether `method`'s controller is compiled out of THIS build by a
+    /// default-ON Cargo feature gate.
+    ///
+    /// The frontend RPC catalog and the alias table below are both **data**:
+    /// they are authored against the full (shipped desktop) surface and cannot
+    /// be `#[cfg]`'d per Rust feature — the frontend is built independently of
+    /// the core's feature set, and the shipped app always enables `mcp`. So in
+    /// a slim build they legitimately still name methods whose controllers no
+    /// longer exist. The drift checks below must therefore ignore exactly those
+    /// namespaces, and keep asserting on everything else — otherwise the whole
+    /// drift signal is lost in slim builds (or, worse, someone "fixes" the
+    /// failure by deleting live frontend methods from the catalog).
+    ///
+    /// Mirrors how the agent loader tolerates the orchestrator TOML's dangling
+    /// `mcp_agent` subagent id (#4799).
+    #[cfg(feature = "mcp")]
+    fn is_compiled_out_method(_method: &str) -> bool {
+        false
+    }
+
+    #[cfg(not(feature = "mcp"))]
+    fn is_compiled_out_method(method: &str) -> bool {
+        // `mcp` feature OFF ⇒ the `mcp_clients` (dynamic registry) and
+        // `mcp_audit` (write log) controllers are unregistered.
+        method.starts_with("openhuman.mcp_clients_") || method.starts_with("openhuman.mcp_audit_")
+    }
+
     #[test]
     fn quoted_value_extracts_single_quoted_string() {
         assert_eq!(quoted_value(": 'hello'"), "hello");
@@ -607,6 +634,7 @@ mod tests {
         let missing: Vec<_> = core_methods
             .values()
             .filter(|method| !registered.contains(*method))
+            .filter(|method| !is_compiled_out_method(method))
             .cloned()
             .collect();
 
@@ -638,6 +666,7 @@ mod tests {
         let missing: Vec<_> = legacy_aliases()
             .iter()
             .filter(|(_, canonical)| !registered.contains(*canonical))
+            .filter(|(_, canonical)| !is_compiled_out_method(canonical))
             .map(|(legacy, canonical)| format!("{legacy} -> {canonical}"))
             .collect();
 

src/openhuman/agent/harness/definition_tests.rs

@@ -367,6 +367,10 @@ fn all_builtin_agent_definitions_have_expected_effective_max_iterations() {
         ("code_executor", 50),
         ("context_scout", 50),
         ("integrations_agent", 50),
+        // `mcp_agent` is compiled out with the `mcp` feature (#4799).
+        // `mcp_setup` is NOT — only its five tools are gated, so the agent
+        // definition still loads in both builds.
+        #[cfg(feature = "mcp")]
         ("mcp_agent", 50),
         ("mcp_setup", 50),
         ("planner", 50),

src/openhuman/agent/harness/subagent_runner/tool_prep.rs

@@ -252,6 +252,11 @@ mod tests {
             "schedule_task",
             "make_presentation",
             "archive_session",
+            // `use_mcp_server` is `mcp_agent`'s `delegate_name`; the agent —
+            // and therefore this delegate tool — is compiled out with the
+            // `mcp` feature (#4799). `setup_mcp_server` belongs to
+            // `mcp_setup`, which stays registered in both builds.
+            #[cfg(feature = "mcp")]
             "use_mcp_server",
             "setup_mcp_server",
         ] {