diff --git a/src/harness/agent_loop/mod.rs b/src/harness/agent_loop/mod.rs index b0e8291..31928c4 100644 --- a/src/harness/agent_loop/mod.rs +++ b/src/harness/agent_loop/mod.rs @@ -85,7 +85,7 @@ use crate::harness::model::{ ChatModel, ModelDelta, ModelRequest, ModelResolutionSource, ModelResponse, ModelStreamItem, ResolvedModel, ResolvedModelBinding, ResponseFormat, StreamAccumulator, ToolChoice, }; -use crate::harness::runtime::{AgentHarness, UnknownToolPolicy}; +use crate::harness::runtime::{AgentHarness, InvalidArgsPolicy, UnknownToolPolicy}; use crate::harness::structured::{StructuredExtractor, StructuredStrategy}; use crate::harness::tool::{Tool, ToolCall, ToolSchema}; use futures::StreamExt; diff --git a/src/harness/agent_loop/test.rs b/src/harness/agent_loop/test.rs index 9627f7a..5336aa4 100644 --- a/src/harness/agent_loop/test.rs +++ b/src/harness/agent_loop/test.rs @@ -28,7 +28,7 @@ use crate::harness::model::{ }; use crate::harness::providers::MockModel; use crate::harness::retry::{FallbackPolicy, RetryPolicy}; -use crate::harness::runtime::{AgentHarness, RunPolicy, UnknownToolPolicy}; +use crate::harness::runtime::{AgentHarness, InvalidArgsPolicy, RunPolicy, UnknownToolPolicy}; use crate::harness::tool::{Tool, ToolCall, ToolResult, ToolSchema}; use crate::harness::usage::Usage; @@ -679,6 +679,50 @@ async fn invalid_tool_arguments_fail_before_tool_execution() { ); } +#[tokio::test] +async fn invalid_tool_arguments_return_tool_error_recovers() { + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness.register_model( + "mock", + Arc::new(MockModel::with_responses(vec![ + // `query` is required to be a string; 42 violates the schema, so + // admission must recover instead of running the tool or aborting. + tool_call_response("call-1", "strict_lookup", json!({ "query": 42 })), + text_response("recovered", 1, 1), + ])), + ); + let calls = Arc::new(Mutex::new(0)); + harness.register_tool(Arc::new(StrictLookupTool { + calls: Arc::clone(&calls), + })); + harness.with_policy(RunPolicy { + invalid_args: InvalidArgsPolicy::ReturnToolError, + ..RunPolicy::default() + }); + + let run = harness + .invoke_default(&(), vec![Message::user("lookup")]) + .await + .expect("invalid arguments are recoverable under ReturnToolError"); + + assert_eq!(run.final_response.unwrap().text(), "recovered"); + assert_eq!( + *calls.lock().unwrap(), + 0, + "the tool implementation must not run on invalid arguments" + ); + // The injected tool-error message carries the validation detail so the + // model can self-correct on the next turn. + let injected = run + .messages + .iter() + .any(|m| format!("{m:?}").contains("invalid arguments for tool `strict_lookup`")); + assert!( + injected, + "recovery message should be injected into the transcript" + ); +} + #[tokio::test] async fn structured_output_is_extracted() { let mut harness: AgentHarness<()> = AgentHarness::new(); diff --git a/src/harness/agent_loop/tools.rs b/src/harness/agent_loop/tools.rs index cf7e563..c09aa15 100644 --- a/src/harness/agent_loop/tools.rs +++ b/src/harness/agent_loop/tools.rs @@ -200,7 +200,35 @@ impl AgentHarness { } } }; - tool.schema().validate_call(call)?; + if let Err(err) = tool.schema().validate_call(call) { + // The model called a registered tool with schema-invalid arguments. + // Apply the run's `InvalidArgsPolicy` instead of unconditionally + // aborting the turn (mirrors the unknown-tool recovery above). + if matches!(self.policy.invalid_args, InvalidArgsPolicy::Fail) { + return Err(err); + } + // `ReturnToolError`: inject a tool-error result carrying the + // validation detail and the tool's expected parameter schema, then + // continue so the model can correct itself. This consumed one + // tool-call budget slot above, bounding the loop. + let call_id = CallId::new(call.id.clone()); + let detail = err.to_string(); + let schema_repr = serde_json::to_string(&tool.schema().parameters) + .unwrap_or_else(|_| "".to_string()); + let message = format!( + "invalid arguments for tool `{}`: {detail}; expected schema: {schema_repr}", + call.name + ); + let record = ctx.emit(AgentEvent::InvalidToolArgs { + call_id, + tool_name: call.name.clone(), + arguments: call.arguments.clone(), + error: detail, + recovery: "tool_error".to_string(), + }); + status.set_last_event(record.id); + return Ok(ResolvedToolCall::ErrorMessage(message)); + } Ok(ResolvedToolCall::Tool(tool)) } diff --git a/src/harness/events/types.rs b/src/harness/events/types.rs index 45fde2d..11dc981 100644 --- a/src/harness/events/types.rs +++ b/src/harness/events/types.rs @@ -182,6 +182,28 @@ pub enum AgentEvent { recovery: String, }, + /// The model called a *registered* tool with arguments that failed schema + /// validation, and the run's + /// [`InvalidArgsPolicy`][crate::harness::runtime::InvalidArgsPolicy] + /// recovered from it instead of aborting. + /// + /// Distinct from a tool that ran and returned an error: no tool was + /// executed. The tool name, arguments, and validation error are preserved + /// so the event stream can drive repair/analysis. + InvalidToolArgs { + /// Identifier of the offending tool call. + call_id: CallId, + /// The registered tool whose schema the supplied arguments violated. + tool_name: String, + /// The raw arguments the model supplied, preserved verbatim so repair + /// middleware or analysis can re-target or replay the invocation. + arguments: serde_json::Value, + /// The schema-validation error detail. + error: String, + /// How the run recovered (currently always `"tool_error"`). + recovery: String, + }, + /// A per-agent isolated workspace/sandbox was prepared. WorkspacePrepared { /// Audit identity of the policy that produced the environment. @@ -575,6 +597,7 @@ impl AgentEvent { AgentEvent::ToolStarted { .. } => "tool.started", AgentEvent::ToolCompleted { .. } => "tool.completed", AgentEvent::UnknownToolCall { .. } => "tool.unknown", + AgentEvent::InvalidToolArgs { .. } => "tool.invalid_args", AgentEvent::BudgetWarning { .. } => "budget.warning", AgentEvent::BudgetReserved { .. } => "budget.reserved", AgentEvent::BudgetReconciled { .. } => "budget.reconciled", diff --git a/src/harness/model/mod.rs b/src/harness/model/mod.rs index aa24cb1..ea3e6b8 100644 --- a/src/harness/model/mod.rs +++ b/src/harness/model/mod.rs @@ -753,10 +753,10 @@ impl StreamAccumulator { fn push_tool_chunk(&mut self, call_id: &str, content: &str, tool_name: Option<&str>) { if let Some(entry) = self.tool_chunks.iter_mut().find(|(id, ..)| id == call_id) { entry.1.push_str(content); - if entry.2.is_none() { - if let Some(name) = tool_name.filter(|n| !n.is_empty()) { - entry.2 = Some(name.to_string()); - } + if entry.2.is_none() + && let Some(name) = tool_name.filter(|n| !n.is_empty()) + { + entry.2 = Some(name.to_string()); } } else { self.tool_chunks.push(( diff --git a/src/harness/runtime/types.rs b/src/harness/runtime/types.rs index 18c7893..2b82fd8 100644 --- a/src/harness/runtime/types.rs +++ b/src/harness/runtime/types.rs @@ -73,6 +73,28 @@ pub enum UnknownToolPolicy { }, } +/// How the agent loop reacts when the model calls a *registered* tool with +/// arguments that fail schema validation. +/// +/// The default is [`InvalidArgsPolicy::Fail`], preserving the historical +/// fail-fast behavior where a missing `required` field, wrong type, or bad +/// `enum` aborts the whole turn. The recoverable variant lets a run keep going +/// so the model can self-correct — the recovery still consumes a tool-call +/// budget slot, so [`RunLimits::max_tool_calls`] bounds any invalid-args loop. +/// Mirrors [`UnknownToolPolicy`] for the schema-validation seam. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub enum InvalidArgsPolicy { + /// Abort the run with + /// [`TinyAgentsError::Validation`][crate::error::TinyAgentsError::Validation] + /// (the default, historical behavior). + #[default] + Fail, + /// Inject a tool-error result (carrying the validation detail and the + /// tool's expected parameter schema) back into the transcript and continue + /// the loop, letting the model retry with corrected arguments. + ReturnToolError, +} + /// Controls whether the agent loop captures model and tool **payloads** /// (prompt/completion text, tool arguments/results) onto the /// [`AgentEvent::ModelCompleted`][crate::harness::events::AgentEvent::ModelCompleted] @@ -126,6 +148,9 @@ pub struct RunPolicy { pub limits: RunLimits, /// How the loop reacts to a model call for an unregistered tool. pub unknown_tool: UnknownToolPolicy, + /// How the loop reacts when a registered tool's arguments fail schema + /// validation. + pub invalid_args: InvalidArgsPolicy, /// Retry policy applied to each model call. pub retry: RetryPolicy, /// Optional ordered model fallback chain. @@ -151,6 +176,7 @@ impl Default for RunPolicy { Self { limits: RunLimits::default(), unknown_tool: UnknownToolPolicy::default(), + invalid_args: InvalidArgsPolicy::default(), retry: RetryPolicy::default(), fallback: None, default_response_format: None, diff --git a/src/harness/stream/mod.rs b/src/harness/stream/mod.rs index 14411f0..1712142 100644 --- a/src/harness/stream/mod.rs +++ b/src/harness/stream/mod.rs @@ -143,6 +143,5 @@ pub fn stream(chunks: &[StreamChunk], modes: &[StreamMode]) -> Vec .collect() } -#[cfg(test)] #[cfg(test)] mod test;