feat(mcp): advertise tool annotations on tools/list

## Summary Adds MCP `ToolAnnotations` (spec 2025-03-26+) to every tool advertised by `openhuman-core mcp`. Clients use `readOnlyHint` / `destructiveHint` / `idempotentHint` / `openWorldHint` to surface accurate safety affordances — e.g. Claude Desktop's "this tool can take destructive actions" confirmation gate. ## Problem `tools/list` today returns the curated tool surface but says nothing about each tool's behavior class. The MCP spec defines `ToolAnnotations` precisely for this — without them: - Claude Desktop / Cursor can't render confirmation gates for destructive actions. - Humans inspecting the tool list (e.g. via MCP Inspector) can't see at a glance which tools are safe to retry. - `agent.run_subagent` — the one Act-policy surface on the MCP server today — looks identical to `memory.search` to clients, even though it can call further tools (including potentially destructive ones) through any sub-agent it spawns. ## Solution - Added `annotations: Value` to `McpToolSpec` (always present, serialized into each tool's `annotations` field in the `tools/list` response). - All read-only tools share a `read_only_local_annotations()` helper (`readOnlyHint: true`, `openWorldHint: false`). - `agent.run_subagent` gets explicit annotations: `readOnlyHint: false / destructiveHint: true / idempotentHint: false / openWorldHint: true`. Sub-agent execution can call further tools, isn't a no-op on repeat, and reaches into the broader OpenHuman environment. - Updated `mcp_server/mod.rs` docstring to no longer claim the surface is purely read-only — it's curated, with `agent.run_subagent` as the one Act-policy surface. - Per spec, `destructiveHint` / `idempotentHint` are only meaningful when `readOnlyHint == false`, so the read-only helper omits them. ## Submission Checklist - [x] Tests added — `list_tools_emits_annotations_for_every_tool` asserts every entry serializes a non-null `annotations` object; a second test pins the read-only / destructive split to each tool's actual behavior. `cargo test --lib mcp_server::` runs 38/38 locally. - [x] **Diff coverage ≥ 80%** — new code is mostly literal annotations + a 6-line helper; both the "every tool has annotations" and "read-only vs destructive" axes are covered. - [x] Coverage matrix updated — `N/A: extends row 11.1.4 (MCP stdio server) rather than adding a new feature.` - [x] All affected feature IDs listed under `## Related` - [x] No new external network dependencies introduced - [x] Manual smoke checklist — `N/A: extends existing MCP surface, not a release-cut path.` - [x] Linked issue closed via `Closes #NNN` — `N/A: capability extension, no issue tracking this.` ## Impact - **Runtime**: `openhuman-core mcp` only. No HTTP RPC, web, or mobile impact. - **Compatibility**: Backward compatible — older MCP clients that don't read `annotations` ignore the field; newer clients pick up the safety affordances. - **Performance**: Negligible — one extra static `Value` per tool, serialized on each `tools/list` call (low frequency). - **Security**: No policy change. `agent.run_subagent` still goes through `enforce_act_policy`; the annotation is an advertised hint, not an enforcement point. ## Related - Feature IDs: `11.1.4` (MCP stdio server) - Spec: https://modelcontextprotocol.io/specification/2025-03-26/server/tools/#tool-annotations - Builds on: #1760, #1790, #1974  ## Summary by CodeRabbit * **New Features** * Tools now include detailed security annotations (readOnly, destructive, idempotent, openWorld hints) so MCP clients can better understand safety and behavior. * Most tools are advertised as read-only; the subagent tool is explicitly advertised as act-capable/destructive, and one search tool is advertised as read-only but open-world. * **Tests** * Added tests ensuring each tool includes annotations and that read-only vs. destructive/open-world hints are correctly serialized.  [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2268?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)   Co-authored-by: justin <justin80605@gmail.com> Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
2026-07-27 21:08:00 +00:00 · 2026-05-20 16:41:11 -07:00
parent 5cbf4adda0
commit 15bdac5be8
2 changed files with 153 additions and 1 deletions
@@ -1,7 +1,13 @@
-//! Stdio MCP server for exposing a curated, read-only OpenHuman tool surface.
+//! Stdio MCP server for exposing a curated OpenHuman tool surface.
 //!
 //! The server is opt-in via `openhuman-core mcp` and writes only JSON-RPC
 //! protocol messages to stdout. Diagnostics go through stderr logging.
+//!
+//! Most tools (memory tree reads, core/agent introspection) are read-only and
+//! gated through `SecurityPolicy` with `ToolOperation::Read`. The one
+//! exception is `agent.run_subagent`, which runs through `ToolOperation::Act`
+//! and is advertised to clients via MCP tool annotations
+//! (`readOnlyHint: false`, `destructiveHint: true`).

 mod protocol;
 mod stdio;
