diff --git a/crates/cli/src/metrics.rs b/crates/cli/src/metrics.rs index bb726e7cb5..179f4f1c18 100644 --- a/crates/cli/src/metrics.rs +++ b/crates/cli/src/metrics.rs @@ -325,15 +325,24 @@ fn read_audit_log(path: &Path, since: Option>, rollup: &mut Rollup stats.elapsed_samples += 1; } - // Success / failure + // Success / failure. A receipt the engine marked + // `executed: false` (a deferred-tool hydration receipt, an + // oversize-hydration refusal) answered the call without the + // tool ever running, so it counts as neither — counting it + // as a success would inflate the tool's real execution rate. let success = v .pointer("/details/success") .or_else(|| v.pointer("/payload/success")) .and_then(|b| b.as_bool()) .unwrap_or(true); - if success { + let executed = v + .pointer("/details/executed") + .or_else(|| v.pointer("/payload/executed")) + .and_then(|b| b.as_bool()) + .unwrap_or(true); + if success && executed { stats.successes += 1; - } else { + } else if !success { stats.failures += 1; } } @@ -1015,6 +1024,39 @@ mod tests { assert_eq!(rollup.tools["read_file"].calls, 1); } + #[test] + fn tool_result_rollup_counts_unexecuted_receipts_as_neither() { + use std::io::Write; + let mut tmp = tempfile::NamedTempFile::new().unwrap(); + // A plain success, a plain failure, an `executed: false` receipt + // (a hydration receipt or an oversize-hydration refusal — the tool + // never ran), and an executed-marked success. + let lines = [ + r#"{"event":"tool.result","ts":"2026-04-01T10:00:00+00:00","details":{"tool_name":"exec_shell","success":true}}"#, + r#"{"event":"tool.result","ts":"2026-04-01T10:01:00+00:00","details":{"tool_name":"exec_shell","success":false}}"#, + r#"{"event":"tool.result","ts":"2026-04-01T10:02:00+00:00","details":{"tool_name":"deferred_tool","success":true,"executed":false}}"#, + r#"{"event":"tool.result","ts":"2026-04-01T10:03:00+00:00","details":{"tool_name":"other_tool","success":true,"executed":true}}"#, + ]; + for line in lines { + writeln!(tmp, "{line}").unwrap(); + } + + let mut rollup = Rollup::default(); + read_audit_log(tmp.path(), None, &mut rollup); + + assert_eq!(rollup.tools["exec_shell"].calls, 2); + assert_eq!(rollup.tools["exec_shell"].successes, 1); + assert_eq!(rollup.tools["exec_shell"].failures, 1); + // The unexecuted receipt counts as neither success nor failure: + // counting it as a success would inflate the tool's real execution + // rate. Deleting the `executed` consult turns this pin red. + assert_eq!(rollup.tools["deferred_tool"].calls, 1); + assert_eq!(rollup.tools["deferred_tool"].successes, 0); + assert_eq!(rollup.tools["deferred_tool"].failures, 0); + assert_eq!(rollup.tools["other_tool"].calls, 1); + assert_eq!(rollup.tools["other_tool"].successes, 1); + } + #[test] fn total_tool_calls_sums_across_tools() { let mut rollup = Rollup::default(); diff --git a/crates/tui/src/client.rs b/crates/tui/src/client.rs index 5f4d278f58..802bc6bb35 100644 --- a/crates/tui/src/client.rs +++ b/crates/tui/src/client.rs @@ -6402,6 +6402,55 @@ mod tests { } } + /// The live-found browser catalog shape (`evaluate_script.args = {"type": + /// "array", "items": {}}`) used to fail MFJS validation and silently + /// disappear from every Kimi request. The wire must carry the tool with + /// the empty `items` keyword repaired away, not drop it. + #[tokio::test] + async fn moonshot_keeps_empty_items_tool_on_the_wire() { + for streaming in [false, true] { + let mut request = + k3_request_fixture(crate::config::KIMI_CODE_K3_MODEL, Some("low"), streaming); + let mut browser = test_tool("mcp_browser_evaluate_script"); + browser.input_schema = json!({ + "type": "object", + "properties": { + "function": {"type": "string"}, + "args": {"type": "array", "items": {}} + }, + "required": ["function"], + "additionalProperties": true + }); + request.tools = Some(vec![browser]); + request.tool_choice = Some(json!("auto")); + + let body = capture_moonshot_chat_request_body( + crate::config::DEFAULT_KIMI_CODE_BASE_URL, + crate::config::KIMI_CODE_K3_MODEL, + request, + ) + .await; + let tools = body["tools"] + .as_array() + .expect("the empty-items tool must stay on the wire"); + assert_eq!( + tools.len(), + 1, + "the repaired tool must not be dropped (streaming={streaming}): {body}" + ); + assert_eq!( + tools[0]["function"]["name"], + json!("mcp_browser_evaluate_script"), + "the surviving tool must be the incident tool itself: {body}" + ); + assert_eq!( + tools[0]["function"]["parameters"]["properties"]["args"], + json!({"type": "array"}), + "the empty items keyword must be repaired to absent: {body}" + ); + } + } + #[tokio::test] async fn moonshot_rejects_named_choice_for_omitted_tool() { for streaming in [false, true] { diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index d7469dcdc8..aa5f3ac492 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -3408,8 +3408,11 @@ impl Engine { // conversation. SyncSession installs a conversation's // identity, history, and workspace (including the // generated-ID new-session path), so never carry the - // previous conversation's toolbox across this edge. + // previous conversation's toolbox across this edge — + // including a parked eviction note whose batch never + // recorded a result. self.session.tool_activation_cache.clear(); + self.session.pending_toolbox_eviction_notes.clear(); let plugin_workspace_changed = self.plugin_registry.workspace() != workspace.as_path(); let previous_session_id = self.session.id.clone(); @@ -8832,7 +8835,7 @@ use self::streaming::{ sleep_gap_detected, stream_read_error_user_message, }; use self::tool_catalog::{ - CODE_EXECUTION_TOOL_NAME, JS_EXECUTION_TOOL_NAME, MULTI_TOOL_PARALLEL_NAME, + CODE_EXECUTION_TOOL_NAME, DeferredHydration, JS_EXECUTION_TOOL_NAME, MULTI_TOOL_PARALLEL_NAME, REQUEST_USER_INPUT_NAME, ToolSurfacePolicy, active_tools_for_request, apply_registry_first_shell_guidance, build_model_tool_catalog_with_surface, default_synthetic_catalog_tool_names, execute_code_execution_tool, is_tool_search_tool, diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 8a52f0c6c2..09b4463738 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -1,6 +1,7 @@ use super::*; use super::context::COMPACTION_SUMMARY_MARKER; +use super::dispatch::ToolExecOutcome; use super::streaming::{TOOL_CALL_END_MARKERS, TOOL_CALL_MARKER_PAIRS}; use super::turn_loop::{ auto_review_block_tool_error, initial_stream_error_user_message, @@ -15,6 +16,7 @@ use crate::prompts::{ system_prompt_for_mode_with_context_skills_and_session, }; use crate::test_support::{EnvVarGuard, lock_test_env}; +use crate::tools::spec::{ToolError, ToolExecutionOutcome}; use serde_json::{Value, json}; use std::collections::{HashMap, HashSet}; use std::ffi::OsString; @@ -4950,6 +4952,17 @@ fn policy_for_catalog( ) } +/// Permissive turn policy for `process_tool_results` tests: no allow/deny +/// gates, so the mid-turn MCP refresh narrows nothing away. +fn permissive_turn_policy() -> ToolSurfacePolicy { + policy_for_catalog( + vec![], + None, + None, + crate::tui::approval::ApprovalMode::Suggest, + ) +} + #[test] fn tool_catalog_scenario() { // Scenario consolidation of: tool_catalog_filter_applies_allow_and_deny_gates, tool_catalog_filter_is_inert_without_gates @@ -9143,11 +9156,14 @@ fn runtime_mcp_refresh_replaces_the_pool_slice() { api_tool("exec_shell"), // engine-owned: never the pool's to remove ]; let mut active = HashSet::new(); + let cache = crate::core::session::ToolActivationCache::default(); // The pool's universe owns the static read (still live) and the // synthetic authenticate entry (its login just succeeded — it must // LEAVE). The refreshed surface adds a new tool and keeps the static - // one; the engine tool is untouched. + // one; the engine tool is untouched. The static read was deferred in + // this conversation, so the refresh carries that marking across instead + // of adopting the pool's eager flag. let universe: HashSet = ["mcp_static_read", "mcp_alpha_authenticate"] .into_iter() .map(str::to_string) @@ -9157,15 +9173,29 @@ fn runtime_mcp_refresh_replaces_the_pool_slice() { &mut active, &universe, vec![api_tool("mcp_static_read"), api_tool("mcp_dynamic_render")], + &cache, ); let names: Vec<&str> = catalog.iter().map(|tool| tool.name.as_str()).collect(); assert_eq!( names, - vec!["exec_shell", "mcp_static_read", "mcp_dynamic_render"], - "the synthetic authenticate tool leaves after its own login; engine tools stay" + vec!["mcp_static_read", "exec_shell", "mcp_dynamic_render"], + "the synthetic authenticate tool leaves after its own login; the replaced \ + entry keeps its own slot, the engine tool never moves, and the new \ + arrival appends at the end" + ); + let static_read = catalog + .iter() + .find(|tool| tool.name == "mcp_static_read") + .expect("the static read survives the refresh"); + assert_eq!( + static_read.defer_loading, + Some(true), + "the conversation's deferral marking must survive the pool's eager flag" ); - assert!(active.contains("mcp_static_read")); + // Deferred and not activated in this conversation: the refresh must not + // put it on the wire — a search activates it. + assert!(!active.contains("mcp_static_read")); assert!(active.contains("mcp_dynamic_render")); assert!(!active.contains("mcp_alpha_authenticate")); @@ -9184,6 +9214,7 @@ fn runtime_mcp_refresh_replaces_the_pool_slice() { &mut active, &universe, vec![api_tool("mcp_alpha_authenticate")], + &cache, ); let names: Vec<&str> = catalog.iter().map(|tool| tool.name.as_str()).collect(); assert_eq!( @@ -9193,6 +9224,95 @@ fn runtime_mcp_refresh_replaces_the_pool_slice() { ); assert!(active.contains("mcp_alpha_authenticate")); assert!(!active.contains("mcp_dynamic_render")); + + // Wire visibility stays the conversation's decision: a deferred tool + // this conversation has not activated must not ride the wire after a + // refresh — it returns when a search activates it, and re-advertising + // it would contradict any eviction note already recorded. + let mut catalog = vec![api_tool("exec_shell")]; + let mut active = HashSet::new(); + let mut deferred = api_tool("mcp_static_read"); + deferred.defer_loading = Some(true); + let universe: HashSet = ["mcp_static_read"] + .into_iter() + .map(str::to_string) + .collect(); + replace_runtime_mcp_tools(&mut catalog, &mut active, &universe, vec![deferred], &cache); + assert!( + !active.contains("mcp_static_read"), + "an unactivated deferred tool must stay off the wire: {active:?}" + ); + assert_eq!(catalog.len(), 2, "the tool still exists for tool_search"); + + // Once a search activates it, the same refresh keeps it on the wire. + let mut deferred = api_tool("mcp_static_read"); + deferred.defer_loading = Some(true); + let mut cache = crate::core::session::ToolActivationCache::default(); + cache.activate(&[deferred.clone()], &["mcp_static_read".to_string()]); + replace_runtime_mcp_tools(&mut catalog, &mut active, &universe, vec![deferred], &cache); + assert!( + active.contains("mcp_static_read"), + "a still-activated deferred tool stays on the wire: {active:?}" + ); + + // A replacement splices back into its own slot: entries after the pool + // slice never move, so a same-name refresh reorders nothing (the cached + // request prefix survives even when a count-keyed declaration would + // have called the refresh a no-op). + let mut catalog = vec![api_tool("mcp_static_read"), api_tool("exec_shell")]; + let mut active = HashSet::new(); + let universe: HashSet = ["mcp_static_read"] + .into_iter() + .map(str::to_string) + .collect(); + let wire_changed = replace_runtime_mcp_tools( + &mut catalog, + &mut active, + &universe, + vec![api_tool("mcp_static_read")], + &cache, + ); + let names: Vec<&str> = catalog.iter().map(|tool| tool.name.as_str()).collect(); + assert_eq!( + names, + vec!["mcp_static_read", "exec_shell"], + "a replaced entry keeps its slot; later entries never relocate" + ); + assert!( + !wire_changed, + "a same-name refresh that changes no wire-visible name is no drift" + ); + + // A 1:1 identity change (a server-side rename) keeps the catalog count + // but changes the wire: the old active name leaves, the eager new name + // joins. The declaration must key on that wire change, not on the + // count, or the C5 guard would see undeclared drift. + let mut deferred = api_tool("mcp_old_name"); + deferred.defer_loading = Some(true); + let mut cache = crate::core::session::ToolActivationCache::default(); + cache.activate(&[deferred.clone()], &["mcp_old_name".to_string()]); + let mut catalog = vec![api_tool("exec_shell"), deferred]; + let mut active: HashSet = ["exec_shell".to_string(), "mcp_old_name".to_string()] + .into_iter() + .collect(); + let universe: HashSet = ["mcp_old_name".to_string(), "mcp_new_name".to_string()] + .into_iter() + .collect(); + let wire_changed = replace_runtime_mcp_tools( + &mut catalog, + &mut active, + &universe, + vec![api_tool("mcp_new_name")], + &cache, + ); + assert!( + wire_changed, + "a rename that swaps a wire-visible name must be declared: {active:?}" + ); + assert!( + !active.contains("mcp_old_name") && active.contains("mcp_new_name"), + "the renamed surface must be reflected in the active set: {active:?}" + ); } #[test] @@ -12277,14 +12397,15 @@ fn deferred_apply_patch_first_use_hydrates_schema_without_execution() { let catalog = vec![apply_patch]; let active_at_batch_start = HashSet::new(); let mut hydrated_this_batch = HashSet::new(); - let result = maybe_hydrate_requested_deferred_tool( + let Some(DeferredHydration::Loaded(result)) = maybe_hydrate_requested_deferred_tool( "apply_patch", &json!({"patch": "*** Begin Patch\n*** End Patch"}), &catalog, &active_at_batch_start, &mut hydrated_this_batch, - ) - .expect("first deferred use should hydrate"); + ) else { + panic!("first deferred use should hydrate, not refuse"); + }; assert!(!active_at_batch_start.contains("apply_patch")); assert!(hydrated_this_batch.contains("apply_patch")); @@ -12298,14 +12419,15 @@ fn deferred_apply_patch_first_use_hydrates_schema_without_execution() { assert_eq!(metadata["executed"], false); assert_eq!(metadata["retry_required"], true); - let second_result = maybe_hydrate_requested_deferred_tool( + let Some(DeferredHydration::Loaded(second_result)) = maybe_hydrate_requested_deferred_tool( "apply_patch", &json!({"patch": "*** Begin Patch\n*** End Patch"}), &catalog, &active_at_batch_start, &mut hydrated_this_batch, - ) - .expect("later calls in the same batch should hydrate instead of executing"); + ) else { + panic!("later calls in the same batch should hydrate instead of executing"); + }; assert_eq!(second_result.metadata.unwrap()["executed"], false); assert_eq!( hydrated_this_batch, @@ -27261,3 +27383,1273 @@ async fn a_booting_turn_keeps_an_already_declared_surface_reason() { last ); } + +// === Toolbox eviction note channels === +// +// The bounded conversation toolbox reports bounds-driven evictions to the +// model through the recorded tool result. These tests pin each channel's +// wiring: the hydration note parked on the session must ride the batch's +// first recorded result (or its error), and the retrieval/refresh evictions +// must ride the triggering result. A regression that drops the take, skips +// the error branch, or misroutes the triggering result turns these red. + +fn synthetic_toolbox_outcome( + id: &str, + name: &str, + result: Result, +) -> ToolExecOutcome { + ToolExecOutcome { + index: 0, + id: id.to_string(), + name: name.to_string(), + input: json!({}), + started_at: Instant::now(), + terminal: ToolExecutionOutcome::from_legacy(result), + content_blocks: Vec::new(), + } +} + +fn heavy_deferred_tool(name: &str, description_bytes: usize) -> crate::models::Tool { + crate::models::Tool { + tool_type: None, + name: name.to_string(), + description: "x".repeat(description_bytes), + input_schema: json!({"type": "object", "properties": {}}), + allowed_callers: None, + defer_loading: Some(true), + input_examples: None, + strict: None, + cache_control: None, + } +} + +fn recorded_tool_result_content(engine: &Engine, tool_use_id: &str) -> String { + engine + .session + .messages + .iter() + .find_map(|message| { + message.content.iter().find_map(|block| match block { + crate::models::ContentBlock::ToolResult { + tool_use_id: id, + content, + .. + } if id == tool_use_id => Some(content.clone()), + _ => None, + }) + }) + .expect("every batch outcome must be recorded as a tool result message") +} + +#[tokio::test] +async fn hydration_eviction_note_rides_the_batch_first_result() { + let (mut engine, _handle) = Engine::new(EngineConfig::default(), &Config::default()); + engine.park_toolbox_eviction_note( + crate::core::session::ToolboxEvictionTrigger::Step, + &["ghost-tool".to_string()], + ); + let mut catalog = Vec::new(); + let mut active = HashSet::new(); + engine + .process_tool_results( + vec![ + Some(synthetic_toolbox_outcome( + "call-first", + "tool_a", + Ok(ToolResult::success("first payload")), + )), + Some(synthetic_toolbox_outcome( + "call-second", + "tool_b", + Ok(ToolResult::success("second payload")), + )), + ], + &mut catalog, + &mut active, + &HashMap::new(), + &permissive_turn_policy(), + ) + .await; + + let first = recorded_tool_result_content(&engine, "call-first"); + assert!( + first.contains("ghost-tool"), + "the parked hydration note must ride the batch's first recorded result: {first}" + ); + let second = recorded_tool_result_content(&engine, "call-second"); + assert!( + !second.contains("ghost-tool"), + "the note rides only the first result of the batch: {second}" + ); +} + +#[tokio::test] +async fn hydration_eviction_note_survives_a_leading_error_result() { + let (mut engine, _handle) = Engine::new(EngineConfig::default(), &Config::default()); + engine.park_toolbox_eviction_note( + crate::core::session::ToolboxEvictionTrigger::Step, + &["ghost-tool".to_string()], + ); + let mut catalog = Vec::new(); + let mut active = HashSet::new(); + engine + .process_tool_results( + vec![ + Some(synthetic_toolbox_outcome( + "call-fails", + "tool_a", + Err(ToolError::ExecutionFailed { + message: "boom".to_string(), + }), + )), + Some(synthetic_toolbox_outcome( + "call-succeeds", + "tool_b", + Ok(ToolResult::success("payload")), + )), + ], + &mut catalog, + &mut active, + &HashMap::new(), + &permissive_turn_policy(), + ) + .await; + + let failed = recorded_tool_result_content(&engine, "call-fails"); + assert!( + failed.contains("boom") && failed.contains("ghost-tool"), + "the note must reach the model even when the batch's first result is an error: {failed}" + ); + let succeeded = recorded_tool_result_content(&engine, "call-succeeds"); + assert!( + !succeeded.contains("ghost-tool"), + "the note rides only the first recorded result: {succeeded}" + ); +} + +#[tokio::test] +async fn retrieval_admission_eviction_note_rides_the_triggering_result() { + let (mut engine, _handle) = Engine::new(EngineConfig::default(), &Config::default()); + // Two cached deferred schemas sit near the collective budget; the + // evidence receipt admits the retrieval schema on top and overflows the + // cache, so the eviction must ride the receipt's own recorded result. + let catalog = vec![ + heavy_deferred_tool("retrieve_tool_result", 3_000), + heavy_deferred_tool("cached-a", 7_000), + heavy_deferred_tool("cached-b", 7_000), + ]; + let mut active = super::tool_catalog::initial_active_tools(&catalog); + let cached = vec!["cached-a".to_string(), "cached-b".to_string()]; + engine + .session + .tool_activation_cache + .activate(&catalog, &cached); + active.extend(cached); + + let mut live_catalog = catalog; + engine + .process_tool_results( + vec![Some(synthetic_toolbox_outcome( + "call-evidence", + "evidence-tool", + Ok(ToolResult { + content: "evidence payload".to_string(), + success: true, + metadata: Some(json!({"evidence_available": true})), + }), + ))], + &mut live_catalog, + &mut active, + &HashMap::new(), + &permissive_turn_policy(), + ) + .await; + + let evidence = recorded_tool_result_content(&engine, "call-evidence"); + assert!( + evidence.contains("cached-a"), + "the retrieval admission's eviction must ride the triggering result: {evidence}" + ); + assert!( + !engine + .session + .tool_activation_cache + .names() + .any(|name| name == "cached-a"), + "the evicted name must leave the cache" + ); +} + +#[tokio::test] +async fn execution_refresh_eviction_note_rides_the_refreshed_result() { + let (mut engine, _handle) = Engine::new(EngineConfig::default(), &Config::default()); + let priming_catalog = vec![ + heavy_deferred_tool("cached-a", 7_000), + heavy_deferred_tool("executed", 7_000), + ]; + let mut active = super::tool_catalog::initial_active_tools(&priming_catalog); + let cached = vec!["cached-a".to_string(), "executed".to_string()]; + engine + .session + .tool_activation_cache + .activate(&priming_catalog, &cached); + active.extend(cached); + + // The catalog's schema for `cached-a` grew after the activation (an MCP + // refresh). Refreshing `executed` revalidates against the grown catalog, + // the collective budget overflows, and the eviction must ride the + // refreshed tool's own recorded result. + let mut grown_catalog = vec![ + heavy_deferred_tool("cached-a", 12_000), + heavy_deferred_tool("executed", 7_000), + ]; + engine + .process_tool_results( + vec![Some(synthetic_toolbox_outcome( + "call-executed", + "executed", + Ok(ToolResult::success("executed payload")), + ))], + &mut grown_catalog, + &mut active, + &HashMap::new(), + &permissive_turn_policy(), + ) + .await; + + let executed = recorded_tool_result_content(&engine, "call-executed"); + assert!( + executed.contains("cached-a"), + "the refresh's revalidation eviction must ride the refreshed result: {executed}" + ); + assert!( + !engine + .session + .tool_activation_cache + .names() + .any(|name| name == "cached-a"), + "the evicted name must leave the cache" + ); +} + +#[tokio::test] +async fn step_note_names_a_victim_the_refresh_cannot_resurrect() { + use crate::mcp::{McpConfig, McpPool}; + use std::sync::Arc; + use tokio::sync::Mutex as AsyncMutex; + + let (mut engine, _handle) = Engine::new(EngineConfig::default(), &Config::default()); + // A server whose last connect failed auth-required: the pool's refreshed + // list serves the synthetic `mcp_probe_authenticate` tool, so the + // outcome's `mcp_catalog_changed` refresh runs in the same arm whose + // touch evicted it. The refresh must not resurrect the evicted entry + // onto the wire: the step note may name it truthfully. + let mut config = McpConfig::default(); + config.servers.insert( + "probe".to_string(), + crate::mcp::McpServerConfig { + command: None, + args: Vec::new(), + env: HashMap::new(), + cwd: None, + url: Some("https://probe.example.test/mcp".to_string()), + transport: None, + connect_timeout: None, + execute_timeout: None, + read_timeout: None, + disabled: false, + enabled: true, + required: false, + enabled_tools: Vec::new(), + disabled_tools: Vec::new(), + headers: HashMap::new(), + env_headers: HashMap::new(), + bearer_token_env_var: None, + scopes: Vec::new(), + oauth: None, + oauth_resource: None, + reviewed_plugin: None, + }, + ); + let mut pool = McpPool::new(config); + pool.note_connect_failure("probe", &anyhow::Error::msg("HTTP 401 unauthorized")); + engine.mcp_pool = Some(Arc::new(AsyncMutex::new(pool))); + + let victim = "mcp_probe_authenticate"; + let priming_catalog = vec![ + heavy_deferred_tool(victim, 7_000), + heavy_deferred_tool("executed", 7_000), + ]; + let mut active = super::tool_catalog::initial_active_tools(&priming_catalog); + let cached = vec![victim.to_string(), "executed".to_string()]; + engine + .session + .tool_activation_cache + .activate(&priming_catalog, &cached); + active.extend(cached); + + // The catalog's schema for the victim grew after the activation (an MCP + // refresh), so refreshing `executed` overflow-pops it — and the same + // outcome's `mcp_catalog_changed` refresh replaces the pool slice. The + // victim is still deferred in this conversation and no longer activated, + // so the refresh must leave it off the wire and the step note may name + // it: the next request must not carry a tool the note declares + // uncallable. + let mut grown_catalog = vec![ + heavy_deferred_tool(victim, 12_000), + heavy_deferred_tool("executed", 7_000), + ]; + engine + .process_tool_results( + vec![Some(synthetic_toolbox_outcome( + "call-executed", + "executed", + Ok( + ToolResult::success("executed payload").with_metadata(json!({ + "mcp_catalog_changed": true, + })), + ), + ))], + &mut grown_catalog, + &mut active, + &HashMap::new(), + &permissive_turn_policy(), + ) + .await; + + // Premise: the refresh must have actually replaced the pool slice — + // otherwise "nothing resurrects the victim" would hold vacuously. + assert!( + grown_catalog + .iter() + .any(|tool| tool.name == "mcp_probe_authenticate"), + "premise: the refresh must have swapped the pool's slice in" + ); + assert!( + !engine + .session + .tool_activation_cache + .names() + .any(|name| name == victim), + "the victim must leave the cache before the note renders" + ); + assert!( + !active.contains(victim), + "the refresh must not resurrect the evicted victim onto the wire: {active:?}" + ); + let executed = recorded_tool_result_content(&engine, "call-executed"); + assert!( + executed.contains("no longer callable") && executed.contains(victim), + "the step note must name the victim truthfully — it is not callable again: {executed}" + ); +} + +/// The cross-outcome window: a note that rides an early outcome of a batch +/// must stay truthful when a later outcome of the same batch refreshes the +/// MCP catalog. The refresh re-advertises eager entries and still-activated +/// deferred tools — never a bounds-evicted one — so the next request cannot +/// carry a tool an earlier result in the same pass declared uncallable. +#[tokio::test] +async fn refresh_between_outcomes_cannot_resurrect_an_evicted_tool() { + use crate::mcp::{McpConfig, McpPool}; + use std::sync::Arc; + use tokio::sync::Mutex as AsyncMutex; + + let (mut engine, _handle) = Engine::new(EngineConfig::default(), &Config::default()); + let mut config = McpConfig::default(); + config.servers.insert( + "probe".to_string(), + crate::mcp::McpServerConfig { + command: None, + args: Vec::new(), + env: HashMap::new(), + cwd: None, + url: Some("https://probe.example.test/mcp".to_string()), + transport: None, + connect_timeout: None, + execute_timeout: None, + read_timeout: None, + disabled: false, + enabled: true, + required: false, + enabled_tools: Vec::new(), + disabled_tools: Vec::new(), + headers: HashMap::new(), + env_headers: HashMap::new(), + bearer_token_env_var: None, + scopes: Vec::new(), + oauth: None, + oauth_resource: None, + reviewed_plugin: None, + }, + ); + let mut pool = McpPool::new(config); + pool.note_connect_failure("probe", &anyhow::Error::msg("HTTP 401 unauthorized")); + engine.mcp_pool = Some(Arc::new(AsyncMutex::new(pool))); + + let victim = "mcp_probe_authenticate"; + // Turn start: the victim's schema grew between turns, so the entry + // revalidation overflow-pops it and parks the turn-start note — the + // exact sequence `run_turn`'s entry runs. + let priming_catalog = vec![ + heavy_deferred_tool(victim, 7_000), + heavy_deferred_tool("filler", 9_000), + ]; + let mut active = super::tool_catalog::initial_active_tools(&priming_catalog); + let cached = vec![victim.to_string(), "filler".to_string()]; + engine + .session + .tool_activation_cache + .activate(&priming_catalog, &cached); + active.extend(cached); + let mut grown_catalog = vec![ + heavy_deferred_tool(victim, 12_000), + heavy_deferred_tool("filler", 9_000), + ]; + let removals = engine + .session + .tool_activation_cache + .revalidate(&grown_catalog); + super::tool_catalog::remove_toolbox_drops( + &mut active, + removals.dropped_names().map(str::to_string), + ); + assert_eq!( + removals.bounds_evicted, + vec![victim.to_string()], + "premise: the turn-start revalidation must evict the victim" + ); + engine.park_toolbox_eviction_note( + crate::core::session::ToolboxEvictionTrigger::TurnStart, + &removals.bounds_evicted, + ); + + // The batch: a first outcome records the parked note, then a later + // outcome in the same pass carries `mcp_catalog_changed`. The refresh + // must not put the victim back onto the wire behind the note's back. + engine + .process_tool_results( + vec![ + Some(synthetic_toolbox_outcome( + "call-filler", + "filler", + Ok(ToolResult::success("filler payload")), + )), + Some(synthetic_toolbox_outcome( + "call-auth", + victim, + Ok(ToolResult::success("auth payload").with_metadata(json!({ + "mcp_catalog_changed": true, + }))), + )), + ], + &mut grown_catalog, + &mut active, + &HashMap::new(), + &permissive_turn_policy(), + ) + .await; + + // Premise: the refresh must have actually replaced the pool slice — + // otherwise "nothing resurrects the victim" would hold vacuously. + assert!( + grown_catalog + .iter() + .any(|tool| tool.name == "mcp_probe_authenticate"), + "premise: the refresh must have swapped the pool's slice in" + ); + + let filler = recorded_tool_result_content(&engine, "call-filler"); + assert!( + filler.contains("no longer callable") && filler.contains(victim), + "the parked note must ride the batch's first result: {filler}" + ); + assert!( + !active.contains(victim), + "a later outcome's refresh must not resurrect the evicted victim onto the wire: {active:?}" + ); + assert!( + !engine + .session + .tool_activation_cache + .names() + .any(|name| name == victim), + "the victim stays unactivated: a search re-activates it" + ); +} + +/// The mid-turn refresh must apply the same allow/deny narrowing the turn +/// build ran over its catalog: a tool the build stripped must not re-enter +/// the catalog and the active set on a `mcp_catalog_changed` outcome, or a +/// live 401 (or a completed login) would defeat the denied-name +/// concealment. Both refresh paths delegate to the same +/// `ToolSurfacePolicy::allows_tool` spelling; deleting the filter here +/// turns this pin red. +#[tokio::test] +async fn mid_turn_refresh_honors_the_turn_policy() { + use crate::mcp::{McpConfig, McpPool}; + use std::sync::Arc; + use tokio::sync::Mutex as AsyncMutex; + + let (mut engine, _handle) = Engine::new(EngineConfig::default(), &Config::default()); + let mut config = McpConfig::default(); + config.servers.insert( + "probe".to_string(), + crate::mcp::McpServerConfig { + command: None, + args: Vec::new(), + env: HashMap::new(), + cwd: None, + url: Some("https://probe.example.test/mcp".to_string()), + transport: None, + connect_timeout: None, + execute_timeout: None, + read_timeout: None, + disabled: false, + enabled: true, + required: false, + enabled_tools: Vec::new(), + disabled_tools: Vec::new(), + headers: HashMap::new(), + env_headers: HashMap::new(), + bearer_token_env_var: None, + scopes: Vec::new(), + oauth: None, + oauth_resource: None, + reviewed_plugin: None, + }, + ); + let mut pool = McpPool::new(config); + pool.note_connect_failure("probe", &anyhow::Error::msg("HTTP 401 unauthorized")); + engine.mcp_pool = Some(Arc::new(AsyncMutex::new(pool))); + + // Premise: the needs-auth pool's refreshed list serves the synthetic + // authenticate tool, so the denial below has something to deny. + { + let pool = engine.mcp_pool.as_ref().expect("pool").lock().await; + assert!( + pool.to_api_tools() + .iter() + .any(|tool| tool.name == "mcp_probe_authenticate"), + "premise: the needs-auth pool must serve the synthetic login tool" + ); + } + + let policy = policy_for_catalog( + vec![], + None, + Some(vec!["mcp_probe_authenticate".to_string()]), + crate::tui::approval::ApprovalMode::Suggest, + ); + let mut catalog = Vec::new(); + let mut active = HashSet::new(); + engine + .process_tool_results( + vec![Some(synthetic_toolbox_outcome( + "call-auth", + "unrelated", + Ok(ToolResult::success("payload").with_metadata(json!({ + "mcp_catalog_changed": true, + }))), + ))], + &mut catalog, + &mut active, + &HashMap::new(), + &policy, + ) + .await; + + assert!( + !catalog + .iter() + .any(|tool| tool.name == "mcp_probe_authenticate"), + "a build-stripped name must not re-enter the catalog on a refresh: {catalog:?}" + ); + assert!( + !active.contains("mcp_probe_authenticate"), + "a build-stripped name must not re-enter the active set on a refresh: {active:?}" + ); +} + +#[tokio::test] +async fn session_sync_drops_a_parked_toolbox_eviction_note() { + use crate::llm_client::mock::{MockLlmClient, canned}; + + let workspace = tempdir().expect("tempdir"); + let config = Config::default(); + let model = std::sync::Arc::new(MockLlmClient::new(vec![ + canned::tool_call_turn("call-after-sync", "get_goal", "{}"), + canned::simple_text_turn("done"), + ])); + let client: crate::core::model_client::SharedModelClient = model.clone(); + let (mut engine, handle) = Engine::new_with_model_client( + deterministic_engine_config(workspace.path()), + &config, + client, + ); + // A turn dropped between planning and result processing leaves the note + // parked. SyncSession installs another conversation: the parked note + // belongs to the previous one and must never ride the new conversation's + // tool results. This test pins the composed mutant only: the turn-entry + // sweep also masks a missing SyncSession clear (the engine owns the + // field behind the op loop, so the clear's effect is not observable + // between the sync and the next turn) — the clear is defense in depth + // beside that sweep, and removing both is what fails here. + engine.park_toolbox_eviction_note( + crate::core::session::ToolboxEvictionTrigger::Step, + &["ghost-tool".to_string()], + ); + let run_task = tokio::spawn(engine.run()); + + handle + .send(Op::SyncSession { + session_id: Some("fresh-session".to_string()), + messages: Vec::new(), + system_prompt: None, + system_prompt_override: false, + model: crate::config::DEFAULT_TEXT_MODEL.to_string(), + workspace: workspace.path().to_path_buf(), + mode: AppMode::Agent, + }) + .await + .expect("sync fresh session"); + handle + .send(external_user_message_op( + "Run a tool in the new conversation.", + AppMode::Agent, + &config, + )) + .await + .expect("send the turn after sync"); + let mut rx = handle.rx_event.write().await; + loop { + let event = tokio::time::timeout(model_turn_event_timeout(), rx.recv()) + .await + .expect("timed out waiting for the post-sync turn") + .expect("engine event channel closed before turn completion"); + if let Event::TurnComplete { status, error, .. } = event { + assert_eq!(status, TurnOutcomeStatus::Completed, "{error:?}"); + break; + } + } + drop(rx); + + let mut saw_tool_result = false; + for request in model.captured_requests() { + for message in &request.messages { + for block in &message.content { + if let crate::models::ContentBlock::ToolResult { content, .. } = block { + saw_tool_result = true; + assert!( + !content.contains("ghost-tool"), + "the previous conversation's parked note must not survive SyncSession: {content}" + ); + } + } + } + } + assert!( + saw_tool_result, + "the post-sync turn must record at least one tool result, or the ghost-tool scan proves nothing" + ); + + handle.send(Op::Shutdown).await.expect("shutdown engine"); + run_task.await.expect("engine task"); +} + +/// A turn-start revalidation that evicts on the collective bounds (a cached +/// schema grew between turns) must park its note so the turn's first recorded +/// result carries it — the eviction may not ride a search that has not +/// happened yet. +#[tokio::test] +async fn turn_start_revalidation_parks_an_eviction_note_for_the_turn() { + use crate::llm_client::mock::{MockLlmClient, canned}; + + let workspace = tempdir().expect("tempdir"); + let mock = std::sync::Arc::new(MockLlmClient::new(vec![canned::simple_text_turn("done")])); + let client: crate::core::model_client::SharedModelClient = mock; + let (mut engine, _handle) = Engine::new_with_model_client( + deterministic_engine_config(workspace.path()), + &Config::default(), + client, + ); + let priming_catalog = vec![ + heavy_deferred_tool("cached-a", 7_000), + heavy_deferred_tool("cached-b", 7_000), + ]; + let cached = vec!["cached-a".to_string(), "cached-b".to_string()]; + engine + .session + .tool_activation_cache + .activate(&priming_catalog, &cached); + + // The catalog's schema for `cached-a` grew between turns (an MCP + // refresh): this turn's start revalidation evicts the LRU victim, and + // the note must be parked for the turn's first result. + let grown_catalog = vec![ + heavy_deferred_tool("cached-a", 12_000), + heavy_deferred_tool("cached-b", 7_000), + ]; + let policy = policy_for_catalog( + grown_catalog, + None, + None, + crate::tui::approval::ApprovalMode::Suggest, + ); + let mut turn = crate::core::turn::TurnContext::new(4); + let _ = engine.run_turn(&mut turn, policy, None, None).await; + + assert!( + !engine + .session + .tool_activation_cache + .names() + .any(|name| name == "cached-a"), + "the turn-start eviction must drop the LRU victim from the cache" + ); + let notes = &engine.session.pending_toolbox_eviction_notes; + assert_eq!( + notes.len(), + 1, + "the turn-start eviction must park its note for the turn's first result: {notes:?}" + ); + assert_eq!( + notes[0].trigger, + crate::core::session::ToolboxEvictionTrigger::TurnStart + ); + assert!( + notes[0].evicted.iter().any(|name| name == "cached-a"), + "the parked note must name the victim: {notes:?}" + ); +} + +/// The ride-time netting pin: a note parked at the turn's start must not +/// name a tool that the very result carrying it re-admits. The natural +/// retry — the model calling the tool the turn-start eviction just dropped +/// — hydrates it back into the cache before any result records (the batch +/// admission in `plan_tool_calls`), so the receipt the note rides must not +/// declare it uncallable. +#[tokio::test] +async fn turn_start_note_does_not_name_a_tool_the_result_re_admits() { + let (mut engine, _handle) = Engine::new(EngineConfig::default(), &Config::default()); + engine.park_toolbox_eviction_note( + crate::core::session::ToolboxEvictionTrigger::TurnStart, + &["cached-a".to_string()], + ); + // The batch hydration re-admitted the victim before any result recorded: + // plan_tool_calls admits the directly-called deferred tool into the cache + // when the model retries it, under the per-tool budget. + let catalog = vec![heavy_deferred_tool("cached-a", 100)]; + engine + .session + .tool_activation_cache + .activate(&catalog, &["cached-a".to_string()]); + + // The batch's catalog (the refresh channel revalidates against it, so it + // must be the same catalog the hydration admitted from). + let mut surface_catalog = vec![heavy_deferred_tool("cached-a", 100)]; + let mut active = HashSet::new(); + engine + .process_tool_results( + vec![Some(synthetic_toolbox_outcome( + "retry-victim", + "cached-a", + Ok(ToolResult::success( + "Tool `cached-a` was deferred and has now been loaded. \ + Retry the call with its visible schema.", + )), + ))], + &mut surface_catalog, + &mut active, + &HashMap::new(), + &permissive_turn_policy(), + ) + .await; + + assert!( + engine.session.pending_toolbox_eviction_notes.is_empty(), + "the note rode the first result (fully netted); it must not stay parked: {:?}", + engine.session.pending_toolbox_eviction_notes + ); + let receipt = recorded_tool_result_content(&engine, "retry-victim"); + assert!( + receipt.contains("has now been loaded"), + "the hydration receipt must reach the model: {receipt}" + ); + assert!( + !receipt.contains("no longer callable"), + "the receipt must not declare a tool uncallable that the same result re-admits: {receipt}" + ); +} + +/// A refused hydration loads nothing: the receipt must answer the call with +/// the shared oversize reason, but no "Loaded deferred tool" status may +/// announce a load that did not happen, and the refused name must stay out +/// of the batch hydration order — the batch activation is never asked to +/// admit it. +#[tokio::test] +async fn refused_hydration_emits_no_load_status_and_never_enters_the_batch() { + use crate::llm_client::mock::{MockLlmClient, canned}; + + let workspace = tempdir().expect("tempdir"); + let mock = std::sync::Arc::new(MockLlmClient::new(vec![canned::simple_text_turn("unused")])); + let client: crate::core::model_client::SharedModelClient = mock.clone(); + let (mut engine, handle) = Engine::new_with_model_client( + deterministic_engine_config(workspace.path()), + &Config::default(), + client, + ); + // `notify` has a registered execution path (so the direct call passes + // the dispatch gate) and its catalog entry is deferred with an oversize + // schema, so the hydration must refuse instead of load. The sibling + // keeps the single-tool fallback from force-activating it. + let mut registry = crate::tools::ToolRegistry::new(crate::tools::ToolContext::new( + workspace.path().to_path_buf(), + )); + registry.register(std::sync::Arc::new(crate::tools::notify::NotifyTool)); + let catalog = vec![ + heavy_deferred_tool("notify", 20_000), + heavy_deferred_tool("cached-b", 100), + ]; + let surface = test_tool_surface(&engine, registry, Some(catalog.clone()), AppMode::Agent); + let tool_registry = Some(&surface.registry); + let mut tool_uses = vec![super::streaming::ToolUseState { + id: "refuse-1".to_string(), + name: "notify".to_string(), + input: json!({}), + caller: None, + thought_signature: None, + input_buffer: String::new(), + input_parse_error: None, + }]; + let mut active = HashSet::new(); + let mut budget = crate::tools::tool_call_budget::ToolCallBudget::new(surface.max_tool_calls); + let mut turn = crate::core::turn::TurnContext::new(4); + let planned = engine + .plan_tool_calls( + mock.as_ref(), + &mut turn, + &surface, + &mut tool_uses, + &catalog, + tool_registry, + &mut active, + &mut budget, + AppMode::Agent, + ) + .await; + + let plan = planned + .plans + .iter() + .find(|plan| plan.id == "refuse-1") + .expect("the refused call must be planned"); + let receipt = plan + .guard_result + .as_ref() + .expect("the refusal must answer the call as a guard result"); + assert!( + receipt.content.contains("was not loaded") + && receipt + .content + .contains(&super::tool_catalog::toolbox_oversize_reason()), + "the receipt must refuse with the shared oversize reason: {}", + receipt.content + ); + assert!( + !receipt.content.contains("has now been loaded"), + "the receipt must not promise a load: {}", + receipt.content + ); + assert!( + !active.contains("notify"), + "the refused name must stay out of the batch hydration: {active:?}" + ); + assert_eq!( + receipt + .metadata + .as_ref() + .and_then(|metadata| metadata.get("executed")) + .and_then(serde_json::Value::as_bool), + Some(false), + "the refusal must be marked unexecuted, or the working set and the audit would record a call that never happened: {:?}", + receipt.metadata + ); + + // Nothing may announce the load: drain the engine's events and require + // that no status claims the tool was loaded. + let mut rx = handle.rx_event.write().await; + while let Ok(event) = rx.try_recv() { + if let crate::core::events::Event::Status { message } = event { + assert!( + !message.contains("Loaded deferred tool"), + "a refused hydration must not announce a load: {message}" + ); + } + } +} + +/// The refusal's `executed: false` marker must reach the `tool.result` audit +/// payload, not just the receipt metadata: the audit is the record that a +/// refused, never-executed call is not a successful execution. Driven through +/// the real refusal receipt this commit's marker reads, the audit line must +/// carry the marker verbatim. +#[tokio::test] +async fn refusal_audit_payload_carries_the_executed_marker() { + use crate::llm_client::mock::{MockLlmClient, canned}; + + let _guard = lock_test_env(); + let workspace = tempdir().expect("tempdir"); + let audit_path = workspace.path().join("tool-audit.jsonl"); + let _audit = EnvVarGuard::set("CODEWHALE_TOOL_AUDIT_LOG", &audit_path); + let mock = std::sync::Arc::new(MockLlmClient::new(vec![canned::simple_text_turn("unused")])); + let client: crate::core::model_client::SharedModelClient = mock.clone(); + let (mut engine, _handle) = Engine::new_with_model_client( + deterministic_engine_config(workspace.path()), + &Config::default(), + client, + ); + let mut registry = crate::tools::ToolRegistry::new(crate::tools::ToolContext::new( + workspace.path().to_path_buf(), + )); + registry.register(std::sync::Arc::new(crate::tools::notify::NotifyTool)); + let catalog = vec![ + heavy_deferred_tool("notify", 20_000), + heavy_deferred_tool("cached-b", 100), + ]; + let surface = test_tool_surface(&engine, registry, Some(catalog.clone()), AppMode::Agent); + let tool_registry = Some(&surface.registry); + let mut tool_uses = vec![super::streaming::ToolUseState { + id: "refuse-audit".to_string(), + name: "notify".to_string(), + input: json!({}), + caller: None, + thought_signature: None, + input_buffer: String::new(), + input_parse_error: None, + }]; + let mut active = HashSet::new(); + let mut budget = crate::tools::tool_call_budget::ToolCallBudget::new(surface.max_tool_calls); + let mut turn = crate::core::turn::TurnContext::new(4); + let planned = engine + .plan_tool_calls( + mock.as_ref(), + &mut turn, + &surface, + &mut tool_uses, + &catalog, + tool_registry, + &mut active, + &mut budget, + AppMode::Agent, + ) + .await; + + let plan = planned + .plans + .iter() + .find(|plan| plan.id == "refuse-audit") + .expect("the refused call must be planned"); + let receipt = plan + .guard_result + .as_ref() + .expect("the refusal must answer the call as a guard result") + .clone(); + + // The results phase consumes the receipt exactly as production does. + engine + .process_tool_results( + vec![Some(synthetic_toolbox_outcome( + "refuse-audit", + "notify", + Ok(receipt), + ))], + &mut catalog.clone(), + &mut active, + &HashMap::new(), + &permissive_turn_policy(), + ) + .await; + + let audit = std::fs::read_to_string(&audit_path).expect("the audit log must exist"); + let line = audit + .lines() + .find_map(|line| { + let value: serde_json::Value = serde_json::from_str(line).ok()?; + (value.get("event")? == "tool.result" && value.get("tool_id")? == "refuse-audit") + .then_some(value) + }) + .expect("the refusal's tool.result audit line must exist"); + assert_eq!( + line.get("executed").and_then(serde_json::Value::as_bool), + Some(false), + "the audit must record the refused call as never executed: {line}" + ); + assert_eq!( + line.get("success").and_then(serde_json::Value::as_bool), + Some(true), + "the receipt travels the success path — only the executed marker distinguishes it: {line}" + ); +} + +/// The hydration batch's eviction park is production wiring, not just the +/// shared helper: a direct call that hydrates a deferred tool can itself +/// overflow the collective bounds and evict a cached victim. Driven through +/// the real `plan_tool_calls`, the batch must park the note for the turn's +/// first recorded result. +#[tokio::test] +async fn hydration_batch_eviction_parks_its_note_through_plan_tool_calls() { + use crate::llm_client::mock::{MockLlmClient, canned}; + + let workspace = tempdir().expect("tempdir"); + let mock = std::sync::Arc::new(MockLlmClient::new(vec![canned::simple_text_turn("unused")])); + let client: crate::core::model_client::SharedModelClient = mock.clone(); + let (mut engine, _handle) = Engine::new_with_model_client( + deterministic_engine_config(workspace.path()), + &Config::default(), + client, + ); + // `notify` has a registered execution path (so the direct call passes + // the dispatch gate) and its catalog entry is deferred and admittable — + // but admitting it on top of the pre-filled cache overflows the + // collective byte budget and evicts the LRU-front victim. + let mut registry = crate::tools::ToolRegistry::new(crate::tools::ToolContext::new( + workspace.path().to_path_buf(), + )); + registry.register(std::sync::Arc::new(crate::tools::notify::NotifyTool)); + let catalog = vec![ + heavy_deferred_tool("notify", 4_000), + heavy_deferred_tool("cached-a", 7_000), + heavy_deferred_tool("cached-b", 7_000), + ]; + engine + .session + .tool_activation_cache + .activate(&catalog, &["cached-a".to_string(), "cached-b".to_string()]); + let surface = test_tool_surface(&engine, registry, Some(catalog.clone()), AppMode::Agent); + let tool_registry = Some(&surface.registry); + let mut tool_uses = vec![super::streaming::ToolUseState { + id: "hydrate-1".to_string(), + name: "notify".to_string(), + input: json!({}), + caller: None, + thought_signature: None, + input_buffer: String::new(), + input_parse_error: None, + }]; + let mut active = HashSet::new(); + let mut budget = crate::tools::tool_call_budget::ToolCallBudget::new(surface.max_tool_calls); + let mut turn = crate::core::turn::TurnContext::new(4); + engine + .plan_tool_calls( + mock.as_ref(), + &mut turn, + &surface, + &mut tool_uses, + &catalog, + tool_registry, + &mut active, + &mut budget, + AppMode::Agent, + ) + .await; + + let notes = &engine.session.pending_toolbox_eviction_notes; + assert_eq!( + notes.len(), + 1, + "the hydration batch must park its eviction note for the turn's first recorded result: {notes:?}" + ); + assert_eq!( + notes[0].trigger, + crate::core::session::ToolboxEvictionTrigger::Step, + "a hydration is a non-search activation step: {notes:?}" + ); + assert_eq!( + notes[0].evicted, + vec!["cached-a".to_string()], + "the note must name the LRU-front victim the admission evicted: {notes:?}" + ); + assert!( + !engine.session.tool_activation_cache.contains("cached-a") + && engine.session.tool_activation_cache.contains("cached-b") + && engine.session.tool_activation_cache.contains("notify"), + "the victim must leave the cache while the sibling and the hydrated tool stay" + ); + assert!( + active.contains("notify"), + "the hydrated tool must join the active set: {active:?}" + ); +} + +/// Turn-start revalidation and a hydration batch can both evict in the same +/// turn; the parked notes must combine, not overwrite. +#[tokio::test] +async fn toolbox_eviction_notes_park_and_combine() { + let (mut engine, _handle) = Engine::new(EngineConfig::default(), &Config::default()); + engine.park_toolbox_eviction_note( + crate::core::session::ToolboxEvictionTrigger::TurnStart, + &["turn-victim".to_string()], + ); + engine.park_toolbox_eviction_note( + crate::core::session::ToolboxEvictionTrigger::Step, + &["step-victim".to_string()], + ); + + let notes = &engine.session.pending_toolbox_eviction_notes; + assert_eq!(notes.len(), 2, "both notes must stay parked: {notes:?}"); + assert_eq!( + notes[0].trigger, + crate::core::session::ToolboxEvictionTrigger::TurnStart + ); + assert!(notes[0].evicted.iter().any(|name| name == "turn-victim")); + assert_eq!( + notes[1].trigger, + crate::core::session::ToolboxEvictionTrigger::Step + ); + assert!(notes[1].evicted.iter().any(|name| name == "step-victim")); + + // An empty eviction list parks nothing and must not clobber the slot. + engine.park_toolbox_eviction_note(crate::core::session::ToolboxEvictionTrigger::Step, &[]); + assert_eq!( + engine.session.pending_toolbox_eviction_notes.len(), + 2, + "an empty eviction list must not clear the parked notes" + ); +} + +/// A note stranded by an interrupted turn must be swept at the next turn's +/// entry, not ride that turn's first result under a false "this step" +/// attribution. +#[tokio::test] +async fn turn_start_sweeps_a_stranded_eviction_note() { + use crate::llm_client::mock::{MockLlmClient, canned}; + + let workspace = tempdir().expect("tempdir"); + let mock = std::sync::Arc::new(MockLlmClient::new(vec![canned::simple_text_turn("done")])); + let client: crate::core::model_client::SharedModelClient = mock; + let (mut engine, _handle) = Engine::new_with_model_client( + deterministic_engine_config(workspace.path()), + &Config::default(), + client, + ); + engine.park_toolbox_eviction_note( + crate::core::session::ToolboxEvictionTrigger::Step, + &["ghost-tool".to_string()], + ); + + // No eviction happens this turn: the sweep must still clear the + // stranded note, and nothing may re-park it. + let policy = policy_for_catalog( + vec![heavy_deferred_tool("cached-a", 7_000)], + None, + None, + crate::tui::approval::ApprovalMode::Suggest, + ); + let mut turn = crate::core::turn::TurnContext::new(4); + let _ = engine.run_turn(&mut turn, policy, None, None).await; + + assert!( + engine.session.pending_toolbox_eviction_notes.is_empty(), + "a stranded note must not survive into the turn to misattribute its evictions" + ); +} + +/// The turn-entry sweep must run before the turn-start park: a note +/// stranded by the previous turn is dropped while this turn's fresh +/// eviction note survives. Sweeping after the park would eat the fresh +/// note and lose the eviction report entirely. +#[tokio::test] +async fn turn_entry_sweep_precedes_the_turn_start_park() { + use crate::llm_client::mock::{MockLlmClient, canned}; + + let workspace = tempdir().expect("tempdir"); + let mock = std::sync::Arc::new(MockLlmClient::new(vec![canned::simple_text_turn("done")])); + let client: crate::core::model_client::SharedModelClient = mock; + let (mut engine, _handle) = Engine::new_with_model_client( + deterministic_engine_config(workspace.path()), + &Config::default(), + client, + ); + // Stranded by an interrupted previous turn. + engine.park_toolbox_eviction_note( + crate::core::session::ToolboxEvictionTrigger::Step, + &["ghost-tool".to_string()], + ); + + // This turn's start evicts on the collective bounds (a cached schema + // grew between turns) and parks its own note. + let priming_catalog = vec![ + heavy_deferred_tool("cached-a", 7_000), + heavy_deferred_tool("cached-b", 7_000), + ]; + let cached = vec!["cached-a".to_string(), "cached-b".to_string()]; + engine + .session + .tool_activation_cache + .activate(&priming_catalog, &cached); + let grown_catalog = vec![ + heavy_deferred_tool("cached-a", 12_000), + heavy_deferred_tool("cached-b", 7_000), + ]; + let policy = policy_for_catalog( + grown_catalog, + None, + None, + crate::tui::approval::ApprovalMode::Suggest, + ); + let mut turn = crate::core::turn::TurnContext::new(4); + let _ = engine.run_turn(&mut turn, policy, None, None).await; + + let notes = &engine.session.pending_toolbox_eviction_notes; + assert_eq!( + notes.len(), + 1, + "the stranded note must be swept and the fresh turn-start note must survive: {notes:?}" + ); + assert_eq!( + notes[0].trigger, + crate::core::session::ToolboxEvictionTrigger::TurnStart + ); + assert!( + notes[0].evicted.iter().any(|name| name == "cached-a"), + "the fresh note names this turn's victim: {notes:?}" + ); + assert!( + !notes[0].evicted.iter().any(|name| name == "ghost-tool"), + "the stranded note must not combine into this turn's report: {notes:?}" + ); +} + +/// A batch whose outcomes all flattened away has no result to carry the +/// parked note; it must be re-parked for a later batch instead of dropped. +#[tokio::test] +async fn toolbox_note_survives_a_batch_without_results() { + let (mut engine, _handle) = Engine::new(EngineConfig::default(), &Config::default()); + engine.park_toolbox_eviction_note( + crate::core::session::ToolboxEvictionTrigger::Step, + &["ghost-tool".to_string()], + ); + let mut catalog = Vec::new(); + let mut active = HashSet::new(); + engine + .process_tool_results( + vec![None, None], + &mut catalog, + &mut active, + &HashMap::new(), + &permissive_turn_policy(), + ) + .await; + + let notes = &engine.session.pending_toolbox_eviction_notes; + assert_eq!( + notes.len(), + 1, + "a resultless batch must re-park the note instead of dropping it: {notes:?}" + ); + assert!( + notes[0].evicted.iter().any(|name| name == "ghost-tool"), + "the re-parked note must keep its content: {notes:?}" + ); +} diff --git a/crates/tui/src/core/engine/tool_catalog.rs b/crates/tui/src/core/engine/tool_catalog.rs index 8863cb9129..d13c13860d 100644 --- a/crates/tui/src/core/engine/tool_catalog.rs +++ b/crates/tui/src/core/engine/tool_catalog.rs @@ -18,7 +18,7 @@ use crate::models::Tool; use crate::tools::spec::{ToolError, ToolResult, optional_str, optional_u64, required_str}; use crate::tui::app::AppMode; -use crate::core::session::ToolActivationCache; +use crate::core::session::{PendingToolboxEviction, ToolActivationCache, ToolboxEvictionTrigger}; use crate::dependencies::ExternalTool; use crate::regex_cache::compile_user_regex; @@ -403,27 +403,190 @@ pub(crate) fn initial_active_tools(catalog: &[Tool]) -> HashSet { active } -/// Remove schemas evicted from the conversation cache without hiding tools -/// that the current catalog now exposes eagerly. +/// Remove names the bounded toolbox dropped — unavailable entries and +/// bounds evictions — from the active set. /// -/// A cached tool can become eager after an explicit `tools_always_load` -/// change or another policy update. Cache revalidation correctly forgets the -/// old deferred entry, but the eager catalog entry must remain active. -pub(crate) fn remove_evicted_cache_activations( - catalog: &[Tool], +/// Eager promotions never appear in these lists: `revalidate` classifies +/// them out because the tool is directly callable at the engine layer, so +/// removing these names cannot hide a tool that the current catalog still +/// exposes eagerly. (A schema the provider wire still rejects is the +/// sanitize layer's concern and outside this vocabulary.) +pub(crate) fn remove_toolbox_drops( active: &mut HashSet, - evicted: impl IntoIterator, + names: impl IntoIterator, ) { - for name in evicted { - let is_eager_now = catalog - .iter() - .any(|tool| tool.name == name && !tool.defer_loading.unwrap_or(false)); - if !is_eager_now { - active.remove(&name); - } + for name in names { + active.remove(&name); + } +} + +/// Why a catalog-shrink stale entry is reported under the unavailable +/// vocabulary instead of the eviction note: the tool left the deferred +/// catalog itself (disconnect, deny rule, mode switch), so the search-result +/// recovery contract ("re-search to re-activate") would be a false promise +/// for it. Oversize drops carry [`toolbox_oversize_reason`] instead. +const TOOLBOX_CATALOG_SHRINK_REASON: &str = "Previously active in this conversation but no longer available in the tool catalog; searching cannot re-activate it while it stays unavailable."; + +/// Why a deferred tool stays out of the bounded conversation toolbox: its +/// serialized form — name, description, and schema measured together — +/// exceeds the per-tool budget, so no activation can admit it while it +/// stays that large. Shared by discovery-time rejections and revalidation +/// drops so the prose cannot drift from the real bounds. Search-context +/// callers append the recovery hint via [`toolbox_oversize_search_hint`]; +/// a direct-call refusal has no query to narrow and states the bounds only. +pub(crate) fn toolbox_oversize_reason() -> String { + format!( + "The serialized tool exceeds the bounded conversation toolbox ({} \ + cached tools, {}KiB of serialized tool definitions).", + crate::core::session::TOOL_ACTIVATION_CACHE_MAX_NAMES, + crate::core::session::TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES / 1024 + ) +} + +/// The search-context tail for [`toolbox_oversize_reason`]: after a +/// discovery rejection the search is the recovery path, so the reason says +/// what a narrower query can do. Only a search result may attach it. +pub(crate) fn toolbox_oversize_search_hint() -> &'static str { + " Narrow the search or use a smaller matching tool." +} + +/// [`toolbox_oversize_reason`] with the search-context recovery hint, for +/// the `tool_search` payload's unavailable references. +fn search_oversize_reason() -> String { + format!( + "{}{}", + toolbox_oversize_reason(), + toolbox_oversize_search_hint() + ) +} + +/// The model-facing sentence naming a bounds-driven toolbox eviction and the +/// recovery contract. `trigger` names the step that surfaced it; the capacity +/// label is derived from the session constants so the prose cannot drift from +/// the real bounds. The eviction may come from the admission itself or from +/// revalidation after a cached schema grew, so the wording stays neutral +/// about the cause. +pub(crate) fn bounded_toolbox_eviction_note( + trigger: ToolboxEvictionTrigger, + evicted: &[String], +) -> String { + format!( + "This {} evicted tools from the bounded conversation toolbox \ + ({} cached tools, {}KiB of serialized tool definitions); the evicted tools \ + are no longer callable until a later search re-activates them: {}", + trigger.label(), + crate::core::session::TOOL_ACTIVATION_CACHE_MAX_NAMES, + crate::core::session::TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES / 1024, + evicted.join(", ") + ) +} + +/// Park a bounds-driven toolbox eviction for a later result to carry, +/// combining with any note already parked — turn-start revalidation and a +/// hydration batch can both evict before a result records. Shared by the +/// parent engine and the child surface so the combine contract is stated +/// and pinned once. +pub(crate) fn park_toolbox_eviction( + notes: &mut Vec, + trigger: ToolboxEvictionTrigger, + evicted: &[String], +) { + if evicted.is_empty() { + return; + } + tracing::debug!( + target: "engine.tool_surface", + trigger = trigger.label(), + ?evicted, + "parked toolbox eviction note for a later result" + ); + notes.push(PendingToolboxEviction { + trigger, + evicted: evicted.to_vec(), + }); +} + +/// Net and render toolbox eviction notes for the result they ride: the +/// turn's parked notes and the step's own touch/retrieval evictions share +/// this attach-time netting. A name that is back in the activation cache is +/// callable again, and so is a name sitting in the request's active tool +/// set; a name an earlier note in the same ride already named is not +/// repeated. Either way the result must not declare the tool "no longer +/// callable": such names are dropped, and a note emptied by the netting is +/// dropped with them. A name that left the catalog between its eviction and +/// this ride can no longer be re-activated by a search, so it is reported +/// under that truth instead of the recovery promise. Consumes the notes — +/// the slot is cleared whether or not anything rendered — so a caller +/// cannot re-ride them on a later result. Returns `None` when nothing +/// remains to report. +pub(crate) fn render_toolbox_eviction_notes( + notes: &mut Vec, + cache: &ToolActivationCache, + active_names: &std::collections::HashSet, + catalog: &[Tool], +) -> Option { + let catalog_names: HashSet<&str> = catalog.iter().map(|tool| tool.name.as_str()).collect(); + let mut rendered_names: HashSet = HashSet::new(); + for note in notes.iter_mut() { + note.evicted.retain(|name| { + !cache.contains(name) && !active_names.contains(name) && !rendered_names.contains(name) + }); + rendered_names.extend(note.evicted.iter().cloned()); + } + let rendered: Vec = notes + .iter() + .filter(|note| !note.evicted.is_empty()) + .map(|note| { + let (recoverable, gone): (Vec, Vec) = note + .evicted + .iter() + .cloned() + .partition(|name| catalog_names.contains(name.as_str())); + let mut text = Vec::new(); + if !recoverable.is_empty() { + text.push(bounded_toolbox_eviction_note(note.trigger, &recoverable)); + } + if !gone.is_empty() { + text.push(format!( + "This {} evicted {} that {} since left the toolbox \ + catalog; a search can no longer re-activate {}: {}", + note.trigger.label(), + if gone.len() == 1 { "a tool" } else { "tools" }, + if gone.len() == 1 { "has" } else { "have" }, + if gone.len() == 1 { "it" } else { "them" }, + gone.join(", ") + )); + } + text.join("\n\n") + }) + .collect(); + let ride_count = rendered.len(); + notes.clear(); + if rendered.is_empty() { + None + } else { + tracing::debug!( + target: "engine.tool_surface", + notes = ride_count, + "toolbox eviction notes ride a recorded result" + ); + Some(rendered.join("\n\n")) } } +/// Outcome of a non-search activation against the bounded conversation +/// toolbox: whether the request-visible tool surface changed, and which +/// previously active tools this activation bounds-evicted — from the +/// admission itself or from revalidation after a cached schema grew. +/// Unavailable drops are handled inside the callee and deliberately absent +/// here — they are reported (where at all) under the unavailable vocabulary, +/// never under the re-activatable eviction contract. +#[derive(Debug, Clone, Default)] +pub(crate) struct ToolSurfaceUpdate { + pub(crate) changed: bool, + pub(crate) evicted: Vec, +} + /// Promote a successfully executed deferred tool only when this conversation /// had already activated it. Execution can update recency, never grant a name. pub(crate) fn touch_cached_tool_after_execution( @@ -431,14 +594,17 @@ pub(crate) fn touch_cached_tool_after_execution( active: &mut HashSet, cache: &mut ToolActivationCache, name: &str, -) -> bool { +) -> ToolSurfaceUpdate { if !cache.names().any(|cached| cached == name) { - return false; + return ToolSurfaceUpdate::default(); } let delta = cache.activate(catalog, &[name.to_string()]); - remove_evicted_cache_activations(catalog, active, delta.evicted); + remove_toolbox_drops(active, delta.dropped_names().map(str::to_string)); active.extend(delta.admitted); - true + ToolSurfaceUpdate { + changed: true, + evicted: delta.evicted, + } } /// Make the recovery schema visible on the next provider step when a tool @@ -450,7 +616,7 @@ pub(crate) fn activate_result_dependencies( active: &mut HashSet, cache: &mut ToolActivationCache, result: &ToolResult, -) -> bool { +) -> ToolSurfaceUpdate { let needs_retrieval = result .metadata .as_ref() @@ -458,12 +624,15 @@ pub(crate) fn activate_result_dependencies( .and_then(Value::as_bool) == Some(true); if !needs_retrieval { - return false; + return ToolSurfaceUpdate::default(); } let delta = cache.activate(catalog, &[TOOL_RESULT_RETRIEVAL_NAME.to_string()]); - remove_evicted_cache_activations(catalog, active, delta.evicted.iter().cloned()); + remove_toolbox_drops(active, delta.dropped_names().map(str::to_string)); active.extend(delta.admitted.iter().cloned()); - !delta.admitted.is_empty() || !delta.evicted.is_empty() + ToolSurfaceUpdate { + changed: !delta.admitted.is_empty() || !delta.evicted.is_empty() || !delta.stale.is_empty(), + evicted: delta.evicted, + } } fn active_tool_list_from_catalog(catalog: &[Tool], active: &HashSet) -> Vec { @@ -1065,15 +1234,55 @@ pub(super) fn maybe_hydrate_requested_deferred_tool( catalog: &[Tool], active_tools_at_batch_start: &HashSet, hydrated_tools_this_batch: &mut HashSet, -) -> Option { +) -> Option { let def = catalog.iter().find(|def| def.name == tool_name)?; if !def.defer_loading.unwrap_or(false) || active_tools_at_batch_start.contains(tool_name) { return None; } + // An oversize tool can never enter the bounded toolbox, so the + // receipt must not promise a load that the admission would refuse — + // that promise turns every retry into another false "has now been + // loaded". Refuse up front with the shared oversize reason, the same + // statement the child surface makes. The name stays out of the + // hydration set, so the batch activation is never asked to admit it. + if ToolActivationCache::exceeds_per_tool_budget(def) { + return Some(DeferredHydration::Refused( + deferred_tool_oversize_hydration_result(def), + )); + } + hydrated_tools_this_batch.insert(tool_name.to_string()); - Some(deferred_tool_schema_hydration_result(def, tool_input)) + Some(DeferredHydration::Loaded( + deferred_tool_schema_hydration_result(def, tool_input), + )) +} + +/// Outcome of a direct call to a deferred tool: either the schema was +/// loaded into the batch hydration (the receipt publishes it and the load +/// is announced), or the tool cannot enter the toolbox and the receipt +/// refuses. The caller must keep the two apart: a refusal is not a load, +/// and announcing one as the other lies to the user and the audit log. +pub(super) enum DeferredHydration { + Loaded(ToolResult), + Refused(ToolResult), +} + +fn deferred_tool_oversize_hydration_result(tool: &Tool) -> ToolResult { + // `executed: false` mirrors the Loaded receipt: the refusal answers a + // tool call without running it, and the results phase defaults the + // executed flag to true — without the marker the working set would + // observe a call that never happened and the audit would log the + // refusal as a successful execution. + ToolResult::success(format!( + "Tool `{}` was not loaded and was not executed: {}", + tool.name, + toolbox_oversize_reason() + )) + .with_metadata(json!({ + "executed": false, + })) } #[cfg(test)] @@ -1085,7 +1294,7 @@ pub(super) fn preflight_requested_deferred_tool( ) -> Option { let active_tools_at_batch_start = active_tools.clone(); let mut hydrated_tools_this_batch = HashSet::new(); - let result = maybe_hydrate_requested_deferred_tool( + let hydration = maybe_hydrate_requested_deferred_tool( tool_name, tool_input, catalog, @@ -1093,7 +1302,9 @@ pub(super) fn preflight_requested_deferred_tool( &mut hydrated_tools_this_batch, ); active_tools.extend(hydrated_tools_this_batch); - result + hydration.map(|hydration| match hydration { + DeferredHydration::Loaded(result) | DeferredHydration::Refused(result) => result, + }) } fn deferred_tool_schema_hydration_result(tool: &Tool, tool_input: &Value) -> ToolResult { @@ -1357,10 +1568,14 @@ fn execute_tool_search_inner( unavailable_core_action_tools_with_bm25_like(catalog, query, remaining_results) }; - let mut cache_rejected = Vec::new(); + let mut cache_rejected: Vec = Vec::new(); + let mut cache_evicted = Vec::new(); + let mut cache_stale = Vec::::new(); if let Some(cache) = cache { let delta = cache.activate(catalog, &discovered); - remove_evicted_cache_activations(catalog, active_tools, delta.evicted); + remove_toolbox_drops(active_tools, delta.dropped_names().map(str::to_string)); + cache_stale = delta.stale; + cache_evicted = delta.evicted; cache_rejected = delta.rejected; discovered = delta.admitted; } @@ -1386,9 +1601,41 @@ fn execute_tool_search_inner( json!({ "type": "unavailable_tool_reference", "tool_name": name, - "reason": "The tool schema exceeds the bounded conversation toolbox (8 cached tools, 16KiB of added serialized schemas). Narrow the search or use a smaller matching tool." + "reason": search_oversize_reason(), }) })); + // Stale evictions travel under the unavailable vocabulary — the eviction + // note's "re-search to re-activate" contract would be a false promise + // for them. The cause picks the truthful reason: a catalog shrink cannot + // be searched back, while an oversize drop can return once its schema + // shrinks. A discovery rejection can only ever mean oversize (discovery + // surfaces catalog members alone), so an Oversize stale entry and its + // rejection would carry the same reason — emit that pair once, chosen + // by cause rather than by name so a CatalogShrink entry for the same + // name would still state its own truth. + let rejected_names: std::collections::HashSet<&str> = + cache_rejected.iter().map(String::as_str).collect(); + unavailable_references.extend(cache_stale.iter().filter_map(|entry| { + if rejected_names.contains(entry.name.as_str()) + && matches!( + entry.cause, + crate::core::session::ToolActivationStaleCause::Oversize + ) + { + return None; + } + let reason = match entry.cause { + crate::core::session::ToolActivationStaleCause::CatalogShrink => { + TOOLBOX_CATALOG_SHRINK_REASON.to_string() + } + crate::core::session::ToolActivationStaleCause::Oversize => search_oversize_reason(), + }; + Some(json!({ + "type": "unavailable_tool_reference", + "tool_name": entry.name, + "reason": reason, + })) + })); // Boot-window status (#588): a bare empty result is byte-identical to // "no server configured", which is how a transient connect window @@ -1401,6 +1648,23 @@ fn execute_tool_search_inner( "tool_references": references, "unavailable_tool_references": unavailable_references.clone(), }); + // The activation cache is bounded (8 cached tools, 16KiB of serialized + // tool definitions), so admitting new tools can silently revoke ones this + // conversation already used. Naming the evictions in the result keeps + // the model's picture of its callable toolbox honest — otherwise it sees + // a previously working tool disappear between turns with no explanation. + if !cache_evicted.is_empty() { + payload["evicted_tool_references"] = json!( + cache_evicted + .iter() + .map(|name| json!({"type": "tool_reference", "tool_name": name})) + .collect::>() + ); + payload["toolbox_note"] = json!(bounded_toolbox_eviction_note( + ToolboxEvictionTrigger::ToolSearch, + &cache_evicted + )); + } if let Some(servers) = boot_status { payload["mcp_boot"] = json!({ "status": "connecting", @@ -1413,6 +1677,10 @@ fn execute_tool_search_inner( "tool_references": discovered, "unavailable_tool_references": unavailable_references, }); + if !cache_evicted.is_empty() { + metadata["evicted_tool_references"] = json!(cache_evicted); + metadata["toolbox_note"] = payload["toolbox_note"].clone(); + } if boot_status.is_some() { metadata["mcp_boot"] = payload["mcp_boot"].clone(); } diff --git a/crates/tui/src/core/engine/tool_catalog/tests.rs b/crates/tui/src/core/engine/tool_catalog/tests.rs index 9154cb7f35..ed3d1a2cf3 100644 --- a/crates/tui/src/core/engine/tool_catalog/tests.rs +++ b/crates/tui/src/core/engine/tool_catalog/tests.rs @@ -1,10 +1,11 @@ use super::{ - CODE_EXECUTION_DESCRIPTION, DEFAULT_ACTIVE_NATIVE_TOOLS, + CODE_EXECUTION_DESCRIPTION, DEFAULT_ACTIVE_NATIVE_TOOLS, DeferredHydration, allowlist_is_native_file_and_shell_only, apply_mcp_tool_deferral, apply_native_tool_deferral, apply_registry_first_shell_guidance, build_model_tool_catalog_with_surface, default_synthetic_catalog_tool_names, ensure_advanced_tooling, execute_tool_search_with_cache, - initial_active_tools, is_synthetic_catalog_tool, remove_evicted_cache_activations, - tool_matches_any_rule, touch_cached_tool_after_execution, + initial_active_tools, is_synthetic_catalog_tool, maybe_hydrate_requested_deferred_tool, + remove_toolbox_drops, render_toolbox_eviction_notes, tool_matches_any_rule, + toolbox_oversize_reason, toolbox_oversize_search_hint, touch_cached_tool_after_execution, }; use crate::core::session::ToolActivationCache; use crate::models::Tool; @@ -135,11 +136,15 @@ fn cache_eviction_does_not_hide_a_tool_that_became_eager() { catalog[0].defer_loading = Some(false); let mut active = initial_active_tools(&catalog); - let evicted = cache.revalidate(&catalog); - remove_evicted_cache_activations(&catalog, &mut active, evicted); + let removals = cache.revalidate(&catalog); + remove_toolbox_drops(&mut active, removals.dropped_names().map(str::to_string)); assert!(active.contains("promoted")); assert_eq!(cache.names().count(), 0); + // The promotion is not an unavailability: reporting it as gone would be + // a false statement, so neither removal bucket may name it. + assert!(removals.stale.is_empty()); + assert!(removals.bounds_evicted.is_empty()); } #[test] @@ -159,24 +164,19 @@ fn successful_cached_execution_updates_lru_without_granting_uncached_names() { let delta = cache.activate(&catalog, &first); active.extend(delta.admitted); - assert!(touch_cached_tool_after_execution( - &catalog, - &mut active, - &mut cache, - "deferred-0" - )); + assert!( + touch_cached_tool_after_execution(&catalog, &mut active, &mut cache, "deferred-0").changed + ); let delta = cache.activate(&catalog, &["deferred-8".to_string()]); - remove_evicted_cache_activations(&catalog, &mut active, delta.evicted); + remove_toolbox_drops(&mut active, delta.dropped_names().map(str::to_string)); active.extend(delta.admitted); assert!(cache.names().any(|name| name == "deferred-0")); assert!(!cache.names().any(|name| name == "deferred-1")); - assert!(!touch_cached_tool_after_execution( - &catalog, - &mut active, - &mut cache, - "never-activated" - )); + assert!( + !touch_cached_tool_after_execution(&catalog, &mut active, &mut cache, "never-activated") + .changed + ); assert!(!active.contains("never-activated")); } @@ -351,3 +351,650 @@ fn settled_search_result_carries_no_booting_block() { ); } } + +/// The activation cache is bounded, so a search that admits new tools can +/// revoke ones the conversation already used. The result must name those +/// evictions — a tool that silently vanishes between turns is exactly the +/// "reference returned but never callable" trap. Searches without evictions +/// keep the legacy result shape with no eviction block at all. +#[test] +fn search_result_names_cache_evictions_instead_of_dropping_them_silently() { + let catalog: Vec = (0..10) + .map(|index| { + let mut definition = tool(&format!("deferred-{index}")); + definition.defer_loading = Some(true); + definition + }) + .collect(); + let mut active = initial_active_tools(&catalog); + let mut cache = ToolActivationCache::default(); + + let first = execute_tool_search_with_cache( + super::TOOL_SEARCH_NAME, + &json!({"query": "deferred", "max_results": 8}), + &catalog, + &mut active, + &mut cache, + None, + ) + .expect("first search should succeed"); + let first_payload: serde_json::Value = + serde_json::from_str(&first.content).expect("payload is JSON"); + assert_eq!( + first_payload["tool_references"].as_array().unwrap().len(), + 8 + ); + assert!( + first_payload.get("evicted_tool_references").is_none(), + "a search without evictions must keep the legacy shape: {first_payload}" + ); + assert!(active.contains("deferred-0")); + assert_eq!(cache.names().count(), 8); + + // Naming deferred-8/9 admits two new tools into the full cache and must + // evict the two least-recently-used activations. + let second = execute_tool_search_with_cache( + super::TOOL_SEARCH_NAME, + &json!({"query": "deferred-8 deferred-9", "max_results": 8}), + &catalog, + &mut active, + &mut cache, + None, + ) + .expect("second search should succeed"); + let second_payload: serde_json::Value = + serde_json::from_str(&second.content).expect("payload is JSON"); + assert_eq!( + second_payload["evicted_tool_references"], + json!([ + {"type": "tool_reference", "tool_name": "deferred-0"}, + {"type": "tool_reference", "tool_name": "deferred-1"}, + ]) + ); + let note = second_payload["toolbox_note"].as_str().expect("note"); + assert!( + note.contains("deferred-0") && note.contains("no longer callable"), + "the note must name the evictions and the recovery contract: {note}" + ); + assert!(!active.contains("deferred-0")); + assert!(!active.contains("deferred-1")); + assert!(active.contains("deferred-8")); + let metadata = second.metadata.expect("search metadata"); + assert_eq!( + metadata["evicted_tool_references"], + json!(["deferred-0", "deferred-1"]) + ); + assert_eq!(metadata["toolbox_note"], second_payload["toolbox_note"]); +} + +/// The serialized-bytes bound evicts too, not just the eight-name bound, and +/// the reporting must not care which bound fired. +#[test] +fn search_result_names_byte_bound_cache_evictions_too() { + let heavy = |name: &str, description_bytes: usize| { + let mut definition = tool(name); + definition.defer_loading = Some(true); + definition.description = "x".repeat(description_bytes); + definition + }; + let catalog = vec![ + heavy("deferred-small", 10_000), + heavy("deferred-large", 8_000), + ]; + let mut active = initial_active_tools(&catalog); + let mut cache = ToolActivationCache::default(); + + execute_tool_search_with_cache( + super::TOOL_SEARCH_NAME, + &json!({"query": "^deferred-small", "match": "regex"}), + &catalog, + &mut active, + &mut cache, + None, + ) + .expect("first search should succeed"); + assert_eq!(cache.names().collect::>(), vec!["deferred-small"]); + + let second = execute_tool_search_with_cache( + super::TOOL_SEARCH_NAME, + &json!({"query": "^deferred-large", "match": "regex"}), + &catalog, + &mut active, + &mut cache, + None, + ) + .expect("second search should succeed"); + let payload: serde_json::Value = + serde_json::from_str(&second.content).expect("payload is JSON"); + // Both tools sit under the per-tool cap, but together they overflow the + // 16KiB cache budget, so admitting the second evicts the first. + assert_eq!( + payload["evicted_tool_references"], + json!([{"type": "tool_reference", "tool_name": "deferred-small"}]) + ); + let note = payload["toolbox_note"].as_str().expect("note"); + assert!( + note.contains("deferred-small") && note.contains("16KiB"), + "the note must name the byte-bound eviction: {note}" + ); + assert!(!active.contains("deferred-small")); + let metadata = second.metadata.expect("search metadata"); + assert_eq!( + metadata["evicted_tool_references"], + json!(["deferred-small"]) + ); +} + +/// A search after the catalog shrank must report the dropped activation under +/// the unavailable vocabulary — the eviction note's "re-search to +/// re-activate" contract would be a false promise for a tool that is gone +/// from the catalog. +#[test] +fn search_reports_catalog_stale_evictions_as_unavailable_not_evicted() { + let catalog_v1: Vec = (0..3) + .map(|index| { + let mut definition = tool(&format!("deferred-{index}")); + definition.defer_loading = Some(true); + definition + }) + .collect(); + let mut active = initial_active_tools(&catalog_v1); + let mut cache = ToolActivationCache::default(); + execute_tool_search_with_cache( + super::TOOL_SEARCH_NAME, + &json!({"query": "deferred", "max_results": 8}), + &catalog_v1, + &mut active, + &mut cache, + None, + ) + .expect("first search should succeed"); + assert!(active.contains("deferred-0")); + + let catalog_v2: Vec = catalog_v1 + .into_iter() + .filter(|definition| definition.name != "deferred-0") + .collect(); + let second = execute_tool_search_with_cache( + super::TOOL_SEARCH_NAME, + &json!({"query": "^deferred-1", "match": "regex"}), + &catalog_v2, + &mut active, + &mut cache, + None, + ) + .expect("second search should succeed"); + let payload: serde_json::Value = + serde_json::from_str(&second.content).expect("payload is JSON"); + let unavailable = payload["unavailable_tool_references"] + .as_array() + .expect("unavailable references"); + let stale_entry = unavailable + .iter() + .find(|reference| reference["tool_name"] == "deferred-0") + .expect("the stale eviction must be reported"); + assert_eq!( + stale_entry["reason"], + super::TOOLBOX_CATALOG_SHRINK_REASON, + "the stale reason must be the catalog-shrink one, not the bounds one: {stale_entry}" + ); + assert!( + payload.get("evicted_tool_references").is_none() && payload.get("toolbox_note").is_none(), + "a staleness eviction is not a bounds eviction: {payload}" + ); + assert!(!active.contains("deferred-0")); + let metadata = second.metadata.expect("search metadata"); + assert!( + metadata["unavailable_tool_references"] + .as_array() + .expect("metadata unavailable references") + .iter() + .any(|reference| reference["tool_name"] == "deferred-0") + ); +} + +/// A cached tool promoted to eager stays directly callable: the search that +/// forgets the cache entry must not report it as unavailable or evicted — +/// the same request already carries its schema in the tools head, so both +/// vocabularies would be false statements. +#[test] +fn search_does_not_report_an_eager_promotion_as_unavailable() { + let mut catalog = vec![tool("promoted")]; + catalog[0].defer_loading = Some(true); + let mut active = initial_active_tools(&catalog); + let mut cache = ToolActivationCache::default(); + execute_tool_search_with_cache( + super::TOOL_SEARCH_NAME, + &json!({"query": "^promoted", "match": "regex"}), + &catalog, + &mut active, + &mut cache, + None, + ) + .expect("first search should succeed"); + assert!(active.contains("promoted")); + + catalog[0].defer_loading = Some(false); + let second = execute_tool_search_with_cache( + super::TOOL_SEARCH_NAME, + &json!({"query": "^promoted", "match": "regex"}), + &catalog, + &mut active, + &mut cache, + None, + ) + .expect("second search should succeed"); + let payload: serde_json::Value = + serde_json::from_str(&second.content).expect("payload is JSON"); + let unavailable = payload["unavailable_tool_references"] + .as_array() + .expect("unavailable references"); + assert!( + !unavailable + .iter() + .any(|reference| reference["tool_name"] == "promoted"), + "an eager promotion is not an unavailability: {unavailable:?}" + ); + assert!( + payload.get("evicted_tool_references").is_none() && payload.get("toolbox_note").is_none(), + "an eager promotion is not a bounds eviction: {payload}" + ); + assert!(active.contains("promoted")); + assert_eq!(cache.names().count(), 0); +} + +/// A cached tool whose schema grows past the collective budget (an MCP +/// refresh shipping a larger schema) is still a cataloged, re-activatable +/// deferred member: the next search must pop it under the eviction note, not +/// as an unavailability. +#[test] +fn search_reports_growth_overflows_as_re_activatable_evictions() { + let heavy = |name: &str, description_bytes: usize| { + let mut definition = tool(name); + definition.defer_loading = Some(true); + definition.description = "x".repeat(description_bytes); + definition + }; + let catalog_v1 = vec![heavy("deferred-left", 8), heavy("deferred-right", 6_000)]; + let mut active = initial_active_tools(&catalog_v1); + let mut cache = ToolActivationCache::default(); + execute_tool_search_with_cache( + super::TOOL_SEARCH_NAME, + &json!({"query": "deferred", "max_results": 8}), + &catalog_v1, + &mut active, + &mut cache, + None, + ) + .expect("first search should succeed"); + assert_eq!( + cache.names().collect::>(), + vec!["deferred-left", "deferred-right"] + ); + + let catalog_v2 = vec![ + heavy("deferred-left", 12_000), + heavy("deferred-right", 6_000), + ]; + let second = execute_tool_search_with_cache( + super::TOOL_SEARCH_NAME, + &json!({"query": "^deferred-right", "match": "regex"}), + &catalog_v2, + &mut active, + &mut cache, + None, + ) + .expect("second search should succeed"); + let payload: serde_json::Value = + serde_json::from_str(&second.content).expect("payload is JSON"); + assert_eq!( + payload["evicted_tool_references"], + json!([{"type": "tool_reference", "tool_name": "deferred-left"}]), + "the growth overflow must be reported as a re-activatable eviction: {payload}" + ); + let unavailable = payload["unavailable_tool_references"] + .as_array() + .expect("unavailable references"); + assert!( + !unavailable + .iter() + .any(|reference| reference["tool_name"] == "deferred-left"), + "the growth victim is still cataloged, so the unavailable vocabulary would lie: {unavailable:?}" + ); + assert!(!active.contains("deferred-left")); + assert!(active.contains("deferred-right")); +} + +/// A cached tool whose schema grows past the per-tool budget is unavailable +/// rather than evicted, and the report must say exactly that — not claim the +/// tool left the catalog. The discovery-time rejection and the revalidation +/// drop describe the same condition, so the payload carries one entry. +#[test] +fn search_reports_oversize_drops_with_the_oversize_reason() { + let heavy = |name: &str, description_bytes: usize| { + let mut definition = tool(name); + definition.defer_loading = Some(true); + definition.description = "x".repeat(description_bytes); + definition + }; + let catalog_v1 = vec![heavy("deferred-left", 8)]; + let mut active = initial_active_tools(&catalog_v1); + let mut cache = ToolActivationCache::default(); + execute_tool_search_with_cache( + super::TOOL_SEARCH_NAME, + &json!({"query": "^deferred-left", "match": "regex"}), + &catalog_v1, + &mut active, + &mut cache, + None, + ) + .expect("first search should succeed"); + assert_eq!(cache.names().collect::>(), vec!["deferred-left"]); + + let catalog_v2 = vec![heavy("deferred-left", 20_000)]; + let second = execute_tool_search_with_cache( + super::TOOL_SEARCH_NAME, + &json!({"query": "^deferred-left", "match": "regex"}), + &catalog_v2, + &mut active, + &mut cache, + None, + ) + .expect("second search should succeed"); + let payload: serde_json::Value = + serde_json::from_str(&second.content).expect("payload is JSON"); + let unavailable = payload["unavailable_tool_references"] + .as_array() + .expect("unavailable references"); + let entries: Vec<&serde_json::Value> = unavailable + .iter() + .filter(|reference| reference["tool_name"] == "deferred-left") + .collect(); + assert_eq!( + entries.len(), + 1, + "the oversize drop must be reported exactly once: {unavailable:?}" + ); + assert_eq!( + entries[0]["reason"], + format!( + "{}{}", + toolbox_oversize_reason(), + toolbox_oversize_search_hint() + ), + "the oversize drop must carry the oversize reason with its search-context recovery hint, not the catalog-shrink one: {entries:?}" + ); + assert!( + payload.get("evicted_tool_references").is_none() && payload.get("toolbox_note").is_none(), + "an oversize drop is not a re-activatable eviction: {payload}" + ); + assert!(!active.contains("deferred-left")); +} + +/// The searched tool itself can be the LRU-front victim of a growth +/// overflow: revalidation pops it, the admission loop re-admits it, and the +/// same payload must not name it evicted while also handing it back in +/// `tool_references`. +#[test] +fn search_does_not_name_its_own_admission_as_evicted() { + let heavy = |name: &str, description_bytes: usize| { + let mut definition = tool(name); + definition.defer_loading = Some(true); + definition.description = "x".repeat(description_bytes); + definition + }; + let catalog_v1 = vec![heavy("deferred-left", 8), heavy("deferred-right", 6_000)]; + let mut active = initial_active_tools(&catalog_v1); + let mut cache = ToolActivationCache::default(); + execute_tool_search_with_cache( + super::TOOL_SEARCH_NAME, + &json!({"query": "deferred", "max_results": 8}), + &catalog_v1, + &mut active, + &mut cache, + None, + ) + .expect("first search should succeed"); + assert_eq!( + cache.names().collect::>(), + vec!["deferred-left", "deferred-right"] + ); + + let catalog_v2 = vec![ + heavy("deferred-left", 12_000), + heavy("deferred-right", 6_000), + ]; + let second = execute_tool_search_with_cache( + super::TOOL_SEARCH_NAME, + &json!({"query": "^deferred-left", "match": "regex"}), + &catalog_v2, + &mut active, + &mut cache, + None, + ) + .expect("second search should succeed"); + let payload: serde_json::Value = + serde_json::from_str(&second.content).expect("payload is JSON"); + let references = payload["tool_references"].as_array().expect("references"); + assert!( + references + .iter() + .any(|reference| reference["tool_name"] == "deferred-left"), + "the searched growth victim must be re-admitted by this very result: {references:?}" + ); + assert_eq!( + payload["evicted_tool_references"], + json!([{"type": "tool_reference", "tool_name": "deferred-right"}]), + "only the net eviction may be named: {payload}" + ); + let note = payload["toolbox_note"].as_str().expect("note"); + assert!( + note.contains("deferred-right") && !note.contains("deferred-left"), + "the note must not claim the re-admitted tool is uncallable: {note}" + ); + assert!(active.contains("deferred-left")); + assert!(!active.contains("deferred-right")); + let metadata = second.metadata.expect("search metadata"); + assert_eq!( + metadata["evicted_tool_references"], + json!(["deferred-right"]) + ); +} + +/// A direct call to an oversize deferred tool can never enter the bounded +/// toolbox, so the hydration receipt must refuse with the shared oversize +/// reason instead of promising a load the admission would refuse. +#[test] +fn hydration_of_an_oversize_tool_reports_the_rejection_not_a_load() { + let mut oversize = tool("huge-deferred"); + oversize.defer_loading = Some(true); + oversize.description = "x".repeat(20_000); + let catalog = vec![oversize]; + // An empty active set: in a real turn core tools fill it, and the + // single-tool fallback must not pre-activate the tool under test. + let active = HashSet::new(); + let mut hydrated = HashSet::new(); + + let Some(DeferredHydration::Refused(receipt)) = maybe_hydrate_requested_deferred_tool( + "huge-deferred", + &json!({}), + &catalog, + &active, + &mut hydrated, + ) else { + panic!("the oversize call must be refused, not loaded"); + }; + assert!( + receipt + .content + .contains("was not loaded and was not executed"), + "the receipt must refuse the load: {}", + receipt.content + ); + assert!( + receipt.content.contains(&toolbox_oversize_reason()), + "the refusal must carry the shared oversize reason: {}", + receipt.content + ); + assert!( + !receipt.content.contains("has now been loaded"), + "the receipt must not promise a load: {}", + receipt.content + ); + assert!( + hydrated.is_empty(), + "an oversize tool must not be offered to the batch activation: {hydrated:?}" + ); +} + +/// Ride-time netting: a parked note must not name a tool the riding result +/// hands back. Names back in the activation cache — or in the request's +/// active tool set, which a mid-turn MCP catalog refresh can repopulate +/// without re-activating the cache — are dropped from the note, a note +/// emptied by the netting is dropped entirely, and the surviving note keeps +/// its own trigger attribution. +#[test] +fn toolbox_note_rendering_nets_re_admitted_names_at_ride_time() { + let mut deferred = tool("deferred-x"); + deferred.defer_loading = Some(true); + let catalog = vec![deferred]; + let mut cache = ToolActivationCache::default(); + cache.activate(&catalog, &["deferred-x".to_string()]); + // The ride-time catalog: every name below is still served by it, so the + // notes keep the recovery vocabulary. The gone-name segment drops the + // name from this catalog on purpose. + let ride_catalog = vec![tool("deferred-x"), tool("deferred-y"), tool("deferred-z")]; + + let note = |trigger, names| crate::core::session::PendingToolboxEviction { + trigger, + evicted: names, + }; + use crate::core::session::ToolboxEvictionTrigger; + let mut notes = vec![ + note( + ToolboxEvictionTrigger::TurnStart, + vec!["deferred-x".to_string(), "deferred-y".to_string()], + ), + note(ToolboxEvictionTrigger::Step, vec!["deferred-x".to_string()]), + ]; + let rendered = + render_toolbox_eviction_notes(&mut notes, &cache, &HashSet::new(), &ride_catalog) + .expect("deferred-y is still evicted"); + assert!( + rendered.contains("deferred-y") && !rendered.contains("deferred-x"), + "re-admitted names must be netted out of the note: {rendered}" + ); + assert!( + rendered.contains("This turn's start evicted"), + "the surviving note keeps its own trigger attribution: {rendered}" + ); + assert_eq!( + rendered + .matches("evicted tools from the bounded conversation toolbox") + .count(), + 1, + "a companion note the netting empties must contribute no prose: {rendered}" + ); + + // A note whose every name is re-admitted is fully absorbed: reporting + // it would claim a callable tool is uncallable. + let mut notes = vec![note( + ToolboxEvictionTrigger::Step, + vec!["deferred-x".to_string()], + )]; + assert!( + render_toolbox_eviction_notes(&mut notes, &cache, &HashSet::new(), &ride_catalog).is_none(), + "a fully netted note must not ride at all" + ); + + // A name the mid-turn MCP refresh re-advertised into the active set is + // callable even though the cache no longer holds it: the note must net + // against the active set too, or it would contradict the next request, + // which carries the tool. + let mut active = HashSet::new(); + active.insert("deferred-y".to_string()); + let mut notes = vec![note( + ToolboxEvictionTrigger::Step, + vec!["deferred-y".to_string()], + )]; + assert!( + render_toolbox_eviction_notes( + &mut notes, + &ToolActivationCache::default(), + &active, + &ride_catalog, + ) + .is_none(), + "an active-set name is callable and must not be declared evicted" + ); + assert!( + notes.is_empty(), + "the render consumes the notes whether or not anything rendered" + ); + + // A name an earlier note in the same ride already named must not be + // repeated by a later note on the same result: the second mention is + // redundant prose about the same eviction. + let mut notes = vec![ + note( + ToolboxEvictionTrigger::TurnStart, + vec!["deferred-z".to_string()], + ), + note(ToolboxEvictionTrigger::Step, vec!["deferred-z".to_string()]), + ]; + let rendered = + render_toolbox_eviction_notes(&mut notes, &cache, &HashSet::new(), &ride_catalog) + .expect("deferred-z is still evicted"); + assert_eq!( + rendered.matches("deferred-z").count(), + 1, + "a name already rendered by an earlier note must not be repeated: {rendered}" + ); + assert!(notes.is_empty()); + + // A name that left the catalog between its eviction and this ride can + // no longer be recovered by a search, so the note must report that + // truth instead of promising a re-activating search — beside a name + // the catalog still serves, which keeps the recovery contract. + let mut gone_catalog = vec![tool("deferred-y")]; + gone_catalog[0].defer_loading = Some(true); + let mut notes = vec![note( + ToolboxEvictionTrigger::Step, + vec!["deferred-y".to_string(), "gone-tool".to_string()], + )]; + let rendered = render_toolbox_eviction_notes( + &mut notes, + &ToolActivationCache::default(), + &HashSet::new(), + &gone_catalog, + ) + .expect("both names are still evicted"); + assert!( + rendered.contains("no longer callable until a later search re-activates them: deferred-y"), + "the served name keeps the recovery contract: {rendered}" + ); + assert!( + rendered.contains("evicted a tool that has since left the toolbox catalog") + && rendered.contains("a search can no longer re-activate it: gone-tool"), + "a catalog-departed name must not be promised a re-activating search: {rendered}" + ); + + // A note whose every name left the catalog renders the unrecoverable + // sentence alone — never an empty recovery list. + let mut notes = vec![note( + ToolboxEvictionTrigger::Step, + vec!["gone-tool".to_string()], + )]; + let rendered = render_toolbox_eviction_notes( + &mut notes, + &ToolActivationCache::default(), + &HashSet::new(), + &gone_catalog, + ) + .expect("gone-tool is still evicted"); + assert!( + rendered.contains("evicted a tool that has since left the toolbox catalog") + && !rendered.contains("no longer callable until a later search"), + "a fully unrecoverable note must drop the recovery promise: {rendered}" + ); +} diff --git a/crates/tui/src/core/engine/turn_loop.rs b/crates/tui/src/core/engine/turn_loop.rs index 00c0c49819..f59ad71d7a 100644 --- a/crates/tui/src/core/engine/turn_loop.rs +++ b/crates/tui/src/core/engine/turn_loop.rs @@ -676,12 +676,12 @@ fn simple_extension_filters(value: &serde_json::Value) -> Option, - hook_contexts: std::collections::HashMap, - batch_sandbox_policy: crate::sandbox::SandboxPolicy, - benchmark_budget_exhausted: bool, - benchmark_skipped_over_budget_tool_ids: std::collections::HashSet, +pub(super) struct PlannedToolCalls { + pub(super) plans: Vec, + pub(super) hook_contexts: std::collections::HashMap, + pub(super) batch_sandbox_policy: crate::sandbox::SandboxPolicy, + pub(super) benchmark_budget_exhausted: bool, + pub(super) benchmark_skipped_over_budget_tool_ids: std::collections::HashSet, } struct StreamOutcome { @@ -1044,26 +1044,87 @@ fn registered_tool_requires_non_bypassable_approval(tool_name: &str) -> bool { /// authenticate` entry would survive its own successful login, and tools /// killed by a live 401 would stay callable in name. `universe` is every /// name the pool can own; entries inside it are the pool's to manage, and -/// the refreshed list is the new truth. +/// the refreshed list is the new truth — for existence. Wire visibility +/// stays the conversation's decision: a deferred tool returns to the active +/// set only if this conversation still has it activated, so a bounds-evicted +/// entry is never resurrected onto the wire by its own server's refresh (the +/// eviction note the model just read — or a search payload in the same +/// batch — would otherwise contradict the next request, and the next turn's +/// active-set re-derivation would drop it anyway). The pool builds every +/// entry eager, so each replaced tool also carries its prior deferral +/// marking across the refresh: adopting the pool's flag would flip deferred +/// tools back onto the head of the wire list and bust the cached prefix. +/// A name the conversation has never seen (the synthetic login tool a +/// mid-turn 401 just started offering, or a server-side rename) arrives +/// with the pool's eager flag and joins the active set: the 401 result's +/// auth hint points straight at that tool, so deferring it would strand +/// the recovery path behind a search. Replaced entries are spliced back +/// into their own catalog slot, so a refresh that serves the same names +/// reorders nothing and the caller's declaration (this return value: did +/// the model-visible wire list change?) stays false — a same-count +/// refresh moves no wire bytes and must not trip the C5 undeclared-drift +/// guard as a false positive. pub(super) fn replace_runtime_mcp_tools( tool_catalog: &mut Vec, active_tool_names: &mut std::collections::HashSet, universe: &std::collections::HashSet, refreshed: Vec, -) -> usize { - let before = tool_catalog.len(); - tool_catalog.retain(|tool| { - let owned = universe.contains(&tool.name); - if owned { - active_tool_names.remove(&tool.name); + activation_cache: &crate::core::session::ToolActivationCache, +) -> bool { + let previous_deferral: std::collections::HashMap = tool_catalog + .iter() + .filter(|tool| universe.contains(&tool.name)) + .map(|tool| (tool.name.clone(), tool.defer_loading.unwrap_or(false))) + .collect(); + // The wire list is the eager head plus the activated deferred tail; + // that identity — not the catalog's length — is what a declared re-pin + // answers for. + let wire_before = wire_visible_tool_names(tool_catalog, active_tool_names); + let mut slot_by_name: std::collections::HashMap = tool_catalog + .iter() + .enumerate() + .filter(|(_, tool)| universe.contains(&tool.name)) + .map(|(i, tool)| (tool.name.clone(), i)) + .collect(); + let mut arrivals: Vec = Vec::new(); + for mut tool in refreshed { + if let Some(was_deferred) = previous_deferral.get(&tool.name) { + tool.defer_loading = Some(*was_deferred); + } + let name = tool.name.clone(); + let re_advertise = !tool.defer_loading.unwrap_or(false) || activation_cache.contains(&name); + active_tool_names.remove(&name); + match slot_by_name.remove(&name) { + Some(slot) => tool_catalog[slot] = tool, + None => arrivals.push(tool), } - !owned - }); - for tool in refreshed { - active_tool_names.insert(tool.name.clone()); - tool_catalog.push(tool); + if re_advertise { + active_tool_names.insert(name); + } + } + // Names the refreshed list stopped serving leave the catalog and the + // active set; the refreshed list is the pool's truth about its slice. + let departed: Vec = slot_by_name.into_keys().collect(); + for name in &departed { + active_tool_names.remove(name); } - tool_catalog.len().abs_diff(before) + tool_catalog.retain(|tool| !departed.contains(&tool.name)); + tool_catalog.extend(arrivals); + wire_visible_tool_names(tool_catalog, active_tool_names) != wire_before +} + +/// Every name the next request would serialize: the eager head plus the +/// activated deferred tail. +fn wire_visible_tool_names( + tool_catalog: &[Tool], + active_tool_names: &std::collections::HashSet, +) -> std::collections::HashSet { + tool_catalog + .iter() + .filter(|tool| !tool.defer_loading.unwrap_or(false)) + .map(|tool| tool.name.clone()) + .chain(active_tool_names.iter().cloned()) + .collect() } impl Engine { @@ -1381,14 +1442,27 @@ impl Engine { let strict_tool_mode = tool_policy.strict_tool_mode; let mut tool_catalog = std::mem::take(&mut tool_policy.catalog); let mut active_tool_names = std::mem::take(&mut tool_policy.active_names); + // A note parked by an earlier turn that never reached a tool result + // (an interrupted turn, a panic) would misattribute its evictions to + // this turn's first result; drop it before revalidating. SyncSession + // already clears it at the conversation edge — this covers the + // same-conversation case. + self.session.pending_toolbox_eviction_notes.clear(); // Search activations belong to the conversation, not just the user // turn. Revalidate names against this turn's already-filtered catalog // before exposing them; stale mode/MCP/allow-list entries disappear. - let evicted = self.session.tool_activation_cache.revalidate(&tool_catalog); - super::tool_catalog::remove_evicted_cache_activations( - &tool_catalog, + let removals = self.session.tool_activation_cache.revalidate(&tool_catalog); + super::tool_catalog::remove_toolbox_drops( &mut active_tool_names, - evicted, + removals.dropped_names().map(str::to_string), + ); + // This revalidation can also evict on the collective bounds when a + // cached schema grew between turns (an MCP refresh, for example). + // Like a hydration it has no result payload of its own: park the + // note so the turn's first recorded result carries it. + self.park_toolbox_eviction_note( + crate::core::session::ToolboxEvictionTrigger::TurnStart, + &removals.bounds_evicted, ); active_tool_names.extend( self.session @@ -3353,6 +3427,7 @@ impl Engine { &mut tool_catalog, &mut active_tool_names, &hook_contexts, + &tool_policy, ) .await; @@ -3473,7 +3548,7 @@ impl Engine { /// deferred-tool activation state. It returns the executable plans together /// with the hook context and batch sandbox policy consumed by later phases. #[allow(clippy::too_many_arguments)] // phase fns mirror the turn pipeline shape - async fn plan_tool_calls( + pub(super) async fn plan_tool_calls( &mut self, client: &dyn crate::core::model_client::ModelClient, turn: &mut TurnContext, @@ -3996,7 +4071,7 @@ impl Engine { let should_emit_hydration_status = !deferred_tools_hydrated_this_batch.contains(&tool_name); if blocked_error.is_none() - && let Some(result) = maybe_hydrate_requested_deferred_tool( + && let Some(hydration) = maybe_hydrate_requested_deferred_tool( &tool_name, &tool_input, tool_catalog, @@ -4004,37 +4079,49 @@ impl Engine { &mut deferred_tools_hydrated_this_batch, ) { - if should_emit_hydration_status { - // Retain first-proposal order separately from the set - // used to deduplicate calls in this batch. LRU bounds - // must not depend on randomized HashSet iteration. - deferred_tools_hydrated_in_order.push(tool_name.clone()); - } - emit_tool_audit_for_policy( - self.active_turn_tool_security.is_some(), - json!({ - "event": "tool.schema_hydrated", - "tool_id": tool_id.clone(), - "tool_name": tool_name.clone(), - "auto_retry_same_turn": false, - "metadata": result.metadata, - }), - ); - if should_emit_hydration_status { - let status = if requested_tool_name == tool_name { - format!( - "Loaded deferred tool '{tool_name}'. Retry the call with its visible schema." - ) - } else { - format!( - "Loaded deferred tool '{tool_name}' after resolving '{requested_tool_name}'. Retry the call with its visible schema." - ) - }; - let _ = self.tx_event.send(Event::status(status)).await; - } + let result = match hydration { + DeferredHydration::Loaded(result) => { + if should_emit_hydration_status { + // Retain first-proposal order separately from the set + // used to deduplicate calls in this batch. LRU bounds + // must not depend on randomized HashSet iteration. + deferred_tools_hydrated_in_order.push(tool_name.clone()); + } + emit_tool_audit_for_policy( + self.active_turn_tool_security.is_some(), + json!({ + "event": "tool.schema_hydrated", + "tool_id": tool_id.clone(), + "tool_name": tool_name.clone(), + "auto_retry_same_turn": false, + "metadata": result.metadata, + }), + ); + if should_emit_hydration_status { + let status = if requested_tool_name == tool_name { + format!( + "Loaded deferred tool '{tool_name}'. Retry the call with its visible schema." + ) + } else { + format!( + "Loaded deferred tool '{tool_name}' after resolving '{requested_tool_name}'. Retry the call with its visible schema." + ) + }; + let _ = self.tx_event.send(Event::status(status)).await; + } + result + } + // A refusal loads nothing: announcing "Loaded deferred + // tool" or auditing `tool.schema_hydrated` would state + // the opposite of the receipt, and the refused name must + // stay out of the hydration order so the batch + // activation is never asked to admit it. + DeferredHydration::Refused(result) => result, + }; // The provider did not advertise this schema in the current // request. Hydration is discovery, never execution authority: - // return the schema now and require a subsequent model call. + // return the schema — or the refusal — now and require a + // subsequent model call. guard_result = Some(result); } @@ -4120,19 +4207,36 @@ impl Engine { .session .tool_activation_cache .activate(tool_catalog, &deferred_tools_hydrated_in_order); - super::tool_catalog::remove_evicted_cache_activations( - tool_catalog, + super::tool_catalog::remove_toolbox_drops( active_tool_names, - activation.evicted.iter().cloned(), + activation.dropped_names().map(str::to_string), ); // Admitting or evicting deferred tools changes the request-visible // tool catalog for the rest of this turn. That is a legitimate, // nameable header change — declare it so the prefix pin re-pins under // `change:tool_surface` instead of tripping the C5 drift guard. - if !activation.admitted.is_empty() || !activation.evicted.is_empty() { + if !activation.admitted.is_empty() + || !activation.evicted.is_empty() + || !activation.stale.is_empty() + { active_tool_names.extend(activation.admitted.iter().cloned()); self.session.pending_prefix_change_reason = Some("tool_surface".to_string()); } + // Unlike a `tool_search` result, a hydration has no result payload of + // its own to name bounds-driven evictions in. Park the note on the + // session so the first recorded tool result of this batch carries it + // to the model. The evictions may come from the admission itself or + // from revalidation after a cached schema grew. Unavailable drops are + // not reported here: an oversize drop is re-reported with its reason + // the next time a search returns the tool (a match ranked past the + // result cap re-reports nothing), while a catalog-shrink drop that a + // non-search path absorbed left the pruned catalog and cannot be + // discovered — the model learns of those only when a search for the + // capability misses. + self.park_toolbox_eviction_note( + crate::core::session::ToolboxEvictionTrigger::Step, + &activation.evicted, + ); PlannedToolCalls { plans, hook_contexts, @@ -4979,18 +5083,34 @@ impl Engine { outcomes } + /// Park a bounded-toolbox eviction note for the next recorded tool + /// result, combining with any note already parked — turn-start + /// revalidation and a hydration batch can both evict in the same turn. + pub(super) fn park_toolbox_eviction_note( + &mut self, + trigger: crate::core::session::ToolboxEvictionTrigger, + evicted: &[String], + ) { + super::tool_catalog::park_toolbox_eviction( + &mut self.session.pending_toolbox_eviction_notes, + trigger, + evicted, + ); + } + /// Commit collected tool outcomes to the session and related runtime state. /// /// This phase activates result dependencies, refreshes a changed MCP catalog, /// updates the working set, runs post-edit LSP diagnostics, appends success or /// error tool-result messages, and refreshes goal state. Its output is these /// side effects; it never plans or executes another tool call. - async fn process_tool_results( + pub(super) async fn process_tool_results( &mut self, outcomes: Vec>, tool_catalog: &mut Vec, active_tool_names: &mut std::collections::HashSet, hook_contexts: &std::collections::HashMap, + tool_policy: &ToolSurfacePolicy, ) { let active_tool_names_before = active_tool_names.clone(); let tool_catalog_len_before = tool_catalog.len(); @@ -5000,6 +5120,12 @@ impl Engine { // sidebar "Goal:" line stays stale for the whole (possibly long) // goal-loop turn while get_goal already reflects the new objective. let mut goal_tool_ran = false; + // Hydration-time and turn-start toolbox evictions have no result + // payload of their own; the notes ride the first recorded result of + // this batch. They stay structured until that attach so names the + // riding result re-admits can be netted out. + let mut pending_eviction_notes = + std::mem::take(&mut self.session.pending_toolbox_eviction_notes); for outcome in outcomes.into_iter().flatten() { let tool_input = outcome.input.clone(); @@ -5011,21 +5137,23 @@ impl Engine { } match result { Ok(output) => { - let mut tool_surface_changed = - super::tool_catalog::activate_result_dependencies( + let dependency = super::tool_catalog::activate_result_dependencies( + tool_catalog, + active_tool_names, + &mut self.session.tool_activation_cache, + &output, + ); + let mut tool_surface_changed = dependency.changed; + let mut evicted_this_result = dependency.evicted; + if output.success { + let touch = super::tool_catalog::touch_cached_tool_after_execution( tool_catalog, active_tool_names, &mut self.session.tool_activation_cache, - &output, + &outcome.name, ); - if output.success { - tool_surface_changed |= - super::tool_catalog::touch_cached_tool_after_execution( - tool_catalog, - active_tool_names, - &mut self.session.tool_activation_cache, - &outcome.name, - ); + tool_surface_changed |= touch.changed; + evicted_this_result.extend(touch.evicted); } // A runtime MCP connection change — a completed login OR // a live 401 that dropped one — rewrites the callable @@ -5042,14 +5170,35 @@ impl Engine { if mcp_catalog_changed && let Some(pool) = self.mcp_pool.as_ref().cloned() { let (universe, refreshed) = { let pool = pool.lock().await; - (pool.model_tool_names(), pool.to_api_tools()) + let (universe, refreshed) = + (pool.model_tool_names(), pool.to_api_tools()); + // Same allow/deny narrowing the turn build ran + // over its catalog: a tool the build stripped + // must not re-enter mid-turn, where a + // `tool_search` hit would defeat the denied-name + // concealment and a declared activation would + // serialize an operator-excluded schema into the + // request. Mirror of the boot-window refresh's + // gate; both delegate to the same + // `ToolSurfacePolicy::allows_tool` spelling. + ( + universe + .into_iter() + .filter(|name| tool_policy.allows_tool(name)) + .collect(), + refreshed + .into_iter() + .filter(|tool| tool_policy.allows_tool(&tool.name)) + .collect(), + ) }; tool_surface_changed |= replace_runtime_mcp_tools( tool_catalog, active_tool_names, &universe, refreshed, - ) > 0; + &self.session.tool_activation_cache, + ); } // Any of the legitimate mid-turn tool-surface changes above // re-pin the header under a declared `change:tool_surface` @@ -5059,6 +5208,12 @@ impl Engine { self.session.pending_prefix_change_reason = Some("tool_surface".to_string()); } + let tool_was_executed = output + .metadata + .as_ref() + .and_then(|metadata| metadata.get("executed")) + .and_then(serde_json::Value::as_bool) + .unwrap_or(true); emit_tool_audit_for_policy( self.active_turn_tool_security.is_some(), json!({ @@ -5067,6 +5222,7 @@ impl Engine { "tool_name": outcome.name.clone(), "status": terminal_status.as_str(), "success": output.success, + "executed": tool_was_executed, }), ); let output_for_context = compact_tool_result_for_route( @@ -5076,12 +5232,6 @@ impl Engine { &outcome.name, &output, ); - let tool_was_executed = output - .metadata - .as_ref() - .and_then(|metadata| metadata.get("executed")) - .and_then(serde_json::Value::as_bool) - .unwrap_or(true); if tool_was_executed { self.session.working_set.observe_tool_call( &tool_name_for_ws, @@ -5112,6 +5262,35 @@ impl Engine { } None => output_for_context, }; + // The batch's parked notes and this result's own + // retrieval/touch evictions share one channel to the + // model: the recorded result content. Both net against + // the live cache and the active tool set at attach so a + // name this result hands back — or that a mid-turn MCP + // refresh re-advertised after the touch dropped it — is + // never declared uncallable. + let mut eviction_notes = Vec::new(); + if !evicted_this_result.is_empty() { + pending_eviction_notes.push(crate::core::session::PendingToolboxEviction { + trigger: crate::core::session::ToolboxEvictionTrigger::Step, + evicted: evicted_this_result, + }); + } + if !pending_eviction_notes.is_empty() { + if let Some(rendered) = super::tool_catalog::render_toolbox_eviction_notes( + &mut pending_eviction_notes, + &self.session.tool_activation_cache, + active_tool_names, + tool_catalog, + ) { + eviction_notes.push(rendered); + } + } + let output_for_context = if eviction_notes.is_empty() { + output_for_context + } else { + format!("{output_for_context}\n\n{}", eviction_notes.join("\n\n")) + }; let content_blocks = outcome.content_blocks; let content_blocks = content_blocks @@ -5150,12 +5329,32 @@ impl Engine { .find(|tool| tool.name == outcome.name) .map(|tool| &tool.input_schema); let error = format_tool_error_with_schema(&e, &outcome.name, input_schema); + // The working set observes the tool's own error, not the + // surface bookkeeping appended below — the same pre-note + // text the success path observes. self.session.working_set.observe_tool_call( &tool_name_for_ws, &tool_input, Some(&error), &self.session.workspace, ); + // A pending eviction note must still reach the model when + // the batch's first result is an error, netted the same + // way as on the success path. The render consumes the + // notes either way. + let mut note_text = None; + if !pending_eviction_notes.is_empty() { + note_text = super::tool_catalog::render_toolbox_eviction_notes( + &mut pending_eviction_notes, + &self.session.tool_activation_cache, + active_tool_names, + tool_catalog, + ); + } + let error = match note_text { + Some(note) => format!("{error}\n\n{note}"), + None => error, + }; self.add_session_message(Message { role: Role::User, content: tool_result_message_content( @@ -5171,6 +5370,19 @@ impl Engine { } } + // A batch whose outcomes all flattened away (a benchmark final-only + // skip, for example) has no result to carry the notes. Re-park them + // so a later batch of this turn still delivers them; a turn that ends + // without another batch strands them, and the next turn's entry sweep + // drops them rather than letting them ride an unrelated result. + // Extending — not assigning — keeps any note parked during the + // results phase instead of silently dropping it. + if !pending_eviction_notes.is_empty() { + self.session + .pending_toolbox_eviction_notes + .extend(pending_eviction_notes); + } + // Reflect a mid-turn goal change on the sidebar immediately (idempotent: // emit_goal_updated only sends when an objective is set, and the UI // applies it behind a `changed` guard). diff --git a/crates/tui/src/core/session.rs b/crates/tui/src/core/session.rs index e654eb8b3d..dd8177963b 100644 --- a/crates/tui/src/core/session.rs +++ b/crates/tui/src/core/session.rs @@ -18,8 +18,9 @@ pub(crate) const TOOL_ACTIVATION_CACHE_MAX_NAMES: usize = 8; /// Maximum serialized bytes added to requests by cached deferred schemas. pub(crate) const TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES: usize = 16 * 1024; -/// Bounded, process-local conversation cache for tools activated by -/// `tool_search`. +/// Bounded, process-local conversation cache for deferred tools activated by +/// `tool_search` or loaded by a direct call (hydration, execution refresh, +/// retrieval admission). /// /// Only names are retained. Every turn revalidates them against the currently /// filtered catalog, so a disconnected MCP server, changed allow/deny rule, or @@ -30,13 +31,123 @@ pub(crate) struct ToolActivationCache { names: VecDeque, } +/// Why `revalidate` dropped a cached activation as unavailable. The cause +/// selects the model-facing reason: a catalog-shrink name cannot come back +/// while the shrink holds, while an oversize name can return once its +/// schema shrinks back under the per-tool budget. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ToolActivationStaleCause { + /// The tool left the filtered catalog entirely (MCP disconnect, deny + /// rule, mode switch). + CatalogShrink, + /// The tool is still in the catalog but its serialized definition + /// (the whole `Tool`, not only its schema) exceeds the per-tool + /// toolbox budget. + Oversize, +} + +/// One cached activation dropped by `revalidate` as unavailable. Eager +/// promotions are deliberately absent: the tool is directly callable and +/// stays active, so there is nothing to explain. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct ToolActivationStale { + pub(crate) name: String, + pub(crate) cause: ToolActivationStaleCause, +} + #[derive(Debug, Clone, Default, Eq, PartialEq)] pub(crate) struct ToolActivationDelta { pub(crate) admitted: Vec, + /// Bounds-driven evictions: collective LRU or serialized-bytes overflow + /// from this admission, or from revalidation after a cached schema grew + /// past the budget. The tools are still deferred catalog members, so a + /// later search can re-activate them — the "evicted" recovery contract. + /// A name this same admission re-admitted is netted out, so reporting + /// sites can quote the list verbatim without disclaiming a tool the same + /// result hands back. pub(crate) evicted: Vec, + /// Unavailable entries dropped by `revalidate` before this admission: + /// the tool left the deferred catalog or outgrew the per-tool budget, + /// so it is unavailable rather than evicted. The cause selects the + /// truthful model-facing reason. + pub(crate) stale: Vec, pub(crate) rejected: Vec, } +impl ToolActivationDelta { + /// Every name this delta dropped from the active toolbox: unavailable + /// entries plus bounds evictions. Eager promotions are classified out + /// by `revalidate` and never appear here, so removing these names can + /// never hide a tool the catalog still exposes eagerly. + pub(crate) fn dropped_names(&self) -> impl Iterator { + self.stale + .iter() + .map(|entry| entry.name.as_str()) + .chain(self.evicted.iter().map(String::as_str)) + } +} + +/// Which step surfaced a toolbox eviction, so the model-facing note can +/// attribute it to the step that caused it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ToolboxEvictionTrigger { + /// A `tool_search` result naming the evictions in its own payload. + ToolSearch, + /// A non-search activation step: a hydration batch, a retrieval + /// admission, or an executed tool's refresh. + Step, + /// The turn-entry revalidation. + TurnStart, +} + +impl ToolboxEvictionTrigger { + pub(crate) fn label(self) -> &'static str { + match self { + Self::ToolSearch => "search", + Self::Step => "step", + Self::TurnStart => "turn's start", + } + } +} + +/// A toolbox eviction parked for a later result to carry, with its names +/// still structured. A step's own touch/retrieval evictions pass through +/// the same struct at attach time so parked and step notes share one +/// netting and rendering path. Names stay unresolved until ride time so +/// the attaching channel can net out names the model can call again by +/// then: between the park and the ride, a search, a hydration, or a +/// retrieval may have re-admitted a victim, and a mid-turn MCP catalog +/// refresh can re-advertise one into the active set without re-activating +/// the cache. A note naming a callable tool "no longer callable" would +/// contradict the very result that hands it back. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct PendingToolboxEviction { + pub(crate) trigger: ToolboxEvictionTrigger, + pub(crate) evicted: Vec, +} + +/// What [`ToolActivationCache::revalidate`] removed, classified by +/// recoverability so callers can report each bucket truthfully. +#[derive(Debug, Clone, Default, Eq, PartialEq)] +pub(crate) struct ToolActivationRemovals { + /// Unavailable drops: the tool left the deferred catalog or outgrew the + /// per-tool budget. See [`ToolActivationStale`]. + pub(crate) stale: Vec, + /// Collective-bounds victims that are still deferred catalog members: + /// a later search can re-activate them. + pub(crate) bounds_evicted: Vec, +} + +impl ToolActivationRemovals { + /// Every name this removal set dropped from the active toolbox. + pub(crate) fn dropped_names(&self) -> impl Iterator { + self.stale + .iter() + .map(|entry| entry.name.as_str()) + .chain(self.bounds_evicted.iter().map(String::as_str)) + } +} + impl ToolActivationCache { /// Forget every deferred tool activated by the current conversation. /// @@ -60,6 +171,16 @@ impl ToolActivationCache { serde_json::to_vec(tool).map_or(usize::MAX, |bytes| bytes.len()) } + /// Whether a tool's serialized definition alone exceeds the per-tool + /// toolbox budget, so no activation can admit it. The measurement is + /// the whole serialized `Tool` — name, description, schema, and every + /// set optional — matching the collective budget's unit. Shared by + /// admission, revalidation, and hydration planning so the gate cannot + /// drift. + pub(crate) fn exceeds_per_tool_budget(tool: &crate::models::Tool) -> bool { + Self::serialized_bytes(tool) > TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES + } + fn total_serialized_bytes(&self, catalog: &[crate::models::Tool]) -> usize { self.names .iter() @@ -68,29 +189,57 @@ impl ToolActivationCache { .fold(0usize, usize::saturating_add) } - /// Drop entries that are no longer deferred members of this turn's - /// filtered catalog and enforce both cache bounds. - pub(crate) fn revalidate(&mut self, catalog: &[crate::models::Tool]) -> Vec { - let mut evicted = Vec::new(); - self.names.retain(|name| { - let keep = Self::catalog_tool(catalog, name).is_some_and(|tool| { - Self::serialized_bytes(tool) <= TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES - }); - if !keep { - evicted.push(name.clone()); - } - keep - }); + /// Pop least-recently-used entries until both collective bounds hold. + /// Shared by revalidation and admission so the bounds condition and the + /// LRU-front-first policy live in exactly one place. + fn pop_collective_overflow(&mut self, catalog: &[crate::models::Tool]) -> Vec { + let mut popped = Vec::new(); while self.names.len() > TOOL_ACTIVATION_CACHE_MAX_NAMES || self.total_serialized_bytes(catalog) > TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES { - if let Some(name) = self.names.pop_front() { - evicted.push(name); - } else { + let Some(name) = self.names.pop_front() else { break; - } + }; + popped.push(name); } - evicted + popped + } + + /// Drop entries that are no longer deferred members of this turn's + /// filtered catalog and enforce both cache bounds, classifying every + /// removal by recoverability. A tool that became eager is forgotten + /// from the cache without being reported: it is directly callable, and + /// the eager catalog entry must stay active. + pub(crate) fn revalidate(&mut self, catalog: &[crate::models::Tool]) -> ToolActivationRemovals { + let mut removals = ToolActivationRemovals::default(); + self.names.retain(|name| { + let Some(tool) = catalog.iter().find(|tool| tool.name == *name) else { + removals.stale.push(ToolActivationStale { + name: name.clone(), + cause: ToolActivationStaleCause::CatalogShrink, + }); + return false; + }; + if !tool.defer_loading.unwrap_or(false) { + return false; + } + if Self::exceeds_per_tool_budget(tool) { + removals.stale.push(ToolActivationStale { + name: name.clone(), + cause: ToolActivationStaleCause::Oversize, + }); + return false; + } + true + }); + removals.bounds_evicted = self.pop_collective_overflow(catalog); + removals + .stale + .sort_by(|left, right| left.name.cmp(&right.name)); + removals.stale.dedup(); + removals.bounds_evicted.sort(); + removals.bounds_evicted.dedup(); + removals } /// Touch requested deferred tools in search-result order. An oversized @@ -101,8 +250,10 @@ impl ToolActivationCache { catalog: &[crate::models::Tool], requested: &[String], ) -> ToolActivationDelta { + let revalidated = self.revalidate(catalog); let mut delta = ToolActivationDelta { - evicted: self.revalidate(catalog), + stale: revalidated.stale, + evicted: revalidated.bounds_evicted, ..ToolActivationDelta::default() }; let mut seen = HashSet::new(); @@ -114,7 +265,7 @@ impl ToolActivationCache { delta.rejected.push(name.clone()); continue; }; - if Self::serialized_bytes(tool) > TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES { + if Self::exceeds_per_tool_budget(tool) { delta.rejected.push(name.clone()); continue; } @@ -122,13 +273,7 @@ impl ToolActivationCache { self.names.remove(index); } self.names.push_back(name.clone()); - while self.names.len() > TOOL_ACTIVATION_CACHE_MAX_NAMES - || self.total_serialized_bytes(catalog) > TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES - { - if let Some(evicted) = self.names.pop_front() { - delta.evicted.push(evicted); - } - } + delta.evicted.extend(self.pop_collective_overflow(catalog)); } let retained = self.names.iter().collect::>(); @@ -137,8 +282,17 @@ impl ToolActivationCache { .filter(|name| retained.contains(name)) .cloned() .collect(); + // A revalidation pop of a name this same activation re-admits (the + // searched tool was the LRU-front victim of a growth overflow, for + // example) is not an eviction: the result that names it evicted also + // hands it back. Net it out so no report lies about it. + delta.evicted.retain(|name| !delta.admitted.contains(name)); delta.evicted.sort(); delta.evicted.dedup(); + delta + .stale + .sort_by(|left, right| left.name.cmp(&right.name)); + delta.stale.dedup(); delta.rejected.sort(); delta.rejected.dedup(); delta @@ -147,6 +301,11 @@ impl ToolActivationCache { pub(crate) fn names(&self) -> impl Iterator { self.names.iter().map(String::as_str) } + + /// Whether `name` is currently cached, and thus callable again. + pub(crate) fn contains(&self, name: &str) -> bool { + self.names.iter().any(|cached| cached == name) + } } /// Session state for the engine. @@ -183,6 +342,13 @@ pub struct Session { /// reason while an undeclared mid-loop change is reported as drift and the /// pin holds. `None` means "no declared change since the last request". pub pending_prefix_change_reason: Option, + /// Evictions parked by non-search activations (a batch's hydration of + /// directly-called deferred tools, the turn-entry revalidation) that + /// have no result payload of their own. The next recorded tool result + /// carries them to the model; names a ride re-admits are netted out at + /// attach time because the model can call them again. Taken and cleared + /// by the turn loop when the notes are attached. + pub(super) pending_toolbox_eviction_notes: Vec, /// The explicit prompt inputs (model, mode, goal, route, translation, /// verbosity) the pinned system prompt was composed from. At each new user /// turn the engine recomposes: if these inputs are unchanged but the @@ -329,6 +495,7 @@ impl Session { }, last_system_prompt_hash: None, pending_prefix_change_reason: None, + pending_toolbox_eviction_notes: Vec::new(), pinned_prompt_context: None, context_update_baseline: None, working_set: WorkingSet::default(), @@ -515,16 +682,58 @@ mod tests { } #[test] - fn revalidate_drops_removed_denied_or_eager_tools() { - let catalog = vec![deferred_tool("kept", 8), deferred_tool("gone", 8)]; + fn revalidate_classifies_catalog_shrink_and_eager_removals() { + let catalog = vec![ + deferred_tool("kept", 8), + deferred_tool("disconnected", 8), + deferred_tool("promoted", 8), + ]; let mut cache = ToolActivationCache::default(); - cache.activate(&catalog, &["kept".to_string(), "gone".to_string()]); - let mut next_catalog = vec![deferred_tool("kept", 8), deferred_tool("gone", 8)]; + cache.activate( + &catalog, + &[ + "kept".to_string(), + "disconnected".to_string(), + "promoted".to_string(), + ], + ); + let mut next_catalog = vec![deferred_tool("kept", 8), deferred_tool("promoted", 8)]; next_catalog[1].defer_loading = Some(false); - let evicted = cache.revalidate(&next_catalog); + let removals = cache.revalidate(&next_catalog); assert_eq!(cache.names().collect::>(), vec!["kept"]); - assert_eq!(evicted, vec!["gone"]); + assert_eq!( + removals.stale, + vec![ToolActivationStale { + name: "disconnected".to_string(), + cause: ToolActivationStaleCause::CatalogShrink, + }] + ); + // The eager promotion is forgotten from the cache without being + // reported: the tool is directly callable, so both the unavailable + // and the evicted vocabulary would be false statements. + assert!(removals.bounds_evicted.is_empty()); + } + + #[test] + fn revalidate_reports_byte_growth_overflows_as_re_activatable_evictions() { + let small = vec![deferred_tool("left", 8), deferred_tool("right", 6 * 1024)]; + let mut cache = ToolActivationCache::default(); + cache.activate(&small, &["left".to_string(), "right".to_string()]); + // The catalog's schema for `left` grows while it is cached (an MCP + // refresh): the collective budget now overflows, but `left` is still + // a deferred catalog member under the per-tool cap, so the LRU + // victim must pop as a re-activatable eviction, not a stale + // unavailability. + let grown = vec![ + deferred_tool("left", 12 * 1024), + deferred_tool("right", 6 * 1024), + ]; + + let removals = cache.revalidate(&grown); + assert_eq!(cache.names().collect::>(), vec!["right"]); + assert!(removals.stale.is_empty()); + assert_eq!(removals.bounds_evicted, vec!["left"]); } #[test] diff --git a/crates/tui/src/tools/schema_sanitize.rs b/crates/tui/src/tools/schema_sanitize.rs index 4a790588c3..a92f87054f 100644 --- a/crates/tui/src/tools/schema_sanitize.rs +++ b/crates/tui/src/tools/schema_sanitize.rs @@ -1272,6 +1272,14 @@ fn sanitize_kimi_parameters_candidate( // only adjusts valid nested unions. sanitize_for_kimi(parameters); + // JSON Schema reads an empty `items` schema as "items are unconstrained", + // which is exactly the MFJS-legal shape with the keyword absent. MFJS + // itself has no empty schema node and fails the whole tool with + // MissingType, so repair the equivalent shape instead of dropping the + // tool from the request (the Pinvou browser `evaluate_script` catalog + // historically shipped `args: {"type": "array", "items": {}}`). + remove_empty_mfjs_items_schemas(parameters); + let Some(root) = parameters.as_object() else { return Err(KimiParameterSchemaError::RootMustBeObject); }; @@ -1287,6 +1295,57 @@ fn sanitize_kimi_parameters_candidate( Ok(constraint_note) } +/// Drop every empty `items` schema node (`"items": {}`) from a Kimi +/// parameters document. +/// +/// Empty-schema nodes sit exactly where schema values live: property values, +/// `items`, object-form `additionalProperties`, `anyOf` branches, and `$defs` +/// entries. `properties` itself is a name map, so its own emptiness is legal +/// and only its values are visited. `oneOf` needs no visit: this runs after +/// `normalize_kimi_compatibility` has converted every non-root `oneOf` to +/// `anyOf`, and root composition was already flattened before that. +/// +/// Children are visited before the node's own `items` check so a nested +/// repair can empty the parent (`"items": {"items": {}}` is vacuously true +/// once the inner keyword is gone, so dropping the outer keyword is still +/// semantics-preserving). Every removal keeps the document equivalent: an +/// absent `items` keyword already means unconstrained items, so a legal +/// document passes through unchanged. +fn remove_empty_mfjs_items_schemas(value: &mut Value) { + let Some(obj) = value.as_object_mut() else { + return; + }; + if let Some(properties) = obj.get_mut("properties").and_then(Value::as_object_mut) { + for child in properties.values_mut() { + remove_empty_mfjs_items_schemas(child); + } + } + if let Some(definitions) = obj.get_mut("$defs").and_then(Value::as_object_mut) { + for child in definitions.values_mut() { + remove_empty_mfjs_items_schemas(child); + } + } + if let Some(branches) = obj.get_mut("anyOf").and_then(Value::as_array_mut) { + for branch in branches.iter_mut() { + remove_empty_mfjs_items_schemas(branch); + } + } + if let Some(additional) = obj.get_mut("additionalProperties") + && additional.is_object() + { + remove_empty_mfjs_items_schemas(additional); + } + if let Some(items) = obj.get_mut("items") { + remove_empty_mfjs_items_schemas(items); + } + if obj + .get("items") + .is_some_and(|items| items.as_object().is_some_and(Map::is_empty)) + { + obj.shift_remove("items"); + } +} + fn inline_internal_kimi_root_ref(parameters: &mut Value) -> Result<(), KimiParameterSchemaError> { let document = parameters.clone(); let Some(document_root) = document.as_object() else { @@ -2452,6 +2511,274 @@ mod kimi_tests { ); } + #[test] + fn kimi_parameters_repairs_empty_items_as_unconstrained() { + // The Pinvou browser `evaluate_script` catalog shipped + // `args: {"type": "array", "items": {}}`. JSON Schema reads the empty + // items schema as unconstrained, but MFJS fails it with MissingType + // and the request builder silently dropped the whole tool — the model + // kept seeing a tool_search reference it could never call. The + // MFJS-legal equivalent shape drops the keyword instead. + let mut schema = json!({ + "type": "object", + "properties": { + "function": {"type": "string"}, + "args": {"type": "array", "items": {}} + }, + "required": ["function"], + "additionalProperties": true + }); + sanitize_for_kimi_parameters(&mut schema) + .expect("empty items must repair to the keyword being absent"); + assert_eq!(schema["properties"]["args"], json!({"type": "array"})); + + // Nested occurrences repair too; other untyped schema nodes still + // fail closed because no semantics-preserving MFJS shape exists. + let mut nested = json!({ + "type": "object", + "properties": { + "outer": { + "type": "object", + "properties": { + "inner": {"type": "array", "items": {}} + }, + "required": ["inner"] + } + } + }); + sanitize_for_kimi_parameters(&mut nested).expect("nested empty items repair"); + assert_eq!( + nested["properties"]["outer"]["properties"]["inner"], + json!({"type": "array"}) + ); + + let mut described_only = json!({ + "type": "object", + "properties": { + "args": {"type": "array", "items": {"description": "anything"}} + } + }); + assert_eq!( + sanitize_for_kimi_parameters(&mut described_only).unwrap_err(), + KimiParameterSchemaError::MissingType + ); + } + + #[test] + fn kimi_parameters_repairs_nested_and_sibling_empty_items() { + // items-in-items: the inner keyword repairs to nothing, which empties + // the outer node; `{"items": X}` is vacuously true for non-arrays and + // `{"items": {}}` accepts everything, so dropping the outer keyword + // preserves the accepted set. The emptiness check must run after the + // recursion or the parent is stranded as `{}` and fails MissingType. + let mut grid = json!({ + "type": "object", + "properties": { + "grid": {"type": "array", "items": {"items": {}}} + } + }); + sanitize_for_kimi_parameters(&mut grid).expect("items-in-items repair"); + assert_eq!(grid["properties"]["grid"], json!({"type": "array"})); + + // A root `items` keyword beside `$defs`: the repair must visit every + // schema-value sibling of the root node, not just the removed key's + // own subtree, or `$defs` stays unrepaired and fails the whole tool. + let mut with_defs = json!({ + "type": "object", + "items": {}, + "$defs": {"rows": {"type": "array", "items": {}}}, + "properties": {"rows": {"$ref": "#/$defs/rows"}} + }); + sanitize_for_kimi_parameters(&mut with_defs).expect("root items beside $defs repair"); + assert!(with_defs.get("items").is_none()); + assert_eq!(with_defs["$defs"]["rows"], json!({"type": "array"})); + + // Every schema-value position the validator recurses into must be + // visited: an empty `items` inside an object-form + // `additionalProperties` or an `anyOf` branch repairs too, and a + // walker that skips either position would strand the node and fail + // the whole tool with MissingType. + let mut nested_positions = json!({ + "type": "object", + "properties": { + "map": { + "type": "object", + "additionalProperties": {"type": "array", "items": {}} + }, + "choice": { + "anyOf": [ + {"type": "string"}, + {"type": "array", "items": {}} + ] + } + } + }); + sanitize_for_kimi_parameters(&mut nested_positions) + .expect("additionalProperties and anyOf empty items repair"); + assert_eq!( + nested_positions["properties"]["map"]["additionalProperties"], + json!({"type": "array"}) + ); + assert_eq!( + nested_positions["properties"]["choice"]["anyOf"][1], + json!({"type": "array"}) + ); + } + + #[test] + fn kimi_parameters_min_items_beside_empty_items_is_position_dependent() { + // Terminator-checked position (a required property): the repaired + // node must still terminate, and with `minItems >= 1` the terminator + // requires an `items` keyword — the tool keeps failing closed, with + // the terminator's variant. + let mut schema = json!({ + "type": "object", + "properties": {"a": {"type": "array", "items": {}, "minItems": 1}}, + "required": ["a"] + }); + let error = sanitize_for_kimi_parameters(&mut schema) + .expect_err("minItems >= 1 cannot terminate without items at a checked position"); + assert!( + matches!(error, KimiParameterSchemaError::InvalidSchemaNode), + "the terminator's variant is InvalidSchemaNode, not MissingType: {error:?}" + ); + + // `minItems: 0` terminates trivially at the same checked position, + // and its only MFJS-legal shape repairs to the equivalent + // unconstrained form. + let mut schema = json!({ + "type": "object", + "properties": {"a": {"type": "array", "items": {}, "minItems": 0}}, + "required": ["a"] + }); + sanitize_for_kimi_parameters(&mut schema).expect("minItems: 0 beside empty items repairs"); + assert_eq!( + schema["properties"]["a"], + json!({"type": "array", "minItems": 0}) + ); + + // A non-required property position is never revisited by the + // terminator, so the repaired tool SHIPS — pre-repair it was dropped + // with MissingType. The shipped shape is instance-set-equivalent: + // the empty `items` was a no-op constraint and `minItems` survives. + let mut schema = json!({ + "type": "object", + "properties": { + "s": {"type": "array", "items": {}, "minItems": 1} + } + }); + sanitize_for_kimi_parameters(&mut schema) + .expect("a non-required position ships the equivalent repaired schema"); + assert_eq!( + schema["properties"]["s"], + json!({"type": "array", "minItems": 1}) + ); + + // The terminator's other two checked positions — an `anyOf` branch + // and a `$ref` target, each reached from a required property — fail + // closed the same way, with the terminator's variant. + let mut schema = json!({ + "type": "object", + "properties": { + "a": { + "anyOf": [ + {"type": "array", "items": {}, "minItems": 1}, + {"type": "string"} + ] + } + }, + "required": ["a"] + }); + let error = sanitize_for_kimi_parameters(&mut schema) + .expect_err("minItems >= 1 cannot terminate in a checked anyOf branch"); + assert!( + matches!(error, KimiParameterSchemaError::InvalidSchemaNode), + "the anyOf branch's variant is InvalidSchemaNode: {error:?}" + ); + + let mut schema = json!({ + "type": "object", + "$defs": {"a": {"type": "array", "items": {}, "minItems": 1}}, + "properties": {"x": {"$ref": "#/$defs/a"}}, + "required": ["x"] + }); + let error = sanitize_for_kimi_parameters(&mut schema) + .expect_err("minItems >= 1 cannot terminate at a checked $ref target"); + assert!( + matches!(error, KimiParameterSchemaError::InvalidSchemaNode), + "the $ref target's variant is InvalidSchemaNode: {error:?}" + ); + } + + #[test] + fn kimi_parameters_fail_closed_shapes_stay_dropped() { + // The body's fail-closed enumeration, each pinned so a validator + // change cannot silence the drop silently: empty `anyOf` branches + // (including one the nested repair itself empties), empty `$defs` + // entries, boolean and tuple `items`. + let mut schema = json!({ + "type": "object", + "properties": { + "choice": {"anyOf": [{"type": "string"}, {}]} + } + }); + let error = sanitize_for_kimi_parameters(&mut schema) + .expect_err("an empty anyOf branch fails closed"); + assert!( + matches!(error, KimiParameterSchemaError::MissingType), + "empty branch drops with MissingType: {error:?}" + ); + + let mut schema = json!({ + "type": "object", + "properties": { + "choice": {"anyOf": [{"type": "string"}, {"items": {"items": {}}}]} + } + }); + let error = sanitize_for_kimi_parameters(&mut schema) + .expect_err("a branch the nested repair empties to {} fails closed"); + assert!( + matches!(error, KimiParameterSchemaError::MissingType), + "nested-emptied branch drops with MissingType: {error:?}" + ); + + let mut schema = json!({ + "type": "object", + "properties": {"a": {"type": "string"}}, + "$defs": {"x": {}} + }); + let error = sanitize_for_kimi_parameters(&mut schema) + .expect_err("an empty $defs entry fails closed"); + assert!( + matches!(error, KimiParameterSchemaError::MissingType), + "empty $defs entry drops with MissingType: {error:?}" + ); + + let mut schema = json!({ + "type": "object", + "properties": {"a": {"type": "array", "items": true}}, + "required": ["a"] + }); + let error = + sanitize_for_kimi_parameters(&mut schema).expect_err("boolean items fail closed"); + assert!( + matches!(error, KimiParameterSchemaError::InvalidSchemaNode), + "boolean items drops with InvalidSchemaNode: {error:?}" + ); + + let mut schema = json!({ + "type": "object", + "properties": {"a": {"type": "array", "items": [{"type": "string"}]}}, + "required": ["a"] + }); + let error = + sanitize_for_kimi_parameters(&mut schema).expect_err("tuple-form items fail closed"); + assert!( + matches!(error, KimiParameterSchemaError::InvalidSchemaNode), + "tuple items drops with InvalidSchemaNode: {error:?}" + ); + } + #[test] fn kimi_parameters_rejects_required_direct_and_mutual_recursion() { let fixtures = [ diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index c783679cc4..2e0ca44e39 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -34,12 +34,13 @@ use crate::client::DeepSeekClient; use crate::config::MAX_SUBAGENTS; use crate::core::engine::tool_catalog::{ TOOL_SEARCH_NAME, active_tools_for_request, apply_native_tool_deferral, - ensure_advanced_tooling, execute_tool_search_with_cache, initial_active_tools, - is_tool_search_tool, remove_evicted_cache_activations, tool_matches_any_rule, + bounded_toolbox_eviction_note, ensure_advanced_tooling, execute_tool_search_with_cache, + initial_active_tools, is_tool_search_tool, park_toolbox_eviction, remove_toolbox_drops, + render_toolbox_eviction_notes, tool_matches_any_rule, toolbox_oversize_reason, touch_cached_tool_after_execution, }; use crate::core::events::{AgentProgressEventMeta, Event}; -use crate::core::session::ToolActivationCache; +use crate::core::session::{PendingToolboxEviction, ToolActivationCache, ToolboxEvictionTrigger}; use crate::dependencies::{ExternalTool, Git}; /// Compatibility re-export: the closed role set lives in /// `crate::fleet::role`, the lightweight role surface every spawn path @@ -14103,10 +14104,14 @@ fn routine_agent_progress_can_preserve_event_headroom(status: AgentWorkerStatus) /// names from this filtered catalog; they cannot make a denied tool executable. /// The active-name snapshot is rebuilt once per model request so a same-batch /// guessed call cannot use a schema that was not present in that request. +/// Bounds evictions observed outside a result payload (a catalog refresh's +/// revalidation, a warm start) park a note here that rides the child's next +/// recorded result, mirroring the parent engine's parked-note channel. struct SubAgentToolSurface { catalog: Vec, active_names: std::collections::HashSet, cache: ToolActivationCache, + pending_eviction_notes: Vec, } impl SubAgentToolSurface { @@ -14115,16 +14120,79 @@ impl SubAgentToolSurface { let activation = cache.activate(&catalog, warm_names); let mut active_names = initial_active_tools(&catalog); active_names.extend(activation.admitted); - Self { + let mut surface = Self { catalog, active_names, cache, + pending_eviction_notes: Vec::new(), + }; + // A warm start that overflows the bounds must not evict silently; + // park the note for the child's first result. + surface.park_eviction_note(ToolboxEvictionTrigger::Step, &activation.evicted); + surface + } + + /// Park (or combine with an already parked) bounded-toolbox eviction + /// note; it rides the child's next recorded result. + fn park_eviction_note(&mut self, trigger: ToolboxEvictionTrigger, evicted: &[String]) { + park_toolbox_eviction(&mut self.pending_eviction_notes, trigger, evicted); + } + + /// Attach the parked eviction notes to this result — the child's ride + /// channel. Names re-admitted since the park are netted out against the + /// child's live cache and its request's active-name set, so a note never + /// declares a tool uncallable that the riding result hands back, and a + /// note emptied by the netting is dropped. The render consumes the + /// notes. Error results carry the notes too, mirroring the parent + /// engine's error path: a note held back on `Err` would strand until a + /// later `Ok` — or forever if the child ends first. + fn ride_pending_eviction_notes(&mut self, result: &mut Result) { + if self.pending_eviction_notes.is_empty() { + return; + } + let Some(rendered) = render_toolbox_eviction_notes( + &mut self.pending_eviction_notes, + &self.cache, + &self.active_names, + &self.catalog, + ) else { + return; + }; + match result.as_mut() { + Ok(rich) => rich.result.content.push_str(&format!("\n\n{rendered}")), + Err(error) => *error = anyhow!("{error}\n\n{rendered}"), + } + } + + /// Refresh an executed cached tool and append its eviction note to the + /// recorded result — the child mirror of the parent engine's + /// post-execution channel. + fn touch_after_execution(&mut self, name: &str, result: &mut Result) { + if result.is_err() { + return; + } + let update = touch_cached_tool_after_execution( + &self.catalog, + &mut self.active_names, + &mut self.cache, + name, + ); + if !update.evicted.is_empty() + && let Ok(rich) = result.as_mut() + { + rich.result.content.push_str(&format!( + "\n\n{}", + bounded_toolbox_eviction_note(ToolboxEvictionTrigger::Step, &update.evicted,) + )); } } fn request_tools(&mut self, catalog: Vec, strict: bool) -> Vec { self.catalog = catalog; - self.cache.revalidate(&self.catalog); + // A refresh that grew a cached schema past the collective bounds + // evicts here, outside any result payload — park the note. + let removals = self.cache.revalidate(&self.catalog); + self.park_eviction_note(ToolboxEvictionTrigger::Step, &removals.bounds_evicted); let mut active_names = initial_active_tools(&self.catalog); active_names.extend(self.cache.names().map(str::to_string)); self.active_names = active_names; @@ -14149,17 +14217,48 @@ impl SubAgentToolSurface { fn hydrate(&mut self, name: &str) -> Result { let activation = self.cache.activate(&self.catalog, &[name.to_string()]); - remove_evicted_cache_activations(&self.catalog, &mut self.active_names, activation.evicted); + remove_toolbox_drops( + &mut self.active_names, + activation.dropped_names().map(str::to_string), + ); self.active_names .extend(activation.admitted.iter().cloned()); if activation.admitted.iter().any(|admitted| admitted == name) { - return Ok(format!( + // The load receipt is the hydration's only result payload, so + // the eviction note rides it to the child model. The evictions + // may come from the admission itself or from revalidation after + // a cached schema grew. + let mut message = format!( "Tool `{name}` was deferred and has now been loaded. Retry the call with the newly available schema." + ); + if !activation.evicted.is_empty() { + message.push_str(&format!( + "\n\n{}", + bounded_toolbox_eviction_note( + ToolboxEvictionTrigger::Step, + &activation.evicted, + ) + )); + } + return Ok(message); + } + // The only rejection here is the per-tool budget: a narrower search + // cannot help, so the recovery hint must say why the tool cannot + // enter instead of promising a narrower query would. The rejection + // runs after the activation, so revalidation may have bounds-evicted + // other tools on the way; this error is the call's only result + // payload, so the note rides it instead of being absorbed. + let mut message = format!( + "Tool {name} could not enter the bounded child toolbox: {}", + toolbox_oversize_reason() + ); + if !activation.evicted.is_empty() { + message.push_str(&format!( + "\n\n{}", + bounded_toolbox_eviction_note(ToolboxEvictionTrigger::Step, &activation.evicted) )); } - Err(anyhow!( - "Tool {name} could not enter the bounded child toolbox; use tool_search with a narrower query" - )) + Err(anyhow!(message)) } } @@ -15582,6 +15681,33 @@ impl SubAgentToolRegistry { request_active_names: &std::collections::HashSet, name: &str, input: Value, + ) -> Result { + let mut result = self + .execute_from_surface_inner( + agent_id, + tool_id, + surface, + request_active_names, + name, + input, + ) + .await; + // A note parked by an earlier catalog refresh (request-time + // revalidation, a warm start) has no payload of its own; it rides + // the child's next recorded result, mirroring the parent engine's + // parked-note channel. + surface.ride_pending_eviction_notes(&mut result); + result + } + + async fn execute_from_surface_inner( + &self, + agent_id: &str, + tool_id: &str, + surface: &mut SubAgentToolSurface, + request_active_names: &std::collections::HashSet, + name: &str, + input: Value, ) -> Result { if is_tool_search_tool(name) { if !request_active_names.contains(name) { @@ -15602,7 +15728,18 @@ impl SubAgentToolRegistry { "Tool {name} is not in this child's policy-filtered catalog" )); }; - if deferred && !request_active_names.contains(name) { + if deferred + && (!request_active_names.contains(name) || !surface.active_names.contains(name)) + { + // A mid-request bounds eviction (a touch or hydrate revalidation) + // removes the tool from the surface's live active set while the + // request snapshot still carries it. Falling through to + // `execute_full` would run a call the surface no longer admits + // and make the eviction note an earlier result recorded a false + // statement for the rest of this request; routing the retry + // through hydration re-admits it under the bounds — or refuses + // it with the shared oversize reason — exactly like the parent + // engine's retry flow. return surface .hydrate(name) .map(|content| RichToolResult::plain(ToolResult::success(content))); @@ -15610,14 +15747,9 @@ impl SubAgentToolRegistry { if !request_active_names.contains(name) { return Err(anyhow!("Tool {name} is not active for this sub-agent")); } - let result = self.execute_full(agent_id, tool_id, name, input).await; - if deferred && result.is_ok() { - touch_cached_tool_after_execution( - &surface.catalog, - &mut surface.active_names, - &mut surface.cache, - name, - ); + let mut result = self.execute_full(agent_id, tool_id, name, input).await; + if deferred { + surface.touch_after_execution(name, &mut result); } result } diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index c33a7eaffb..83fee07599 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -7595,6 +7595,466 @@ fn synthetic_deferred_tool(name: &str, description_bytes: usize) -> Tool { } } +fn synthetic_eager_tool(name: &str) -> Tool { + Tool { + tool_type: Some("function".to_string()), + name: name.to_string(), + description: "eager always-present tool".to_string(), + input_schema: json!({"type": "object", "properties": {}}), + allowed_callers: None, + defer_loading: Some(false), + input_examples: None, + strict: None, + cache_control: None, + } +} + +/// The child hydration receipt is the hydration's only result payload, so a +/// bounds eviction caused by the load must reach the child model through it. +#[test] +fn child_hydration_receipt_carries_the_eviction_note() { + // Two large warm activations sit near the collective budget; hydrating a + // third overflows the child cache. The 8,050-byte descriptions serialize + // to ~8,170-byte tools, so the warm pair totals ~16,340 bytes — roughly + // 40 bytes under the 16,384 collective budget: loading big-c overflows + // it, but the warm pair alone must not. If a Tool field or serde + // attribute changes serialized size, re-tune these numbers before + // trusting either half of this test. + let warm_catalog = vec![ + synthetic_deferred_tool("big-a", 8_050), + synthetic_deferred_tool("big-b", 8_050), + synthetic_deferred_tool("big-c", 8), + ]; + let mut premise = ToolActivationCache::default(); + assert!( + premise + .activate(&warm_catalog, &["big-a".to_string(), "big-b".to_string()]) + .evicted + .is_empty(), + "premise: the warm pair alone must fit the collective budget" + ); + let mut surface = + SubAgentToolSurface::new(warm_catalog, &["big-a".to_string(), "big-b".to_string()]); + + let receipt = surface.hydrate("big-c").expect("third activation"); + assert!( + receipt.contains("`big-c` was deferred and has now been loaded"), + "the receipt must confirm the load: {receipt}" + ); + assert!( + receipt.contains("evicted tools from the bounded conversation toolbox") + && receipt.contains("big-a"), + "the load receipt must carry the eviction note naming the LRU victim: {receipt}" + ); + assert!(!surface.active_names.contains("big-a")); +} + +/// The oversize hydration rejection runs after the activation, so its +/// revalidation may have bounds-evicted other tools. This error is the +/// call's only result payload: the rejection must carry both the shared +/// oversize reason and those evictions instead of absorbing them. +#[test] +fn child_hydrate_rejection_carries_the_revalidations_evictions() { + let warm_catalog = vec![ + synthetic_deferred_tool("big-a", 8_050), + synthetic_deferred_tool("big-b", 8_050), + synthetic_deferred_tool("big-c", 20_000), + ]; + let mut surface = + SubAgentToolSurface::new(warm_catalog, &["big-a".to_string(), "big-b".to_string()]); + // The catalog's schema for `big-a` grew after the warm start (a + // refresh): the rejection's activation revalidates against it and the + // collective budget overflows, while `big-c` itself is per-tool + // oversize and is refused. + surface.catalog = vec![ + synthetic_deferred_tool("big-a", 12_000), + synthetic_deferred_tool("big-b", 8_050), + synthetic_deferred_tool("big-c", 20_000), + ]; + + let error = surface + .hydrate("big-c") + .expect_err("big-c is per-tool oversize"); + let message = error.to_string(); + assert!( + message.contains("could not enter the bounded child toolbox") + && message.contains(&toolbox_oversize_reason()), + "the rejection must carry the shared oversize reason: {message}" + ); + assert!( + message.contains("no longer callable") && message.contains("big-a"), + "the rejection must carry the revalidation's eviction note: {message}" + ); + assert!( + !message.contains("has now been loaded"), + "the rejection must not promise a load: {message}" + ); + assert!( + surface.pending_eviction_notes.is_empty(), + "the evictions rode the error directly; nothing may stay parked" + ); +} + +/// The child's ride channel mirrors the parent's: parked notes ride success +/// and error results alike (a note held back on `Err` would strand until a +/// later `Ok` — or forever if the child ends first), and names the ride +/// re-admits are netted out against the child's live cache. +#[test] +fn child_parked_note_rides_success_and_error_results_and_nets_re_admissions() { + // A parked eviction note names cache members, and cache members are + // catalog members — the premise the ride-time catalog check relies on. + // The eager lead defeats `initial_active_tools`' single-tool fallback, + // which would otherwise force-activate `big-a` and the ride would net + // it out of the note. + let mut lead = synthetic_deferred_tool("lead", 8); + lead.defer_loading = Some(false); + let catalog = vec![ + lead, + synthetic_deferred_tool("big-a", 8), + synthetic_deferred_tool("big-b", 8), + synthetic_deferred_tool("big-c", 8), + ]; + let mut surface = SubAgentToolSurface::new(catalog, &[]); + surface.park_eviction_note( + ToolboxEvictionTrigger::Step, + vec!["big-a".to_string(), "big-b".to_string()].as_slice(), + ); + + let mut ok = Ok(RichToolResult::plain(ToolResult::success("payload"))); + surface.ride_pending_eviction_notes(&mut ok); + let ok = ok.expect("stays ok"); + assert!( + ok.result.content.contains("payload") + && ok.result.content.contains("no longer callable") + && ok.result.content.contains("big-a") + && ok.result.content.contains("big-b"), + "the note rides a success result: {}", + ok.result.content + ); + + surface.park_eviction_note(ToolboxEvictionTrigger::Step, &["big-c".to_string()]); + let mut failure: Result = Err(anyhow::anyhow!("boom")); + surface.ride_pending_eviction_notes(&mut failure); + let message = failure.expect_err("stays an error").to_string(); + assert!( + message.contains("boom") + && message.contains("no longer callable") + && message.contains("big-c"), + "the note must ride an error result too: {message}" + ); + assert!(surface.pending_eviction_notes.is_empty()); + + // A name the ride hands back is netted out; a fully netted note does + // not ride at all. + let mut deferred = synthetic_deferred_tool("big-a", 8); + deferred.name = "big-a".to_string(); + surface.cache.activate(&[deferred], &["big-a".to_string()]); + surface.park_eviction_note(ToolboxEvictionTrigger::Step, &["big-a".to_string()]); + let mut netted = Ok(RichToolResult::plain(ToolResult::success("clean"))); + surface.ride_pending_eviction_notes(&mut netted); + let netted = netted.expect("stays ok"); + assert!( + netted.result.content == "clean", + "a note naming only re-admitted tools must be absorbed: {}", + netted.result.content + ); +} + +/// An executed tool's refresh can evict on the collective bounds when a +/// cached schema grew; the child mirror of the parent's post-execution +/// channel must append the eviction note to the recorded result. +#[test] +fn child_execution_refresh_eviction_note_rides_the_child_result() { + let warm_catalog = vec![ + synthetic_deferred_tool("big-a", 8_050), + synthetic_deferred_tool("big-b", 8_050), + ]; + let mut surface = + SubAgentToolSurface::new(warm_catalog, &["big-a".to_string(), "big-b".to_string()]); + // The catalog's schema for `big-a` grew after the warm start (a + // refresh): refreshing `big-b` revalidates against the grown catalog + // and the collective budget overflows. + surface.catalog = vec![ + synthetic_deferred_tool("big-a", 12_000), + synthetic_deferred_tool("big-b", 8_050), + ]; + + let mut result: Result = + Ok(RichToolResult::plain(ToolResult::success("payload"))); + surface.touch_after_execution("big-b", &mut result); + + let rich = result.expect("the refresh result stays ok"); + assert!( + rich.result.content.contains("big-a") + && rich + .result + .content + .contains("evicted tools from the bounded conversation toolbox"), + "the refresh result must carry the eviction note naming the LRU victim: {}", + rich.result.content + ); + assert!(!surface.active_names.contains("big-a")); + assert!( + surface.pending_eviction_notes.is_empty(), + "the touch eviction rides the result directly, not the parked slot" + ); +} + +/// A note parked outside any result payload (a catalog refresh's +/// revalidation) must ride the child's next recorded result instead of +/// vanishing. +#[test] +fn child_parked_note_rides_the_next_result_and_refresh_parks_one() { + let warm_catalog = vec![ + synthetic_deferred_tool("big-a", 8_050), + synthetic_deferred_tool("big-b", 8_050), + ]; + let mut surface = + SubAgentToolSurface::new(warm_catalog, &["big-a".to_string(), "big-b".to_string()]); + assert!( + surface.pending_eviction_notes.is_empty(), + "a warm start inside the bounds must not park anything" + ); + + // The refresh grows `big-a` past the collective bounds: request_tools + // revalidates, the LRU victim pops, and the note must park for the + // child's next recorded result. The eager lead tool keeps the + // single-tool fallback from force-activating `big-a`. + let refreshed_catalog = vec![ + synthetic_eager_tool("small-eager"), + synthetic_deferred_tool("big-a", 12_000), + synthetic_deferred_tool("big-b", 8_050), + ]; + let tools = surface.request_tools(refreshed_catalog, false); + assert!( + !tools.iter().any(|tool| tool.name == "big-a"), + "the evicted tool must leave the child's request surface" + ); + let parked = &surface.pending_eviction_notes; + assert_eq!( + parked.len(), + 1, + "the refresh eviction must park a note: {parked:?}" + ); + assert!( + parked[0].evicted.iter().any(|name| name == "big-a"), + "the parked note must name the LRU victim: {parked:?}" + ); +} + +/// The park side is covered above; this pin is the consumption side — +/// `execute_from_surface` must hand parked notes to the child's recorded +/// result, the production edge that once discarded every report. Driven +/// through the same wrapper a real child tool call takes. +#[tokio::test] +async fn parked_note_rides_through_the_execute_from_surface_glue() { + let registry = small_surface_registry(FleetRole::Builder); + let catalog = registry.deferred_catalog_for_model(&FleetRole::Builder); + let mut surface = SubAgentToolSurface::new(catalog, &["get_goal".to_string()]); + // A parked eviction note names a cache member, and a cache member is a + // catalog member — the premise the ride-time catalog check relies on. + surface + .catalog + .push(synthetic_deferred_tool("ghost-tool", 8)); + surface.park_eviction_note( + ToolboxEvictionTrigger::TurnStart, + &["ghost-tool".to_string()], + ); + + let content = execute_surface_tool(®istry, &mut surface, "get_goal", json!({})) + .await + .expect("cached read tool executes"); + assert!( + content.contains("no longer callable") && content.contains("ghost-tool"), + "the production ride glue must attach the parked note to the recorded result: {content}" + ); + assert!( + surface.pending_eviction_notes.is_empty(), + "the ride consumes the parked notes: {:?}", + surface.pending_eviction_notes + ); +} + +/// The hydrate dispatch is production wiring too: a deferred tool outside +/// the request's active set must load through `execute_from_surface` — the +/// glue edge whose regression would dead-end every first call. Driven +/// through the same wrapper a real child tool call takes. +#[tokio::test] +async fn hydrate_dispatch_loads_a_deferred_tool_through_the_execute_from_surface_glue() { + let registry = small_surface_registry(FleetRole::Builder); + let catalog = vec![ + synthetic_deferred_tool("big-a", 8_050), + synthetic_deferred_tool("big-b", 8_050), + synthetic_deferred_tool("big-c", 8), + ]; + // Premise: the warm pair alone must fit the collective budget, so the + // overflow below is attributable to the third load (see the byte-tuning + // note on the receipt test above). + let mut premise = ToolActivationCache::default(); + assert!( + premise + .activate(&catalog, &["big-a".to_string(), "big-b".to_string()]) + .evicted + .is_empty(), + "premise: the warm pair must fit the collective budget" + ); + let mut surface = + SubAgentToolSurface::new(catalog, &["big-a".to_string(), "big-b".to_string()]); + + let content = execute_surface_tool(®istry, &mut surface, "big-c", json!({})) + .await + .expect("the deferred tool must hydrate through the glue, not dead-end"); + assert!( + content.contains("`big-c` was deferred and has now been loaded"), + "the glue's hydrate dispatch must load the tool: {content}" + ); + assert!( + surface.active_names.contains("big-c"), + "the hydration must activate the tool for the rest of this request: {:?}", + surface.active_names + ); +} + +/// The refresh channel's reporting is production wiring: an executed +/// deferred tool's touch eviction must ride the recorded result through +/// `execute_from_surface` itself — deleting the dispatch there must turn +/// this pin red, the exact round-2 defect shape (the child discarding its +/// helpers' reports). +#[tokio::test] +async fn touch_glue_rides_the_refresh_eviction_through_execute_from_surface() { + let registry = small_surface_registry(FleetRole::Builder); + let catalog = vec![ + synthetic_deferred_tool("big-a", 8_050), + synthetic_deferred_tool("get_goal", 8_050), + ]; + let mut surface = + SubAgentToolSurface::new(catalog, &["big-a".to_string(), "get_goal".to_string()]); + // The catalog's schema for `get_goal` grew after the warm start (a + // refresh): the executed tool's touch revalidates against it and the + // collective budget overflows. + surface.catalog = vec![ + synthetic_deferred_tool("big-a", 8_050), + synthetic_deferred_tool("get_goal", 8_400), + ]; + + let content = execute_surface_tool(®istry, &mut surface, "get_goal", json!({})) + .await + .expect("the cached tool executes"); + assert!( + content.contains("no longer callable") && content.contains("big-a"), + "the touch eviction must ride the recorded result through the glue: {content}" + ); + assert!( + !surface.active_names.contains("big-a"), + "the evicted victim must leave the child's active set" + ); +} + +/// A mid-request bounds eviction must not leave the evicted tool +/// dispatchable for the rest of the request: the request's active-name +/// snapshot still carries it, but the surface's live active set no longer +/// does. Routing the retry through hydration re-admits it under the bounds +/// with a truthful receipt (or refuses it with the shared oversize reason) +/// instead of executing a call the surface no longer admits — the eviction +/// note an earlier result recorded would otherwise be a false statement for +/// the rest of this request. Deleting the live-set disjunct in the dispatch +/// turns this pin red. +#[tokio::test] +async fn mid_request_eviction_routes_the_retry_through_hydration() { + let registry = small_surface_registry(FleetRole::Builder); + let catalog = vec![ + synthetic_deferred_tool("big-a", 8_050), + synthetic_deferred_tool("big-b", 8_050), + synthetic_deferred_tool("big-c", 8), + ]; + let mut surface = + SubAgentToolSurface::new(catalog, &["big-a".to_string(), "big-b".to_string()]); + // The request's active-name snapshot is taken once per request; call i + // (hydrating big-c) evicts big-a behind it. + let request_active: HashSet = ["big-a".to_string(), "big-b".to_string()] + .into_iter() + .collect(); + + let load = execute_surface_tool(®istry, &mut surface, "big-c", json!({})) + .await + .expect("big-c hydrates"); + assert!( + load.contains("`big-c` was deferred and has now been loaded") && load.contains("big-a"), + "premise: the mid-request hydration evicts big-a and says so: {load}" + ); + assert!( + !surface.active_names.contains("big-a"), + "premise: big-a left the surface's live active set" + ); + + // Call j retries big-a under the stale request snapshot. + let retry = registry + .execute_from_surface( + "agent_test", + "", + &mut surface, + &request_active, + "big-a", + json!({}), + ) + .await + .expect("the retry must rehydrate, not execute behind the eviction"); + assert!( + retry + .result + .content + .contains("`big-a` was deferred and has now been loaded"), + "the evicted retry must route through hydration: {}", + retry.result.content + ); + assert!( + retry.result.content.contains("big-b"), + "the rehydration's own eviction must be reported truthfully: {}", + retry.result.content + ); + assert!(surface.active_names.contains("big-a")); + assert!(!surface.active_names.contains("big-b")); +} + +/// The warm-start overflow park is defensive today (the only production +/// spawn passes no warm names) but it is the constructor's invariant guard: +/// deleting the park must turn this pin red. +#[tokio::test] +async fn warm_start_overflow_park_rides_the_first_result() { + let registry = small_surface_registry(FleetRole::Builder); + let catalog = registry.deferred_catalog_for_model(&FleetRole::Builder); + let deferred: Vec = catalog + .iter() + .filter(|tool| tool.defer_loading == Some(true) && tool.name != "get_goal") + .map(|tool| tool.name.clone()) + .take(8) + .collect(); + // Nine warm names exceed the eight-name cache: the LRU front victim + // pops at construction and its note must park for the first result. + // `get_goal` is warmed last (most recently used) so it survives and the + // pinned ride runs down the executed-tool path. + let mut warm = deferred.clone(); + warm.push("get_goal".to_string()); + let victim = warm[0].clone(); + let mut surface = SubAgentToolSurface::new(catalog, &warm); + let parked = &surface.pending_eviction_notes; + assert!( + parked + .iter() + .any(|note| note.evicted.iter().any(|name| name == &victim)), + "premise: the warm start must overflow and park the LRU victim: {parked:?}" + ); + + let content = execute_surface_tool(®istry, &mut surface, "get_goal", json!({})) + .await + .expect("cached read tool executes"); + assert!( + content.contains("no longer callable") && content.contains(&victim), + "the warm-start overflow note must ride the child's first result: {content}" + ); + assert!(surface.pending_eviction_notes.is_empty()); +} + #[test] fn small_surface_caches_are_independent_bounded_and_revalidated() { let mut catalog = (0..9) diff --git a/docs/TOOL_SURFACE.md b/docs/TOOL_SURFACE.md index 641eed374d..d777a3ce15 100644 --- a/docs/TOOL_SURFACE.md +++ b/docs/TOOL_SURFACE.md @@ -76,11 +76,53 @@ user explicitly names them in `[tools].always_load`. A successful search activation is remembered by name for the current conversation. The cache holds at most eight deferred names and 16 KiB of -serialized schemas, evicts least-recently-used entries, and revalidates every +serialized tool definitions, evicts least-recently-used entries, and +revalidates every entry against the current catalog and policy before advertising it again. A session sync clears it. The cache cannot resurrect a removed, denied, or newly-eager tool. +Bounds-driven evictions are reported to the model best-effort — in the +`tool_search` result when a search causes it, otherwise appended to a tool +result: a hydration's note rides the batch's first recorded result (or its +error), a retrieval activation or a refresh of an executed tool rides the +triggering result, and a turn's start revalidation parks its note on the +session so the turn's first recorded result carries it (a refresh only +evicts when a cached tool's serialized definition grew since its last +activation enough that the collective budget overflows). A note that never reaches a recorded result is dropped rather +than riding an unrelated later result: a session sync clears it at the +conversation edge, and a turn that ends without a recorded result has it +swept at the next turn's entry. Child agents report the same way on their +own surface (they have no conversation-level sync or turn-entry sweep): a +hydration receipt (or its refusal error) and an executed tool's result +carry their own eviction notes, and a note parked outside any result — a +catalog refresh's revalidation or a warm-start overflow — rides the child's +next recorded result (or its error); a call the child cancels on its tool +timeout is recorded without the note, deferring it to a later result; a +retry of a tool a mid-request eviction removed rehydrates it under the +bounds instead of executing behind the eviction; a note a child never +delivers dies with the child's surface. A note never +names a tool the model can call +again: names the ride re-admits — into the activation cache or the +request's active tool set — are netted out of it, a name that left the +catalog between the eviction and the ride is reported under that +unrecoverable truth instead of the search promise, and a mid-turn MCP +catalog refresh respects the same rule from the other side, re-advertising +only the eager entries its servers serve and the deferred tools this +conversation still has activated. An evicted deferred tool therefore stays +off the wire and the note naming it stays truthful. Tools +the model cannot get back by searching are reported differently: +`tool_search` results surface them as unavailable references, naming +whether the tool left the catalog (a disconnect, a changed allow/deny rule, +a mode switch — any catalog-shrink cause) or outgrew the per-tool +serialized-size budget. A cached tool that became +eager is not reported at all — it stays directly callable. Unavailable drops +observed by non-search paths are absorbed silently: an oversize drop is +re-reported with its reason the next time a search returns the tool (a +match ranked past the result cap re-reports nothing), while a +catalog-shrink drop left the catalog entirely and no later search can name +it — the model learns of those only when a search for the capability misses. + Each subagent gets its own policy-filtered deferred catalog, always-present `tool_search`, and bounded activation cache. Forked messages and instructions remain in context, but the child cache starts empty and discovers tools locally;