feat(mcp): preserve typed results and policy boundaries

Author: claude

CHANGELOG.md

@@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Preserved standard MCP tool metadata and call results end to end, including
+  output schemas, annotations, icons, `_meta`, `structuredContent`, decoded
+  images, embedded resources, and bounded content-addressed artifacts.
+
+### Security
+
+- Made MCP confirmation annotations escalation-only: tool metadata can require
+  HITL but cannot weaken a host Allow/Ask/Deny decision.
+- Allowed an explicitly scoped delegated worker to see a parent-hidden tool
+  while keeping both parent and worker execution policies authoritative.
+
 ## [5.3.5] - 2026-07-17
 
 ### Added

core/src/agent/tool_invoker.rs

@@ -83,6 +83,11 @@ impl ScopedToolInvoker {
                 tool_name: &invocation.name,
                 args: &invocation.args,
                 pre_tool_block,
+                tool_requires_confirmation: self
+                    .agent
+                    .tool_executor
+                    .registry()
+                    .requires_confirmation(&invocation.name, &invocation.args),
             })
             .await
     }
@@ -395,7 +400,11 @@ impl ToolInvoker for ScopedToolInvoker {
     }
 
     fn available_tools(&self) -> Vec<String> {
-        self.agent.tool_executor.registry().list()
+        let mut tools = self.agent.tool_executor.registry().list();
+        if let Some(permission_checker) = &self.agent.config.permission_checker {
+            tools.retain(|tool| permission_checker.expose_to_model(tool));
+        }
+        tools
     }
 
     fn capabilities(&self, name: &str, args: &serde_json::Value) -> Option<ToolCapabilities> {

core/src/child_run.rs

@@ -30,7 +30,7 @@
 use crate::agent::AgentConfig;
 use crate::hitl::ConfirmationProvider;
 use crate::hooks::HookExecutor;
-use crate::permissions::{PermissionChecker, PermissionPolicy};
+use crate::permissions::{PermissionChecker, PermissionDecision, PermissionPolicy};
 use crate::security::SecurityProvider;
 use crate::skills::SkillRegistry;
 use std::sync::Arc;
@@ -60,6 +60,56 @@ pub struct ChildRunContext {
     pub budget_guard: Option<Arc<dyn crate::budget::BudgetGuard>>,
 }
 
+struct DelegatedPermissionChecker {
+    child: Arc<dyn PermissionChecker>,
+    child_policy: Option<PermissionPolicy>,
+    parent: Arc<dyn PermissionChecker>,
+    parent_policy: Option<PermissionPolicy>,
+}
+
+impl PermissionChecker for DelegatedPermissionChecker {
+    fn expose_to_model(&self, tool_name: &str) -> bool {
+        if !self.child.expose_to_model(tool_name) {
+            return false;
+        }
+
+        if self
+            .child_policy
+            .as_ref()
+            .is_some_and(|policy| policy.declares_tool_access(tool_name))
+        {
+            // A worker's explicitly declared capability may cross a host's
+            // ordinary parent-only visibility filter. A serializable parent
+            // deny remains authoritative and keeps the tool hidden.
+            return self
+                .parent_policy
+                .as_ref()
+                .map(|policy| policy.expose_to_model(tool_name))
+                .unwrap_or_else(|| self.parent.expose_to_model(tool_name));
+        }
+
+        self.parent.expose_to_model(tool_name)
+    }
+
+    fn check(&self, tool_name: &str, args: &serde_json::Value) -> PermissionDecision {
+        stricter_decision(
+            self.child.check(tool_name, args),
+            self.parent.check(tool_name, args),
+        )
+    }
+}
+
+const fn stricter_decision(
+    left: PermissionDecision,
+    right: PermissionDecision,
+) -> PermissionDecision {
+    match (left, right) {
+        (PermissionDecision::Deny, _) | (_, PermissionDecision::Deny) => PermissionDecision::Deny,
+        (PermissionDecision::Ask, _) | (_, PermissionDecision::Ask) => PermissionDecision::Ask,
+        (PermissionDecision::Allow, PermissionDecision::Allow) => PermissionDecision::Allow,
+    }
+}
+
 impl ChildRunContext {
     /// Apply inherited capabilities to a child AgentConfig.
     ///
@@ -75,10 +125,26 @@ impl ChildRunContext {
         if config.skill_registry.is_none() {
             config.skill_registry = self.skill_registry.clone();
         }
-        if config.permission_checker.is_none() {
-            config.permission_checker = self.permission_checker.clone();
-            config.permission_policy = self.permission_policy.clone();
-        } else if config.permission_policy.is_none() {
+        match (
+            config.permission_checker.take(),
+            self.permission_checker.clone(),
+        ) {
+            (Some(child), Some(parent)) => {
+                config.permission_checker = Some(Arc::new(DelegatedPermissionChecker {
+                    child,
+                    child_policy: config.permission_policy.clone(),
+                    parent,
+                    parent_policy: self.permission_policy.clone(),
+                }));
+            }
+            (Some(child), None) => config.permission_checker = Some(child),
+            (None, Some(parent)) => {
+                config.permission_checker = Some(parent);
+                config.permission_policy = self.permission_policy.clone();
+            }
+            (None, None) => {}
+        }
+        if config.permission_policy.is_none() {
             config.permission_policy = self.permission_policy.clone();
         }
         if config.tool_timeout_ms.is_none() {
@@ -110,3 +176,77 @@ impl ChildRunContext {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[derive(Clone)]
+    struct ParentVisibility {
+        policy: PermissionPolicy,
+        hide_use_from_primary: bool,
+    }
+
+    impl PermissionChecker for ParentVisibility {
+        fn expose_to_model(&self, tool_name: &str) -> bool {
+            !(self.hide_use_from_primary && tool_name.starts_with("mcp__use_"))
+                && self.policy.expose_to_model(tool_name)
+        }
+
+        fn check(&self, tool_name: &str, args: &serde_json::Value) -> PermissionDecision {
+            self.policy.check(tool_name, args)
+        }
+    }
+
+    fn delegated(
+        child_policy: PermissionPolicy,
+        parent_policy: PermissionPolicy,
+    ) -> DelegatedPermissionChecker {
+        DelegatedPermissionChecker {
+            child: Arc::new(child_policy.clone()),
+            child_policy: Some(child_policy),
+            parent: Arc::new(ParentVisibility {
+                policy: parent_policy.clone(),
+                hide_use_from_primary: true,
+            }),
+            parent_policy: Some(parent_policy),
+        }
+    }
+
+    #[test]
+    fn explicitly_scoped_worker_can_see_parent_hidden_tool() {
+        let mut child = PermissionPolicy::new().allow("mcp__use_*");
+        child.default_decision = PermissionDecision::Deny;
+        let parent = PermissionPolicy::new().allow("mcp__use_*");
+        let checker = delegated(child, parent);
+
+        assert!(checker.expose_to_model("mcp__use_browser__browser_snapshot"));
+        assert_eq!(
+            checker.check("mcp__use_browser__browser_snapshot", &serde_json::json!({})),
+            PermissionDecision::Allow
+        );
+    }
+
+    #[test]
+    fn unrelated_worker_does_not_inherit_parent_hidden_use_tools() {
+        let child = PermissionPolicy::new().allow("read(*)");
+        let parent = PermissionPolicy::new().allow("mcp__use_*");
+        let checker = delegated(child, parent);
+
+        assert!(!checker.expose_to_model("mcp__use_browser__browser_snapshot"));
+    }
+
+    #[test]
+    fn parent_deny_remains_authoritative_for_explicit_worker_capability() {
+        let mut child = PermissionPolicy::new().allow("mcp__use_*");
+        child.default_decision = PermissionDecision::Deny;
+        let parent = PermissionPolicy::new().deny("mcp__use_ocr__ocr_extract");
+        let checker = delegated(child, parent);
+
+        assert!(!checker.expose_to_model("mcp__use_ocr__ocr_extract"));
+        assert_eq!(
+            checker.check("mcp__use_ocr__ocr_extract", &serde_json::json!({})),
+            PermissionDecision::Deny
+        );
+    }
+}

core/src/mcp/manager.rs

@@ -5,7 +5,7 @@
 use crate::mcp::client::McpClient;
 use crate::mcp::oauth;
 use crate::mcp::protocol::{
-    CallToolResult, McpServerConfig, McpTool, McpTransportConfig, OAuthConfig, ToolContent,
+    CallToolResult, McpServerConfig, McpTool, McpTransportConfig, OAuthConfig,
 };
 use crate::mcp::transport::http_sse::HttpSseTransport;
 use crate::mcp::transport::stdio::StdioTransport;
@@ -16,6 +16,8 @@ use std::collections::HashMap;
 use std::sync::Arc;
 use tokio::sync::RwLock;
 
+pub use crate::mcp::result::tool_result_to_string;
+
 /// MCP server status
 #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
 pub struct McpServerStatus {
@@ -489,33 +491,6 @@ fn now_epoch_ms() -> u64 {
         .unwrap_or(0)
 }
 
-/// Convert MCP tool result to string output
-pub fn tool_result_to_string(result: &CallToolResult) -> String {
-    let mut output = String::new();
-
-    for content in &result.content {
-        match content {
-            ToolContent::Text { text } => {
-                output.push_str(text);
-                output.push('\n');
-            }
-            ToolContent::Image { data: _, mime_type } => {
-                output.push_str(&format!("[Image: {}]\n", mime_type));
-            }
-            ToolContent::Resource { resource } => {
-                if let Some(text) = &resource.text {
-                    output.push_str(text);
-                    output.push('\n');
-                } else {
-                    output.push_str(&format!("[Resource: {}]\n", resource.uri));
-                }
-            }
-        }
-    }
-
-    output.trim_end().to_string()
-}
-
 #[cfg(test)]
 #[path = "manager/tests.rs"]
 mod tests;

core/src/mcp/manager/tests.rs

@@ -1,4 +1,5 @@
 use super::*;
+use crate::mcp::protocol::ToolContent;
 
 #[test]
 fn test_parse_tool_name() {
@@ -32,6 +33,7 @@ fn test_tool_result_to_string() {
             },
         ],
         is_error: false,
+        ..CallToolResult::default()
     };
 
     let output = tool_result_to_string(&result);
@@ -136,6 +138,7 @@ fn test_tool_result_to_string_single_text() {
             text: "Hello World".to_string(),
         }],
         is_error: false,
+        ..CallToolResult::default()
     };
     let output = tool_result_to_string(&result);
     assert_eq!(output, "Hello World");
@@ -153,6 +156,7 @@ fn test_tool_result_to_string_multiple_text() {
             },
         ],
         is_error: false,
+        ..CallToolResult::default()
     };
     let output = tool_result_to_string(&result);
     assert!(output.contains("First line"));
@@ -164,6 +168,7 @@ fn test_tool_result_to_string_empty() {
     let result = CallToolResult {
         content: vec![],
         is_error: false,
+        ..CallToolResult::default()
     };
     let output = tool_result_to_string(&result);
     assert_eq!(output, "");
@@ -177,6 +182,7 @@ fn test_tool_result_to_string_image() {
             mime_type: "image/png".to_string(),
         }],
         is_error: false,
+        ..CallToolResult::default()
     };
     let output = tool_result_to_string(&result);
     assert!(output.contains("[Image: image/png]"));