@@ -34,6 +34,13 @@ pub struct McpToolSpec {
    pub description: &'static str,
    pub rpc_method: Option<&'static str>,
    pub input_schema: Value,
+    /// MCP `ToolAnnotations` per the 2025-03-26+ spec — `readOnlyHint`,
+    /// `destructiveHint`, `idempotentHint`, `openWorldHint`. Hints, not
+    /// guarantees; clients use them to surface accurate safety affordances
+    /// (e.g. Claude Desktop's "this tool can take destructive actions"
+    /// confirmation gate). Per spec, destructive/idempotent are meaningful
+    /// only when `readOnlyHint == false`, so read-only tools omit them.
+    pub annotations: Value,
 }

 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -87,6 +94,7 @@ fn base_tool_specs() -> Vec<McpToolSpec> {
            description: "List the live core agent tool catalog that OpenHuman exposes to its orchestrator session.",
            rpc_method: None,
            input_schema: no_args_schema(),
+            annotations: read_only_local_annotations(),
        },
        McpToolSpec {
            name: "core.tool_instructions",
@@ -94,6 +102,7 @@ fn base_tool_specs() -> Vec<McpToolSpec> {
            description: "Emit the markdown tool-use instructions block that OpenHuman injects into prompt-guided agents.",
            rpc_method: None,
            input_schema: no_args_schema(),
+            annotations: read_only_local_annotations(),
        },
        McpToolSpec {
            name: "agent.list_subagents",
@@ -101,6 +110,7 @@ fn base_tool_specs() -> Vec<McpToolSpec> {
            description: "List registered sub-agent definitions that the core can dispatch for specialized work.",
            rpc_method: None,
            input_schema: no_args_schema(),
+            annotations: read_only_local_annotations(),
        },
        McpToolSpec {
            name: "agent.run_subagent",
@@ -122,6 +132,17 @@ fn base_tool_specs() -> Vec<McpToolSpec> {
                "required": ["agent_id", "prompt"],
                "additionalProperties": false
            }),
+            // Sub-agent execution is the one Act-policy surface on the MCP
+            // server today (see `enforce_act_policy` dispatch in `call_tool`).
+            // Sub-agents can call further tools, so destructive/openWorld are
+            // both true; running the same agent twice is not a no-op so
+            // idempotent is false.
+            annotations: json!({
+                "readOnlyHint": false,
+                "destructiveHint": true,
+                "idempotentHint": false,
+                "openWorldHint": true
+            }),
        },
        McpToolSpec {
            name: "memory.search",
@@ -129,6 +150,7 @@ fn base_tool_specs() -> Vec<McpToolSpec> {
            description: "Keyword-search OpenHuman's local memory tree and return matching chunks ordered by recency.",
            rpc_method: Some("openhuman.memory_tree_search"),
            input_schema: query_schema("Substring to match against stored memory chunks."),
+            annotations: read_only_local_annotations(),
        },
        McpToolSpec {
            name: "memory.recall",
@@ -136,6 +158,7 @@ fn base_tool_specs() -> Vec<McpToolSpec> {
            description: "Semantically recall local memory-tree chunks relevant to a natural-language query.",
            rpc_method: Some("openhuman.memory_tree_recall"),
            input_schema: query_schema("Natural-language query to embed and rerank against memory summaries."),
+            annotations: read_only_local_annotations(),
        },
        McpToolSpec {
            name: "tree.read_chunk",
@@ -153,6 +176,7 @@ fn base_tool_specs() -> Vec<McpToolSpec> {
                "required": ["chunk_id"],
                "additionalProperties": false
            }),
+            annotations: read_only_local_annotations(),
        },
        McpToolSpec {
            name: "tree.browse",
@@ -165,6 +189,7 @@ fn base_tool_specs() -> Vec<McpToolSpec> {
                          pagination.",
            rpc_method: Some("openhuman.memory_tree_list_chunks"),
            input_schema: tree_browse_schema(),
+            annotations: read_only_local_annotations(),
        },
        McpToolSpec {
            name: "tree.top_entities",
@@ -175,6 +200,7 @@ fn base_tool_specs() -> Vec<McpToolSpec> {
                          or `memory.search`. Returns entities ordered by reference count.",
            rpc_method: Some("openhuman.memory_tree_top_entities"),
            input_schema: tree_top_entities_schema(),
+            annotations: read_only_local_annotations(),
        },
        McpToolSpec {
            name: "tree.list_sources",
@@ -186,10 +212,24 @@ fn base_tool_specs() -> Vec<McpToolSpec> {
                          `tree.browse`.",
            rpc_method: Some("openhuman.memory_tree_list_sources"),
            input_schema: tree_list_sources_schema(),
+            annotations: read_only_local_annotations(),
        },
    ]
 }

+/// Annotation preset for the read-only, closed-world tools that just read
+/// OpenHuman's local memory tree or agent registry. The MCP spec defaults are
+/// `readOnlyHint: false` / `openWorldHint: true`, so both fields must be set
+/// explicitly to communicate the actual shape to clients. Destructive and
+/// idempotent hints are deliberately omitted — per the spec they are
+/// meaningful only when `readOnlyHint == false`.
+fn read_only_local_annotations() -> Value {
+    json!({
+        "readOnlyHint": true,
+        "openWorldHint": false
+    })
+}
+
 fn searxng_tool_spec() -> McpToolSpec {
    McpToolSpec {
        name: "searxng_search",
@@ -197,6 +237,14 @@ fn searxng_tool_spec() -> McpToolSpec {
        description: "Search the configured self-hosted SearXNG instance and return normalized title, URL, snippet, and source results. Requires searxng.enabled=true in OpenHuman config.",
        rpc_method: Some("openhuman.tools_searxng_search"),
        input_schema: searxng_search_schema(),
+        // SearXNG queries an external (self-hosted but network-reachable)
+        // search engine: read-only (no state mutation), open-world (results
+        // come from outside OpenHuman). Per spec, destructive/idempotent
+        // hints are meaningful only when readOnlyHint=false, so omit them.
+        annotations: json!({
+            "readOnlyHint": true,
+            "openWorldHint": true
+        }),
    }
 }

@@ -350,6 +398,7 @@ fn list_tools_result_from_specs(specs: Vec<McpToolSpec>) -> Value {
                "title": tool.title,
                "description": tool.description,
                "inputSchema": tool.input_schema,
+                "annotations": tool.annotations,
            })
        })
        .collect::<Vec<_>>();
@@ -1030,6 +1079,103 @@ mod tests {
        );
    }

