From 929e4509aa8a579d045f4947bf8a7434e1f55cb9 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Wed, 15 Jul 2026 18:08:23 +0530 Subject: [PATCH 01/16] build(mcp): add default-ON `mcp` Cargo feature (#4799) Empty feature list on purpose: there is no MCP SDK in this crate. The whole protocol stack is hand-rolled over tokio process stdio + reqwest + axum, all load-bearing for non-MCP domains, so the gate drops ~20k LOC and ~19 agent tools but ZERO dependencies. The comment records that fact so nobody "fixes" the empty list into `dep:` entries later. --- Cargo.toml | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 2b6bff7f03..a866f47401 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -334,7 +334,7 @@ tokio = { version = "1", features = ["test-util"] } proptest = "1" [features] -default = ["tokenjuice-treesitter", "voice"] +default = ["tokenjuice-treesitter", "voice", "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 = [ @@ -351,6 +351,27 @@ tokenjuice-treesitter = [ # inference domain (shared with accessibility for cpal) and await a separate # `inference` gate. voice = ["dep:hound", "dep:lettre"] +# 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"] From c4bb04956226fe2038a5869281ac524cd42d1986 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Wed, 15 Jul 2026 18:08:28 +0530 Subject: [PATCH 02/16] refactor(mcp_registry): move ConnectedServerOverview into dep-free types (#4799) The always-compiled orchestrator prompt builder consumes this type, but it lived in connections.rs alongside the tokio/stdio connection map that the `mcp` feature gates. Relocating it to the inert serde-only types module lets both builds share ONE real definition instead of a hand-mirrored stub copy, so the struct's fields can never drift between them. connections re-exports it, so mcp_registry::connections::ConnectedServerOverview keeps resolving for every existing caller. --- src/openhuman/mcp_registry/connections.rs | 21 +++++------------- src/openhuman/mcp_registry/types.rs | 27 +++++++++++++++++++++++ 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/openhuman/mcp_registry/connections.rs b/src/openhuman/mcp_registry/connections.rs index fdc964593c..1bffe5ea96 100644 --- a/src/openhuman/mcp_registry/connections.rs +++ b/src/openhuman/mcp_registry/connections.rs @@ -179,21 +179,12 @@ impl Connection { } } -/// One connected server's identity + advertised tools, for prompt-surface -/// discovery (the orchestrator's "## Connected MCP Servers" block). Sourced -/// entirely from the live connection map — no `Config`, no store read. -#[derive(Debug, Clone)] -pub struct ConnectedServerOverview { - pub server_id: String, - pub qualified_name: String, - pub display_name: String, - /// Short registry description — the primary capability hint surfaced in - /// the orchestrator prompt (mirrors Composio's per-toolkit description). - pub description: Option, - /// Advertised tools — retained for a tool-count fallback when a server - /// has no description, and for any caller that wants the full list. - pub tools: Vec, -} +/// Re-exported from [`super::types`] so the long-standing +/// `mcp_registry::connections::ConnectedServerOverview` path keeps resolving. +/// The definition moved to `types.rs` (an inert, dep-free module that survives +/// the `mcp` feature gate) because the always-compiled orchestrator prompt +/// builder consumes this type while `connections` itself is gated. +pub use super::types::ConnectedServerOverview; // ── Global registry ────────────────────────────────────────────────────────── diff --git a/src/openhuman/mcp_registry/types.rs b/src/openhuman/mcp_registry/types.rs index 7aae8a567a..c41de76a86 100644 --- a/src/openhuman/mcp_registry/types.rs +++ b/src/openhuman/mcp_registry/types.rs @@ -170,6 +170,33 @@ pub struct McpTool { pub input_schema: Value, } +// ── ConnectedServerOverview ───────────────────────────────────────────────── + +/// One connected server's identity + advertised tools, for prompt-surface +/// discovery (the orchestrator's "## Connected MCP Servers" block). Sourced +/// entirely from the live connection map — no `Config`, no store read. +/// +/// Lives here (rather than beside the live connection map in [`super::connections`], +/// where it was originally defined) because it is an inert, dependency-free +/// data type consumed by the ALWAYS-COMPILED orchestrator prompt builder. The +/// `mcp` Cargo feature gates `connections` — which owns the tokio/stdio +/// machinery — but this type must survive that gate. Keeping the single +/// definition here and re-exporting it from `connections` means the disabled +/// build reuses the real type instead of a hand-mirrored stub copy, so the +/// struct's fields can never drift between the two builds. +#[derive(Debug, Clone)] +pub struct ConnectedServerOverview { + pub server_id: String, + pub qualified_name: String, + pub display_name: String, + /// Short registry description — the primary capability hint surfaced in + /// the orchestrator prompt (mirrors Composio's per-toolkit description). + pub description: Option, + /// Advertised tools — retained for a tool-count fallback when a server + /// has no description, and for any caller that wants the full list. + pub tools: Vec, +} + // ── ConnStatus ────────────────────────────────────────────────────────────── /// Connection status summary for one installed server. From 1bd03f982488cfe3738eee03e1e3d2bb878e9dc1 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Wed, 15 Jul 2026 18:08:36 +0530 Subject: [PATCH 03/16] feat(mcp_registry): gate dynamic registry behind `mcp` feature (#4799) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Facade + stub: types stays ungated (inert serde data consumed by the always-compiled orchestrator prompt), behaviour is gated. The stub's aggregator returns an empty Vec, which is what keeps src/core/all.rs — the epic's hottest multi-agent file — free of any #[cfg] for this gate. boot/bus/supervisor no-op, oauth::complete errors, connected_overview and all_connected_tools return empty; every always-on caller already tolerates those shapes with zero call-site edits. --- src/openhuman/mcp_registry/mod.rs | 42 ++++++++++ src/openhuman/mcp_registry/stub.rs | 127 +++++++++++++++++++++++++++++ 2 files changed, 169 insertions(+) create mode 100644 src/openhuman/mcp_registry/stub.rs diff --git a/src/openhuman/mcp_registry/mod.rs b/src/openhuman/mcp_registry/mod.rs index 5a481c276b..e241c73303 100644 --- a/src/openhuman/mcp_registry/mod.rs +++ b/src/openhuman/mcp_registry/mod.rs @@ -50,22 +50,55 @@ //! backwards compatibility with existing frontend code and on-disk state. //! The Rust module path is `mcp_registry`. +//! ## Compile-time gate (`mcp` feature) +//! +//! `pub mod mcp_registry;` is ALWAYS compiled — it is a facade. Everything +//! that carries behaviour (Smithery HTTP client, SQLite store, live +//! connection map, boot spawn, supervisor, OAuth, RPC surface) is gated +//! behind the default-ON `mcp` Cargo feature; when the feature is off, +//! [`stub`] mirrors the subset of the surface that always-compiled callers +//! depend on with no-op / `Err` / empty-`Vec` bodies, so those callers need no +//! `#[cfg]` of their own. +//! +//! [`types`] stays UNGATED on purpose: it is inert serde data (`serde` + +//! `serde_json` only) consumed by the always-compiled orchestrator prompt +//! builder. Sharing the one real definition across both builds means the +//! disabled build cannot drift from the enabled one — the stub carries +//! behaviour, never duplicated types. + +#[cfg(feature = "mcp")] pub mod boot; +#[cfg(feature = "mcp")] pub mod bus; +#[cfg(feature = "mcp")] pub mod connections; +#[cfg(feature = "mcp")] mod curation; +#[cfg(feature = "mcp")] pub mod oauth; +#[cfg(feature = "mcp")] pub mod ops; +#[cfg(feature = "mcp")] mod registries; +#[cfg(feature = "mcp")] mod registry; +#[cfg(feature = "mcp")] mod schemas; +#[cfg(feature = "mcp")] pub mod setup; +#[cfg(feature = "mcp")] pub mod setup_ops; +#[cfg(feature = "mcp")] pub mod store; +#[cfg(feature = "mcp")] pub mod supervisor; +#[cfg(feature = "mcp")] pub mod tools; + +// Inert serde types — always compiled (see the module note above). pub mod types; +#[cfg(feature = "mcp")] pub use schemas::{ all_controller_schemas as all_mcp_registry_controller_schemas, all_registered_controllers as all_mcp_registry_registered_controllers, @@ -73,3 +106,12 @@ pub use schemas::{ }; pub use types::{ConnStatus, InstalledServer, McpTool}; + +// --------------------------------------------------------------------------- +// Disabled facade — compiled only when the `mcp` feature is OFF. +// --------------------------------------------------------------------------- + +#[cfg(not(feature = "mcp"))] +mod stub; +#[cfg(not(feature = "mcp"))] +pub use stub::*; diff --git a/src/openhuman/mcp_registry/stub.rs b/src/openhuman/mcp_registry/stub.rs new file mode 100644 index 0000000000..9fe9f72b5c --- /dev/null +++ b/src/openhuman/mcp_registry/stub.rs @@ -0,0 +1,127 @@ +//! Disabled-MCP facade for [`super`] (the dynamic Smithery registry). +//! +//! Compiled only when the `mcp` Cargo feature is OFF (see the gate in +//! [`super`]). It mirrors the subset of the real `mcp_registry` public surface +//! that always-compiled callers depend on, with no-op / disabled-error / +//! empty-collection bodies so the crate still compiles, boots, and serves +//! `/rpc` without the MCP domains. +//! +//! Note what is NOT here: [`super::types`] is ungated, so this module defines +//! no data types at all — it carries behaviour only. Every type an always-on +//! caller names ([`ConnectedServerOverview`], [`McpTool`]) is the *real* one, +//! which is why the two builds cannot drift in shape. +//! +//! The signatures here MUST match the real ones exactly (return types and +//! async-ness included). The disabled build +//! (`cargo check --no-default-features --features tokenjuice-treesitter`) is +//! the only thing that catches drift — if a real signature changes, update the +//! mirror below until that build is green again. + +use crate::openhuman::config::Config; + +/// Error text returned by every disabled-path operation that must yield a +/// `Result`. Shared so callers / log-greps see one stable string. +const DISABLED_MSG: &str = "mcp feature disabled at compile time"; + +// --------------------------------------------------------------------------- +// Controller registration (mirrors `schemas::all_registered_controllers`) +// --------------------------------------------------------------------------- + +/// No controllers: the `mcp_clients` RPC namespace does not exist in this +/// build, so every `openhuman.mcp_clients_*` method is an unknown method over +/// `/rpc` and absent from `/schema`. +/// +/// `src/core/all.rs` pushes this straight into its controller vec with no +/// `#[cfg]` of its own — the empty vec is what keeps that (very hot, +/// multi-agent) file untouched by this gate. +pub fn all_mcp_registry_registered_controllers() -> Vec { + Vec::new() +} + +// --------------------------------------------------------------------------- +// Boot / lifecycle (mirrors `boot`, `bus`, `supervisor`) +// --------------------------------------------------------------------------- + +/// Boot-time spawn of installed local MCP servers. +pub mod boot { + use super::*; + + /// No-op: no installed-server store exists in this build, so there is + /// nothing to spawn. The real one already treats every per-server failure + /// as non-fatal and never blocks boot, so doing nothing is shape-identical + /// to "every server failed to spawn". + pub async fn spawn_installed_servers(_config: &Config) { + log::debug!("[mcp_registry] {DISABLED_MSG} — skipping installed-server spawn"); + } +} + +/// DomainEvent subscriber for MCP lifecycle logging. +pub mod bus { + use super::*; + + /// No-op: with no connection lifecycle there are no events to subscribe to. + pub fn init() { + log::debug!("[mcp_registry] {DISABLED_MSG} — event subscriber not registered"); + } +} + +/// Reconnect supervisor for installed local-spawn servers. +pub mod supervisor { + use super::*; + + /// Returns immediately instead of running the reconnect tick loop. + /// + /// The real `run` never returns (it is an infinite `interval` loop), and + /// its caller in `core/runtime/services.rs` spawns it as a background + /// task and does not await completion — so returning at once simply means + /// that task finishes rather than idling forever. + pub async fn run(_config: Config) { + log::debug!("[mcp_registry] {DISABLED_MSG} — supervisor not started"); + } +} + +// --------------------------------------------------------------------------- +// OAuth callback (mirrors `oauth::complete`) +// --------------------------------------------------------------------------- + +/// OAuth authorization-code completion for HTTP-remote MCP servers. +pub mod oauth { + use super::*; + + /// Always errors: no pending-authorization map exists in this build, so a + /// callback can only be a stale/blind hit. The real function returns the + /// same `Err(String)` shape for an unknown state, and its `core/jsonrpc.rs` + /// caller already renders that as an error page. + pub async fn complete(_config: &Config, _state: &str, _code: &str) -> Result { + Err(DISABLED_MSG.to_string()) + } +} + +// --------------------------------------------------------------------------- +// Live connection map (mirrors `connections`) +// --------------------------------------------------------------------------- + +/// Global in-process registry of connected MCP servers. +pub mod connections { + /// Re-exported from the ungated `types` module — the SAME type the enabled + /// build uses, not a mirrored copy, so the orchestrator prompt builder's + /// field access can never drift between builds. + pub use super::super::types::ConnectedServerOverview; + use super::super::types::McpTool; + + /// Empty: nothing can be connected when the registry is compiled out. + /// + /// Always-on callers already handle this shape with zero `#[cfg]` — the + /// orchestrator prompt skips its "## Connected MCP Servers" block on an + /// empty slice, and the turn loop seeds an empty announced-server set. + pub async fn connected_overview() -> Vec { + Vec::new() + } + + /// Empty: no connected servers means no advertised tools. `tool_registry` + /// folds this into its entry map, which simply gains no `mcp-client::*` + /// entries. + pub async fn all_connected_tools() -> Vec<(String, String, McpTool)> { + Vec::new() + } +} From ec078b24014c851064c5089d912793a4a647c5ee Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Wed, 15 Jul 2026 18:08:37 +0530 Subject: [PATCH 04/16] feat(mcp_audit): gate write-audit log behind `mcp` feature (#4799) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same facade+stub+ungated-types shape as mcp_registry. The audit log records calls made through the MCP server, so with MCP compiled out there is no writer: list_writes returns Ok(empty) rather than Err — the query genuinely succeeded and the log is genuinely empty. --- src/openhuman/mcp_audit/mod.rs | 24 ++++++++++++++++ src/openhuman/mcp_audit/stub.rs | 50 +++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 src/openhuman/mcp_audit/stub.rs diff --git a/src/openhuman/mcp_audit/mod.rs b/src/openhuman/mcp_audit/mod.rs index 648d618752..5cb47a1d0f 100644 --- a/src/openhuman/mcp_audit/mod.rs +++ b/src/openhuman/mcp_audit/mod.rs @@ -3,15 +3,39 @@ //! The audit table is stored in the existing memory-tree SQLite database so //! writes and their query surface reuse the same local workspace persistence. +//! ## Compile-time gate (`mcp` feature) +//! +//! `pub mod mcp_audit;` is ALWAYS compiled — it is a facade. The SQLite store +//! and RPC surface are gated behind the default-ON `mcp` Cargo feature; when +//! it is off, [`stub`] mirrors the consumed surface with no-op / empty bodies. +//! +//! [`types`] stays UNGATED: it is inert serde data (`serde` + `serde_json` +//! only), so both builds share the one real definition and cannot drift. + +#[cfg(feature = "mcp")] mod schemas; +#[cfg(feature = "mcp")] pub mod store; + +// Inert serde types — always compiled (see the module note above). pub mod types; +#[cfg(feature = "mcp")] pub use schemas::{ all_controller_schemas as all_mcp_audit_controller_schemas, all_internal_controllers as all_mcp_audit_internal_controllers, all_registered_controllers as all_mcp_audit_registered_controllers, schemas as mcp_audit_schemas, }; +#[cfg(feature = "mcp")] pub use store::{list_writes, record_write}; pub use types::{McpWriteListQuery, McpWriteRecord, NewMcpWriteRecord}; + +// --------------------------------------------------------------------------- +// Disabled facade — compiled only when the `mcp` feature is OFF. +// --------------------------------------------------------------------------- + +#[cfg(not(feature = "mcp"))] +mod stub; +#[cfg(not(feature = "mcp"))] +pub use stub::*; diff --git a/src/openhuman/mcp_audit/stub.rs b/src/openhuman/mcp_audit/stub.rs new file mode 100644 index 0000000000..f8e03535ba --- /dev/null +++ b/src/openhuman/mcp_audit/stub.rs @@ -0,0 +1,50 @@ +//! Disabled-MCP facade for [`super`] (the MCP write-audit log). +//! +//! Compiled only when the `mcp` Cargo feature is OFF (see the gate in +//! [`super`]). The audit log records calls made *through the MCP server*, so +//! with MCP compiled out there is no writer and nothing to read back — the +//! disabled surface is naturally empty rather than an error. +//! +//! As in the sibling `mcp_registry` stub, [`super::types`] is ungated, so this +//! module defines no data types — only behaviour. The signatures MUST match +//! the real ones exactly; the disabled build +//! (`cargo check --no-default-features --features tokenjuice-treesitter`) is +//! the only thing that catches drift. + +use anyhow::Result; + +use crate::openhuman::config::Config; + +use super::types::{McpWriteListQuery, McpWriteRecord, NewMcpWriteRecord}; + +// --------------------------------------------------------------------------- +// Controller registration (mirrors `schemas::all_internal_controllers`) +// --------------------------------------------------------------------------- + +/// No controllers: the internal `mcp_audit` list method is unregistered, so +/// the desktop UI/CLI sees an unknown method rather than an empty history. +/// +/// `src/core/all.rs` pushes this straight into its internal-controller vec +/// with no `#[cfg]` of its own — the empty vec keeps that file untouched. +pub fn all_mcp_audit_internal_controllers() -> Vec { + Vec::new() +} + +// --------------------------------------------------------------------------- +// Store surface (mirrors `store::{record_write, list_writes}`) +// --------------------------------------------------------------------------- + +/// No-op that reports a synthetic row id: nothing can call an MCP write tool +/// in this build, so this is unreachable in practice. Returns `Ok` rather than +/// `Err` because the real callers treat a failure here as a logged anomaly — +/// an "audit write failed" warning would be actively misleading when the +/// audited subsystem does not exist. +pub fn record_write(_config: &Config, _record: NewMcpWriteRecord) -> Result { + Ok(0) +} + +/// Empty history: no MCP writes can have occurred in this build. `Ok(vec![])` +/// (not `Err`) keeps the shape honest — the query succeeded, the log is empty. +pub fn list_writes(_config: &Config, _query: &McpWriteListQuery) -> Result> { + Ok(Vec::new()) +} From adc0a58e46baed534e51f80d2947822821adde57 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Wed, 15 Jul 2026 18:08:43 +0530 Subject: [PATCH 05/16] feat(mcp_server): gate MCP server behind `mcp` feature, keep CLI honest (#4799) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tools::types stays ungated so McpToolSpec — inert &'static str + Value data named by the always-compiled tool_registry — is the same real type in both builds. src/core/cli.rs is deliberately NOT touched. The naive gate is to delete the "mcp" | "mcp-server" match arm, but then `mcp` falls through to generic namespace resolution and dies with "unknown namespace: mcp" — reading like a user typo rather than a build fact. Instead the arm resolves to the stub, which bails with a message naming the cause and the fix, so an MCP host spawning `openhuman mcp` gets a non-zero exit + diagnostic instead of hanging on stdout that never speaks JSON-RPC. --- src/openhuman/mcp_server/mod.rs | 44 +++++++++++++- src/openhuman/mcp_server/stub.rs | 85 +++++++++++++++++++++++++++ src/openhuman/mcp_server/tools/mod.rs | 33 ++++++++--- 3 files changed, 152 insertions(+), 10 deletions(-) create mode 100644 src/openhuman/mcp_server/stub.rs diff --git a/src/openhuman/mcp_server/mod.rs b/src/openhuman/mcp_server/mod.rs index c9a8d89e1c..ed2ae49723 100644 --- a/src/openhuman/mcp_server/mod.rs +++ b/src/openhuman/mcp_server/mod.rs @@ -10,18 +10,58 @@ //! and is advertised to clients via MCP tool annotations //! (`readOnlyHint: false`, `destructiveHint: true`). +//! ## Compile-time gate (`mcp` feature) +//! +//! `pub mod mcp_server;` is ALWAYS compiled — it is a facade. The protocol, +//! transports, session, and dispatch machinery are gated behind the default-ON +//! `mcp` Cargo feature; when it is off, [`stub`] mirrors the surface that +//! always-compiled callers reach (`run_stdio_from_cli`, `ensure_local_http`, +//! `LocalMcpEndpoint`, `tool_specs`) with disabled-error / empty bodies. +//! +//! `tools::types` stays UNGATED so [`McpToolSpec`] — an inert `&'static str` + +//! `Value` record consumed by the always-compiled `tool_registry` — is the +//! same real type in both builds and cannot drift. + +#[cfg(feature = "mcp")] mod http; +#[cfg(feature = "mcp")] mod local; +#[cfg(feature = "mcp")] mod protocol; +#[cfg(feature = "mcp")] mod resources; +#[cfg(feature = "mcp")] mod session; +#[cfg(feature = "mcp")] mod stdio; +#[cfg(feature = "mcp")] mod subagent_depth; -mod tools; +#[cfg(feature = "mcp")] mod write_dispatch; +// Facade: gates its own behavioural submodules but always compiles `types` +// so `McpToolSpec` survives the gate (see the module note above). +mod tools; + +#[cfg(feature = "mcp")] pub use http::{run_http, run_http_reporting, HttpServerConfig}; +#[cfg(feature = "mcp")] pub use local::{ensure_local_http, LocalMcpEndpoint}; +#[cfg(feature = "mcp")] pub use stdio::run_stdio_from_cli; +#[cfg(feature = "mcp")] pub use subagent_depth::{current_depth as current_subagent_depth, HEADER_SUBAGENT_DEPTH}; -pub use tools::{tool_specs, McpToolSpec}; +#[cfg(feature = "mcp")] +pub use tools::tool_specs; + +// Inert tool-spec type — always compiled (see the module note above). +pub use tools::McpToolSpec; + +// --------------------------------------------------------------------------- +// Disabled facade — compiled only when the `mcp` feature is OFF. +// --------------------------------------------------------------------------- + +#[cfg(not(feature = "mcp"))] +mod stub; +#[cfg(not(feature = "mcp"))] +pub use stub::*; diff --git a/src/openhuman/mcp_server/stub.rs b/src/openhuman/mcp_server/stub.rs new file mode 100644 index 0000000000..9e9401d9b1 --- /dev/null +++ b/src/openhuman/mcp_server/stub.rs @@ -0,0 +1,85 @@ +//! Disabled-MCP facade for [`super`] (the OpenHuman-as-an-MCP-server surface). +//! +//! Compiled only when the `mcp` Cargo feature is OFF (see the gate in +//! [`super`]). It mirrors the subset of the real `mcp_server` public surface +//! that always-compiled callers reach, with disabled-error / empty bodies. +//! +//! [`super::tools::types`] is ungated, so [`McpToolSpec`] here is the real +//! type, not a mirrored copy — this module carries behaviour only. +//! +//! The signatures MUST match the real ones exactly (return types and +//! async-ness included). The disabled build +//! (`cargo check --no-default-features --features tokenjuice-treesitter`) is +//! the only thing that catches drift. + +use super::tools::McpToolSpec; + +/// Error text returned by every disabled-path operation that must yield a +/// `Result`. Shared so callers / log-greps see one stable string. +const DISABLED_MSG: &str = "mcp feature disabled at compile time"; + +// --------------------------------------------------------------------------- +// CLI entry point (mirrors `stdio::run_stdio_from_cli`) +// --------------------------------------------------------------------------- + +/// Fails with a build-fact diagnostic instead of serving MCP over stdio. +/// +/// This is deliberately a stub rather than a `#[cfg]` on the `"mcp"` match arm +/// in `src/core/cli.rs`. Deleting the arm is the naive move and is WRONG: the +/// `mcp` token 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 deliberate property of this build. Keeping the arm and failing +/// here means: +/// +/// * an MCP host (Claude Desktop, Cursor, …) that spawns `openhuman mcp` gets +/// a non-zero exit and a one-line stderr diagnostic naming the fix, instead +/// of hanging forever on an stdout stream that never speaks JSON-RPC; +/// * `cli.rs` needs no `#[cfg]` at all, so the gate stays invisible to the +/// transport layer. +/// +/// Banner suppression in `cli.rs` is a `matches!` on the raw string, so it +/// keeps working here without touching a gated symbol. +pub fn run_stdio_from_cli(_args: &[String]) -> anyhow::Result<()> { + anyhow::bail!( + "{DISABLED_MSG}: this build was compiled without the `mcp` feature, so the MCP stdio \ + server is unavailable. Rebuild with `--features mcp`." + ) +} + +// --------------------------------------------------------------------------- +// In-process local HTTP server (mirrors `local::{ensure_local_http, LocalMcpEndpoint}`) +// --------------------------------------------------------------------------- + +/// Address + bearer token of the in-process MCP HTTP server. +/// +/// Mirrors [`super::local::LocalMcpEndpoint`] (real build). No value of this +/// type is ever constructed here — [`ensure_local_http`] always errors — but +/// the type must stay nameable for call sites that bind its `Ok` variant. +#[derive(Debug, Clone)] +pub struct LocalMcpEndpoint { + pub addr: std::net::SocketAddr, + pub token: String, +} + +/// Always errors: there is no MCP server to stand up in this build. +/// +/// The sole always-on caller (`inference::provider::claude_code::driver`) +/// already handles the `Err` arm by logging "…CC running without OpenHuman MCP +/// tools" and continuing — so Claude Code still runs, just without our tool +/// surface injected. That call site needs no `#[cfg]`. +pub async fn ensure_local_http() -> anyhow::Result { + Err(anyhow::anyhow!(DISABLED_MSG)) +} + +// --------------------------------------------------------------------------- +// Tool catalog (mirrors `tools::tool_specs`) +// --------------------------------------------------------------------------- + +/// Empty catalog: this build advertises no tools over MCP. +/// +/// `tool_registry::ops::registry_entries` folds this into a `BTreeMap`, so an +/// empty vec simply means the registry gains no `mcp_stdio`-transport entries. +/// No `#[cfg]` needed at that call site. +pub fn tool_specs() -> Vec { + Vec::new() +} diff --git a/src/openhuman/mcp_server/tools/mod.rs b/src/openhuman/mcp_server/tools/mod.rs index da91c54dcc..3377ae216f 100644 --- a/src/openhuman/mcp_server/tools/mod.rs +++ b/src/openhuman/mcp_server/tools/mod.rs @@ -6,31 +6,48 @@ //! - `params` — argument parsing and RPC param construction //! - `dispatch` — `call_tool`, `list_tools_result`, agent/subagent handlers +//! ## Compile-time gate (`mcp` feature) +//! +//! `types` is ALWAYS compiled: [`McpToolSpec`] is inert data (`&'static str` + +//! `Value`, no deps beyond `serde_json`) that the always-compiled +//! `tool_registry` names. The behavioural siblings — which reach into the RPC +//! surface, security policy, and every gated domain — are gated. + +#[cfg(feature = "mcp")] mod dispatch; +#[cfg(feature = "mcp")] mod params; +#[cfg(feature = "mcp")] mod specs; + +// Inert type module — always compiled (see the module note above). mod types; // Public API consumed by the rest of `mcp_server` +#[cfg(feature = "mcp")] pub use dispatch::{call_tool, list_tools_result, tool_error, tool_success}; +#[cfg(feature = "mcp")] pub use specs::tool_specs; -pub use types::{McpToolSpec, ToolCallError}; +#[cfg(feature = "mcp")] +pub use types::ToolCallError; + +pub use types::McpToolSpec; // Re-exports needed by the companion test module via `use super::*`. // Guarded by `#[cfg(test)]` so they do not pollute the production namespace. -#[cfg(test)] +#[cfg(all(test, feature = "mcp"))] pub use crate::core::all; -#[cfg(test)] +#[cfg(all(test, feature = "mcp"))] pub use crate::openhuman::config::rpc as config_rpc; -#[cfg(test)] +#[cfg(all(test, feature = "mcp"))] pub use crate::openhuman::tools::SEARXNG_MAX_RESULTS; -#[cfg(test)] +#[cfg(all(test, feature = "mcp"))] pub use params::{build_rpc_params, slug_from}; -#[cfg(test)] +#[cfg(all(test, feature = "mcp"))] pub use serde_json::{json, Value}; -#[cfg(test)] +#[cfg(all(test, feature = "mcp"))] pub use types::{DEFAULT_LIMIT, MAX_LIMIT, TREE_TAG_MAX_TAGS, TREE_TAG_MAX_TAG_LENGTH}; -#[cfg(test)] +#[cfg(all(test, feature = "mcp"))] #[path = "../tools_tests.rs"] mod tests; From e5a52ba3cb4006d1311ce3a40305bea693d3872c Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Wed, 15 Jul 2026 18:08:53 +0530 Subject: [PATCH 06/16] =?UTF-8?q?feat(mcp=5Fclient):=20split-gate=20?= =?UTF-8?q?=E2=80=94=20keep=20mis-housed=20shared=20utilities=20compiled?= =?UTF-8?q?=20(#4799)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mcp_client is NOT gated wholesale, because the directory does not match the real dependency graph. Two submodules stay always compiled: sanitize — orchestrator/prompt.rs runs *skill* descriptions through sanitize_for_llm. Nothing to do with MCP; stubbing it would corrupt the orchestrator prompt in slim builds. client — the gitbooks docs tool dials McpHttpClient + redact_endpoint directly (GitBook is modelled as a legacy MCP server); stubbing it would break a docs tool users still reach. Only registry/stdio/spawn_env/setup_agent are gated. The gate follows the real dependency graph, not the directory name. Keeping client compiled also keeps McpUnauthorizedError — and so the McpServerNeedsAuth classifier coupling test — always compiled, with no #[cfg] and no wording-drift leak. --- src/openhuman/mcp_client/mod.rs | 38 ++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/src/openhuman/mcp_client/mod.rs b/src/openhuman/mcp_client/mod.rs index 5826b1bbd7..68adb5e12b 100644 --- a/src/openhuman/mcp_client/mod.rs +++ b/src/openhuman/mcp_client/mod.rs @@ -36,13 +36,46 @@ //! - `registry` — [`McpServerRegistry`] built from //! [`crate::openhuman::config::McpClientConfig`] +//! ## Compile-time gate (`mcp` feature) — SPLIT facade +//! +//! Unlike the sibling MCP modules, this one is NOT gated wholesale, because +//! the `mcp_client` directory does not match the real dependency graph. Two of +//! its submodules are **mis-housed shared utilities** with live consumers that +//! have nothing to do with the MCP subsystem, so they stay ALWAYS COMPILED: +//! +//! * [`sanitize`] — the orchestrator prompt builder runs *skill* descriptions +//! through `sanitize::sanitize_for_llm`. Nothing to do with MCP; stubbing it +//! would silently corrupt the orchestrator prompt in slim builds. +//! * [`client`] — the bespoke `gitbooks` docs tool dials [`McpHttpClient`] + +//! [`redact_endpoint`] directly (GitBook is modelled as a legacy MCP server). +//! Stubbing it would break a docs tool that users reach in slim builds. +//! Keeping it compiled also keeps [`McpUnauthorizedError`] — and therefore +//! the `McpServerNeedsAuth` classifier coupling test in +//! `core::observability` — always compiled, with no `#[cfg]` and no +//! wording-drift leak. +//! +//! Gated behind the default-ON `mcp` feature: [`registry`] (the static, +//! config-declared server set), `stdio`, `spawn_env`, and `setup_agent` — the +//! parts that genuinely constitute MCP-subsystem behaviour. +//! +//! In short: **the gate follows the real dependency graph, not the directory +//! name.** Relocating `sanitize` + `client` out of `mcp_client` is worthwhile +//! follow-up, but is deliberately out of scope here. +//! +//! There is no `stub` module: every gated item's consumers are themselves +//! gated, so nothing needs a disabled mirror. + mod client; +#[cfg(feature = "mcp")] mod registry; pub mod sanitize; +#[cfg(feature = "mcp")] pub mod setup_agent; -#[cfg(test)] +#[cfg(all(test, feature = "mcp"))] mod setup_agent_integration_test; +#[cfg(feature = "mcp")] mod spawn_env; +#[cfg(feature = "mcp")] mod stdio; pub use client::{ @@ -50,6 +83,9 @@ pub use client::{ McpHttpClient, McpInitializeResult, McpRemoteTool, McpServerToolResult, McpSseEvent, McpUnauthorizedError, ProtectedResourceMetadata, }; +#[cfg(feature = "mcp")] pub(crate) use registry::apply_safety_filter; +#[cfg(feature = "mcp")] pub use registry::{McpRegistrySource, McpServerDefinition, McpServerRegistry, McpTransportClient}; +#[cfg(feature = "mcp")] pub use stdio::McpStdioClient; From 7bd9cfe99ee6c6b73fc0923a52c40a5c0d7db1e8 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Wed, 15 Jul 2026 18:08:59 +0530 Subject: [PATCH 07/16] feat(tools): gate the ~19 MCP agent tools behind `mcp` (#4799) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three clusters, both halves of the (confusingly named) MCP surface: mcp_registry_* × 11 — DYNAMIC, sqlite-installed servers mcp_setup_* × 5 — setup-agent surface mcp_* × 3 — STATIC, config-declared servers (mcp_client) The naming is inverted from intuition, so gating only one half would leave the gate half-applied. gitbooks deliberately stays — it dials McpHttpClient but is a docs tool, not MCP-subsystem code. network/{mcp,mcp_setup}.rs are leaf-gated: their only consumers are the gated blocks above, so no stub is needed. --- src/openhuman/tools/impl/network/mod.rs | 8 +++ src/openhuman/tools/mod.rs | 1 + src/openhuman/tools/ops.rs | 66 +++++++++++++++++-------- 3 files changed, 54 insertions(+), 21 deletions(-) diff --git a/src/openhuman/tools/impl/network/mod.rs b/src/openhuman/tools/impl/network/mod.rs index 76db69598b..5d89269a4a 100644 --- a/src/openhuman/tools/impl/network/mod.rs +++ b/src/openhuman/tools/impl/network/mod.rs @@ -3,7 +3,13 @@ mod curl; mod gitbooks; mod gmail_unsubscribe; mod http_request; +// Leaf-gated: the only consumers of these two are the `#[cfg(feature = "mcp")]` +// blocks in `tools/ops.rs`, so no stub is needed — nothing names them when the +// feature is off. (`gitbooks` is deliberately NOT gated: it dials `McpHttpClient` +// but is a docs tool, not MCP-subsystem code. See `mcp_client`'s split facade.) +#[cfg(feature = "mcp")] mod mcp; +#[cfg(feature = "mcp")] mod mcp_setup; mod polymarket; mod polymarket_orders; @@ -14,7 +20,9 @@ pub use curl::CurlTool; pub use gitbooks::{GitbooksGetPageTool, GitbooksSearchTool}; pub use gmail_unsubscribe::GmailUnsubscribeTool; pub use http_request::HttpRequestTool; +#[cfg(feature = "mcp")] pub use mcp::{McpCallTool, McpListServersTool, McpListToolsTool}; +#[cfg(feature = "mcp")] pub use mcp_setup::{ McpSetupGetTool, McpSetupInstallAndConnectTool, McpSetupRequestSecretTool, McpSetupSearchTool, McpSetupTestConnectionTool, diff --git a/src/openhuman/tools/mod.rs b/src/openhuman/tools/mod.rs index 58aef6e4a6..af29ba6c2e 100644 --- a/src/openhuman/tools/mod.rs +++ b/src/openhuman/tools/mod.rs @@ -32,6 +32,7 @@ pub use crate::openhuman::flows::tools::*; pub use crate::openhuman::health::tools::*; pub use crate::openhuman::integrations::tools::*; pub use crate::openhuman::learning::tools::*; +#[cfg(feature = "mcp")] pub use crate::openhuman::mcp_registry::tools::*; pub use crate::openhuman::memory::tools::*; pub use crate::openhuman::memory_diff::tools::*; diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index 95fb2736bb..3aad647ea9 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -646,16 +646,30 @@ pub fn all_tools_with_runtime( Box::new(ScreenGlobeStopTool), Box::new(ScreenRequestPermissionsTool), Box::new(ScreenRequestPermissionTool), + // MCP registry (dynamic, user-installed servers) — compiled out with + // the `mcp` feature. Per-element attrs inside the `vec![]` mirror the + // voice idiom used earlier in this same literal. + #[cfg(feature = "mcp")] Box::new(McpRegistrySearchTool::new(config.clone())), + #[cfg(feature = "mcp")] Box::new(McpRegistryGetTool::new(config.clone())), + #[cfg(feature = "mcp")] Box::new(McpRegistryInstalledListTool::new(config.clone())), + #[cfg(feature = "mcp")] Box::new(McpRegistryStatusTool::new(config.clone())), + #[cfg(feature = "mcp")] Box::new(McpRegistryListToolsTool), + #[cfg(feature = "mcp")] Box::new(McpRegistryConnectTool::new(config.clone())), + #[cfg(feature = "mcp")] Box::new(McpRegistryDisconnectTool), + #[cfg(feature = "mcp")] Box::new(McpRegistryToolCallTool), + #[cfg(feature = "mcp")] Box::new(McpRegistryConfigAssistTool::new(config.clone())), + #[cfg(feature = "mcp")] Box::new(McpRegistryInstallTool::new(config.clone())), + #[cfg(feature = "mcp")] Box::new(McpRegistryUninstallTool::new(config.clone())), Box::new(WorkspaceReadPersonaTool::new(config.clone())), Box::new(WorkspaceUpdatePersonaTool::new(config.clone())), @@ -834,6 +848,8 @@ pub fn all_tools_with_runtime( // Registered unconditionally — the `mcp_setup` sub-agent filters to just // these via its `[tools] named = [...]` allowlist, and the host agent's // own tool list is wide enough that the extra five entries are negligible. + // Compiled out entirely with the `mcp` feature. + #[cfg(feature = "mcp")] { let cfg = Arc::new(root_config.clone()); tools.push(Box::new(McpSetupSearchTool::new(Arc::clone(&cfg)))); @@ -847,28 +863,36 @@ pub fn all_tools_with_runtime( // Generic remote MCP bridge tools. These let the agent enumerate // named MCP servers and forward `tools/call` through the core // instead of hardcoding one bespoke MCP integration per server. - let mcp_registry = { - let base = crate::openhuman::mcp_client::McpServerRegistry::from_config(root_config); - // Scope the MCP surface to the active profile's allowlist. `None` keeps - // every configured server; `Some(&[])` yields an empty registry. - match mcp_allowlist { - Some(allowed) => Arc::new(base.retaining_servers(allowed)), - None => Arc::new(base), + // + // Backed by the STATIC, config-declared server set (`[[mcp_client.servers]]` + // in TOML) — despite the local binding's name, this is NOT the dynamic + // `mcp_registry` domain gated above. Both are compiled out by the `mcp` + // feature; see the static-vs-dynamic note in AGENTS.md. + #[cfg(feature = "mcp")] + { + let mcp_registry = { + let base = crate::openhuman::mcp_client::McpServerRegistry::from_config(root_config); + // Scope the MCP surface to the active profile's allowlist. `None` keeps + // every configured server; `Some(&[])` yields an empty registry. + match mcp_allowlist { + Some(allowed) => Arc::new(base.retaining_servers(allowed)), + None => Arc::new(base), + } + }; + if !mcp_registry.is_empty() { + tools.push(Box::new(McpListServersTool::new(Arc::clone(&mcp_registry)))); + tools.push(Box::new(McpListToolsTool::new(Arc::clone(&mcp_registry)))); + tools.push(Box::new(McpCallTool::new( + Arc::clone(&mcp_registry), + security.clone(), + ))); + tracing::debug!( + count = mcp_registry.list().len(), + "[mcp_client] registered generic MCP bridge tools" + ); + } else { + tracing::debug!("[mcp_client] no MCP servers registered — bridge tools skipped"); } - }; - if !mcp_registry.is_empty() { - tools.push(Box::new(McpListServersTool::new(Arc::clone(&mcp_registry)))); - tools.push(Box::new(McpListToolsTool::new(Arc::clone(&mcp_registry)))); - tools.push(Box::new(McpCallTool::new( - Arc::clone(&mcp_registry), - security.clone(), - ))); - tracing::debug!( - count = mcp_registry.list().len(), - "[mcp_client] registered generic MCP bridge tools" - ); - } else { - tracing::debug!("[mcp_client] no MCP servers registered — bridge tools skipped"); } tools.extend(crate::openhuman::search::build_search_tools(root_config)); From b9657c156b4adfbd4c8cfc3e8e0c70a39ef7d4f0 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Wed, 15 Jul 2026 18:09:10 +0530 Subject: [PATCH 08/16] feat(agent_registry): gate mcp_agent, pin the dangling-subagent contract (#4799) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drops delegate_use_mcp_server from the orchestrator's synthesised belt. The orchestrator's agent.toml still lists mcp_agent in subagents — TOML is data, it cannot be #[cfg]'d, and forking it per-feature would invite exactly the drift this gate avoids. That dangling id is safe: collect_orchestrator_tools warns and skips unknown subagent ids, and validate_tier_hierarchy continues past them rather than failing the boot. Because the whole gate rests on that tolerance, pin it from both build configurations: orchestrator_tolerates_unresolvable_subagent_id and orchestrator_tolerates_absent_mcp_agent. A future "unknown subagent is a hard error" change now fails loudly here instead of silently breaking the slim build's boot — a failure mode CI's cargo check lane cannot catch. --- src/openhuman/agent_registry/agents/loader.rs | 83 +++++++++++++++++++ src/openhuman/agent_registry/agents/mod.rs | 1 + 2 files changed, 84 insertions(+) diff --git a/src/openhuman/agent_registry/agents/loader.rs b/src/openhuman/agent_registry/agents/loader.rs index bcaa34a0cf..a409586845 100644 --- a/src/openhuman/agent_registry/agents/loader.rs +++ b/src/openhuman/agent_registry/agents/loader.rs @@ -257,6 +257,20 @@ pub const BUILTINS: &[BuiltinAgent] = &[ prompt_fn: super::mcp_setup::prompt::build, graph_fn: None, }, + // Connected-server execution specialist. Compiled out with the `mcp` + // feature, which drops the `delegate_use_mcp_server` tool from the + // orchestrator's synthesised belt. + // + // The orchestrator's `agent.toml` still lists `mcp_agent` in `subagents` + // (TOML is data — it cannot be `cfg`'d, and forking it per-feature would + // invite exactly the data drift this gate is meant to avoid). That + // dangling reference is SAFE and already handled: `collect_orchestrator_tools` + // logs a warn and skips subagent ids that are not in the registry, and + // `validate_tier_hierarchy` explicitly `continue`s past unknown ids rather + // than failing the boot. `orchestrator_tolerates_absent_mcp_agent` in the + // test module below pins that contract so a future "strict unknown + // subagent" change cannot silently break the slim build's boot. + #[cfg(feature = "mcp")] BuiltinAgent { id: "mcp_agent", toml: include_str!("mcp_agent/agent.toml"), @@ -1843,6 +1857,71 @@ mod tests { ); } + /// The `mcp` gate's load-bearing safety contract (#4799). + /// + /// `agent.toml` is DATA — it cannot be `#[cfg]`'d, so the orchestrator goes + /// on listing `mcp_agent` in `subagents` even in builds where the `mcp` + /// feature dropped `mcp_agent` from [`BUILTINS`]. That leaves a subagent id + /// that resolves to nothing, and the whole gate rests on the loader + /// TOLERATING it rather than failing the boot. + /// + /// Two independent sites provide that tolerance today: + /// * `orchestrator_tools::collect_orchestrator_tools` warns + skips + /// subagent ids absent from the registry; + /// * [`validate_tier_hierarchy`] `continue`s past unknown ids instead of + /// reporting a tier error. + /// + /// This test pins the second one (the boot-blocking one) from BOTH build + /// configurations, so a future "unknown subagent ids are a hard error" + /// change fails here loudly instead of silently breaking the slim build's + /// boot — the failure mode would otherwise only appear in a + /// `--no-default-features` run, which CI's `cargo check` lane cannot catch. + #[test] + fn orchestrator_tolerates_unresolvable_subagent_id() { + let mut def = find("orchestrator"); + def.subagents.push(SubagentEntry::AgentId( + "definitely_not_a_compiled_in_agent".into(), + )); + + validate_tier_hierarchy(&[def]).expect( + "validate_tier_hierarchy must tolerate an unresolvable subagent id — the `mcp` \ + feature gate relies on it (orchestrator's agent.toml lists `mcp_agent` even in \ + builds that compile `mcp_agent` out)", + ); + } + + /// Companion to the above, asserting the real gated shape rather than a + /// synthetic id: with `mcp` compiled out, `mcp_agent` is genuinely absent + /// from the loaded set while the orchestrator still lists it — and + /// `load_builtins` (which runs `validate_tier_hierarchy` internally) must + /// still succeed, i.e. the core boots. + #[test] + #[cfg(not(feature = "mcp"))] + fn orchestrator_tolerates_absent_mcp_agent() { + let defs = load_builtins().expect( + "load_builtins must succeed with `mcp` compiled out — the orchestrator's dangling \ + `mcp_agent` subagent reference must not fail the boot", + ); + + assert!( + !defs.iter().any(|d| d.id == "mcp_agent"), + "`mcp_agent` must be compiled out when the `mcp` feature is off" + ); + + let orchestrator = defs + .iter() + .find(|d| d.id == "orchestrator") + .expect("orchestrator must still load"); + assert!( + orchestrator.subagents.iter().any(|e| matches!( + e, + SubagentEntry::AgentId(id) if id == "mcp_agent" + )), + "orchestrator.agent.toml is data and still lists `mcp_agent` — this dangling \ + reference is exactly what the loader must tolerate" + ); + } + /// The orchestrator gets lightweight MCP discovery (`mcp_registry_status`, /// like `composio_list_connections`) but must NOT carry the per-server /// enumerate/execute tools — those belong to `mcp_agent`, keeping the @@ -1873,7 +1952,11 @@ mod tests { /// the discover + call surface and a stable `use_mcp_server` delegate name, /// but must NOT hold the secret-handling install/uninstall tools (those are /// `mcp_setup`'s) or any shell/file/network capability. + /// + /// Gated: `find` panics on a missing id, and the `mcp` feature drops + /// `mcp_agent` from [`BUILTINS`] entirely. #[test] + #[cfg(feature = "mcp")] fn mcp_agent_drives_connected_servers_without_install_or_shell() { let def = find("mcp_agent"); assert_eq!(def.agent_tier, AgentTier::Worker); diff --git a/src/openhuman/agent_registry/agents/mod.rs b/src/openhuman/agent_registry/agents/mod.rs index aa30640cbc..4d584f1e0f 100644 --- a/src/openhuman/agent_registry/agents/mod.rs +++ b/src/openhuman/agent_registry/agents/mod.rs @@ -16,6 +16,7 @@ pub mod help; pub mod image_agent; pub mod integrations_agent; pub mod markets_agent; +#[cfg(feature = "mcp")] pub mod mcp_agent; pub mod mcp_setup; pub mod morning_briefing; From 8ee2825800b4353cef6ebfdce8e0a22118f58c8f Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Wed, 15 Jul 2026 18:09:19 +0530 Subject: [PATCH 09/16] test(mcp): pin the gate in both directions + the CLI build-fact error (#4799) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit all_tests: mcp_clients + mcp_audit registered when the gate is on, absent when off. An assert that only ever runs in one build configuration cannot prove a gate works. cli_tests: `openhuman mcp` must fail naming the BUILD as the cause and must NOT degrade into "unknown namespace" — the exact confusing failure the issue forbids. Covers the mcp-server alias too. These only run under --no-default-features, which CI never test-runs. --- src/core/all_tests.rs | 55 +++++++++++++++++++++++++++++++++++++++++++ src/core/cli_tests.rs | 55 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) diff --git a/src/core/all_tests.rs b/src/core/all_tests.rs index 08412d4d26..d1c6d3a6fb 100644 --- a/src/core/all_tests.rs +++ b/src/core/all_tests.rs @@ -253,6 +253,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!( @@ -864,5 +865,59 @@ fn group_mapping_smoke() { assert_eq!(group_for_namespace("wallet"), Some(DomainGroup::Web3)); 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" + ); +} diff --git a/src/core/cli_tests.rs b/src/core/cli_tests.rs index 1dcf86e21c..adf6d7c302 100644 --- a/src/core/cli_tests.rs +++ b/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`" + ); +} From 7bf78de8dd0dba0bb552bc91c817aac611c38a7a Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Wed, 15 Jul 2026 18:09:19 +0530 Subject: [PATCH 10/16] test(mcp): keep non-MCP coverage alive in slim builds (#4799) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rather than gating whole tests away and losing their non-MCP coverage, gate only the MCP-specific assertions: - desktop tool tables: per-element #[cfg] on the mcp_registry_* rows - tool_registry: MCP-transport half gated, controller half keeps asserting; trim/list tests fall back to a controller-transport id - diagnostics: mcp_stdio_tools > 0 when on, == 0 when off Adds all_tools_omits_mcp_tools_when_gate_off, which asserts by PREFIX rather than naming the ~19 tools — a new MCP tool added later must not be able to leak into slim builds just because nobody extended a hardcoded list. --- src/openhuman/tool_registry/ops_tests.rs | 49 +++++++++++++---- src/openhuman/tool_registry/schemas.rs | 12 ++++- src/openhuman/tools/ops_tests.rs | 68 ++++++++++++++++++++++++ 3 files changed, 119 insertions(+), 10 deletions(-) diff --git a/src/openhuman/tool_registry/ops_tests.rs b/src/openhuman/tool_registry/ops_tests.rs index be2b96da08..6287a527cd 100644 --- a/src/openhuman/tool_registry/ops_tests.rs +++ b/src/openhuman/tool_registry/ops_tests.rs @@ -8,13 +8,30 @@ use crate::openhuman::config::schema::{ fn registry_entries_include_mcp_and_controller_tools() { let entries = registry_entries(); - let memory_search = entries - .iter() - .find(|entry| entry.tool_id == "memory.search") - .expect("memory.search mcp tool"); - assert_eq!(memory_search.transport, ToolRegistryTransport::McpStdio); - assert_eq!(memory_search.route["method"], json!("tools/call")); - assert_eq!(memory_search.health, ToolRegistryHealth::Available); + // The MCP-transport half of the inventory is sourced from + // `mcp_server::tool_specs()`, which the `mcp` feature compiles out (the + // stub returns an empty catalog). Only this half is gated — the + // controller half below must keep its coverage in BOTH builds. + #[cfg(feature = "mcp")] + { + let memory_search = entries + .iter() + .find(|entry| entry.tool_id == "memory.search") + .expect("memory.search mcp tool"); + assert_eq!(memory_search.transport, ToolRegistryTransport::McpStdio); + assert_eq!(memory_search.route["method"], json!("tools/call")); + assert_eq!(memory_search.health, ToolRegistryHealth::Available); + } + + // With `mcp` compiled out the registry must contain NO MCP-transport + // entries at all — the inventory degrades to controller tools only. + #[cfg(not(feature = "mcp"))] + assert!( + !entries + .iter() + .any(|entry| entry.transport == ToolRegistryTransport::McpStdio), + "no MCP-transport tools may be registered when the `mcp` feature is off" + ); let web_search = entries .iter() @@ -48,7 +65,12 @@ fn diagnostics_reports_inventory_and_policy_surfaces() { assert!(outcome.value.total_tools > 0); assert_eq!(outcome.value.total_tools, outcome.value.enabled_tools); + // MCP-transport tools only exist when the `mcp` feature is compiled in; + // with it off the count is legitimately zero (see `mcp_server::stub`). + #[cfg(feature = "mcp")] assert!(outcome.value.mcp_stdio_tools > 0); + #[cfg(not(feature = "mcp"))] + assert_eq!(outcome.value.mcp_stdio_tools, 0); assert!(outcome.value.json_rpc_tools > 0); assert!(outcome .value @@ -207,8 +229,17 @@ fn insert_registry_entry_skips_duplicate_tool_id() { #[test] fn get_tool_trims_and_returns_exact_entry() { - let outcome = get_tool(" memory.search ").expect("registry lookup"); - assert_eq!(outcome.value.tool_id, "memory.search"); + // `memory.search` is an MCP-transport entry, so it is absent when the `mcp` + // feature is compiled out. The behaviour under test here is id *trimming*, + // not MCP — so fall back to a controller-transport entry rather than gating + // the whole test away and losing that coverage in slim builds. + #[cfg(feature = "mcp")] + let tool_id = "memory.search"; + #[cfg(not(feature = "mcp"))] + let tool_id = "tools.web_search"; + + let outcome = get_tool(&format!(" {tool_id} ")).expect("registry lookup"); + assert_eq!(outcome.value.tool_id, tool_id); } #[test] diff --git a/src/openhuman/tool_registry/schemas.rs b/src/openhuman/tool_registry/schemas.rs index ca90d57505..a8fcae6f09 100644 --- a/src/openhuman/tool_registry/schemas.rs +++ b/src/openhuman/tool_registry/schemas.rs @@ -192,9 +192,19 @@ mod tests { .get("tools") .and_then(Value::as_array) .expect("tools array"); + + // `memory.search` is an MCP-transport entry, absent when the `mcp` + // feature is compiled out. The behaviour under test is that `list` + // returns a populated registry object, so assert against an entry that + // exists in the build at hand rather than gating the test away. + #[cfg(feature = "mcp")] + let expected = "memory.search"; + #[cfg(not(feature = "mcp"))] + let expected = "tools.web_search"; + assert!(tools .iter() - .any(|tool| { tool.get("tool_id").and_then(Value::as_str) == Some("memory.search") })); + .any(|tool| { tool.get("tool_id").and_then(Value::as_str) == Some(expected) })); } #[tokio::test] diff --git a/src/openhuman/tools/ops_tests.rs b/src/openhuman/tools/ops_tests.rs index 3e1e0d5b7e..dd1a033c45 100644 --- a/src/openhuman/tools/ops_tests.rs +++ b/src/openhuman/tools/ops_tests.rs @@ -298,6 +298,11 @@ fn all_tools_registers_gitbooks_when_enabled() { } #[test] +// Wholly about the static MCP bridge surface, which the `mcp` feature compiles +// out — no meaningful residue to assert in the disabled build (the +// "no MCP tools registered" direction is covered by +// `all_tools_omits_mcp_tools_when_gate_off` below). +#[cfg(feature = "mcp")] fn all_tools_registers_generic_mcp_bridge_tools_when_servers_exist() { let tmp = TempDir::new().unwrap(); let mut cfg = test_config(&tmp); @@ -327,6 +332,50 @@ fn all_tools_registers_generic_mcp_bridge_tools_when_servers_exist() { ); } +/// The disabled direction of the `mcp` gate (#4799): even with MCP servers +/// declared in config, a build without the `mcp` feature registers NO MCP tool +/// of any family — neither the static bridge (`mcp_*`), the dynamic registry +/// (`mcp_registry_*`), nor the setup-agent surface (`mcp_setup_*`). +/// +/// Deliberately asserts by prefix rather than naming the ~19 tools: a new MCP +/// tool added later must not be able to leak into slim builds just because +/// nobody remembered to extend a hardcoded list here. +#[test] +#[cfg(not(feature = "mcp"))] +fn all_tools_omits_mcp_tools_when_gate_off() { + let tmp = TempDir::new().unwrap(); + let mut cfg = test_config(&tmp); + cfg.gitbooks.enabled = false; + cfg.mcp_client + .servers + .push(crate::openhuman::config::McpServerConfig { + name: "docs".into(), + endpoint: "https://example.com/mcp".into(), + command: String::new(), + args: Vec::new(), + env: std::collections::HashMap::new(), + cwd: None, + description: Some("Example docs MCP".into()), + enabled: true, + allowed_tools: Vec::new(), + disallowed_tools: Vec::new(), + timeout_secs: 30, + auth: crate::openhuman::config::McpAuthConfig::None, + }); + + let names = tool_names(&integration_tools_for_config(&tmp, &cfg)); + let leaked: Vec<&String> = names + .iter() + .filter(|n| n.starts_with("mcp_") || n.starts_with("mcp_registry_")) + .collect(); + + assert!( + leaked.is_empty(), + "no MCP tool may be registered when the `mcp` feature is compiled out, \ + even with `[[mcp_client.servers]]` declared in config; leaked: {leaked:?}" + ); +} + #[test] fn all_tools_skips_gitbooks_when_disabled() { let tmp = TempDir::new().unwrap(); @@ -2092,15 +2141,29 @@ const DESKTOP_TOOLS: &[&str] = &[ "screen_intelligence_globe_listener_stop", "screen_intelligence_request_permissions", "screen_intelligence_request_permission", + // The `mcp_registry_*` desktop surface is compiled out with the `mcp` + // feature, so these expectations are gated per-element rather than gating + // the three tests below away wholesale — the non-MCP desktop tools must + // keep their coverage in both builds. + #[cfg(feature = "mcp")] "mcp_registry_search", + #[cfg(feature = "mcp")] "mcp_registry_get", + #[cfg(feature = "mcp")] "mcp_registry_installed_list", + #[cfg(feature = "mcp")] "mcp_registry_status", + #[cfg(feature = "mcp")] "mcp_registry_connect", + #[cfg(feature = "mcp")] "mcp_registry_disconnect", + #[cfg(feature = "mcp")] "mcp_registry_tool_call", + #[cfg(feature = "mcp")] "mcp_registry_config_assist", + #[cfg(feature = "mcp")] "mcp_registry_install", + #[cfg(feature = "mcp")] "mcp_registry_uninstall", "workspace_read_persona", "workspace_update_persona", @@ -2111,7 +2174,9 @@ const DESKTOP_TOOLS: &[&str] = &[ const DESKTOP_DEFAULT_OFF: &[&str] = &[ "screen_intelligence_request_permissions", "screen_intelligence_request_permission", + #[cfg(feature = "mcp")] "mcp_registry_install", + #[cfg(feature = "mcp")] "mcp_registry_uninstall", "workspace_update_persona", "workspace_reset_persona", @@ -2121,8 +2186,11 @@ const DESKTOP_DEFAULT_OFF: &[&str] = &[ const DESKTOP_ALWAYS_ON: &[&str] = &[ "screen_intelligence_status", "screen_intelligence_capture_now", + #[cfg(feature = "mcp")] "mcp_registry_search", + #[cfg(feature = "mcp")] "mcp_registry_tool_call", + #[cfg(feature = "mcp")] "mcp_registry_connect", "workspace_read_persona", ]; From ff003a7e7cb3022d43fb2c4449e077b010881f0e Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Wed, 15 Jul 2026 18:09:27 +0530 Subject: [PATCH 11/16] test(mcp): tolerate gate-compiled-out methods in data-vs-registry checks (#4799) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit legacy_aliases: the frontend RPC catalog and the alias table are DATA — they are authored against the full shipped surface and are built independently of the core's Cargo features, so in a slim build they legitimately name openhuman.mcp_clients_* methods whose controllers no longer exist. Ignore exactly the gated namespaces and keep asserting on everything else, so the drift signal survives instead of tempting someone to delete live frontend methods from the catalog. Same data-vs-code shape as the orchestrator TOML. definition_tests: gate the mcp_agent max-iterations row. mcp_setup stays — only its tools are gated, the agent definition still loads in both builds. tool_prep: use_mcp_server is mcp_agent's delegate_name; setup_mcp_server is mcp_setup's and stays. --- src/core/legacy_aliases.rs | 29 +++++++++++++++++++ .../agent/harness/definition_tests.rs | 4 +++ .../harness/subagent_runner/tool_prep.rs | 5 ++++ 3 files changed, 38 insertions(+) diff --git a/src/core/legacy_aliases.rs b/src/core/legacy_aliases.rs index 20574aff48..f08bf5850d 100644 --- a/src/core/legacy_aliases.rs +++ b/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(); diff --git a/src/openhuman/agent/harness/definition_tests.rs b/src/openhuman/agent/harness/definition_tests.rs index ed905d26dc..790dd421cc 100644 --- a/src/openhuman/agent/harness/definition_tests.rs +++ b/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), diff --git a/src/openhuman/agent/harness/subagent_runner/tool_prep.rs b/src/openhuman/agent/harness/subagent_runner/tool_prep.rs index 8e1602711a..e0f5309558 100644 --- a/src/openhuman/agent/harness/subagent_runner/tool_prep.rs +++ b/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", ] { From f2f3a4059381be7681e3600d82b6f9e0393c523d Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Wed, 15 Jul 2026 18:09:35 +0530 Subject: [PATCH 12/16] build(tauri): enable the `mcp` feature for the desktop shell (#4799) default-features = false means the shell must opt in explicitly or the shipped app silently loses MCP. Multi-line array so sibling gates append cleanly. --- app/src-tauri/Cargo.toml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/src-tauri/Cargo.toml b/app/src-tauri/Cargo.toml index 177210c7e1..88478b3cc9 100644 --- a/app/src-tauri/Cargo.toml +++ b/app/src-tauri/Cargo.toml @@ -132,7 +132,9 @@ cef = { version = "=146.4.1", default-features = false } # by tying the core's lifetime to the GUI process. The existing port-7788 # probe in `core_process::ensure_running` still attaches to a running # `openhuman-core run` harness when one is already listening. -openhuman_core = { path = "../..", package = "openhuman", default-features = false } +openhuman_core = { path = "../..", package = "openhuman", default-features = false, features = [ + "mcp", +] } tinyjuice = { version = "0.2.1", default-features = false } [target.'cfg(unix)'.dependencies] From a74f15ad79a098485c6470c25ba1e41f2b7d5222 Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Wed, 15 Jul 2026 18:09:35 +0530 Subject: [PATCH 13/16] docs(agents): document the `mcp` gate, its scope correction and inversions (#4799) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the three things most likely to be got wrong later: - the gate drops ZERO dependencies (there is no MCP SDK; the DoD line claiming it sheds one is superseded), so mcp = [] is empty on purpose; - static vs dynamic is INVERTED from intuition — mcp_client is the STATIC config-declared set, mcp_registry the DYNAMIC installed one; gating only one leaves the gate half-applied; - mcp_client is split-gated because the gate follows the real dependency graph, not the directory name. Plus the type carve-out, why cli.rs stays untouched, and why the orchestrator TOML's dangling mcp_agent reference is expected and safe. --- AGENTS.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 03ecc7e77c..8ba3273ae5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -233,11 +233,34 @@ GGML_NATIVE=OFF cargo check --manifest-path Cargo.toml \ | Feature | Default | Gates | Drops deps | | ------- | ------- | ----- | ---------- | | `voice` | ON | `openhuman::voice` + `openhuman::audio_toolkit` domains — STT/TTS providers, dictation server, always-on listening, podcast audio + email | `hound`, `lettre` | +| `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. **Scope note:** the `voice` gate does **not** drop `whisper-rs` / `llama` / `cpal`. Those live in the inference domain (`src/openhuman/inference/local/service/whisper_engine.rs`; `cpal` is shared with accessibility) and await a separate future `inference` gate. The issue-level DoD line claiming whisper is dropped is superseded by this scope correction. +#### 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 sanitises **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: `grep -i mcp Cargo.toml` matches only the `test-mcp-stub` 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. From 64eca0d3c2f8f81f0ab873a2238293d07831957d Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Wed, 15 Jul 2026 20:35:19 +0530 Subject: [PATCH 14/16] docs(mcp): fix sanitize spelling variant and stale grep claim (#4799) - Use the American "sanitizes" consistently (the file already uses "sanitize" 4x); the lone British variant tripped LanguageTool. - The "grep -i mcp Cargo.toml matches only test-mcp-stub" claim went stale in this PR: the new mcp = [] feature definition, the default list and the gate comments all match it now. Scope the statement to dependency declarations, which is what it was actually asserting. --- AGENTS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b587e6c00b..bd6c4852e1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -246,9 +246,9 @@ GGML_NATIVE=OFF cargo check --manifest-path Cargo.toml \ 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 sanitises **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. +- **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: `grep -i mcp Cargo.toml` matches only the `test-mcp-stub` 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. +**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: From cfdb11b391efeb4ea0d8cf9c56e914aaa2ee564e Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Thu, 16 Jul 2026 13:59:16 +0530 Subject: [PATCH 15/16] test(mcp): gate MCP-only integration tests + instrument disabled stubs Maintainer review follow-ups for the mcp compile-time gate (#4799): - Add `#![cfg(feature = "mcp")]` to the four MCP integration test files (mcp_registry_e2e, mcp_registry_multi_server, mcp_setup_e2e, mcp_stdio_integration). They import gated APIs (mcp_registry::{store, boot,ops,connections,setup}, mcp_client::McpStdioClient) unconditionally and are auto-discovered, so `cargo test --no-default-features --features tokenjuice-treesitter --tests` failed to compile in slim builds. Gating the whole file compiles them to nothing when mcp is off. - Add debug/warn diagnostics to the mcp_audit and mcp_server disabled stubs, matching the sibling mcp_registry stub and the repo's Rust-flow observability rule (no secrets/PII logged). --- src/openhuman/mcp_audit/stub.rs | 3 +++ src/openhuman/mcp_server/stub.rs | 5 +++++ tests/mcp_registry_e2e.rs | 6 ++++++ tests/mcp_registry_multi_server.rs | 6 ++++++ tests/mcp_setup_e2e.rs | 7 +++++++ tests/mcp_stdio_integration.rs | 6 ++++++ 6 files changed, 33 insertions(+) diff --git a/src/openhuman/mcp_audit/stub.rs b/src/openhuman/mcp_audit/stub.rs index f8e03535ba..be12e11c69 100644 --- a/src/openhuman/mcp_audit/stub.rs +++ b/src/openhuman/mcp_audit/stub.rs @@ -27,6 +27,7 @@ use super::types::{McpWriteListQuery, McpWriteRecord, NewMcpWriteRecord}; /// `src/core/all.rs` pushes this straight into its internal-controller vec /// with no `#[cfg]` of its own — the empty vec keeps that file untouched. pub fn all_mcp_audit_internal_controllers() -> Vec { + log::debug!("[mcp_audit] {DISABLED_MSG} — no internal controllers registered"); Vec::new() } @@ -40,11 +41,13 @@ pub fn all_mcp_audit_internal_controllers() -> Vec Result { + log::debug!("[mcp_audit] {DISABLED_MSG} — record_write is a no-op (no MCP writer exists)"); Ok(0) } /// Empty history: no MCP writes can have occurred in this build. `Ok(vec![])` /// (not `Err`) keeps the shape honest — the query succeeded, the log is empty. pub fn list_writes(_config: &Config, _query: &McpWriteListQuery) -> Result> { + log::debug!("[mcp_audit] {DISABLED_MSG} — list_writes returning empty history"); Ok(Vec::new()) } diff --git a/src/openhuman/mcp_server/stub.rs b/src/openhuman/mcp_server/stub.rs index 9e9401d9b1..364ab94787 100644 --- a/src/openhuman/mcp_server/stub.rs +++ b/src/openhuman/mcp_server/stub.rs @@ -40,6 +40,9 @@ const DISABLED_MSG: &str = "mcp feature disabled at compile time"; /// Banner suppression in `cli.rs` is a `matches!` on the raw string, so it /// keeps working here without touching a gated symbol. pub fn run_stdio_from_cli(_args: &[String]) -> anyhow::Result<()> { + log::warn!( + "[mcp_server] {DISABLED_MSG} — `openhuman mcp` rejected; rebuild with `--features mcp`" + ); anyhow::bail!( "{DISABLED_MSG}: this build was compiled without the `mcp` feature, so the MCP stdio \ server is unavailable. Rebuild with `--features mcp`." @@ -68,6 +71,7 @@ pub struct LocalMcpEndpoint { /// tools" and continuing — so Claude Code still runs, just without our tool /// surface injected. That call site needs no `#[cfg]`. pub async fn ensure_local_http() -> anyhow::Result { + log::debug!("[mcp_server] {DISABLED_MSG} — local HTTP endpoint not stood up"); Err(anyhow::anyhow!(DISABLED_MSG)) } @@ -81,5 +85,6 @@ pub async fn ensure_local_http() -> anyhow::Result { /// empty vec simply means the registry gains no `mcp_stdio`-transport entries. /// No `#[cfg]` needed at that call site. pub fn tool_specs() -> Vec { + log::debug!("[mcp_server] {DISABLED_MSG} — advertising empty MCP tool catalog"); Vec::new() } diff --git a/tests/mcp_registry_e2e.rs b/tests/mcp_registry_e2e.rs index 4e8d2dd8de..e5aaa2b228 100644 --- a/tests/mcp_registry_e2e.rs +++ b/tests/mcp_registry_e2e.rs @@ -7,6 +7,12 @@ //! → `connections::disconnect` round-trips correctly through the unified //! `mcp_client::McpStdioClient` transport. +// Exercises the gated `mcp_registry` / `mcp_client` surface, so the whole suite +// is compiled only when the `mcp` feature is on. Without this gate the slim +// build's `cargo test --no-default-features --features tokenjuice-treesitter +// --tests` fails to compile against the removed APIs (#4799). +#![cfg(feature = "mcp")] + use openhuman_core::openhuman::config::Config; use openhuman_core::openhuman::mcp_registry::connections; use openhuman_core::openhuman::mcp_registry::store; diff --git a/tests/mcp_registry_multi_server.rs b/tests/mcp_registry_multi_server.rs index 129e6f4821..5bba039c52 100644 --- a/tests/mcp_registry_multi_server.rs +++ b/tests/mcp_registry_multi_server.rs @@ -12,6 +12,12 @@ //! temp-dir workspace so they do not share SQLite state with each other //! or with `mcp_registry_e2e.rs`. +// Exercises the gated `mcp_registry` surface, so the whole suite is compiled +// only when the `mcp` feature is on — otherwise the slim build's +// `cargo test --no-default-features --features tokenjuice-treesitter --tests` +// fails to compile against the removed APIs (#4799). +#![cfg(feature = "mcp")] + use openhuman_core::openhuman::config::Config; use openhuman_core::openhuman::mcp_registry::types::{CommandKind, InstalledServer, Transport}; use openhuman_core::openhuman::mcp_registry::{connections, ops, store}; diff --git a/tests/mcp_setup_e2e.rs b/tests/mcp_setup_e2e.rs index b2e34f8985..caf4d34427 100644 --- a/tests/mcp_setup_e2e.rs +++ b/tests/mcp_setup_e2e.rs @@ -7,6 +7,13 @@ //! `registry::registry_get`. The transport itself is the same //! `test-mcp-stub` binary used by `mcp_registry_e2e.rs`. +// Exercises the gated `mcp_registry::setup` + `mcp_client` surface, so the +// whole suite is compiled only when the `mcp` feature is on — otherwise the +// slim build's `cargo test --no-default-features --features +// tokenjuice-treesitter --tests` fails to compile against the removed APIs +// (#4799). +#![cfg(feature = "mcp")] + use std::collections::HashMap; use std::time::Duration; diff --git a/tests/mcp_stdio_integration.rs b/tests/mcp_stdio_integration.rs index 684a289757..742bb2800d 100644 --- a/tests/mcp_stdio_integration.rs +++ b/tests/mcp_stdio_integration.rs @@ -4,6 +4,12 @@ //! the test graph and exposes it through `CARGO_BIN_EXE_openhuman-core`. Running //! a nested `cargo build` from a lib unit test is prone to CI disk exhaustion. +// Exercises the gated `mcp_client::McpStdioClient` transport, so the whole +// suite is compiled only when the `mcp` feature is on — otherwise the slim +// build's `cargo test --no-default-features --features tokenjuice-treesitter +// --tests` fails to compile against the removed API (#4799). +#![cfg(feature = "mcp")] + use openhuman_core::openhuman::config::McpClientIdentityConfig; use openhuman_core::openhuman::mcp_client::McpStdioClient; use std::path::PathBuf; From 6ecb0951952b424134550501c77ba261c6e7780f Mon Sep 17 00:00:00 2001 From: oxoxDev Date: Thu, 16 Jul 2026 14:53:34 +0530 Subject: [PATCH 16/16] fix(mcp_audit): define DISABLED_MSG in the disabled-MCP stub --- src/openhuman/mcp_audit/stub.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/openhuman/mcp_audit/stub.rs b/src/openhuman/mcp_audit/stub.rs index be12e11c69..f2103cbfbe 100644 --- a/src/openhuman/mcp_audit/stub.rs +++ b/src/openhuman/mcp_audit/stub.rs @@ -17,6 +17,11 @@ use crate::openhuman::config::Config; use super::types::{McpWriteListQuery, McpWriteRecord, NewMcpWriteRecord}; +/// Error/log text shared by every disabled-path operation. Owned locally +/// (mirroring the sibling `mcp_registry` / `mcp_server` stubs) so the module +/// stays self-contained rather than importing a private const across modules. +const DISABLED_MSG: &str = "mcp feature disabled at compile time"; + // --------------------------------------------------------------------------- // Controller registration (mirrors `schemas::all_internal_controllers`) // ---------------------------------------------------------------------------