@@ -195,6 +201,7 @@ fn test_tool_result_to_string_resource() {
             },
         }],
         is_error: false,
+        ..CallToolResult::default()
     };
     let output = tool_result_to_string(&result);
     assert!(output.contains("Resource content"));
@@ -222,6 +229,7 @@ fn test_tool_result_to_string_mixed_content() {
             },
         ],
         is_error: false,
+        ..CallToolResult::default()
     };
     let output = tool_result_to_string(&result);
     assert!(output.contains("Text content"));

core/src/mcp/mod.rs

@@ -61,13 +61,15 @@ pub mod client;
 pub mod manager;
 pub mod oauth;
 pub mod protocol;
+mod result;
 pub mod tools;
 pub mod transport;
 
 pub use client::McpClient;
-pub use manager::{tool_result_to_string, McpManager, McpServerStatus};
+pub use manager::{McpManager, McpServerStatus};
 pub use protocol::{
-    CallToolResult, McpNotification, McpResource, McpServerConfig, McpTool, McpTransportConfig,
-    OAuthConfig, ServerCapabilities, ToolContent,
+    CallToolResult, McpNotification, McpResource, McpServerConfig, McpTool, McpToolAnnotations,
+    McpTransportConfig, OAuthConfig, ServerCapabilities, ToolContent,
 };
+pub use result::tool_result_to_string;
 pub use tools::{create_mcp_tools, McpToolWrapper};