From c92b38de76cd7d7ee52760328e9dd074b2915c74 Mon Sep 17 00:00:00 2001 From: CodeWhale Bot Date: Wed, 23 Sep 2026 08:47:47 -0700 Subject: [PATCH] feat(tools): run well-formed first calls to deferred tools Deferred tools kept every unused schema out of the pinned prefix, but a deferred tool's first call never ran: it returned the schema and told the model to retry, so ordinary investigation (list_dir, grep_files, Web fetch, Run) spent a turn plus a tool-surface re-pin before doing anything. Children and Workflow leaves also started with none of the tools their assignment named explicitly. - Parent and child: a first call that already matches the unseen schema (object, every required field, no undeclared field) executes and activates at the catalog tail. Malformed calls still get the schema. Hydration runs after every authority gate (deny/allow lists, Plan, repo law, approval), and dispatch still requires a registry spec, so this changes turn cost, not authority. The executed path emits a `tool.deferred_first_use_executed` audit record. - Children warm their explicit `allowed_tools` onto the first request through the existing bounded activation cache (8 names / 16 KiB). Visibility only: grants, role posture and envelopes are unchanged. Known limitation: argument types are left to each tool's own validation. Evidence: 60 passed, 0 failed (13,192 skipped) across deferred, small_surface, tool_catalog, hydrat and tool_search selections, including a Scout whose well-formed first list_dir runs, whose malformed call gets the schema, and whose write/edit stay refused. Two tests that encoded the old never-execute behavior now use malformed input to keep covering the schema-hint path. TUI all-target/all-feature Clippy with CI flags and fmt passed. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/tui/src/core/engine/tests.rs | 84 ++++++++++++---------- crates/tui/src/core/engine/tool_catalog.rs | 28 +++++++- crates/tui/src/core/engine/turn_loop.rs | 30 +++++--- crates/tui/src/tools/subagent/mod.rs | 32 ++++++--- crates/tui/src/tools/subagent/tests.rs | 83 +++++++++++++++++++-- 5 files changed, 196 insertions(+), 61 deletions(-) diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 08b8ab2107..8a91323686 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -12008,7 +12008,7 @@ fn print_skill_discovery_turn_metrics() { } #[test] -fn deferred_apply_patch_first_use_hydrates_schema_without_execution() { +fn deferred_first_use_executes_well_formed_calls_and_hydrates_malformed_ones() { let mut apply_patch = api_tool("apply_patch"); apply_patch.defer_loading = Some(true); apply_patch.input_schema = json!({ @@ -12022,40 +12022,48 @@ 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( - "apply_patch", - &json!({"patch": "*** Begin Patch\n*** End Patch"}), - &catalog, - &active_at_batch_start, - &mut hydrated_this_batch, - ) - .expect("first deferred use should hydrate"); - - assert!(!active_at_batch_start.contains("apply_patch")); - assert!(hydrated_this_batch.contains("apply_patch")); - assert!(result.success); - assert!(result.content.contains("Tool `apply_patch` was deferred")); - assert!(result.content.contains("patch: string")); - assert!(result.content.contains("The tool was not executed")); - - let metadata = result.metadata.expect("metadata"); - assert_eq!(metadata["event"], "tool.schema_hydrated"); - assert_eq!(metadata["executed"], false); - assert_eq!(metadata["retry_required"], true); - - let 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"); - assert_eq!(second_result.metadata.unwrap()["executed"], false); - assert_eq!( - hydrated_this_batch, - HashSet::from(["apply_patch".to_string()]) + // A call already shaped like the unseen schema must not lose its turn. + assert!( + maybe_hydrate_requested_deferred_tool( + "apply_patch", + &json!({"patch": "*** Begin Patch\n*** End Patch"}), + &catalog, + &active_at_batch_start, + &mut hydrated_this_batch, + ) + .is_none(), + "a well-formed first call executes" ); + assert!( + hydrated_this_batch.contains("apply_patch"), + "the executed tool still activates for later requests" + ); + + for malformed in [ + json!({}), + json!({"diff": "*** Begin Patch\n*** End Patch"}), + json!({"patch": "x", "path": "src/lib.rs"}), + json!("*** Begin Patch"), + ] { + let mut hydrated = HashSet::new(); + let result = maybe_hydrate_requested_deferred_tool( + "apply_patch", + &malformed, + &catalog, + &active_at_batch_start, + &mut hydrated, + ) + .unwrap_or_else(|| panic!("{malformed} must return the schema instead of executing")); + assert!(hydrated.contains("apply_patch")); + assert!(result.success); + assert!(result.content.contains("Tool `apply_patch` was deferred")); + assert!(result.content.contains("patch: string")); + assert!(result.content.contains("The tool was not executed")); + let metadata = result.metadata.expect("metadata"); + assert_eq!(metadata["event"], "tool.schema_hydrated"); + assert_eq!(metadata["executed"], false); + assert_eq!(metadata["retry_required"], true); + } let mut active_next_batch = active_at_batch_start.clone(); active_next_batch.extend(hydrated_this_batch); @@ -12063,13 +12071,13 @@ fn deferred_apply_patch_first_use_hydrates_schema_without_execution() { assert!( maybe_hydrate_requested_deferred_tool( "apply_patch", - &json!({"patch": "*** Begin Patch\n*** End Patch"}), + &json!({}), &catalog, &active_next_batch, &mut hydrated_next_batch, ) .is_none(), - "tools hydrated in a previous batch should execute normally" + "tools hydrated in a previous batch execute normally, even malformed" ); } @@ -12088,7 +12096,9 @@ async fn deferred_tool_first_use_does_not_emit_a_retry_status() { let tool_call_sse = concat!( "data: {\"id\":\"chatcmpl-e3\",\"choices\":[{\"index\":0,\"delta\":{\"tool_calls\":[", "{\"index\":0,\"id\":\"call_e3_map\",\"type\":\"function\",\"function\":{\"name\":\"project_map\",", - "\"arguments\":\"{}\"}}", + // Malformed on purpose: a well-formed first call now executes, and + // this test covers the schema hint returned for a malformed one. + "\"arguments\":\"{\\\"not_a_project_map_field\\\":true}\"}}", "]},\"finish_reason\":null}]}\n\n", "data: {\"id\":\"chatcmpl-e3\",\"choices\":[{\"index\":0,\"delta\":{},", "\"finish_reason\":\"tool_calls\"}]}\n\n", diff --git a/crates/tui/src/core/engine/tool_catalog.rs b/crates/tui/src/core/engine/tool_catalog.rs index b8534a44fd..43d2c1cbed 100644 --- a/crates/tui/src/core/engine/tool_catalog.rs +++ b/crates/tui/src/core/engine/tool_catalog.rs @@ -1226,9 +1226,35 @@ pub(super) fn maybe_hydrate_requested_deferred_tool( } hydrated_tools_this_batch.insert(tool_name.to_string()); + if deferred_first_call_matches_schema(def, tool_input) { + // Progressive disclosure keeps unused schemas out of the prefix; it + // must not cost a well-formed call its turn. Every authority gate has + // already run for this call, so executing it grants nothing new, and + // the tool still activates at the tail for later requests. + return None; + } Some(deferred_tool_schema_hydration_result(def, tool_input)) } +/// Whether a call to a tool whose schema the model has not yet been shown is +/// shaped like that schema: an object carrying every required field and, when +/// the schema declares properties, no field outside them. Known limitation: +/// field types are left to the tool's own input validation, which reports a +/// wrong type as an ordinary tool error after the schema has been activated. +pub(crate) fn deferred_first_call_matches_schema(tool: &Tool, tool_input: &Value) -> bool { + let Some(input) = tool_input.as_object() else { + return false; + }; + let expected = schema_fields(&tool.input_schema); + let required = schema_required_fields(&tool.input_schema); + required.iter().all(|field| input.contains_key(field)) + && (expected.is_empty() && input.is_empty() + || !expected.is_empty() + && input + .keys() + .all(|key| expected.iter().any(|field| &field.name == key))) +} + #[cfg(test)] pub(super) fn preflight_requested_deferred_tool( tool_name: &str, @@ -1249,7 +1275,7 @@ pub(super) fn preflight_requested_deferred_tool( result } -fn deferred_tool_schema_hydration_result(tool: &Tool, tool_input: &Value) -> ToolResult { +pub(crate) fn deferred_tool_schema_hydration_result(tool: &Tool, tool_input: &Value) -> ToolResult { let expected = schema_fields(&tool.input_schema); let required = schema_required_fields(&tool.input_schema); let received = received_field_names(tool_input); diff --git a/crates/tui/src/core/engine/turn_loop.rs b/crates/tui/src/core/engine/turn_loop.rs index 0dc3367b14..7af9afdeee 100644 --- a/crates/tui/src/core/engine/turn_loop.rs +++ b/crates/tui/src/core/engine/turn_loop.rs @@ -3509,21 +3509,33 @@ impl Engine { let first_hydration_this_batch = !deferred_tools_hydrated_this_batch.contains(&tool_name); - if blocked_error.is_none() - && let Some(result) = maybe_hydrate_requested_deferred_tool( + let hydration = if blocked_error.is_none() { + maybe_hydrate_requested_deferred_tool( &tool_name, &tool_input, tool_catalog, &active_tools_at_batch_start, &mut deferred_tools_hydrated_this_batch, ) + } else { + None + }; + if first_hydration_this_batch && deferred_tools_hydrated_this_batch.contains(&tool_name) { - if first_hydration_this_batch { - // 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()); + // Retain first-proposal order separately from the set used to + // deduplicate calls in this batch. LRU bounds must not depend + // on randomized HashSet iteration. A well-formed first call + // executes below and activates exactly like a hydrated one. + deferred_tools_hydrated_in_order.push(tool_name.clone()); + if hydration.is_none() { + emit_tool_audit(json!({ + "event": "tool.deferred_first_use_executed", + "tool_id": tool_id.clone(), + "tool_name": tool_name.clone(), + })); } + } + if let Some(result) = hydration { emit_tool_audit(json!({ "event": "tool.schema_hydrated", "tool_id": tool_id.clone(), @@ -3536,8 +3548,8 @@ impl Engine { // receives it in the tool result below (E3). The audit // record above is the receipt. // 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. + // request and the call does not match it: return the schema + // now and require a corrected model call. guard_result = Some(result); } diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index 851a9e0c2a..ba21caa59a 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -34,6 +34,7 @@ use crate::client::CodewhaleClient; use crate::config::{MAX_SUBAGENTS, SubagentModelOverride}; use crate::core::engine::tool_catalog::{ TOOL_SEARCH_NAME, ToolMode, active_tools_for_request, apply_native_tool_deferral, + deferred_first_call_matches_schema, deferred_tool_schema_hydration_result, ensure_advanced_tooling, execute_tool_search_with_cache, initial_active_tools, is_tool_search_tool, remove_evicted_cache_activations, tool_denied, touch_cached_tool_after_execution, @@ -13204,7 +13205,12 @@ async fn run_subagent( )); } let tool_catalog = tool_registry.deferred_catalog_for_model(&agent_type); - let mut tool_surface = SubAgentToolSurface::new(tool_catalog, &[]); + // Tools the assignment named explicitly are the ones it expects to use: + // put them on the first request instead of behind a discovery hop. The + // activation cache keeps its own size bound, and dispatch still enforces + // every grant; this changes visibility, never authority. + let mut tool_surface = + SubAgentToolSurface::new(tool_catalog, allowed_tools.as_deref().unwrap_or_default()); let mut steps = 0; let mut final_result: Option = None; let mut pending_inputs: VecDeque = VecDeque::new(); @@ -16801,14 +16807,23 @@ impl SubAgentToolSurface { .map_err(|error| anyhow!(error)) } - fn hydrate(&mut self, name: &str) -> Result { + /// Activate a deferred tool on its first call. `Ok(None)`: the call + /// already matches the schema and should execute now. `Ok(Some(text))`: + /// the schema the model must retry against. + fn hydrate(&mut self, name: &str, input: &Value) -> Result> { let activation = self.cache.activate(&self.catalog, &[name.to_string()]); remove_evicted_cache_activations(&self.catalog, &mut self.active_names, activation.evicted); self.active_names .extend(activation.admitted.iter().cloned()); if activation.admitted.iter().any(|admitted| admitted == name) { - return Ok(format!( - "Tool `{name}` was deferred and has now been loaded. Retry the call with the newly available schema." + let Some(tool) = self.catalog.iter().find(|tool| tool.name == name) else { + return Err(anyhow!("Tool {name} left this child's catalog")); + }; + if deferred_first_call_matches_schema(tool, input) { + return Ok(None); + } + return Ok(Some( + deferred_tool_schema_hydration_result(tool, input).content, )); } Err(anyhow!( @@ -18308,11 +18323,10 @@ impl SubAgentToolRegistry { )); }; if deferred && !request_active_names.contains(name) { - return surface - .hydrate(name) - .map(|content| RichToolResult::plain(ToolResult::success(content))); - } - if !request_active_names.contains(name) { + if let Some(schema) = surface.hydrate(name, &input)? { + return Ok(RichToolResult::plain(ToolResult::success(schema))); + } + } else 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; diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index 553fec7d63..dbb38f18cc 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -8111,14 +8111,85 @@ async fn small_surface_read_only_child_discovers_web_deferred() { &mut surface, &request_active, "Web", - json!({"action": "search", "query": "codewhale"}), + // Malformed on purpose: a well-formed first use now executes. + json!({"not_a_web_field": "codewhale"}), ) .await - .expect("same-batch first use hydrates instead of executing"); + .expect("a malformed first use hydrates instead of executing"); assert!(same_batch.result.content.contains("deferred")); assert!(model_tool_names(model_request_tools(&mut surface)).contains("Web")); } +/// First-call tool policy: a read-only investigator's well-formed first call +/// to a deferred inspection tool runs immediately instead of costing a +/// discovery turn, while mutation stays refused at dispatch. +#[tokio::test] +async fn small_surface_read_only_child_runs_well_formed_deferred_first_call() { + let registry = small_surface_registry(FleetRole::Scout); + let catalog = registry.deferred_catalog_for_model(&FleetRole::Scout); + assert_eq!( + catalog + .iter() + .find(|tool| tool.name == "list_dir") + .and_then(|tool| tool.defer_loading), + Some(true), + "list_dir is a deferred evidence tool for Scouts" + ); + let mut surface = SubAgentToolSurface::new(catalog, &[]); + assert!(!model_tool_names(model_request_tools(&mut surface)).contains("list_dir")); + let listing = execute_surface_tool(®istry, &mut surface, "list_dir", json!({})) + .await + .expect("well-formed first call executes"); + assert!( + !listing.contains("was deferred"), + "the call ran instead of returning its schema: {listing}" + ); + assert!(model_tool_names(model_request_tools(&mut surface)).contains("list_dir")); + + let hint = execute_surface_tool( + ®istry, + &mut SubAgentToolSurface::new(registry.deferred_catalog_for_model(&FleetRole::Scout), &[]), + "list_dir", + json!({"directory": "."}), + ) + .await + .expect("malformed first call returns the schema"); + assert!( + hint.contains("was deferred") && hint.contains("path"), + "{hint}" + ); + + for mutation in [ + ("write", json!({"path": "scout.txt", "content": "x"})), + ( + "edit", + json!({"path": "scout.txt", "old_string": "x", "new_string": "y"}), + ), + ] { + assert!( + execute_surface_tool(®istry, &mut surface, mutation.0, mutation.1) + .await + .is_err(), + "a Scout must not {}", + mutation.0 + ); + } +} + +/// Tools an assignment names explicitly start on the child's first request. +#[test] +fn small_surface_warms_explicitly_allowed_deferred_tools() { + let registry = small_surface_registry(FleetRole::Scout); + let catalog = registry.deferred_catalog_for_model(&FleetRole::Scout); + let mut surface = + SubAgentToolSurface::new(catalog, &["list_dir".to_string(), "read".to_string()]); + let names = model_tool_names(model_request_tools(&mut surface)); + assert!( + names.contains("list_dir") && names.contains("read"), + "{names:?}" + ); +} + #[tokio::test] async fn small_surface_fork_context_survives_fresh_child_discovery() { let registry = small_surface_registry(FleetRole::Builder); @@ -8218,7 +8289,7 @@ async fn small_surface_denied_warm_tool_is_not_resurrected() { .await .expect("search remains available"); assert!(!searched.contains("\"tool_name\":\"Web\"")); - assert!(surface.hydrate("Web").is_err()); + assert!(surface.hydrate("Web", &json!({})).is_err()); } fn synthetic_deferred_tool(name: &str, description_bytes: usize) -> Tool { @@ -8262,7 +8333,7 @@ fn small_surface_caches_are_independent_bounded_and_revalidated() { first .catalog .push(synthetic_deferred_tool("oversized", 17 * 1024)); - assert!(first.hydrate("oversized").is_err()); + assert!(first.hydrate("oversized", &json!({})).is_err()); let mut byte_catalog = (0..3) .map(|index| synthetic_deferred_tool(&format!("bytes_{index}"), 6 * 1024)) @@ -8306,7 +8377,9 @@ async fn small_surface_successful_cached_use_touches_lru() { execute_surface_tool(®istry, &mut surface, "get_goal", json!({})) .await .expect("cached read tool executes"); - surface.hydrate(&ninth).expect("ninth activation"); + surface + .hydrate(&ninth, &json!({})) + .expect("ninth activation"); assert!(model_tool_names(model_request_tools(&mut surface)).contains("get_goal")); }