From fe724e8aa70d4fee4c05aa25aed83a623dfe053c Mon Sep 17 00:00:00 2001 From: limityan Date: Tue, 19 May 2026 17:56:51 +0800 Subject: [PATCH] refactor(tools): harden H1 migration baselines --- docs/plans/core-decomposition-plan.md | 17 +++- scripts/check-core-boundaries.mjs | 22 +++++ src/crates/agent-tools/src/framework.rs | 34 ++++++- src/crates/agent-tools/src/lib.rs | 14 +-- .../agent-tools/tests/tool_contracts.rs | 53 +++++++++-- .../src/agentic/execution/execution_engine.rs | 65 +++++++------- .../src/agentic/tools/manifest_resolver.rs | 24 +++++ .../agentic/tools/pipeline/tool_pipeline.rs | 89 ++++++++++++++++--- src/crates/core/src/agentic/tools/registry.rs | 31 +++++++ 9 files changed, 288 insertions(+), 61 deletions(-) diff --git a/docs/plans/core-decomposition-plan.md b/docs/plans/core-decomposition-plan.md index 0c78d8eb9..418dc531f 100644 --- a/docs/plans/core-decomposition-plan.md +++ b/docs/plans/core-decomposition-plan.md @@ -1076,10 +1076,13 @@ pub fn create_tool_registry() -> ToolRegistry { - [x] runtime manifest assembly / `GetToolSpec` 执行迁移前的 baseline 已作为 H1 进入条件： 保留并扩展 expanded/collapsed manifest、 prompt-visible stub、unlock state 和 desktop/MCP/ACP catalog 等价测试。 +- [x] H1 解锁契约切片只抽出 `GetToolSpec` 结果到 collapsed 工具名集合的纯收集规则； + `ToolUseContext.unlocked_collapsed_tools`、执行消息解析、runtime manifest assembly 和 + `GetToolSpecTool` 执行仍由 core 拥有。 **当前安全迁移状态（2026-05-18）：** -- 已迁移到 `bitfun-agent-tools`：`ToolResult`、`ValidationResult`、`InputValidator`、dynamic tool metadata、tool render options、runtime restriction DTO、path resolution DTO、`ToolContextFacts` / `ToolWorkspaceKind` 轻量上下文事实、`PortableToolContextProvider` 只读 facts provider、不依赖 core service 的 `ToolRegistry` / `ToolRegistryItem` generic registry container，以及 `StaticToolProvider` / `install_static_provider` 安装合约。dynamic tool provider / decorator contract 已通过 `agent-tools` 提供兼容 re-export，原 `runtime-ports` 路径保持可用；core 旧路径继续 re-export，并只保留 `BitFunError` 映射、路径 containment helper 与 `ToolUseContext` 到 facts 的只读投影。 +- 已迁移到 `bitfun-agent-tools`：`ToolResult`、`ValidationResult`、`InputValidator`、dynamic tool metadata、tool render options、runtime restriction DTO、path resolution DTO、`ToolContextFacts` / `ToolWorkspaceKind` 轻量上下文事实、`PortableToolContextProvider` 只读 facts provider、不依赖 core service 的 `ToolRegistry` / `ToolRegistryItem` generic registry container、`StaticToolProvider` / `install_static_provider` 安装合约，以及 `GetToolSpec` load observation 到 collapsed 工具名集合的纯收集 helper。dynamic tool provider / decorator contract 已通过 `agent-tools` 提供兼容 re-export，原 `runtime-ports` 路径保持可用；core 旧路径继续 re-export，并只保留 `BitFunError` 映射、路径 containment helper、`ToolUseContext` 到 facts 的只读投影和 runtime unlock state。 - `bitfun-core::agentic::tools` 现在保留 core-owned product provider groups、snapshot decorator 组装、旧构造函数、`dyn Tool` 到 generic registry 的适配、`ToolUseContext` runtime handle / service owner，以及最新主干新增的 runtime manifest assembly / context filtering / `GetToolSpec` 执行；dynamic metadata map、tool map、dynamic descriptor assembly、static provider 安装合约、portable context facts、纯 manifest/exposure 契约和 GetToolSpec presentation/schema 纯 helper 由 `bitfun-agent-tools` 拥有。 - 已新增 `bitfun-tool-packs` feature scaffold，默认 feature 为空，`product-full` 只聚合 feature；当前只提供 `ToolPackFeatureGroup` / `all_feature_groups` / `enabled_feature_groups` 元数据，不注册或迁移任何工具实现。 - 已通过 boundary check 锁定 `agent-tools` / `tool-packs` 暂不拥有 product tool runtime assembly、`GetToolSpecTool` 执行或 collapsed-tool unlock state；`tool-packs` 也不得拥有 manifest/exposure 契约。`agent-tools` 只允许拥有纯 manifest/exposure helper、GetToolSpec presentation/schema helper 和不依赖具体工具的 provider 安装合约，core product tool runtime 继续负责产品 registry snapshot、context-aware discovery、unlock state 和执行路径。 @@ -1092,6 +1095,13 @@ pub fn create_tool_registry() -> ToolRegistry { provider group 顺序装配。该切片不移动 concrete tool implementation、`ToolUseContext`、 runtime manifest assembly 或 `GetToolSpec` 执行；provider id、工具顺序与 manifest 快照由 `bitfun-agent-tools` contract test、core registry snapshot 和 boundary check 共同保护。 +- H1 follow-up（2026-05-19）：下一步先补迁移前 baseline，而不是直接移动 runtime owner。 + 新增保护应覆盖完整 collapsed 工具清单、`ToolExecutionContext` 到 core-owned + `ToolUseContext` 的运行时状态传递，以及 `ToolContextFacts` / `PortableToolContextProvider` + 不携带 `unlocked_collapsed_tools`、custom data、cancellation token 或 workspace services 的边界； + core `resolve_tool_manifest` 还要保留当前显式允许 `GetToolSpec` 时的 runtime insertion 快照。 + 这仍不声明 `ToolUseContext`、runtime manifest assembly、`GetToolSpecTool` 执行或 concrete + tools 已经外移。 **验证：** @@ -1821,8 +1831,9 @@ git diff -- package.json scripts/dev.cjs scripts/desktop-tauri-build.mjs scripts 15. 已完成：agent tools + `tool-packs` owner 化低风险闭环；tool contract / DTO、runtime restriction、path resolution、portable context facts/provider、generic registry / static provider installation / dynamic provider container 已归属 `bitfun-agent-tools`，`tool-packs` 只提供计划内 feature-group scaffold，core 保留 core-owned product provider groups、snapshot decorator、`ToolUseContext` 和 concrete tool implementation，后续外移需单独 service port/provider 设计。 16. 已完成：关键语义回归 baseline，不移动 runtime owner。覆盖 MCP config failure / catalog invalidation / 既有 list-changed helper / dynamic manifest、tool manifest / `GetToolSpec`、product-domains adapter equivalence、remote workspace search fallback 的 focused tests 或 snapshots。 17. 已完成：remote-connect runtime 当前批次收口。已基于当前 port baseline 记录 remote command/response、remote model catalog、poll response、model catalog delta、session restore、active turn、cancel、image context、tracker event、queue/event fanout 的输入输出和验证命令；tracker state / registry lifecycle、legacy image context fallback / preference、restore target decision、cancel decision 与 remote file transfer size/chunk/name policy 已迁入 `bitfun-services-integrations`。dispatcher / product execution、`ImageContextData` adapter、file IO/path resolution、terminal pre-warm 与 workspace/session restore 执行显式保留在 core-owned runtime；后续只有在另起 port/provider 设计且 focused regression 继续通过时才允许继续移动这些 runtime owner，不能把 generic attachment guard 当作已接入多模态行为。 -18. 当前阶段：`product-domains` runtime port/facade closure。已迁入 MiniApp storage-backed runtime-state facade、built-in seed plan / marker wire helper 与 function-agent Git/AI port-backed runtime facade，并补充 focused contract tests；core 只对 MiniApp deps/restart/recompile/sync/rollback 的状态持久化委托 facade，仍保留 `PathManager` 注入、filesystem IO、worker process execution、host dispatch 执行、built-in asset seeding/source-hash lookup、prompt template、JSON extraction 和 error mapping adapter。Git commit-message 与 Startchat work-state 产品路径已通过 core-owned Git/AI adapter 接入 function-agent facade；extracted JSON string 可委托 product-domain parser，但 JSON 提取、日志与错误映射仍在 core。Startchat 接线已用 no-HEAD diff fallback、非 Git 目录空状态和 `analyze_git=false` time-info 保护旧行为，`analyzed_at` 仍由 core 在 AI 分析完成后赋值。 -19. 后续独立评估：`bitfun-core default = []`、per-product feature set、依赖版本收敛或构建收益优化；任何收益声明都需要记录 `cargo check -p bitfun-core`、workspace check 和目标 crate check 的前后数据。 +18. 已完成：`product-domains` runtime port/facade closure。已迁入 MiniApp storage-backed runtime-state facade、built-in seed plan / marker wire helper 与 function-agent Git/AI port-backed runtime facade，并补充 focused contract tests；core 只对 MiniApp deps/restart/recompile/sync/rollback 的状态持久化委托 facade，仍保留 `PathManager` 注入、filesystem IO、worker process execution、host dispatch 执行、built-in asset seeding/source-hash lookup、prompt template、JSON extraction 和 error mapping adapter。Git commit-message 与 Startchat work-state 产品路径已通过 core-owned Git/AI adapter 接入 function-agent facade；extracted JSON string 可委托 product-domain parser，但 JSON 提取、日志与错误映射仍在 core。Startchat 接线已用 no-HEAD diff fallback、非 Git 目录空状态和 `analyze_git=false` time-info 保护旧行为，`analyzed_at` 仍由 core 在 AI 分析完成后赋值。 +19. 当前阶段：H1 tool runtime migration baseline。已开始把纯 helper 从 runtime owner 中剥离：`StaticToolProviderGroup` 和 `GetToolSpec` collapsed-load 纯收集规则可由 `bitfun-agent-tools` 拥有；完整 collapsed 工具清单、runtime context 传递和 portable facts 边界回归仍需继续补齐，再评估单一 owner 外移。不得把 `ToolUseContext`、`GetToolSpecTool` 执行、runtime manifest assembly、snapshot decorator 或 concrete tools 的迁移混成隐式结构调整。 +20. 后续独立评估：`bitfun-core default = []`、per-product feature set、依赖版本收敛或构建收益优化；任何收益声明都需要记录 `cargo check -p bitfun-core`、workspace check 和目标 crate check 的前后数据。 冗余清理 PR 不进入上述主线序号。只有在满足 `0A.6` 的绝对等价要求时，才可以插入到相邻里程碑之间，并且不得与主线拆分 PR 混合。 diff --git a/scripts/check-core-boundaries.mjs b/scripts/check-core-boundaries.mjs index 8ef7c20c2..376795063 100644 --- a/scripts/check-core-boundaries.mjs +++ b/scripts/check-core-boundaries.mjs @@ -1161,6 +1161,14 @@ const requiredContentRules = [ regex: /\bpub fn build_get_tool_spec_assistant_detail\b/, message: 'missing pure GetToolSpec assistant detail rendering contract', }, + { + regex: /\bpub struct GetToolSpecLoadObservation\b/, + message: 'missing pure GetToolSpec load observation contract', + }, + { + regex: /\bpub fn collect_loaded_collapsed_tool_names\b/, + message: 'missing pure collapsed-tool load collection contract', + }, { regex: /\bpub fn sort_tool_manifest_definitions\b/, message: 'missing prompt-visible manifest ordering helper', @@ -1215,6 +1223,10 @@ const requiredContentRules = [ regex: /\bget_collapsed_tool_names\b/, message: 'missing collapsed-tool catalog owner', }, + { + regex: /\bregistry_preserves_collapsed_tool_manifest_for_owner_migration\b/, + message: 'missing collapsed-tool manifest migration baseline', + }, { regex: /\bToolExposure::Collapsed\b/, message: 'missing collapsed exposure lookup', @@ -1326,6 +1338,10 @@ const requiredContentRules = [ regex: /\bbuild_collapsed_tool_stub_definition\b/, message: 'missing collapsed-tool prompt stub contract use', }, + { + regex: /\bmanifest_preserves_explicit_get_tool_spec_runtime_contract\b/, + message: 'missing core GetToolSpec manifest insertion regression', + }, ], }, { @@ -1391,6 +1407,10 @@ const requiredContentRules = [ regex: /\bunlocked_collapsed_tools\b/, message: 'missing collapsed-tool unlock state propagation', }, + { + regex: /\bpipeline_preserves_core_owned_tool_context_without_portable_runtime_leak\b/, + message: 'missing ToolUseContext runtime boundary regression', + }, { regex: /\bGetToolSpec\b/, message: 'missing GetToolSpec gating contract', @@ -2991,6 +3011,8 @@ function runManifestParserSelfTest() { 'get_tool_spec_input_schema', 'validate_get_tool_spec_input', 'build_get_tool_spec_assistant_detail', + 'GetToolSpecLoadObservation', + 'collect_loaded_collapsed_tool_names', 'sort_tool_manifest_definitions', ], }, diff --git a/src/crates/agent-tools/src/framework.rs b/src/crates/agent-tools/src/framework.rs index f6acb4dcb..ffc5bdda4 100644 --- a/src/crates/agent-tools/src/framework.rs +++ b/src/crates/agent-tools/src/framework.rs @@ -6,7 +6,7 @@ use bitfun_core_types::ToolImageAttachment; use indexmap::IndexMap; use serde::{Deserialize, Serialize}; use serde_json::Value; -use std::collections::BTreeSet; +use std::collections::{BTreeSet, HashSet}; use std::fmt; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -259,6 +259,38 @@ pub fn build_get_tool_spec_duplicate_load_hint(tool_name: &str) -> String { ) } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct GetToolSpecLoadObservation<'a> { + pub tool_name: &'a str, + pub loaded_tool_name: Option<&'a str>, + pub is_error: bool, +} + +pub fn collect_loaded_collapsed_tool_names( + observations: &[GetToolSpecLoadObservation<'_>], + collapsed_tool_names: &[String], + get_tool_spec_tool_name: &str, +) -> Vec { + let collapsed_set: HashSet<&str> = collapsed_tool_names.iter().map(String::as_str).collect(); + let mut loaded = BTreeSet::new(); + + for observation in observations { + if observation.is_error || observation.tool_name != get_tool_spec_tool_name { + continue; + } + + let Some(tool_name) = observation.loaded_tool_name else { + continue; + }; + + if collapsed_set.contains(tool_name) { + loaded.insert(tool_name.to_string()); + } + } + + loaded.into_iter().collect() +} + pub fn build_get_tool_spec_assistant_detail(description: &str, input_schema: &Value) -> String { format!( "\n{}\n\n\n{}\n", diff --git a/src/crates/agent-tools/src/lib.rs b/src/crates/agent-tools/src/lib.rs index 9562614f1..2e4ebc3f5 100644 --- a/src/crates/agent-tools/src/lib.rs +++ b/src/crates/agent-tools/src/lib.rs @@ -11,15 +11,15 @@ pub use bitfun_runtime_ports::{ DynamicToolDescriptor, DynamicToolProvider, PortError, PortErrorKind, PortResult, ToolDecorator, }; pub use framework::{ - DynamicMcpToolInfo, DynamicToolInfo, GET_TOOL_SPEC_TOOL_NAME, PortableToolContextProvider, - StaticToolProvider, StaticToolProviderGroup, ToolContextFacts, ToolExposure, - ToolManifestDefinition, ToolManifestPolicyResolution, ToolManifestPolicyTool, ToolPathBackend, - ToolPathOperation, ToolPathPolicy, ToolPathResolution, ToolRef, ToolRegistry, ToolRegistryItem, - ToolRenderOptions, ToolRestrictionError, ToolResult, ToolRuntimeRestrictions, + DynamicMcpToolInfo, DynamicToolInfo, GET_TOOL_SPEC_TOOL_NAME, GetToolSpecLoadObservation, + PortableToolContextProvider, StaticToolProvider, StaticToolProviderGroup, ToolContextFacts, + ToolExposure, ToolManifestDefinition, ToolManifestPolicyResolution, ToolManifestPolicyTool, + ToolPathBackend, ToolPathOperation, ToolPathPolicy, ToolPathResolution, ToolRef, ToolRegistry, + ToolRegistryItem, ToolRenderOptions, ToolRestrictionError, ToolResult, ToolRuntimeRestrictions, ToolWorkspaceKind, ValidationResult, build_collapsed_tool_stub_definition, build_get_tool_spec_assistant_detail, build_get_tool_spec_collapsed_tool_entry, build_get_tool_spec_description, build_get_tool_spec_duplicate_load_hint, - get_tool_spec_input_schema, resolve_tool_manifest_policy, sort_tool_manifest_definitions, - tool_manifest_sort_rank, validate_get_tool_spec_input, + collect_loaded_collapsed_tool_names, get_tool_spec_input_schema, resolve_tool_manifest_policy, + sort_tool_manifest_definitions, tool_manifest_sort_rank, validate_get_tool_spec_input, }; pub use input_validator::InputValidator; diff --git a/src/crates/agent-tools/tests/tool_contracts.rs b/src/crates/agent-tools/tests/tool_contracts.rs index 9daad164c..e82145eda 100644 --- a/src/crates/agent-tools/tests/tool_contracts.rs +++ b/src/crates/agent-tools/tests/tool_contracts.rs @@ -1,10 +1,11 @@ use bitfun_agent_tools::{ - DynamicMcpToolInfo, DynamicToolInfo, GET_TOOL_SPEC_TOOL_NAME, InputValidator, ToolContextFacts, - ToolExposure, ToolImageAttachment, ToolManifestDefinition, ToolManifestPolicyTool, - ToolPathBackend, ToolPathResolution, ToolRenderOptions, ToolResult, ToolRuntimeRestrictions, - ToolWorkspaceKind, ValidationResult, build_collapsed_tool_stub_definition, - build_get_tool_spec_assistant_detail, build_get_tool_spec_collapsed_tool_entry, - build_get_tool_spec_description, build_get_tool_spec_duplicate_load_hint, + DynamicMcpToolInfo, DynamicToolInfo, GET_TOOL_SPEC_TOOL_NAME, GetToolSpecLoadObservation, + InputValidator, ToolContextFacts, ToolExposure, ToolImageAttachment, ToolManifestDefinition, + ToolManifestPolicyTool, ToolPathBackend, ToolPathResolution, ToolRenderOptions, ToolResult, + ToolRuntimeRestrictions, ToolWorkspaceKind, ValidationResult, + build_collapsed_tool_stub_definition, build_get_tool_spec_assistant_detail, + build_get_tool_spec_collapsed_tool_entry, build_get_tool_spec_description, + build_get_tool_spec_duplicate_load_hint, collect_loaded_collapsed_tool_names, get_tool_spec_input_schema, resolve_tool_manifest_policy, sort_tool_manifest_definitions, validate_get_tool_spec_input, }; @@ -419,6 +420,46 @@ fn tool_manifest_policy_preserves_explicit_get_tool_spec_duplicate_runtime_contr assert_eq!(policy.collapsed_tool_names, vec!["WebFetch"]); } +#[test] +fn get_tool_spec_load_collector_preserves_collapsed_runtime_contract() { + let collapsed_tools = vec!["WebFetch".to_string(), "GetFileDiff".to_string()]; + let observations = vec![ + GetToolSpecLoadObservation { + tool_name: GET_TOOL_SPEC_TOOL_NAME, + loaded_tool_name: Some("WebFetch"), + is_error: false, + }, + GetToolSpecLoadObservation { + tool_name: GET_TOOL_SPEC_TOOL_NAME, + loaded_tool_name: Some("Read"), + is_error: false, + }, + GetToolSpecLoadObservation { + tool_name: GET_TOOL_SPEC_TOOL_NAME, + loaded_tool_name: Some("GetFileDiff"), + is_error: true, + }, + GetToolSpecLoadObservation { + tool_name: "Read", + loaded_tool_name: Some("WebFetch"), + is_error: false, + }, + GetToolSpecLoadObservation { + tool_name: GET_TOOL_SPEC_TOOL_NAME, + loaded_tool_name: Some("WebFetch"), + is_error: false, + }, + ]; + + let loaded = collect_loaded_collapsed_tool_names( + &observations, + &collapsed_tools, + GET_TOOL_SPEC_TOOL_NAME, + ); + + assert_eq!(loaded, vec!["WebFetch".to_string()]); +} + #[test] fn collapsed_tool_stub_definition_preserves_prompt_visible_guardrail() { let stub = build_collapsed_tool_stub_definition( diff --git a/src/crates/core/src/agentic/execution/execution_engine.rs b/src/crates/core/src/agentic/execution/execution_engine.rs index 25bd8d513..5bc76a79d 100644 --- a/src/crates/core/src/agentic/execution/execution_engine.rs +++ b/src/crates/core/src/agentic/execution/execution_engine.rs @@ -5,23 +5,23 @@ use super::round_executor::RoundExecutor; use super::types::{ExecutionContext, ExecutionResult, RoundContext, RoundResult}; use crate::agentic::agents::{ - get_agent_registry, PromptBuilder, PromptBuilderContext, RemoteExecutionHints, + PromptBuilder, PromptBuilderContext, RemoteExecutionHints, get_agent_registry, }; use crate::agentic::context_profile::{ContextProfilePolicy, ModelCapabilityProfile}; use crate::agentic::core::{ - render_system_reminder, Message, MessageContent, MessageHelper, MessageRole, - MessageSemanticKind, RequestReasoningTokenPolicy, Session, + Message, MessageContent, MessageHelper, MessageRole, MessageSemanticKind, + RequestReasoningTokenPolicy, Session, render_system_reminder, }; use crate::agentic::events::{AgenticEvent, EventPriority, EventQueue}; use crate::agentic::execution::types::FinishReason; use crate::agentic::image_analysis::{ - build_multimodal_message_with_images, process_image_contexts_for_provider, ImageContextData, - ImageLimits, + ImageContextData, ImageLimits, build_multimodal_message_with_images, + process_image_contexts_for_provider, }; use crate::agentic::round_preempt::RoundInjectionKind; use crate::agentic::session::{CompressionTailPolicy, ContextCompressor, SessionManager}; use crate::agentic::tools::{ - resolve_tool_manifest, ResolvedToolManifest, SubagentParentInfo, ToolRuntimeRestrictions, + ResolvedToolManifest, SubagentParentInfo, ToolRuntimeRestrictions, resolve_tool_manifest, }; use crate::agentic::util::build_remote_workspace_layout_preview; use crate::agentic::{WorkspaceBackend, WorkspaceBinding}; @@ -34,9 +34,10 @@ use crate::util::token_counter::TokenCounter; use crate::util::types::Message as AIMessage; use crate::util::types::ToolDefinition; use crate::util::{elapsed_ms_u64, truncate_at_char_boundary}; +use bitfun_agent_tools::{GetToolSpecLoadObservation, collect_loaded_collapsed_tool_names}; use log::{debug, error, info, trace, warn}; use sha2::{Digest, Sha256}; -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{HashMap, HashSet}; use std::path::Path; use std::sync::Arc; use tokio_util::sync::CancellationToken; @@ -501,34 +502,32 @@ impl ExecutionEngine { messages: &[Message], collapsed_tools: &[String], ) -> Vec { - let collapsed_set: HashSet<&str> = collapsed_tools.iter().map(String::as_str).collect(); - let mut unlocked = BTreeSet::new(); - - for message in messages { - let MessageContent::ToolResult { - tool_name, - result, - is_error, - .. - } = &message.content - else { - continue; - }; - - if *is_error || tool_name != "GetToolSpec" { - continue; - } - - let Some(tool_name) = result.get("tool_name").and_then(|v| v.as_str()) else { - continue; - }; + let observations = messages + .iter() + .filter_map(|message| { + let MessageContent::ToolResult { + tool_name, + result, + is_error, + .. + } = &message.content + else { + return None; + }; - if collapsed_set.contains(tool_name) { - unlocked.insert(tool_name.to_string()); - } - } + Some(GetToolSpecLoadObservation { + tool_name, + loaded_tool_name: result.get("tool_name").and_then(|v| v.as_str()), + is_error: *is_error, + }) + }) + .collect::>(); - unlocked.into_iter().collect() + collect_loaded_collapsed_tool_names( + &observations, + collapsed_tools, + crate::agentic::tools::registry::GET_TOOL_SPEC_TOOL_NAME, + ) } async fn build_prompt_context( diff --git a/src/crates/core/src/agentic/tools/manifest_resolver.rs b/src/crates/core/src/agentic/tools/manifest_resolver.rs index d00628965..1a9014163 100644 --- a/src/crates/core/src/agentic/tools/manifest_resolver.rs +++ b/src/crates/core/src/agentic/tools/manifest_resolver.rs @@ -316,6 +316,30 @@ mod tests { ); } + #[tokio::test] + async fn manifest_preserves_explicit_get_tool_spec_runtime_contract() { + let allowed_tools = vec![GET_TOOL_SPEC_TOOL_NAME.to_string(), "WebFetch".to_string()]; + + let manifest = resolve_tool_manifest( + &allowed_tools, + &AgentToolPolicyOverrides::default(), + &tool_context(), + ) + .await; + + assert_eq!(manifest.allowed_tool_names, allowed_tools); + assert_eq!(manifest.collapsed_tool_names, vec!["WebFetch".to_string()]); + assert_eq!( + manifest + .tool_definitions + .iter() + .map(|tool| tool.name.as_str()) + .collect::>(), + vec!["WebFetch", "GetToolSpec", "GetToolSpec"], + "core runtime currently mirrors the pure policy contract when GetToolSpec is already allowed" + ); + } + #[tokio::test] async fn manifest_expands_tool_when_agent_override_requests_it() { let allowed_tools = vec!["Read".to_string(), "WebFetch".to_string()]; diff --git a/src/crates/core/src/agentic/tools/pipeline/tool_pipeline.rs b/src/crates/core/src/agentic/tools/pipeline/tool_pipeline.rs index 02a677e9c..3c7d21186 100644 --- a/src/crates/core/src/agentic/tools/pipeline/tool_pipeline.rs +++ b/src/crates/core/src/agentic/tools/pipeline/tool_pipeline.rs @@ -19,8 +19,8 @@ use log::{debug, error, info, warn}; use std::collections::{HashMap, VecDeque}; use std::sync::Arc; use std::time::{Instant, SystemTime}; -use tokio::sync::{oneshot, RwLock as TokioRwLock}; -use tokio::time::{timeout, Duration}; +use tokio::sync::{RwLock as TokioRwLock, oneshot}; +use tokio::time::{Duration, timeout}; use tokio_util::sync::CancellationToken; /// A batch of tool tasks to execute together. @@ -1577,10 +1577,18 @@ impl ToolPipeline { #[cfg(test)] mod tests { use super::*; + use crate::agentic::events::{EventQueue, EventQueueConfig}; use crate::agentic::tools::ToolRuntimeRestrictions; use serde_json::json; use std::collections::HashMap; + fn test_tool_pipeline() -> ToolPipeline { + let registry = Arc::new(TokioRwLock::new(ToolRegistry::new())); + let event_queue = Arc::new(EventQueue::new(EventQueueConfig::default())); + let state_manager = Arc::new(ToolStateManager::new(event_queue)); + ToolPipeline::new(registry, state_manager, None) + } + fn test_tool_task(tool_id: &str, tool_name: &str) -> ToolTask { ToolTask::new( ToolCall { @@ -1643,12 +1651,14 @@ mod tests { result.result.result["provided_arguments"], serde_json::Value::String("{\"operation\":\"log\"".to_string()) ); - assert!(result - .result - .result_for_assistant - .as_deref() - .unwrap_or_default() - .contains("Provided arguments: {\"operation\":\"log\"")); + assert!( + result + .result + .result_for_assistant + .as_deref() + .unwrap_or_default() + .contains("Provided arguments: {\"operation\":\"log\"") + ); } #[test] @@ -1675,6 +1685,62 @@ mod tests { assert!(!assistant_text.contains("completed with error")); } + #[test] + fn pipeline_preserves_core_owned_tool_context_without_portable_runtime_leak() { + let pipeline = test_tool_pipeline(); + let mut task = test_tool_task("tool_context_1", "WebFetch"); + task.context + .context_vars + .insert("turn_index".to_string(), "7".to_string()); + task.context + .context_vars + .insert("primary_model_provider".to_string(), "openai".to_string()); + task.context.context_vars.insert( + "primary_model_supports_image_understanding".to_string(), + "true".to_string(), + ); + task.context.collapsed_tools = vec!["WebFetch".to_string()]; + task.context.unlocked_collapsed_tools = vec!["WebFetch".to_string()]; + task.context.runtime_tool_restrictions = ToolRuntimeRestrictions { + allowed_tool_names: ["WebFetch"].into_iter().map(str::to_string).collect(), + denied_tool_names: ["Bash"].into_iter().map(str::to_string).collect(), + path_policy: Default::default(), + }; + + let context = pipeline.build_tool_use_context(&task, CancellationToken::new()); + + assert_eq!(context.tool_call_id.as_deref(), Some("tool_context_1")); + assert_eq!(context.agent_type.as_deref(), Some("agent")); + assert_eq!(context.session_id.as_deref(), Some("session_1")); + assert_eq!(context.dialog_turn_id.as_deref(), Some("turn_1")); + assert_eq!(context.unlocked_collapsed_tools, vec!["WebFetch"]); + assert!(context.cancellation_token.is_some()); + assert!( + context + .runtime_tool_restrictions + .is_tool_allowed("WebFetch") + ); + assert!(!context.runtime_tool_restrictions.is_tool_allowed("Bash")); + assert_eq!(context.custom_data["turn_index"], json!(7)); + assert_eq!( + context.custom_data["primary_model_provider"], + json!("openai") + ); + assert_eq!( + context.custom_data["primary_model_supports_image_understanding"], + json!(true) + ); + + let facts = context.to_tool_context_facts(); + let value = serde_json::to_value(&facts).expect("serialize context facts"); + assert_eq!(value["toolCallId"], "tool_context_1"); + assert_eq!(value["sessionId"], "session_1"); + assert!(value.get("unlockedCollapsedTools").is_none()); + assert!(value.get("customData").is_none()); + assert!(value.get("cancellationToken").is_none()); + assert!(value.get("workspaceServices").is_none()); + } + #[test] fn collapsed_tool_requires_tool_catalog_unlock() { let mut task = test_tool_task("tool_1", "WebFetch"); @@ -1683,9 +1749,10 @@ mod tests { let err = ToolPipeline::validate_collapsed_tool_usage(&task) .expect_err("collapsed tool should require GetToolSpec unlock"); - assert!(err - .to_string() - .contains("Call GetToolSpec first with {\"tool_name\":\"WebFetch\"}")); + assert!( + err.to_string() + .contains("Call GetToolSpec first with {\"tool_name\":\"WebFetch\"}") + ); } #[test] diff --git a/src/crates/core/src/agentic/tools/registry.rs b/src/crates/core/src/agentic/tools/registry.rs index 6abd38b94..c3b7e041d 100644 --- a/src/crates/core/src/agentic/tools/registry.rs +++ b/src/crates/core/src/agentic/tools/registry.rs @@ -430,6 +430,37 @@ mod tests { assert!(registry.is_tool_collapsed("Git")); } + #[test] + fn registry_preserves_collapsed_tool_manifest_for_owner_migration() { + let registry = create_tool_registry(); + + assert_eq!( + registry.get_collapsed_tool_names(), + vec![ + "GetFileDiff", + "Log", + "TerminalControl", + "SessionControl", + "SessionMessage", + "SessionHistory", + "Cron", + "WebSearch", + "WebFetch", + "ListMCPResources", + "ReadMCPResource", + "ListMCPPrompts", + "GetMCPPrompt", + "GenerativeUI", + "Git", + "InitMiniApp", + "ControlHub", + "ComputerUse", + "Playbook", + ], + "collapsed tool manifest must stay stable before moving registry or manifest ownership" + ); + } + #[tokio::test] async fn registry_preserves_readonly_tool_manifest_for_owner_migration() { let readonly_names = super::get_readonly_tools()