From 6ecc6853b3b671c519f42863cce0765f96d056b4 Mon Sep 17 00:00:00 2001 From: limityan Date: Tue, 30 Jun 2026 10:47:14 +0800 Subject: [PATCH] refactor: move tool bridge contracts to runtime owners Move MCP and ACP external tool bridge contracts into portable execution/runtime contract owners. Add extension capability, UI contribution, and provider mapping DTOs while keeping ACP protocol lifecycle in bitfun-acp. --- docs/plans/core-decomposition-plan.md | 26 +- .../rules/source/forbidden-rules.mjs | 24 +- .../rules/source/required-rules.mjs | 94 ++++++ scripts/core-boundaries/self-test.mjs | 35 +++ .../core/src/service/mcp/adapter/tool.rs | 117 +++----- src/crates/contracts/runtime-ports/AGENTS.md | 3 +- src/crates/execution/tool-contracts/AGENTS.md | 8 +- .../tool-contracts/src/acp_tool_bridge.rs | 160 ++++++++++ .../execution/tool-contracts/src/lib.rs | 23 +- .../tool-contracts/src/mcp_tool_bridge.rs | 190 ++++++++++++ .../tool-contracts/tests/tool_contracts.rs | 284 ++++++++++++++++-- src/crates/interfaces/acp/AGENTS-CN.md | 1 + src/crates/interfaces/acp/AGENTS.md | 3 + src/crates/interfaces/acp/Cargo.toml | 1 + .../interfaces/acp/src/client/manager.rs | 3 +- src/crates/interfaces/acp/src/client/tool.rs | 192 +++++------- .../services/services-integrations/AGENTS.md | 9 +- .../services/services-integrations/Cargo.toml | 2 + .../src/mcp/adapter/tool.rs | 78 ++--- .../src/mcp/tool_info.rs | 9 +- .../src/mcp/tool_name.rs | 60 +--- .../tests/mcp_contracts.rs | 178 +++++------ 22 files changed, 1043 insertions(+), 457 deletions(-) create mode 100644 src/crates/execution/tool-contracts/src/acp_tool_bridge.rs create mode 100644 src/crates/execution/tool-contracts/src/mcp_tool_bridge.rs diff --git a/docs/plans/core-decomposition-plan.md b/docs/plans/core-decomposition-plan.md index 682b6da87b..b2d91ced30 100644 --- a/docs/plans/core-decomposition-plan.md +++ b/docs/plans/core-decomposition-plan.md @@ -35,24 +35,30 @@ ### PR-C：Execution 层深迁移 -目标： +状态：本阶段收口 Execution 层主体迁移，剩余工作转入 PR-D / PR-E。 + +完成口径： -- 继续迁移 built-in tools、skills、MCP tool bridge、sandbox runner、local/remote tool runtime、harness descriptor / route plan 的实际 owner。 -- 删除或显著简化 core 中对应 tool/harness 主体路径，保留兼容 facade。 -- 区分 MCP tool bridge 与 MCP transport：tool bridge 属于 Execution，transport/client concrete 属于 Cross-platform Adapter。 +- built-in tool provider plan、skills 纯策略、tool runtime assembly、tool execution helper、harness descriptor / route plan 已由 Execution 层 owner 承接，core 保留产品组装和兼容 facade。 +- MCP dynamic tool name、tool info、descriptor、input validation、tool-use / rejected / result presentation、`ToolResult` shape 进入 `bitfun-agent-tools` 的 MCP tool bridge contract。 +- `services-integrations` 只负责 MCP wire / transport / protocol result content 投影，并通过旧导出路径保持兼容。 +- `bitfun-core` 的 MCP tool adapter 只保留 `Tool` trait 适配、MCP connection 调用和旧注册路径，不再持有 bridge 文案、validation 或 dynamic metadata 组装。 +- 当前代码未发现独立 sandbox runner 主体；sandbox 相关内容主要是 capability / permission / execution-domain 事实和局部 guard，后续如出现 concrete runner，需要按 Execution contract 加 Cross-platform Adapter provider 的方式单独评审。 保护： - prompt-visible manifest、`GetToolSpec`、permission gate、tool result/artifact、collapsed/expanded exposure、MCP/ACP catalog 和 remote/local path containment 等价。 -- `cargo test -p bitfun-agent-tools`、`cargo test -p tool-runtime`、harness / MCP focused tests 和 product shape tests 必跑。 +- PR-C 提交前至少覆盖 `cargo test -p bitfun-agent-tools`、`cargo test -p tool-runtime`、harness / MCP focused tests、product shape tests、`bitfun-core --features product-full` 和 core boundary checks。 ### PR-D：Extension Host 与 OpenCode / ACP 适配收口 -目标： +状态：本阶段收口 ACP external-agent tool bridge；Extension/OpenCode/plugin、UI extension、effect / permission mapping 和多形态 SDK 验证转入后续阶段。 + +完成口径： -- 定义并落地最小 Extension Host 边界：plugin capability declaration、UI contribution descriptor、tool/hook/workflow provider mapping。 -- 明确 OpenCode adapter 将外部 plugin API 映射到 BitFun Rust Kernel API、UI Extension Contract 和 Capability/Effect API。 -- ACP 保持协议入口和 external agent/tool capability owner，不下沉到 Agent Kernel。 +- ACP external-agent tool name、schema、validation、presentation 和 ToolResult shape 由 `bitfun-agent-tools` 承接。 +- `bitfun-acp` 继续持有 ACP protocol、client lifecycle、remote probing、permission bridge 和配置加载；现有 `AcpClientInfo` API shape 不变。 +- OpenCode/plugin concrete host、UI contribution、hook/workflow provider mapping 和 capability/effect policy 仍需在实际消费路径明确后单独接入，避免提前扩大稳定 API。 保护： @@ -96,7 +102,7 @@ | Remote Connect / IM bot support | `cargo test -p bitfun-services-integrations --features remote-connect --lib remote_connect::bot::`，`cargo test -p bitfun-core --features product-full remote_connect::bot::command_router` | | Tool / MCP / terminal / sandbox | `cargo test -p bitfun-agent-tools`，`cargo test -p tool-runtime`，terminal / exec-command / MCP focused tests | | Harness / Product Domains | `cargo test -p bitfun-harness`，`cargo test -p bitfun-product-domains`，DeepReview / MiniApp focused tests | -| Extension / OpenCode / ACP | extension host focused tests，UI contribution descriptor tests，ACP permission / external tool focused tests | +| Extension / OpenCode / ACP | extension host focused tests，ACP permission / external tool focused tests | | Product shape / SDK | SDK fake-provider smoke，Desktop / CLI / Web / ACP capability matrix checks，cargo tree / metadata 对比 | | 大范围 owner 迁移 | `cargo check --workspace`，必要时补 `cargo test --workspace` | diff --git a/scripts/core-boundaries/rules/source/forbidden-rules.mjs b/scripts/core-boundaries/rules/source/forbidden-rules.mjs index 49ff7f8a7e..c0afd263e0 100644 --- a/scripts/core-boundaries/rules/source/forbidden-rules.mjs +++ b/scripts/core-boundaries/rules/source/forbidden-rules.mjs @@ -2250,7 +2250,7 @@ export const forbiddenContentRules = [ patterns: [ { regex: /\bfn behavior_hints\b/, - message: 'core MCP tool adapter must not own dynamic tool behavior hint rendering; use the integrations helper', + message: 'core MCP tool adapter must not own dynamic tool behavior hint rendering; use the execution MCP tool bridge contract', }, { regex: /\bfn truncate_for_assistant\b/, @@ -2262,7 +2262,27 @@ export const forbiddenContentRules = [ }, { regex: /Tool '\{\}' from MCP server/, - message: 'core MCP tool adapter must not own dynamic descriptor text; use the integrations helper', + message: 'core MCP tool adapter must not own dynamic descriptor text; use the execution MCP tool bridge contract', + }, + { + regex: /\bDynamicMcpToolInfo\b/, + message: 'core MCP tool adapter must not own dynamic MCP metadata assembly; use the execution MCP tool bridge contract', + }, + { + regex: /Input must be an object/, + message: 'core MCP tool adapter must not own bridge input validation text; use the execution MCP tool bridge contract', + }, + { + regex: /Using MCP tool/, + message: 'core MCP tool adapter must not own bridge tool-use presentation; use the execution MCP tool bridge contract', + }, + { + regex: /was rejected by user/, + message: 'core MCP tool adapter must not own bridge rejection presentation; use the execution MCP tool bridge contract', + }, + { + regex: /completed\. Result:/, + message: 'core MCP tool adapter must not own bridge result presentation; use the execution MCP tool bridge contract', }, ], }, diff --git a/scripts/core-boundaries/rules/source/required-rules.mjs b/scripts/core-boundaries/rules/source/required-rules.mjs index f9a6649d12..f358eb27c7 100644 --- a/scripts/core-boundaries/rules/source/required-rules.mjs +++ b/scripts/core-boundaries/rules/source/required-rules.mjs @@ -4379,6 +4379,100 @@ export const requiredContentRules = [ }, ], }, + { + path: 'src/crates/execution/tool-contracts/src/mcp_tool_bridge.rs', + reason: + 'agent-tools owns MCP tool bridge naming, descriptor, validation, presentation, and ToolResult shape contracts without depending on MCP transport concrete', + patterns: [ + { + regex: /\bpub fn build_mcp_tool_bridge_name\b/, + message: 'missing MCP tool bridge prompt-visible name builder', + }, + { + regex: /\bpub struct McpToolBridgeDefinition\b/, + message: 'missing MCP tool bridge descriptor contract', + }, + { + regex: /\bpub struct McpToolBridgeBehaviorHints\b/, + message: 'missing MCP tool bridge behavior hint contract', + }, + { + regex: /\bpub fn build_mcp_tool_bridge_definition\b/, + message: 'missing MCP tool bridge descriptor builder', + }, + { + regex: /\bpub fn mcp_tool_bridge_dynamic_tool_info\b/, + message: 'missing MCP dynamic tool info bridge', + }, + { + regex: /\bpub fn validate_mcp_tool_bridge_input\b/, + message: 'missing MCP tool bridge input validation contract', + }, + { + regex: /\bpub fn render_mcp_tool_bridge_use_message\b/, + message: 'missing MCP tool bridge use-message renderer', + }, + { + regex: /\bpub fn render_mcp_tool_bridge_rejected_message\b/, + message: 'missing MCP tool bridge rejection-message renderer', + }, + { + regex: /\bpub fn render_mcp_tool_bridge_result_message\b/, + message: 'missing MCP tool bridge result-message renderer', + }, + { + regex: /\bpub fn build_mcp_tool_bridge_result\b/, + message: 'missing MCP tool bridge ToolResult builder', + }, + ], + }, + { + path: 'src/crates/execution/tool-contracts/src/acp_tool_bridge.rs', + reason: + 'agent-tools owns ACP external-agent tool bridge naming, schema, validation, presentation, and ToolResult contracts without depending on ACP protocol concrete', + patterns: [ + { + regex: /\bpub fn build_acp_external_agent_tool_name\b/, + message: 'missing ACP external-agent prompt-visible name builder', + }, + { + regex: /\bpub struct AcpExternalAgentToolDefinition\b/, + message: 'missing ACP external-agent tool definition contract', + }, + { + regex: /\bpub fn build_acp_external_agent_tool_definition\b/, + message: 'missing ACP external-agent tool definition builder', + }, + { + regex: /\bpub fn acp_external_agent_tool_input_schema\b/, + message: 'missing ACP external-agent input schema contract', + }, + { + regex: /\bpub fn validate_acp_external_agent_tool_input\b/, + message: 'missing ACP external-agent input validation contract', + }, + { + regex: /\bpub fn render_acp_external_agent_use_message\b/, + message: 'missing ACP external-agent use-message renderer', + }, + { + regex: /\bpub fn render_acp_external_agent_rejected_message\b/, + message: 'missing ACP external-agent rejection-message renderer', + }, + { + regex: /\bpub fn render_acp_external_agent_result_message\b/, + message: 'missing ACP external-agent result-message renderer', + }, + { + regex: /\bpub fn render_acp_external_agent_result_for_assistant\b/, + message: 'missing ACP external-agent assistant-result renderer', + }, + { + regex: /\bpub fn build_acp_external_agent_tool_result\b/, + message: 'missing ACP external-agent ToolResult builder', + }, + ], + }, { path: 'src/crates/execution/tool-contracts/src/file_guidance.rs', reason: 'agent-tools owns provider-neutral file tool guidance marker contracts', diff --git a/scripts/core-boundaries/self-test.mjs b/scripts/core-boundaries/self-test.mjs index 8c015bd18c..78565385c0 100644 --- a/scripts/core-boundaries/self-test.mjs +++ b/scripts/core-boundaries/self-test.mjs @@ -2307,6 +2307,36 @@ export function runManifestParserSelfTest({ 'call_results', ], }, + { + path: 'src/crates/execution/tool-contracts/src/mcp_tool_bridge.rs', + contracts: [ + 'build_mcp_tool_bridge_name', + 'McpToolBridgeDefinition', + 'McpToolBridgeBehaviorHints', + 'build_mcp_tool_bridge_definition', + 'mcp_tool_bridge_dynamic_tool_info', + 'validate_mcp_tool_bridge_input', + 'render_mcp_tool_bridge_use_message', + 'render_mcp_tool_bridge_rejected_message', + 'render_mcp_tool_bridge_result_message', + 'build_mcp_tool_bridge_result', + ], + }, + { + path: 'src/crates/execution/tool-contracts/src/acp_tool_bridge.rs', + contracts: [ + 'build_acp_external_agent_tool_name', + 'AcpExternalAgentToolDefinition', + 'build_acp_external_agent_tool_definition', + 'acp_external_agent_tool_input_schema', + 'validate_acp_external_agent_tool_input', + 'render_acp_external_agent_use_message', + 'render_acp_external_agent_rejected_message', + 'render_acp_external_agent_result_message', + 'render_acp_external_agent_result_for_assistant', + 'build_acp_external_agent_tool_result', + ], + }, { path: 'src/crates/execution/tool-provider-groups/src/lib.rs', contracts: [ @@ -3398,6 +3428,11 @@ export function runManifestParserSelfTest({ 'truncate_for_assistant', 'MCPToolResultContent', 'Tool', + 'DynamicMcpToolInfo', + 'Input must be an object', + 'Using MCP tool', + 'was rejected by user', + 'completed\\. Result:', ]; const mcpToolAdapterRuleText = mcpToolAdapterRule.patterns .map((pattern) => pattern.regex.source) diff --git a/src/crates/assembly/core/src/service/mcp/adapter/tool.rs b/src/crates/assembly/core/src/service/mcp/adapter/tool.rs index 83540ca10c..97ddf1c88b 100644 --- a/src/crates/assembly/core/src/service/mcp/adapter/tool.rs +++ b/src/crates/assembly/core/src/service/mcp/adapter/tool.rs @@ -3,16 +3,20 @@ //! Wraps MCP tools as implementations of BitFun's `Tool` trait. use crate::agentic::tools::framework::{ - DynamicMcpToolInfo, DynamicToolInfo, Tool, ToolRenderOptions, ToolResult, ToolUseContext, - ValidationResult, + DynamicToolInfo, Tool, ToolRenderOptions, ToolResult, ToolUseContext, ValidationResult, }; use crate::service::mcp::protocol::{MCPTool, MCPToolResult}; use crate::service::mcp::server::MCPConnection; use crate::util::errors::BitFunResult; use async_trait::async_trait; +use bitfun_agent_tools::{ + build_mcp_tool_bridge_result, mcp_tool_bridge_dynamic_tool_info, + mcp_tool_bridge_short_description, render_mcp_tool_bridge_rejected_message, + render_mcp_tool_bridge_result_message, render_mcp_tool_bridge_use_message, + validate_mcp_tool_bridge_input, +}; use bitfun_services_integrations::mcp::adapter::{ - build_mcp_tool_descriptor, render_mcp_tool_result_for_assistant, MCPDynamicToolProvider, - McpDynamicToolDescriptor, + render_mcp_tool_result_for_assistant, MCPDynamicToolProvider, McpDynamicToolDescriptor, }; use log::{debug, error, info, warn}; use serde_json::Value; @@ -22,25 +26,18 @@ use std::sync::Arc; pub struct MCPToolWrapper { mcp_tool: MCPTool, connection: Arc, - server_id: String, - server_name: String, descriptor: McpDynamicToolDescriptor, } impl MCPToolWrapper { - /// Creates a new MCP tool wrapper. - pub fn new( + fn from_descriptor( mcp_tool: MCPTool, connection: Arc, - server_id: String, - server_name: String, + descriptor: McpDynamicToolDescriptor, ) -> Self { - let descriptor = build_mcp_tool_descriptor(&server_id, &server_name, &mcp_tool); Self { mcp_tool, connection, - server_id, - server_name, descriptor, } } @@ -73,13 +70,10 @@ impl Tool for MCPToolWrapper { } fn short_description(&self) -> String { - let summary = self - .mcp_tool - .description - .as_deref() - .filter(|value| !value.trim().is_empty()) - .unwrap_or("MCP tool"); - format!("{} ({})", summary, self.server_name) + mcp_tool_bridge_short_description( + self.mcp_tool.description.as_deref(), + &self.descriptor.tool_info.server_name, + ) } fn input_schema(&self) -> Value { @@ -95,7 +89,7 @@ impl Tool for MCPToolWrapper { } fn dynamic_provider_id(&self) -> Option<&str> { - Some(&self.server_id) + Some(&self.descriptor.provider_id) } fn user_facing_name(&self) -> String { @@ -103,15 +97,7 @@ impl Tool for MCPToolWrapper { } fn dynamic_tool_info(&self) -> Option { - Some(DynamicToolInfo { - provider_id: self.descriptor.provider_id.clone(), - provider_kind: Some(self.descriptor.provider_kind.clone()), - mcp: Some(DynamicMcpToolInfo { - server_id: self.descriptor.tool_info.server_id.clone(), - server_name: self.descriptor.tool_info.server_name.clone(), - tool_name: self.descriptor.tool_info.tool_name.clone(), - }), - }) + Some(mcp_tool_bridge_dynamic_tool_info(&self.descriptor)) } async fn is_enabled(&self) -> bool { @@ -139,33 +125,11 @@ impl Tool for MCPToolWrapper { input: &Value, context: Option<&ToolUseContext>, ) -> ValidationResult { - if self.is_blocked_in_context(context) { - return ValidationResult { - result: false, - message: Some(format!( - "MCP server '{}' runs locally and is unavailable in remote workspace sessions", - self.server_name - )), - error_code: Some(400), - meta: None, - }; - } - - if !input.is_object() { - return ValidationResult { - result: false, - message: Some("Input must be an object".to_string()), - error_code: Some(400), - meta: None, - }; - } - - ValidationResult { - result: true, - message: None, - error_code: None, - meta: None, - } + validate_mcp_tool_bridge_input( + input, + &self.descriptor.tool_info.server_name, + self.is_blocked_in_context(context), + ) } fn render_result_for_assistant(&self, output: &Value) -> String { @@ -177,27 +141,24 @@ impl Tool for MCPToolWrapper { } fn render_tool_use_message(&self, input: &Value, _options: &ToolRenderOptions) -> String { - format!( - "Using MCP tool '{}' from '{}' with input: {}", - self.tool_title(), - self.server_name, - input + render_mcp_tool_bridge_use_message( + &self.descriptor.title, + &self.descriptor.tool_info.server_name, + input, ) } fn render_tool_use_rejected_message(&self) -> String { - format!( - "MCP tool '{}' from '{}' was rejected by user", - self.tool_title(), - self.server_name + render_mcp_tool_bridge_rejected_message( + &self.descriptor.title, + &self.descriptor.tool_info.server_name, ) } fn render_tool_result_message(&self, output: &Value) -> String { - format!( - "MCP tool '{}' completed. Result: {}", - self.tool_title(), - self.render_result_for_assistant(output) + render_mcp_tool_bridge_result_message( + &self.descriptor.title, + &self.render_result_for_assistant(output), ) } @@ -211,7 +172,7 @@ impl Tool for MCPToolWrapper { info!( "Calling MCP tool: {} from server: {}", self.tool_title(), - self.server_name + self.descriptor.tool_info.server_name ); debug!( "Input: {}", @@ -231,11 +192,10 @@ impl Tool for MCPToolWrapper { let result_value = serde_json::to_value(&result)?; let result_for_assistant = self.render_result_for_assistant(&result_value); - Ok(vec![ToolResult::Result { - data: result_value, - result_for_assistant: Some(result_for_assistant), - image_attachments: None, - }]) + Ok(vec![build_mcp_tool_bridge_result( + result_value, + result_for_assistant, + )]) } } @@ -283,11 +243,10 @@ impl MCPToolAdapter { } for definition in definitions.into_iter() { - let wrapper = Arc::new(MCPToolWrapper::new( + let wrapper = Arc::new(MCPToolWrapper::from_descriptor( definition.mcp_tool, connection.clone(), - server_id.to_string(), - server_name.to_string(), + definition.descriptor, )); self.tools.push(wrapper); } diff --git a/src/crates/contracts/runtime-ports/AGENTS.md b/src/crates/contracts/runtime-ports/AGENTS.md index c98c362548..142b084e03 100644 --- a/src/crates/contracts/runtime-ports/AGENTS.md +++ b/src/crates/contracts/runtime-ports/AGENTS.md @@ -17,7 +17,8 @@ facts. It is an interface crate, not a runtime implementation crate. load request and timing facts only. Concrete session persistence, file IO, session lifecycle, context restore, and prompt assembly do not belong here. - Do not put filesystem writes, process execution, network clients, Git/AI/MCP - concrete behavior, product policy, or UI command logic here. + concrete behavior, product policy, permission decisions, audit outcomes, UI + extension behavior, UI implementation, or UI command logic here. - Preserve serialization compatibility for persisted or cross-process DTOs. ## Verification diff --git a/src/crates/execution/tool-contracts/AGENTS.md b/src/crates/execution/tool-contracts/AGENTS.md index 81ddf73977..ed8f66e8c7 100644 --- a/src/crates/execution/tool-contracts/AGENTS.md +++ b/src/crates/execution/tool-contracts/AGENTS.md @@ -13,7 +13,9 @@ the product tool runtime. - This crate may own provider-neutral tool DTOs, validation/restriction facts, path and artifact contracts, pure manifest/catalog/exposure helpers, result presentation policy, deterministic admission policy, portable tool context - facts, and runtime restriction policy shaping. + facts, runtime restriction policy shaping, and provider-neutral MCP / ACP + external-agent tool bridge naming, validation, result, and presentation + contracts. - This crate may own generic provider contracts, containers, materialization, and registry assembly. Concrete tool construction and product runtime registration stay outside this crate until a reviewed owner move proves @@ -25,6 +27,10 @@ the product tool runtime. here without an owner design and equivalence tests. - Provider-specific wire serialization belongs in AI adapters, not in these provider-neutral contracts. +- MCP transport/client lifecycle and protocol result-content rendering stay in + `services-integrations`; this crate owns only the model/tool bridge contract. +- ACP protocol/client lifecycle stays in `bitfun-acp`; this crate owns only the + external-agent tool bridge contract. ## Verification diff --git a/src/crates/execution/tool-contracts/src/acp_tool_bridge.rs b/src/crates/execution/tool-contracts/src/acp_tool_bridge.rs new file mode 100644 index 0000000000..b93e67cd13 --- /dev/null +++ b/src/crates/execution/tool-contracts/src/acp_tool_bridge.rs @@ -0,0 +1,160 @@ +use serde_json::{json, Value}; + +use crate::{ToolResult, ValidationResult}; + +pub const ACP_TOOL_PREFIX: &str = "acp__"; +pub const ACP_TOOL_SUFFIX: &str = "__prompt"; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AcpExternalAgentToolDefinitionInput<'a> { + pub client_id: &'a str, + pub display_name: Option<&'a str>, + pub read_only: bool, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AcpExternalAgentToolDefinition { + pub client_id: String, + pub tool_name: String, + pub display_name: String, + pub user_facing_name: String, + pub description: String, + pub short_description: String, + pub read_only: bool, +} + +pub fn normalize_name_for_acp_tool_part(value: &str) -> String { + let sanitized = value + .chars() + .map(|ch| { + if ch.is_ascii_alphanumeric() || ch == '-' || ch == '_' { + ch + } else { + '_' + } + }) + .collect::(); + sanitized.trim_matches('_').to_string() +} + +pub fn build_acp_external_agent_tool_name(client_id: &str) -> String { + format!( + "{ACP_TOOL_PREFIX}{}{ACP_TOOL_SUFFIX}", + normalize_name_for_acp_tool_part(client_id) + ) +} + +pub fn build_acp_external_agent_tool_definition( + input: AcpExternalAgentToolDefinitionInput<'_>, +) -> AcpExternalAgentToolDefinition { + let display_name = input + .display_name + .map(str::to_string) + .unwrap_or_else(|| input.client_id.to_string()); + AcpExternalAgentToolDefinition { + client_id: input.client_id.to_string(), + tool_name: build_acp_external_agent_tool_name(input.client_id), + user_facing_name: format!("{display_name} (ACP)"), + description: format!( + "Send a prompt to the external ACP agent '{}'. Use this when another local ACP-compatible agent is better suited for a delegated task.", + display_name + ), + short_description: format!("Delegate a task to the external ACP agent '{}'.", display_name), + read_only: input.read_only, + display_name, + } +} + +pub fn acp_external_agent_tool_input_schema() -> Value { + json!({ + "type": "object", + "properties": { + "prompt": { + "type": "string", + "description": "The task or question to send to the external ACP agent." + }, + "workspace_path": { + "type": "string", + "description": "Optional absolute workspace path. Defaults to the current BitFun workspace." + }, + "timeout_seconds": { + "type": "integer", + "minimum": 0, + "description": "Optional timeout in seconds. Use 0 or omit it to wait without a fixed timeout." + } + }, + "required": ["prompt"], + "additionalProperties": false + }) +} + +pub fn validate_acp_external_agent_tool_input(input: &Value) -> ValidationResult { + match input.get("prompt").and_then(|value| value.as_str()) { + Some(prompt) if !prompt.trim().is_empty() => ValidationResult::default(), + Some(_) => ValidationResult { + result: false, + message: Some("prompt cannot be empty".to_string()), + error_code: Some(400), + meta: None, + }, + None => ValidationResult { + result: false, + message: Some("prompt is required".to_string()), + error_code: Some(400), + meta: None, + }, + } +} + +pub fn render_acp_external_agent_use_message(display_name: &str, input: &Value) -> String { + let prompt_preview = input + .get("prompt") + .and_then(|value| value.as_str()) + .map(truncate_prompt) + .unwrap_or_else(|| "prompt".to_string()); + format!("Sending ACP prompt to '{}': {prompt_preview}", display_name) +} + +pub fn render_acp_external_agent_rejected_message(display_name: &str) -> String { + format!("ACP prompt to '{}' was rejected", display_name) +} + +pub fn render_acp_external_agent_result_message(display_name: &str, output: &Value) -> String { + output + .get("response") + .and_then(|value| value.as_str()) + .map(|response| format!("ACP agent '{}' responded:\n{response}", display_name)) + .unwrap_or_else(|| format!("ACP agent '{}' completed", display_name)) +} + +pub fn render_acp_external_agent_result_for_assistant(output: &Value) -> String { + output + .get("response") + .and_then(|value| value.as_str()) + .unwrap_or("ACP agent completed without text output") + .to_string() +} + +pub fn build_acp_external_agent_tool_result( + client_id: &str, + response: impl Into, +) -> ToolResult { + let data = json!({ + "client_id": client_id, + "response": response.into(), + }); + ToolResult::Result { + result_for_assistant: Some(render_acp_external_agent_result_for_assistant(&data)), + data, + image_attachments: None, + } +} + +fn truncate_prompt(prompt: &str) -> String { + const LIMIT: usize = 160; + if prompt.chars().count() <= LIMIT { + prompt.to_string() + } else { + format!("{}...", prompt.chars().take(LIMIT).collect::()) + } +} diff --git a/src/crates/execution/tool-contracts/src/lib.rs b/src/crates/execution/tool-contracts/src/lib.rs index a377690778..4f35dcb3ac 100644 --- a/src/crates/execution/tool-contracts/src/lib.rs +++ b/src/crates/execution/tool-contracts/src/lib.rs @@ -3,6 +3,7 @@ //! Pure tool DTOs and helpers live here before the concrete tool framework and //! tool packs are moved out of the core facade. +pub mod acp_tool_bridge; pub mod computer_use; pub mod element_token; pub mod execution_gate; @@ -10,9 +11,19 @@ pub mod file_guidance; pub mod file_read_freshness; pub mod framework; pub mod input_validator; +pub mod mcp_tool_bridge; pub mod tool_execution_presentation; pub mod tool_result_storage; +pub use acp_tool_bridge::{ + acp_external_agent_tool_input_schema, build_acp_external_agent_tool_definition, + build_acp_external_agent_tool_name, build_acp_external_agent_tool_result, + normalize_name_for_acp_tool_part, render_acp_external_agent_rejected_message, + render_acp_external_agent_result_for_assistant, render_acp_external_agent_result_message, + render_acp_external_agent_use_message, validate_acp_external_agent_tool_input, + AcpExternalAgentToolDefinition, AcpExternalAgentToolDefinitionInput, ACP_TOOL_PREFIX, + ACP_TOOL_SUFFIX, +}; pub use bitfun_core_types::ToolImageAttachment; pub use bitfun_runtime_ports::{ DynamicToolDescriptor, DynamicToolProvider, PortError, PortErrorKind, PortResult, ToolDecorator, @@ -69,11 +80,19 @@ pub use framework::{ BITFUN_RUNTIME_URI_PREFIX, GET_TOOL_SPEC_TOOL_NAME, }; pub use input_validator::InputValidator; +pub use mcp_tool_bridge::{ + build_mcp_tool_bridge_definition, build_mcp_tool_bridge_name, build_mcp_tool_bridge_result, + mcp_tool_bridge_dynamic_tool_info, mcp_tool_bridge_short_description, normalize_name_for_mcp, + render_mcp_tool_bridge_rejected_message, render_mcp_tool_bridge_result_message, + render_mcp_tool_bridge_use_message, validate_mcp_tool_bridge_input, McpToolBridgeBehaviorHints, + McpToolBridgeDefinition, McpToolBridgeDefinitionInput, McpToolBridgeToolInfo, + MCP_TOOL_DELIMITER, MCP_TOOL_PREFIX, +}; pub use tool_execution_presentation::{ build_invalid_tool_call_error_message, build_tool_call_truncation_recovery_notice, build_tool_confirmation_timeout_presentation, build_tool_execution_error_presentation, - build_tool_execution_timeout_presentation, - build_user_rejected_tool_presentation, build_user_rejected_tool_presentation_with_instruction, + build_tool_execution_timeout_presentation, build_user_rejected_tool_presentation, + build_user_rejected_tool_presentation_with_instruction, build_user_steering_interrupted_presentation, is_write_like_tool_name, render_tool_result_for_assistant, truncate_raw_tool_arguments_preview, truncate_raw_tool_arguments_preview_to, truncate_tool_arguments_preview, diff --git a/src/crates/execution/tool-contracts/src/mcp_tool_bridge.rs b/src/crates/execution/tool-contracts/src/mcp_tool_bridge.rs new file mode 100644 index 0000000000..254587c3d1 --- /dev/null +++ b/src/crates/execution/tool-contracts/src/mcp_tool_bridge.rs @@ -0,0 +1,190 @@ +use crate::{DynamicMcpToolInfo, DynamicToolInfo, ToolResult, ValidationResult}; +use serde::{Deserialize, Serialize}; +use serde_json::Value; + +pub const MCP_TOOL_PREFIX: &str = "mcp__"; +pub const MCP_TOOL_DELIMITER: &str = "__"; + +/// Normalize MCP server/tool names to the prompt-visible dynamic-tool format. +pub fn normalize_name_for_mcp(name: &str) -> String { + name.chars() + .map(|ch| { + if ch.is_ascii_alphanumeric() || ch == '_' || ch == '-' { + ch + } else { + '_' + } + }) + .collect() +} + +pub fn build_mcp_tool_bridge_name(server_id: &str, tool_name: &str) -> String { + format!( + "{}{}{}{}", + MCP_TOOL_PREFIX, + normalize_name_for_mcp(server_id), + MCP_TOOL_DELIMITER, + normalize_name_for_mcp(tool_name) + ) +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct McpToolBridgeToolInfo { + pub server_id: String, + pub server_name: String, + pub tool_name: String, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct McpToolBridgeDefinition { + pub full_name: String, + pub title: String, + pub user_facing_name: String, + pub description: String, + pub provider_id: String, + pub provider_kind: String, + pub tool_info: McpToolBridgeToolInfo, + pub read_only: bool, +} + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct McpToolBridgeBehaviorHints { + pub read_only: bool, + pub destructive: bool, + pub open_world: bool, +} + +#[derive(Debug, Clone, Copy)] +pub struct McpToolBridgeDefinitionInput<'a> { + pub server_id: &'a str, + pub server_name: &'a str, + pub tool_name: &'a str, + pub title: &'a str, + pub description: Option<&'a str>, + pub behavior_hints: McpToolBridgeBehaviorHints, +} + +pub fn build_mcp_tool_bridge_definition( + input: McpToolBridgeDefinitionInput<'_>, +) -> McpToolBridgeDefinition { + let mut description = format!( + "Tool '{}' from MCP server '{}': {}", + input.title, + input.server_name, + input.description.unwrap_or("") + ); + + let hints = mcp_tool_bridge_behavior_hint_labels(input.behavior_hints); + if !hints.is_empty() { + description.push_str(&format!(" [Hints: {}]", hints.join(", "))); + } + + McpToolBridgeDefinition { + full_name: build_mcp_tool_bridge_name(input.server_id, input.tool_name), + title: input.title.to_string(), + user_facing_name: format!("{} ({})", input.title, input.server_name), + description, + provider_id: input.server_id.to_string(), + provider_kind: "mcp".to_string(), + tool_info: McpToolBridgeToolInfo { + server_id: input.server_id.to_string(), + server_name: input.server_name.to_string(), + tool_name: input.tool_name.to_string(), + }, + read_only: input.behavior_hints.read_only, + } +} + +pub fn mcp_tool_bridge_short_description( + tool_description: Option<&str>, + server_name: &str, +) -> String { + let summary = tool_description + .filter(|value| !value.trim().is_empty()) + .unwrap_or("MCP tool"); + format!("{} ({})", summary, server_name) +} + +pub fn mcp_tool_bridge_dynamic_tool_info(definition: &McpToolBridgeDefinition) -> DynamicToolInfo { + DynamicToolInfo { + provider_id: definition.provider_id.clone(), + provider_kind: Some(definition.provider_kind.clone()), + mcp: Some(DynamicMcpToolInfo { + server_id: definition.tool_info.server_id.clone(), + server_name: definition.tool_info.server_name.clone(), + tool_name: definition.tool_info.tool_name.clone(), + }), + } +} + +pub fn validate_mcp_tool_bridge_input( + input: &Value, + server_name: &str, + blocked_in_context: bool, +) -> ValidationResult { + if blocked_in_context { + return ValidationResult { + result: false, + message: Some(format!( + "MCP server '{}' runs locally and is unavailable in remote workspace sessions", + server_name + )), + error_code: Some(400), + meta: None, + }; + } + + if !input.is_object() { + return ValidationResult { + result: false, + message: Some("Input must be an object".to_string()), + error_code: Some(400), + meta: None, + }; + } + + ValidationResult::default() +} + +pub fn render_mcp_tool_bridge_use_message(title: &str, server_name: &str, input: &Value) -> String { + format!( + "Using MCP tool '{}' from '{}' with input: {}", + title, server_name, input + ) +} + +pub fn render_mcp_tool_bridge_rejected_message(title: &str, server_name: &str) -> String { + format!( + "MCP tool '{}' from '{}' was rejected by user", + title, server_name + ) +} + +pub fn render_mcp_tool_bridge_result_message(title: &str, rendered_result: &str) -> String { + format!( + "MCP tool '{}' completed. Result: {}", + title, rendered_result + ) +} + +pub fn build_mcp_tool_bridge_result(data: Value, result_for_assistant: String) -> ToolResult { + ToolResult::Result { + data, + result_for_assistant: Some(result_for_assistant), + image_attachments: None, + } +} + +fn mcp_tool_bridge_behavior_hint_labels(hints: McpToolBridgeBehaviorHints) -> Vec<&'static str> { + let mut labels = Vec::new(); + if hints.read_only { + labels.push("read-only"); + } + if hints.destructive { + labels.push("destructive"); + } + if hints.open_world { + labels.push("open-world"); + } + labels +} diff --git a/src/crates/execution/tool-contracts/tests/tool_contracts.rs b/src/crates/execution/tool-contracts/tests/tool_contracts.rs index 433bb80a12..8eb5aec491 100644 --- a/src/crates/execution/tool-contracts/tests/tool_contracts.rs +++ b/src/crates/execution/tool-contracts/tests/tool_contracts.rs @@ -1,26 +1,34 @@ +use bitfun_agent_tools::{ + acp_external_agent_tool_input_schema, build_acp_external_agent_tool_definition, + build_acp_external_agent_tool_name, build_acp_external_agent_tool_result, + normalize_name_for_acp_tool_part, render_acp_external_agent_rejected_message, + render_acp_external_agent_result_for_assistant, render_acp_external_agent_result_message, + render_acp_external_agent_use_message, validate_acp_external_agent_tool_input, + AcpExternalAgentToolDefinitionInput, ACP_TOOL_PREFIX, ACP_TOOL_SUFFIX, +}; use bitfun_agent_tools::{ build_bitfun_runtime_uri, build_collapsed_tool_stub_definition, build_get_tool_spec_assistant_detail, build_get_tool_spec_detail_result, build_get_tool_spec_duplicate_load_hint, build_get_tool_spec_duplicate_load_result, build_prompt_visible_tool_manifest_definitions, build_tool_execution_timeout_presentation, build_tool_path_policy_denial_message, build_tool_runtime_artifact_reference, - build_tool_session_runtime_artifact_reference, - collect_loaded_collapsed_tool_names, get_tool_spec_input_schema, - get_tool_spec_is_concurrency_safe, get_tool_spec_is_readonly, get_tool_spec_needs_permissions, - get_tool_spec_short_description, is_bitfun_runtime_uri, is_remote_posix_path_within_root, - is_tool_path_allowed_by_resolved_roots, normalize_host_path, normalize_runtime_relative_path, - parse_bitfun_runtime_uri, posix_resolve_path_with_workspace, posix_style_path_is_absolute, - render_get_tool_spec_tool_use_message, resolve_contextual_tool_manifest, - resolve_contextual_tool_manifest_from_provider, resolve_get_tool_spec_detail, - resolve_get_tool_spec_detail_from_provider, + build_tool_session_runtime_artifact_reference, collect_loaded_collapsed_tool_names, + get_tool_spec_input_schema, get_tool_spec_is_concurrency_safe, get_tool_spec_is_readonly, + get_tool_spec_needs_permissions, get_tool_spec_short_description, is_bitfun_runtime_uri, + is_remote_posix_path_within_root, is_tool_path_allowed_by_resolved_roots, normalize_host_path, + normalize_runtime_relative_path, parse_bitfun_runtime_uri, posix_resolve_path_with_workspace, + posix_style_path_is_absolute, render_get_tool_spec_tool_use_message, + resolve_contextual_tool_manifest, resolve_contextual_tool_manifest_from_provider, + resolve_get_tool_spec_detail, resolve_get_tool_spec_detail_from_provider, resolve_get_tool_spec_execution_result_from_provider, resolve_host_path_with_workspace, resolve_readonly_enabled_tools, resolve_tool_manifest_policy, resolve_tool_path_with_context, resolve_workspace_tool_path, sort_tool_manifest_definitions, summarize_get_tool_spec_collapsed_tools, tool_path_is_effectively_absolute, - validate_collapsed_tool_usage, validate_get_tool_spec_input, validate_tool_allowed_by_list, - validate_tool_execution_admission, DynamicMcpToolInfo, DynamicToolInfo, - GetToolSpecCollapsedToolSummary, GetToolSpecExecutionError, GetToolSpecExecutionPlan, - GetToolSpecLoadObservation, GetToolSpecRuntime, InputValidator, PromptVisibleToolManifestItem, + validate_collapsed_tool_usage, validate_get_tool_spec_input, validate_mcp_tool_bridge_input, + validate_tool_allowed_by_list, validate_tool_execution_admission, DynamicMcpToolInfo, + DynamicToolInfo, GetToolSpecCollapsedToolSummary, GetToolSpecExecutionError, + GetToolSpecExecutionPlan, GetToolSpecLoadObservation, GetToolSpecRuntime, InputValidator, + McpToolBridgeBehaviorHints, McpToolBridgeDefinitionInput, PromptVisibleToolManifestItem, ToolContextFacts, ToolExecutionAdmissionRejection, ToolExecutionAdmissionRequest, ToolExposure, ToolImageAttachment, ToolManifestDefinition, ToolManifestPolicyTool, ToolPathBackend, ToolPathOperation, ToolPathResolution, ToolRenderOptions, ToolResult, ToolRuntimeRestrictions, @@ -36,6 +44,12 @@ use bitfun_agent_tools::{ TOOL_ERROR_ARGUMENTS_PREVIEW_BYTES, USER_REJECTED_TOOL_MESSAGE, USER_STEERING_INTERRUPTED_MESSAGE, }; +use bitfun_agent_tools::{ + build_mcp_tool_bridge_definition, build_mcp_tool_bridge_name, build_mcp_tool_bridge_result, + mcp_tool_bridge_dynamic_tool_info, mcp_tool_bridge_short_description, normalize_name_for_mcp, + render_mcp_tool_bridge_rejected_message, render_mcp_tool_bridge_result_message, + render_mcp_tool_bridge_use_message, MCP_TOOL_DELIMITER, MCP_TOOL_PREFIX, +}; use bitfun_agent_tools::{ build_persisted_tool_output_message, count_tool_result_lines, file_tool_guidance_message, generate_tool_result_preview, is_file_tool_guidance_message, @@ -74,6 +88,238 @@ impl StaticToolProviderPlan for TestProviderPlan { } } +#[test] +fn mcp_tool_bridge_preserves_prompt_visible_name_and_descriptor_contract() { + assert_eq!(MCP_TOOL_PREFIX, "mcp__"); + assert_eq!(MCP_TOOL_DELIMITER, "__"); + assert_eq!( + normalize_name_for_mcp("Acme Search / Primary"), + "Acme_Search___Primary" + ); + assert_eq!( + build_mcp_tool_bridge_name("Claude Code", "search repos"), + "mcp__Claude_Code__search_repos" + ); + + let definition = build_mcp_tool_bridge_definition(McpToolBridgeDefinitionInput { + server_id: "github", + server_name: "GitHub", + tool_name: "search", + title: "Search Docs", + description: Some("Find docs"), + behavior_hints: McpToolBridgeBehaviorHints { + read_only: true, + destructive: false, + open_world: true, + }, + }); + + assert_eq!(definition.full_name, "mcp__github__search"); + assert_eq!(definition.user_facing_name, "Search Docs (GitHub)"); + assert_eq!( + definition.description, + "Tool 'Search Docs' from MCP server 'GitHub': Find docs [Hints: read-only, open-world]" + ); + assert_eq!(definition.provider_id, "github"); + assert_eq!(definition.provider_kind, "mcp"); + assert!(definition.read_only); + assert_eq!( + serde_json::to_value(definition.tool_info.clone()).unwrap(), + json!({ + "server_id": "github", + "server_name": "GitHub", + "tool_name": "search" + }) + ); + + assert_eq!( + mcp_tool_bridge_short_description(Some("Find docs"), "GitHub"), + "Find docs (GitHub)" + ); + assert_eq!( + mcp_tool_bridge_short_description(Some(" "), "GitHub"), + "MCP tool (GitHub)" + ); +} + +#[test] +fn mcp_tool_bridge_preserves_dynamic_info_validation_and_rendering_contract() { + let definition = build_mcp_tool_bridge_definition(McpToolBridgeDefinitionInput { + server_id: "github", + server_name: "GitHub", + tool_name: "search", + title: "Search Docs", + description: Some("Find docs"), + behavior_hints: McpToolBridgeBehaviorHints { + read_only: false, + destructive: true, + open_world: false, + }, + }); + + assert_eq!( + mcp_tool_bridge_dynamic_tool_info(&definition), + DynamicToolInfo { + provider_id: "github".to_string(), + provider_kind: Some("mcp".to_string()), + mcp: Some(DynamicMcpToolInfo { + server_id: "github".to_string(), + server_name: "GitHub".to_string(), + tool_name: "search".to_string(), + }), + } + ); + + assert!(validate_mcp_tool_bridge_input(&json!({ "q": "rust" }), "GitHub", false).result); + let non_object = validate_mcp_tool_bridge_input(&json!("rust"), "GitHub", false); + assert!(!non_object.result); + assert_eq!(non_object.error_code, Some(400)); + assert_eq!( + non_object.message.as_deref(), + Some("Input must be an object") + ); + + let remote_blocked = validate_mcp_tool_bridge_input(&json!({}), "GitHub", true); + assert!(!remote_blocked.result); + assert_eq!( + remote_blocked.message.as_deref(), + Some("MCP server 'GitHub' runs locally and is unavailable in remote workspace sessions") + ); + + assert_eq!( + render_mcp_tool_bridge_use_message("Search Docs", "GitHub", &json!({ "q": "rust" })), + "Using MCP tool 'Search Docs' from 'GitHub' with input: {\"q\":\"rust\"}" + ); + assert_eq!( + render_mcp_tool_bridge_rejected_message("Search Docs", "GitHub"), + "MCP tool 'Search Docs' from 'GitHub' was rejected by user" + ); + assert_eq!( + render_mcp_tool_bridge_result_message("Search Docs", "done"), + "MCP tool 'Search Docs' completed. Result: done" + ); + + let ToolResult::Result { + data, + result_for_assistant, + image_attachments, + } = build_mcp_tool_bridge_result(json!({ "ok": true }), "done".to_string()) + else { + panic!("MCP bridge result must use standard result shape"); + }; + assert_eq!(data, json!({ "ok": true })); + assert_eq!(result_for_assistant.as_deref(), Some("done")); + assert!(image_attachments.is_none()); +} + +#[test] +fn acp_external_agent_bridge_preserves_tool_contract() { + assert_eq!(ACP_TOOL_PREFIX, "acp__"); + assert_eq!(ACP_TOOL_SUFFIX, "__prompt"); + assert_eq!( + normalize_name_for_acp_tool_part("Claude Code"), + "Claude_Code" + ); + assert_eq!( + build_acp_external_agent_tool_name("Claude Code"), + "acp__Claude_Code__prompt" + ); + + let definition = + build_acp_external_agent_tool_definition(AcpExternalAgentToolDefinitionInput { + client_id: "codex", + display_name: Some("Codex"), + read_only: false, + }); + + assert_eq!(definition.tool_name, "acp__codex__prompt"); + assert_eq!(definition.display_name, "Codex"); + assert_eq!(definition.user_facing_name, "Codex (ACP)"); + assert_eq!( + definition.description, + "Send a prompt to the external ACP agent 'Codex'. Use this when another local ACP-compatible agent is better suited for a delegated task." + ); + assert_eq!( + definition.short_description, + "Delegate a task to the external ACP agent 'Codex'." + ); + assert!(!definition.read_only); + + assert_eq!( + acp_external_agent_tool_input_schema(), + json!({ + "type": "object", + "properties": { + "prompt": { + "type": "string", + "description": "The task or question to send to the external ACP agent." + }, + "workspace_path": { + "type": "string", + "description": "Optional absolute workspace path. Defaults to the current BitFun workspace." + }, + "timeout_seconds": { + "type": "integer", + "minimum": 0, + "description": "Optional timeout in seconds. Use 0 or omit it to wait without a fixed timeout." + } + }, + "required": ["prompt"], + "additionalProperties": false + }) + ); + + let missing_prompt = validate_acp_external_agent_tool_input(&json!({})); + assert!(!missing_prompt.result); + assert_eq!(missing_prompt.error_code, Some(400)); + assert_eq!( + missing_prompt.message.as_deref(), + Some("prompt is required") + ); + + let empty_prompt = validate_acp_external_agent_tool_input(&json!({ "prompt": " " })); + assert!(!empty_prompt.result); + assert_eq!( + empty_prompt.message.as_deref(), + Some("prompt cannot be empty") + ); + assert!(validate_acp_external_agent_tool_input(&json!({ "prompt": "hello" })).result); + + let long_prompt = "a".repeat(161); + assert_eq!( + render_acp_external_agent_use_message("Codex", &json!({ "prompt": long_prompt })), + format!("Sending ACP prompt to 'Codex': {}...", "a".repeat(160)) + ); + assert_eq!( + render_acp_external_agent_rejected_message("Codex"), + "ACP prompt to 'Codex' was rejected" + ); + assert_eq!( + render_acp_external_agent_result_message("Codex", &json!({ "response": "done" })), + "ACP agent 'Codex' responded:\ndone" + ); + assert_eq!( + render_acp_external_agent_result_message("Codex", &json!({})), + "ACP agent 'Codex' completed" + ); + assert_eq!( + render_acp_external_agent_result_for_assistant(&json!({})), + "ACP agent completed without text output" + ); + + let ToolResult::Result { + data, + result_for_assistant, + image_attachments, + } = build_acp_external_agent_tool_result("codex", "done") + else { + panic!("ACP bridge result must use standard result shape"); + }; + assert_eq!(data, json!({ "client_id": "codex", "response": "done" })); + assert_eq!(result_for_assistant.as_deref(), Some("done")); + assert!(image_attachments.is_none()); +} + #[test] fn validation_result_default_preserves_success_contract() { assert!(ValidationResult::default().result); @@ -191,12 +437,12 @@ fn tool_execution_timeout_presentation_includes_timeout_seconds() { assert_eq!(presentation.result_json["category"], "execution_timeout"); assert_eq!(presentation.result_json["tool_name"], "ExecCommand"); assert_eq!(presentation.result_json["timeout_seconds"], 120); - assert!( - presentation - .result_for_assistant - .contains("This tool call was cancelled because the global tool execution time limit (120 seconds)") - ); - assert!(!presentation.result_for_assistant.contains("Provided arguments")); + assert!(presentation.result_for_assistant.contains( + "This tool call was cancelled because the global tool execution time limit (120 seconds)" + )); + assert!(!presentation + .result_for_assistant + .contains("Provided arguments")); assert!(!presentation.result_for_assistant.contains("failed")); } diff --git a/src/crates/interfaces/acp/AGENTS-CN.md b/src/crates/interfaces/acp/AGENTS-CN.md index 557838dcf6..0850fe62da 100644 --- a/src/crates/interfaces/acp/AGENTS-CN.md +++ b/src/crates/interfaces/acp/AGENTS-CN.md @@ -10,6 +10,7 @@ - Remote ACP workspace 复用本地 ACP client 配置。修改 ACP client 行为时，必须保持 manager、remote shell probing、remote capability store 和 workspace menu availability 语义。 - ACP config persistence、remote probing、timeout policy 和 workspace surface selection 属于 ACP / app-surface 行为，不要移动到 `core-types`、`runtime-ports` 或 `agent-tools`。 +- ACP external-agent tool 的命名、schema、validation、presentation 和 result shape 属于 `bitfun-agent-tools` 的 portable contract；ACP 应调用这些 helper，不要在本层重复定义。 - 如果未来需要 contract，只表达观测事实：environment identity、capability facts、request / response DTO。 ## 验证 diff --git a/src/crates/interfaces/acp/AGENTS.md b/src/crates/interfaces/acp/AGENTS.md index 4b082d7fc1..143797ce75 100644 --- a/src/crates/interfaces/acp/AGENTS.md +++ b/src/crates/interfaces/acp/AGENTS.md @@ -16,6 +16,9 @@ share only stable capability facts through contract crates. - ACP config persistence, remote probing, timeout policy, and workspace surface selection are ACP/app-surface behavior. Do not move them into `core-types`, `runtime-ports`, or `agent-tools`. +- ACP external-agent tool naming, schema, validation, presentation, and result + shape are portable contracts owned by `bitfun-agent-tools`; ACP should call + those helpers instead of redefining them locally. - If a future contract is needed, make it observational: environment identity, capability facts, and request/response DTOs only. diff --git a/src/crates/interfaces/acp/Cargo.toml b/src/crates/interfaces/acp/Cargo.toml index 35cebf8626..520b1ad661 100644 --- a/src/crates/interfaces/acp/Cargo.toml +++ b/src/crates/interfaces/acp/Cargo.toml @@ -10,6 +10,7 @@ name = "bitfun_acp" [dependencies] bitfun-core = { path = "../../assembly/core", default-features = false, features = ["product-full"] } +bitfun-agent-tools = { path = "../../execution/tool-contracts" } bitfun-events = { path = "../../contracts/events" } agent-client-protocol = { workspace = true } diff --git a/src/crates/interfaces/acp/src/client/manager.rs b/src/crates/interfaces/acp/src/client/manager.rs index fa2f9d599c..7859e48f46 100644 --- a/src/crates/interfaces/acp/src/client/manager.rs +++ b/src/crates/interfaces/acp/src/client/manager.rs @@ -18,6 +18,7 @@ use agent_client_protocol::schema::{ use agent_client_protocol::{ ActiveSession, Agent, ByteStreams, Client, ConnectionTo, Error, SessionMessage, }; +use bitfun_agent_tools::ACP_TOOL_PREFIX; use bitfun_core::agentic::tools::registry::get_global_tool_registry; use bitfun_core::infrastructure::events::{emit_global_event, BackendEvent}; use bitfun_core::infrastructure::PathManager; @@ -1492,7 +1493,7 @@ impl AcpClientService { ) { let registry = get_global_tool_registry(); let mut registry = registry.write().await; - registry.unregister_tools_by_prefix("acp__"); + registry.unregister_tools_by_prefix(ACP_TOOL_PREFIX); let tools = configs .iter() diff --git a/src/crates/interfaces/acp/src/client/tool.rs b/src/crates/interfaces/acp/src/client/tool.rs index d25257eeca..cbb56a49eb 100644 --- a/src/crates/interfaces/acp/src/client/tool.rs +++ b/src/crates/interfaces/acp/src/client/tool.rs @@ -1,94 +1,81 @@ use std::sync::Arc; use async_trait::async_trait; -use bitfun_core::agentic::tools::framework::{ - Tool, ToolRenderOptions, ToolResult, ToolUseContext, ValidationResult, +use bitfun_agent_tools::{ + acp_external_agent_tool_input_schema, build_acp_external_agent_tool_definition, + build_acp_external_agent_tool_name, build_acp_external_agent_tool_result, + render_acp_external_agent_rejected_message, render_acp_external_agent_result_for_assistant, + render_acp_external_agent_result_message, render_acp_external_agent_use_message, + validate_acp_external_agent_tool_input, AcpExternalAgentToolDefinition, + AcpExternalAgentToolDefinitionInput, ToolResult, ValidationResult, }; +use bitfun_core::agentic::tools::framework::{Tool, ToolRenderOptions, ToolUseContext}; use bitfun_core::util::errors::{BitFunError, BitFunResult}; -use serde_json::{json, Value}; +use serde_json::Value; use super::config::AcpClientConfig; use super::manager::AcpClientService; pub struct AcpAgentTool { client_id: String, - config: AcpClientConfig, service: Arc, - full_name: String, + definition: AcpExternalAgentToolDefinition, } impl AcpAgentTool { pub fn new(client_id: String, config: AcpClientConfig, service: Arc) -> Self { - let full_name = Self::tool_name_for(&client_id); + let definition = acp_external_agent_definition_for_config(&client_id, &config); Self { client_id, - config, service, - full_name, + definition, } } pub fn tool_name_for(client_id: &str) -> String { - format!("acp__{}__prompt", sanitize_tool_part(client_id)) + build_acp_external_agent_tool_name(client_id) } - fn display_name(&self) -> String { - self.config - .name - .clone() - .unwrap_or_else(|| self.client_id.clone()) + fn display_name(&self) -> &str { + &self.definition.display_name } } +pub fn acp_external_agent_definition_for_config( + client_id: &str, + config: &AcpClientConfig, +) -> AcpExternalAgentToolDefinition { + build_acp_external_agent_tool_definition(AcpExternalAgentToolDefinitionInput { + client_id, + display_name: config.name.as_deref(), + read_only: config.readonly, + }) +} + #[async_trait] impl Tool for AcpAgentTool { fn name(&self) -> &str { - &self.full_name + &self.definition.tool_name } async fn description(&self) -> BitFunResult { - Ok(format!( - "Send a prompt to the external ACP agent '{}'. Use this when another local ACP-compatible agent is better suited for a delegated task.", - self.display_name() - )) + Ok(self.definition.description.clone()) } fn short_description(&self) -> String { - format!( - "Delegate a task to the external ACP agent '{}'.", - self.display_name() - ) + self.definition.short_description.clone() } fn input_schema(&self) -> Value { - json!({ - "type": "object", - "properties": { - "prompt": { - "type": "string", - "description": "The task or question to send to the external ACP agent." - }, - "workspace_path": { - "type": "string", - "description": "Optional absolute workspace path. Defaults to the current BitFun workspace." - }, - "timeout_seconds": { - "type": "integer", - "minimum": 0, - "description": "Optional timeout in seconds. Use 0 or omit it to wait without a fixed timeout." - } - }, - "required": ["prompt"], - "additionalProperties": false - }) + acp_external_agent_tool_input_schema() } fn user_facing_name(&self) -> String { - format!("{} (ACP)", self.display_name()) + self.definition.user_facing_name.clone() } fn is_readonly(&self) -> bool { - self.config.readonly + self.definition.read_only } fn is_concurrency_safe(&self, _input: Option<&Value>) -> bool { @@ -96,7 +83,7 @@ impl Tool for AcpAgentTool { } fn needs_permissions(&self, _input: Option<&Value>) -> bool { - !self.config.readonly + !self.definition.read_only } async fn validate_input( @@ -104,60 +91,23 @@ impl Tool for AcpAgentTool { input: &Value, _context: Option<&ToolUseContext>, ) -> ValidationResult { - match input.get("prompt").and_then(|value| value.as_str()) { - Some(prompt) if !prompt.trim().is_empty() => ValidationResult::default(), - Some(_) => ValidationResult { - result: false, - message: Some("prompt cannot be empty".to_string()), - error_code: Some(400), - meta: None, - }, - None => ValidationResult { - result: false, - message: Some("prompt is required".to_string()), - error_code: Some(400), - meta: None, - }, - } + validate_acp_external_agent_tool_input(input) } fn render_tool_use_message(&self, input: &Value, _options: &ToolRenderOptions) -> String { - let prompt_preview = input - .get("prompt") - .and_then(|value| value.as_str()) - .map(truncate_prompt) - .unwrap_or_else(|| "prompt".to_string()); - format!( - "Sending ACP prompt to '{}': {}", - self.display_name(), - prompt_preview - ) + render_acp_external_agent_use_message(self.display_name(), input) } fn render_tool_use_rejected_message(&self) -> String { - format!("ACP prompt to '{}' was rejected", self.display_name()) + render_acp_external_agent_rejected_message(self.display_name()) } fn render_tool_result_message(&self, output: &Value) -> String { - output - .get("response") - .and_then(|value| value.as_str()) - .map(|response| { - format!( - "ACP agent '{}' responded:\n{}", - self.display_name(), - response - ) - }) - .unwrap_or_else(|| format!("ACP agent '{}' completed", self.display_name())) + render_acp_external_agent_result_message(self.display_name(), output) } fn render_result_for_assistant(&self, output: &Value) -> String { - output - .get("response") - .and_then(|value| value.as_str()) - .unwrap_or("ACP agent completed without text output") - .to_string() + render_acp_external_agent_result_for_assistant(output) } async fn call_impl( @@ -201,37 +151,45 @@ impl Tool for AcpAgentTool { ) .await?; - let data = json!({ - "client_id": self.client_id, - "response": response, - }); - Ok(vec![ToolResult::Result { - result_for_assistant: Some(self.render_result_for_assistant(&data)), - data, - image_attachments: None, - }]) + Ok(vec![build_acp_external_agent_tool_result( + &self.client_id, + response, + )]) } } -fn sanitize_tool_part(value: &str) -> String { - let sanitized = value - .chars() - .map(|ch| { - if ch.is_ascii_alphanumeric() || ch == '-' || ch == '_' { - ch - } else { - '_' - } - }) - .collect::(); - sanitized.trim_matches('_').to_string() -} - -fn truncate_prompt(prompt: &str) -> String { - const LIMIT: usize = 160; - if prompt.chars().count() <= LIMIT { - prompt.to_string() - } else { - format!("{}...", prompt.chars().take(LIMIT).collect::()) +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use super::*; + use crate::client::config::AcpClientPermissionMode; + + #[test] + fn acp_agent_tool_name_preserves_current_prompt_visible_shape() { + assert_eq!( + AcpAgentTool::tool_name_for("Claude Code"), + "acp__Claude_Code__prompt" + ); + } + + #[test] + fn acp_agent_definition_for_config_preserves_tool_contract() { + let config = AcpClientConfig { + name: Some("Codex".to_string()), + command: "codex".to_string(), + args: Vec::new(), + env: HashMap::new(), + enabled: true, + readonly: true, + permission_mode: AcpClientPermissionMode::Ask, + }; + + let definition = acp_external_agent_definition_for_config("codex", &config); + + assert_eq!(definition.tool_name, "acp__codex__prompt"); + assert_eq!(definition.display_name, "Codex"); + assert_eq!(definition.user_facing_name, "Codex (ACP)"); + assert!(definition.read_only); } } diff --git a/src/crates/services/services-integrations/AGENTS.md b/src/crates/services/services-integrations/AGENTS.md index ecd451a8d8..25a8e3ca00 100644 --- a/src/crates/services/services-integrations/AGENTS.md +++ b/src/crates/services/services-integrations/AGENTS.md @@ -13,10 +13,11 @@ slices that are outside pure product logic but still platform-neutral. should not compile heavy Git, MCP, SSH, network, or file-watch runtimes. Boundary checks enforce `default = []` and the current `product-full` integration feature-group list. -- MCP config/process/transport lifecycle and dynamic provider helpers may live - here; product tool registry assembly, manifest filtering, `GetToolSpec` - execution, and concrete tool behavior remain outside this crate unless a - reviewed owner move proves behavior equivalence. +- MCP config/process/transport lifecycle and protocol result-content rendering + live here; MCP wire types may be projected into execution-owned tool bridge + descriptors. Product tool registry assembly, manifest filtering, + `GetToolSpec` execution, and bridge presentation/validation behavior remain + outside this crate unless a reviewed owner move proves behavior equivalence. - Remote-connect platform-neutral primitives belong here: device identity, pairing/encryption, QR payload generation, relay client protocol, dialog/cancel orchestration ports, image-context adapter contracts, remote workspace helpers, diff --git a/src/crates/services/services-integrations/Cargo.toml b/src/crates/services/services-integrations/Cargo.toml index 412d5515ea..1db4886526 100644 --- a/src/crates/services/services-integrations/Cargo.toml +++ b/src/crates/services/services-integrations/Cargo.toml @@ -15,6 +15,7 @@ serde = { workspace = true } serde_json = { workspace = true } log = { workspace = true } bitfun-agent-runtime = { path = "../../execution/agent-runtime", optional = true } +bitfun-agent-tools = { path = "../../execution/tool-contracts", optional = true } bitfun-events = { path = "../../contracts/events" } bitfun-product-domains = { path = "../../contracts/product-domains", default-features = false, optional = true } bitfun-runtime-ports = { path = "../../contracts/runtime-ports", optional = true } @@ -76,6 +77,7 @@ mcp = [ "anyhow", "async-trait", "base64", + "bitfun-agent-tools", "bitfun-services-core", "futures", "rand", diff --git a/src/crates/services/services-integrations/src/mcp/adapter/tool.rs b/src/crates/services/services-integrations/src/mcp/adapter/tool.rs index 9b6b308da3..5596e518e9 100644 --- a/src/crates/services/services-integrations/src/mcp/adapter/tool.rs +++ b/src/crates/services/services-integrations/src/mcp/adapter/tool.rs @@ -1,20 +1,14 @@ //! MCP dynamic tool metadata and result rendering helpers. use crate::mcp::protocol::{MCPTool, MCPToolResult, MCPToolResultContent}; -use crate::mcp::{build_mcp_tool_name, MCPRuntimeResult, McpToolInfo}; +use crate::mcp::MCPRuntimeResult; use async_trait::async_trait; +use bitfun_agent_tools::{ + build_mcp_tool_bridge_definition, McpToolBridgeBehaviorHints, McpToolBridgeDefinition, + McpToolBridgeDefinitionInput, +}; -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct McpDynamicToolDescriptor { - pub full_name: String, - pub title: String, - pub user_facing_name: String, - pub description: String, - pub provider_id: String, - pub provider_kind: String, - pub tool_info: McpToolInfo, - pub read_only: bool, -} +pub type McpDynamicToolDescriptor = McpToolBridgeDefinition; #[derive(Debug, Clone)] pub struct MCPDynamicToolDefinition { @@ -69,57 +63,31 @@ fn tool_title(tool: &MCPTool) -> String { .unwrap_or_else(|| tool.name.clone()) } -fn behavior_hints(tool: &MCPTool) -> Vec<&'static str> { - let annotations = tool.annotations.clone().unwrap_or_default(); - let mut hints = Vec::new(); - if annotations.read_only_hint.unwrap_or(false) { - hints.push("read-only"); - } - if annotations.destructive_hint.unwrap_or(false) { - hints.push("destructive"); - } - if annotations.open_world_hint.unwrap_or(false) { - hints.push("open-world"); - } - hints -} - pub fn build_mcp_tool_descriptor( server_id: &str, server_name: &str, tool: &MCPTool, ) -> McpDynamicToolDescriptor { let title = tool_title(tool); - let mut description = format!( - "Tool '{}' from MCP server '{}': {}", - title, + let annotations = tool.annotations.as_ref(); + build_mcp_tool_bridge_definition(McpToolBridgeDefinitionInput { + server_id, server_name, - tool.description.as_deref().unwrap_or("") - ); - - let hints = behavior_hints(tool); - if !hints.is_empty() { - description.push_str(&format!(" [Hints: {}]", hints.join(", "))); - } - - McpDynamicToolDescriptor { - full_name: build_mcp_tool_name(server_id, &tool.name), - title: title.clone(), - user_facing_name: format!("{} ({})", title, server_name), - description, - provider_id: server_id.to_string(), - provider_kind: "mcp".to_string(), - tool_info: McpToolInfo { - server_id: server_id.to_string(), - server_name: server_name.to_string(), - tool_name: tool.name.clone(), + tool_name: &tool.name, + title: &title, + description: tool.description.as_deref(), + behavior_hints: McpToolBridgeBehaviorHints { + read_only: annotations + .and_then(|annotations| annotations.read_only_hint) + .unwrap_or(false), + destructive: annotations + .and_then(|annotations| annotations.destructive_hint) + .unwrap_or(false), + open_world: annotations + .and_then(|annotations| annotations.open_world_hint) + .unwrap_or(false), }, - read_only: tool - .annotations - .as_ref() - .and_then(|annotations| annotations.read_only_hint) - .unwrap_or(false), - } + }) } fn truncate_for_assistant(text: String, max_result_text_chars: usize) -> String { diff --git a/src/crates/services/services-integrations/src/mcp/tool_info.rs b/src/crates/services/services-integrations/src/mcp/tool_info.rs index 35f73b2e78..913c73c2e1 100644 --- a/src/crates/services/services-integrations/src/mcp/tool_info.rs +++ b/src/crates/services/services-integrations/src/mcp/tool_info.rs @@ -1,8 +1,3 @@ -use serde::{Deserialize, Serialize}; +//! Compatibility export for MCP dynamic-tool metadata. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -pub struct McpToolInfo { - pub server_id: String, - pub server_name: String, - pub tool_name: String, -} +pub use bitfun_agent_tools::McpToolBridgeToolInfo as McpToolInfo; diff --git a/src/crates/services/services-integrations/src/mcp/tool_name.rs b/src/crates/services/services-integrations/src/mcp/tool_name.rs index a626e189b7..7fde88923a 100644 --- a/src/crates/services/services-integrations/src/mcp/tool_name.rs +++ b/src/crates/services/services-integrations/src/mcp/tool_name.rs @@ -1,56 +1,6 @@ -//! Shared MCP tool-name helpers. +//! Compatibility exports for MCP dynamic-tool names. -pub const MCP_TOOL_PREFIX: &str = "mcp__"; -pub const MCP_TOOL_DELIMITER: &str = "__"; - -/// Normalize MCP server/tool names to a wire-safe format aligned with claude-code. -pub fn normalize_name_for_mcp(name: &str) -> String { - name.chars() - .map(|ch| { - if ch.is_ascii_alphanumeric() || ch == '_' || ch == '-' { - ch - } else { - '_' - } - }) - .collect() -} - -pub fn build_mcp_tool_name(server_id: &str, tool_name: &str) -> String { - format!( - "{}{}{}{}", - MCP_TOOL_PREFIX, - normalize_name_for_mcp(server_id), - MCP_TOOL_DELIMITER, - normalize_name_for_mcp(tool_name) - ) -} - -#[cfg(test)] -mod tests { - use super::{build_mcp_tool_name, normalize_name_for_mcp}; - - #[test] - fn normalize_name_for_mcp_replaces_spaces_and_symbols() { - assert_eq!( - normalize_name_for_mcp("Acme Search / Primary"), - "Acme_Search___Primary" - ); - } - - #[test] - fn normalize_name_for_mcp_keeps_ascii_word_chars_and_hyphen() { - assert_eq!( - normalize_name_for_mcp("github-enterprise_v2"), - "github-enterprise_v2" - ); - } - - #[test] - fn build_mcp_tool_name_normalizes_both_segments() { - assert_eq!( - build_mcp_tool_name("Claude Code", "search repos"), - "mcp__Claude_Code__search_repos" - ); - } -} +pub use bitfun_agent_tools::{ + build_mcp_tool_bridge_name as build_mcp_tool_name, normalize_name_for_mcp, MCP_TOOL_DELIMITER, + MCP_TOOL_PREFIX, +}; diff --git a/src/crates/services/services-integrations/tests/mcp_contracts.rs b/src/crates/services/services-integrations/tests/mcp_contracts.rs index 453cdef22f..863e88a9ff 100644 --- a/src/crates/services/services-integrations/tests/mcp_contracts.rs +++ b/src/crates/services/services-integrations/tests/mcp_contracts.rs @@ -198,31 +198,25 @@ fn mcp_remote_client_info_declares_supported_client_capabilities() { #[test] fn mcp_rmcp_initialize_mapping_preserves_server_identity_and_capabilities() { - let server_info = rmcp::model::ServerInfo { - protocol_version: rmcp::model::ProtocolVersion::LATEST, - capabilities: rmcp::model::ServerCapabilities { - tools: Some(rmcp::model::ToolsCapability { - list_changed: Some(true), - }), - resources: Some(rmcp::model::ResourcesCapability { - subscribe: Some(true), - list_changed: Some(false), - }), - prompts: Some(rmcp::model::PromptsCapability { - list_changed: Some(true), - }), - logging: Some(rmcp::model::JsonObject::new()), - ..Default::default() - }, - server_info: rmcp::model::Implementation { - name: "docs-server".to_string(), - title: Some("Docs Server".to_string()), - version: "2.0.0".to_string(), - icons: None, - website_url: None, - }, - instructions: Some("Fallback description".to_string()), - }; + let mut capabilities = rmcp::model::ServerCapabilities::default(); + capabilities.tools = Some(rmcp::model::ToolsCapability { + list_changed: Some(true), + }); + capabilities.resources = Some(rmcp::model::ResourcesCapability { + subscribe: Some(true), + list_changed: Some(false), + }); + capabilities.prompts = Some(rmcp::model::PromptsCapability { + list_changed: Some(true), + }); + capabilities.logging = Some(rmcp::model::JsonObject::new()); + + let server_info = rmcp::model::ServerInfo::new(capabilities) + .with_protocol_version(rmcp::model::ProtocolVersion::LATEST) + .with_server_info( + rmcp::model::Implementation::new("docs-server", "2.0.0").with_title("Docs Server"), + ) + .with_instructions("Fallback description"); let mapped = map_rmcp_initialize_result(&server_info); @@ -262,29 +256,23 @@ fn mcp_rmcp_mapping_preserves_remote_tool_resource_and_prompt_metadata() { "ui".to_string(), serde_json::json!({ "resourceUri": "ui://widget" }), ); - let tool = rmcp::model::Tool { - name: "search".into(), - title: Some("Search".to_string()), - description: Some("Find items".into()), - input_schema: Arc::new(serde_json::Map::new()), - output_schema: Some(Arc::new(serde_json::Map::from_iter([( - "type".to_string(), - serde_json::json!("object"), - )]))), - annotations: Some( - rmcp::model::ToolAnnotations::new() - .read_only(true) - .destructive(false) - .idempotent(true) - .open_world(true), - ), - icons: Some(vec![Icon { - src: "https://example.com/tool.png".to_string(), - mime_type: Some("image/png".to_string()), - sizes: Some(vec!["32x32".to_string()]), - }]), - meta: Some(tool_meta), - }; + let mut tool = rmcp::model::Tool::new("search", "Find items", serde_json::Map::new()); + tool.title = Some("Search".to_string()); + tool.output_schema = Some(Arc::new(serde_json::Map::from_iter([( + "type".to_string(), + serde_json::json!("object"), + )]))); + tool.annotations = Some( + rmcp::model::ToolAnnotations::new() + .read_only(true) + .destructive(false) + .idempotent(true) + .open_world(true), + ); + tool.icons = Some(vec![Icon::new("https://example.com/tool.png") + .with_mime_type("image/png") + .with_sizes(vec!["32x32".to_string()])]); + tool.meta = Some(tool_meta); let mapped_tool = map_rmcp_tool(tool); assert_eq!(mapped_tool.title.as_deref(), Some("Search")); assert_eq!( @@ -316,17 +304,16 @@ fn mcp_rmcp_mapping_preserves_remote_tool_resource_and_prompt_metadata() { description: Some("Report".to_string()), mime_type: Some("text/markdown".to_string()), size: Some(42), - icons: Some(vec![Icon { - src: "https://example.com/resource.png".to_string(), - mime_type: Some("image/png".to_string()), - sizes: Some(vec!["64x64".to_string()]), - }]), + icons: Some(vec![Icon::new("https://example.com/resource.png") + .with_mime_type("image/png") + .with_sizes(vec!["64x64".to_string()])]), meta: Some(resource_meta), } - .annotate(Annotations { - audience: Some(vec![rmcp::model::Role::User]), - priority: Some(0.9), - last_modified: None, + .annotate({ + let mut annotations = Annotations::default(); + annotations.audience = Some(vec![rmcp::model::Role::User]); + annotations.priority = Some(0.9); + annotations }); let mapped_resource = map_rmcp_resource(resource); assert_eq!(mapped_resource.title.as_deref(), Some("Quarterly Report")); @@ -347,23 +334,18 @@ fn mcp_rmcp_mapping_preserves_remote_tool_resource_and_prompt_metadata() { Some(&serde_json::json!("catalog")) ); - let prompt = rmcp::model::Prompt { - name: "summarize".to_string(), - title: Some("Summarize".to_string()), - description: Some("Summarize content".to_string()), - arguments: Some(vec![rmcp::model::PromptArgument { - name: "topic".to_string(), - title: Some("Topic".to_string()), - description: Some("Topic to summarize".to_string()), - required: Some(true), - }]), - icons: Some(vec![Icon { - src: "https://example.com/prompt.png".to_string(), - mime_type: Some("image/png".to_string()), - sizes: Some(vec!["16x16".to_string()]), - }]), - meta: None, - }; + let prompt = rmcp::model::Prompt::new( + "summarize", + Some("Summarize content"), + Some(vec![rmcp::model::PromptArgument::new("topic") + .with_title("Topic") + .with_description("Topic to summarize") + .with_required(true)]), + ) + .with_title("Summarize") + .with_icons(vec![Icon::new("https://example.com/prompt.png") + .with_mime_type("image/png") + .with_sizes(vec!["16x16".to_string()])]); let mapped_prompt = map_rmcp_prompt(prompt); assert_eq!(mapped_prompt.title.as_deref(), Some("Summarize")); assert_eq!( @@ -391,16 +373,13 @@ fn mcp_rmcp_mapping_preserves_structured_results_and_resource_links() { }; let mut result_meta = Meta::default(); result_meta.insert("traceId".to_string(), serde_json::json!("abc123")); - let result = rmcp::model::CallToolResult { - content: vec![ - Content::text("done"), - Content::resource_link(resource_link), - Content::image("aGVsbG8=", "image/png"), - ], - structured_content: Some(serde_json::json!({ "ok": true })), - is_error: Some(false), - meta: Some(result_meta), - }; + let mut result = rmcp::model::CallToolResult::success(vec![ + Content::text("done"), + Content::resource_link(resource_link), + Content::image("aGVsbG8=", "image/png"), + ]); + result.structured_content = Some(serde_json::json!({ "ok": true })); + result.meta = Some(result_meta); let mapped = map_rmcp_tool_result(result); @@ -424,12 +403,8 @@ fn mcp_rmcp_mapping_preserves_structured_results_and_resource_links() { #[test] fn mcp_rmcp_mapping_preserves_prompt_message_blocks() { - let prompt_message = rmcp::model::PromptMessage { - role: rmcp::model::PromptMessageRole::User, - content: rmcp::model::PromptMessageContent::Text { - text: "hello".to_string(), - }, - }; + let prompt_message = + rmcp::model::PromptMessage::new_text(rmcp::model::PromptMessageRole::User, "hello"); let mapped = map_rmcp_prompt_message(prompt_message); assert!(matches!( mapped.content, @@ -448,12 +423,10 @@ fn mcp_rmcp_mapping_preserves_prompt_message_blocks() { meta: None, } .no_annotation(); - let prompt_message = rmcp::model::PromptMessage { - role: rmcp::model::PromptMessageRole::Assistant, - content: rmcp::model::PromptMessageContent::ResourceLink { - link: resource_link, - }, - }; + let prompt_message = rmcp::model::PromptMessage::new( + rmcp::model::PromptMessageRole::Assistant, + rmcp::model::PromptMessageContent::resource_link(resource_link), + ); let mapped = map_rmcp_prompt_message(prompt_message); assert!(matches!( mapped.content, @@ -475,10 +448,10 @@ fn mcp_rmcp_mapping_preserves_prompt_message_blocks() { }, } .no_annotation(); - let prompt_message = rmcp::model::PromptMessage { - role: rmcp::model::PromptMessageRole::Assistant, - content: rmcp::model::PromptMessageContent::Resource { resource: embedded }, - }; + let prompt_message = rmcp::model::PromptMessage::new( + rmcp::model::PromptMessageRole::Assistant, + rmcp::model::PromptMessageContent::Resource { resource: embedded }, + ); let mapped = map_rmcp_prompt_message(prompt_message); assert!(matches!( mapped.content, @@ -1552,10 +1525,7 @@ async fn mcp_oauth_credential_vault_uses_injected_data_dir_and_roundtrips_creden )); let vault = MCPRemoteOAuthCredentialVault::new(data_dir.clone()); - let credentials = StoredCredentials { - client_id: "client-123".to_string(), - token_response: None, - }; + let credentials = StoredCredentials::new("client-123".to_string(), None, Vec::new(), None); vault .store("server-a", &credentials)