+    #[test]
+    fn list_tools_emits_annotations_for_every_tool() {
+        // Exercise the searxng-enabled config so the annotation contract covers
+        // every shipping tool, not just the base set.
+        let mut config = crate::openhuman::config::Config::default();
+        config.searxng.enabled = true;
+        let result = list_tools_result_for_config(&config);
+        let tools = result["tools"].as_array().expect("tools array");
+        for tool in tools {
+            let name = tool["name"].as_str().expect("tool name");
+            assert!(
+                tool.get("annotations")
+                    .map(Value::is_object)
+                    .unwrap_or(false),
+                "tool `{name}` is missing a serialized `annotations` object",
+            );
+        }
+    }
+
+    #[test]
+    fn read_only_tools_are_marked_read_only_and_closed_world() {
+        // Every tool except the act-capable ones reads local OpenHuman state
+        // (memory tree / agent registry) or queries an external read-only
+        // search engine. Per MCP spec defaults these would be
+        // `readOnlyHint: false` and `openWorldHint: true`, so we MUST set
+        // `readOnlyHint` explicitly to communicate accurate safety affordances
+        // to clients. (`searxng_search` is read-only but openWorld, so it
+        // verifies the read-only axis here and is exempt from the
+        // openWorld=false check below.)
+        let act_tool_names = ["agent.run_subagent"];
+        let open_world_read_only = ["searxng_search"];
+        for spec in tool_specs() {
+            if act_tool_names.contains(&spec.name) {
+                continue;
+            }
+            let annotations = &spec.annotations;
+            assert_eq!(
+                annotations.get("readOnlyHint").and_then(Value::as_bool),
+                Some(true),
+                "expected `{}` to advertise readOnlyHint=true",
+                spec.name
+            );
+            let expected_open_world = open_world_read_only.contains(&spec.name);
+            assert_eq!(
+                annotations.get("openWorldHint").and_then(Value::as_bool),
+                Some(expected_open_world),
+                "expected `{}` to advertise openWorldHint={}",
+                spec.name,
+                expected_open_world
+            );
+            // Per spec these are meaningful only when readOnlyHint == false.
+            // Emitting them on a read-only tool would be misleading.
+            assert!(
+                annotations.get("destructiveHint").is_none(),
+                "read-only tool `{}` should not emit destructiveHint",
+                spec.name
+            );
+            assert!(
+                annotations.get("idempotentHint").is_none(),
+                "read-only tool `{}` should not emit idempotentHint",
+                spec.name
+            );
+        }
+    }
+
+    #[test]
+    fn run_subagent_annotations_signal_act_semantics() {
+        let spec = tool_specs()
+            .into_iter()
+            .find(|spec| spec.name == "agent.run_subagent")
+            .expect("agent.run_subagent must be registered");
+        assert_eq!(
+            spec.annotations
+                .get("readOnlyHint")
+                .and_then(Value::as_bool),
+            Some(false)
+        );
+        assert_eq!(
+            spec.annotations
+                .get("destructiveHint")
+                .and_then(Value::as_bool),
+            Some(true)
+        );
+        assert_eq!(
+            spec.annotations
+                .get("idempotentHint")
+                .and_then(Value::as_bool),
+            Some(false)
+        );
+        assert_eq!(
+            spec.annotations
+                .get("openWorldHint")
+                .and_then(Value::as_bool),
+            Some(true)
+        );
+    }
+
    #[test]
    fn list_tools_includes_searxng_when_enabled() {
        let mut config = crate::openhuman::config::Config::default();