From 3552e60ed887bde6bf7583d76f180201b2678cda Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Tue, 27 Oct 2026 18:45:20 +0530 Subject: [PATCH 1/2] fix(harness): keep provider-invalid tool arguments for admission repair ArgRecoveryMiddleware runs in before_tool, ahead of admission's relaxed-JSON repair. For a call the provider marked invalid it coerced the raw string to {} whenever the schema was permissive or unknown, so repair had nothing to fix and the call was still answered with the bare parse error. Leave such calls untouched. When repair fails, answer with the parse detail plus a corrective: re-send one well-formed JSON object, and build a large document incrementally and pass its reference instead of inlining it again. Production traces showed a model re-sending the same malformed inline workflow graph until the failure breaker halted the run. --- .../src/agent_loop/mod_tests.rs | 67 +++++++++++++++++++ .../src/agent_loop/tools.rs | 16 ++++- .../src/middleware/library/arg_recovery.rs | 22 ++++-- .../middleware/library/arg_recovery_tests.rs | 10 +++ 4 files changed, 110 insertions(+), 5 deletions(-) diff --git a/crates/tinyagents-harness/src/agent_loop/mod_tests.rs b/crates/tinyagents-harness/src/agent_loop/mod_tests.rs index 61f801b0a..40d215f43 100644 --- a/crates/tinyagents-harness/src/agent_loop/mod_tests.rs +++ b/crates/tinyagents-harness/src/agent_loop/mod_tests.rs @@ -3693,6 +3693,73 @@ async fn provider_invalid_arguments_recoverable_by_relaxed_json_are_repaired_and ); } +/// The host stack installs `ArgRecoveryMiddleware`, whose `before_tool` runs +/// before admission's relaxed-JSON repair. It used to coerce a provider-invalid +/// call's raw string to `{}` (the tool's schema is unknown to it here, so it +/// counts as permissive), which left repair nothing to fix and answered a +/// repairable call with the bare parse error. +#[tokio::test] +async fn arg_recovery_middleware_leaves_provider_invalid_arguments_for_repair() { + let tool = Arc::new(crate::testkit::FakeTool::returning("lookup", "found it")); + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness.with_model_resolver(Arc::new(crate::host::FixedModelResolver::named( + "mock", + Arc::new(MockModel::with_responses(vec![ + invalid_tool_call_response("call-x", "lookup", "{query:\"weather\"}"), + text_response("found it", 1, 1), + ])), + ))); + harness.register_tool(tool.clone()); + harness.push_middleware(Arc::new(crate::middleware::ArgRecoveryMiddleware::new( + Vec::new(), + ))); + + let run = harness + .invoke_in_context( + &(), + RunContext::new(RunConfig::new("arg-recovery-repair"), ()), + vec![Message::user("lookup the weather")], + ) + .await + .expect("repaired arguments let the call execute"); + + assert_eq!(run.text().as_deref(), Some("found it")); + assert_eq!(tool.calls(), vec![json!({"query": "weather"})]); +} + +/// An unrepairable call is answered with the parse detail plus a corrective +/// the model can act on, not the bare serde error alone. +#[tokio::test] +async fn unrepairable_arguments_are_answered_with_an_actionable_error() { + let tool = Arc::new(crate::testkit::FakeTool::returning("lookup", "found it")); + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness.with_model_resolver(Arc::new(crate::host::FixedModelResolver::named( + "mock", + Arc::new(MockModel::with_responses(vec![ + invalid_tool_call_response("call-x", "lookup", "{\"query\": [1 2]}"), + text_response("gave up", 1, 1), + ])), + ))); + harness.register_tool(tool.clone()); + + let run = harness + .invoke_in_context( + &(), + RunContext::new(RunConfig::new("unrepairable-args"), ()), + vec![Message::user("lookup")], + ) + .await + .expect("unparseable arguments do not fail the run"); + + assert!(tool.calls().is_empty()); + let transcript = format!("{:?}", run.messages); + assert!(transcript.contains("invalid JSON arguments"), "{transcript}"); + assert!( + transcript.contains("Re-send it as one well-formed JSON object"), + "{transcript}" + ); +} + #[tokio::test] async fn structured_output_is_extracted() { let mut harness: AgentHarness<()> = AgentHarness::new(); diff --git a/crates/tinyagents-harness/src/agent_loop/tools.rs b/crates/tinyagents-harness/src/agent_loop/tools.rs index a34177577..b36362a1b 100644 --- a/crates/tinyagents-harness/src/agent_loop/tools.rs +++ b/crates/tinyagents-harness/src/agent_loop/tools.rs @@ -869,7 +869,7 @@ impl AgentHarness { }); status.set_last_event(record.id); return Ok(ResolvedToolCall::Answered(tinytools::ToolResult::error( - detail, + unparseable_tool_call_message(&call.name, &detail), ))); } @@ -2342,6 +2342,20 @@ fn repeated_truncated_tool_call_message(tool_name: &str) -> String { ) } +/// The corrective for a call whose arguments the provider could not parse and +/// relaxed-JSON repair could not fix. A bare parse error ("expected `,` or `]` +/// at line 1 column 911") gave the model nothing to act on: production traces +/// show it re-sending the same large inline document until the failure breaker +/// halted the run. +fn unparseable_tool_call_message(tool_name: &str, detail: &str) -> String { + format!( + "Tool call `{tool_name}` was not run: its arguments are not valid JSON ({detail}). \ + Re-send it as one well-formed JSON object. If the arguments carry a large document, \ + do not inline it again: build it in smaller incremental calls (for example with an \ + editing tool that returns a handle or id) and pass that reference instead." + ) +} + /// Appends a batch's follow-up user messages (B2) after its last tool row, /// in the calls' original order. /// diff --git a/crates/tinyagents-harness/src/middleware/library/arg_recovery.rs b/crates/tinyagents-harness/src/middleware/library/arg_recovery.rs index 85e44a276..fa25e2458 100644 --- a/crates/tinyagents-harness/src/middleware/library/arg_recovery.rs +++ b/crates/tinyagents-harness/src/middleware/library/arg_recovery.rs @@ -24,10 +24,12 @@ use tinytools::Tool; /// 1. Already a JSON object → leave it (the common, valid case). /// 2. A JSON-encoded string (optionally fenced) that decodes to an object → /// decode and use it. -/// 3. Otherwise a non-object whose tool schema declares **no** required fields → +/// 3. A call the provider marked invalid (`ToolCall::invalid`) keeps its raw +/// string, so admission can run its relaxed-JSON repair on it. +/// 4. Otherwise a non-object whose tool schema declares **no** required fields → /// coerce to `{}` (legacy-engine parity: the tool runs and produces its own /// recoverable error). -/// 4. Otherwise (non-object + schema has required fields) → leave the arguments +/// 5. Otherwise (non-object + schema has required fields) → leave the arguments /// untouched so the crate's `InvalidArgsPolicy::ReturnToolError` path reports /// the original validation failure. Coercing to `{}` would discard the raw /// malformed value without improving recovery. @@ -78,7 +80,19 @@ impl Middleware<(), C> for ArgRecoveryMiddleware { return Ok(()); } - // (3) Non-object with a permissive schema (no required fields): coerce to + // (3) A call the provider marked unparseable keeps its raw string: admission + // runs the relaxed-JSON repair on it next, and otherwise answers with the + // parse error. Coercing it to `{}` here destroyed the input that repair + // needs while the call was still answered as invalid. + if call.invalid.is_some() { + tracing::debug!( + tool = call.name.as_str(), + "[tinyagents::mw] arg_recovery: leaving provider-invalid tool arguments for admission repair" + ); + return Ok(()); + } + + // (4) Non-object with a permissive schema (no required fields): coerce to // `{}` so the tool runs and produces its own recoverable error — engine // parity for tools that predate the schema gate. let has_required = self @@ -94,7 +108,7 @@ impl Middleware<(), C> for ArgRecoveryMiddleware { return Ok(()); } - // (4) Non-object + schema has required fields: leave untouched. The + // (5) Non-object + schema has required fields: leave untouched. The // crate admission policy surfaces a descriptive, recoverable tool // result without executing the real tool. tracing::debug!( diff --git a/crates/tinyagents-harness/src/middleware/library/arg_recovery_tests.rs b/crates/tinyagents-harness/src/middleware/library/arg_recovery_tests.rs index 0d8df8306..47994c3ac 100644 --- a/crates/tinyagents-harness/src/middleware/library/arg_recovery_tests.rs +++ b/crates/tinyagents-harness/src/middleware/library/arg_recovery_tests.rs @@ -97,3 +97,13 @@ async fn a_non_object_for_a_schema_with_required_fields_is_left_for_the_crate_ga async fn an_unknown_tool_is_treated_as_permissive() { assert_eq!(recover("nope", json!(3)).await, json!({})); } + +#[tokio::test] +async fn provider_invalid_arguments_keep_their_raw_string_for_admission_repair() { + let mut ctx: RunContext = RunContext::new(RunConfig::new("mw-test"), ()); + let raw = "{query:\"weather\"}"; + let mut call = ToolCall::invalid("c1", "loose", raw, "key must be a string"); + mw().before_tool(&mut ctx, &(), &mut call).await.unwrap(); + assert_eq!(call.arguments, json!(raw)); + assert!(call.invalid.is_some()); +} From b3deec6efdde4b0f4d0e843f7869b830053f3327 Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Tue, 27 Oct 2026 19:06:55 +0530 Subject: [PATCH 2/2] fix(harness): clear the invalid mark on a decoded fenced object A provider marks a markdown-fenced JSON object invalid. ArgRecoveryMiddleware decoded it to an object but left the mark, so admission answered the now-valid call with the stale parse error. Clear it on a successful decode. Word the unparseable-arguments corrective against the tool's argument schema rather than demanding an object, since string- and array-valued schemas are supported, and apply rustfmt to the new test. --- crates/tinyagents-harness/src/agent_loop/mod_tests.rs | 7 +++++-- crates/tinyagents-harness/src/agent_loop/tools.rs | 7 ++++--- .../src/middleware/library/arg_recovery.rs | 5 ++++- .../src/middleware/library/arg_recovery_tests.rs | 10 ++++++++++ 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/crates/tinyagents-harness/src/agent_loop/mod_tests.rs b/crates/tinyagents-harness/src/agent_loop/mod_tests.rs index 40d215f43..d8dc41532 100644 --- a/crates/tinyagents-harness/src/agent_loop/mod_tests.rs +++ b/crates/tinyagents-harness/src/agent_loop/mod_tests.rs @@ -3753,9 +3753,12 @@ async fn unrepairable_arguments_are_answered_with_an_actionable_error() { assert!(tool.calls().is_empty()); let transcript = format!("{:?}", run.messages); - assert!(transcript.contains("invalid JSON arguments"), "{transcript}"); assert!( - transcript.contains("Re-send it as one well-formed JSON object"), + transcript.contains("invalid JSON arguments"), + "{transcript}" + ); + assert!( + transcript.contains("matches the tool's argument schema"), "{transcript}" ); } diff --git a/crates/tinyagents-harness/src/agent_loop/tools.rs b/crates/tinyagents-harness/src/agent_loop/tools.rs index b36362a1b..4fae1acc6 100644 --- a/crates/tinyagents-harness/src/agent_loop/tools.rs +++ b/crates/tinyagents-harness/src/agent_loop/tools.rs @@ -2350,9 +2350,10 @@ fn repeated_truncated_tool_call_message(tool_name: &str) -> String { fn unparseable_tool_call_message(tool_name: &str, detail: &str) -> String { format!( "Tool call `{tool_name}` was not run: its arguments are not valid JSON ({detail}). \ - Re-send it as one well-formed JSON object. If the arguments carry a large document, \ - do not inline it again: build it in smaller incremental calls (for example with an \ - editing tool that returns a handle or id) and pass that reference instead." + Re-send it as well-formed JSON that matches the tool's argument schema. If the \ + arguments carry a large document, do not inline it again: build it in smaller \ + incremental calls (for example with an editing tool that returns a handle or id) and \ + pass that reference instead." ) } diff --git a/crates/tinyagents-harness/src/middleware/library/arg_recovery.rs b/crates/tinyagents-harness/src/middleware/library/arg_recovery.rs index fa25e2458..9850e2117 100644 --- a/crates/tinyagents-harness/src/middleware/library/arg_recovery.rs +++ b/crates/tinyagents-harness/src/middleware/library/arg_recovery.rs @@ -68,7 +68,9 @@ impl Middleware<(), C> for ArgRecoveryMiddleware { } // (2) JSON-encoded-string arguments (optionally markdown-fenced): decode - // and adopt the inner object. + // and adopt the inner object. A provider marks a fenced object invalid, + // so the decode also clears that mark; otherwise admission would answer + // the now-valid call with the stale parse error. if let Some(raw) = call.arguments.as_str() && let Some(obj) = recover_object_from_json_string(raw) { @@ -77,6 +79,7 @@ impl Middleware<(), C> for ArgRecoveryMiddleware { "[tinyagents::mw] arg_recovery: decoded JSON-encoded-string tool arguments to object" ); call.arguments = obj; + call.invalid = None; return Ok(()); } diff --git a/crates/tinyagents-harness/src/middleware/library/arg_recovery_tests.rs b/crates/tinyagents-harness/src/middleware/library/arg_recovery_tests.rs index 47994c3ac..a7b0a1f4d 100644 --- a/crates/tinyagents-harness/src/middleware/library/arg_recovery_tests.rs +++ b/crates/tinyagents-harness/src/middleware/library/arg_recovery_tests.rs @@ -107,3 +107,13 @@ async fn provider_invalid_arguments_keep_their_raw_string_for_admission_repair() assert_eq!(call.arguments, json!(raw)); assert!(call.invalid.is_some()); } + +#[tokio::test] +async fn a_provider_invalid_fenced_object_is_decoded_and_cleared() { + let mut ctx: RunContext = RunContext::new(RunConfig::new("mw-test"), ()); + let raw = "```json\n{\"q\": \"x\"}\n```"; + let mut call = ToolCall::invalid("c1", "strict", raw, "expected value"); + mw().before_tool(&mut ctx, &(), &mut call).await.unwrap(); + assert_eq!(call.arguments, json!({"q": "x"})); + assert!(call.invalid.is_none()); +}