Fix: surface actionable errors for truncated tool-call arguments

[vincenzopalazzo]

Summary

• When a model (e.g. glm-5.2 via custom_zai) hits its output-token limit mid-tool-call, the resulting truncated JSON was either silently dropped (pure data loss in the Anthropic streaming path) or silently repaired by safely_parse_json — invoking the tool with truncated content (e.g. writing a half-finished file). Both cases produced a cryptic -32602: Could not parse tool arguments error with no actionable guidance.
• This adds parse_tool_arguments() — a tiered parser that tries strict JSON first, falls back to safely_parse_json for non-truncated malformation (e.g. unescaped control characters, which models commonly emit), and returns None for truncation so callers surface an error instead of silent data loss.
• truncation_error_message() produces actionable guidance distinguishing truncation from malformation, with a character count and snippet of where parsing broke: "The model's response was truncated — it hit the output token limit... Try increasing max_tokens or breaking the task into smaller steps."
• Anthropic streaming path gets a post-loop guard that catches tool blocks which never received content_block_stop (the realistic truncation scenario where the model is cut off mid-tool-call).
• Agent-layer message now passes through the provider's actionable error text instead of a generic canned string.

Review Notes

• Production-safety review passed (2 rounds). A critical regression was caught in round 1: the initial implementation used strict serde_json::from_str only, which broke safely_parse_json's control-character repair that some models rely on. Fixed with the tiered parse_tool_arguments approach.
• The Anthropic provider always streams (non-streaming returns an error), so the streaming path is the only one custom_zai/GLM uses.
• Pre-existing test failure databricks::test_create_request_enabled_thinking_budget_tracks_effort confirmed on clean main, unrelated to this change.

Decision Log

Hardest decision: Whether to let safely_parse_json repair truncated JSON (closing open strings/braces to produce syntactically valid JSON) or block it. Allowing repair means goose silently invokes the tool with truncated content — e.g. a write tool call that writes half a file with no error. Blocking it means some edge cases that could have been salvaged now produce an error. I chose to block truncation repair entirely (looks_truncated check in parse_tool_arguments), because silent data loss is strictly worse than a visible error the user can act on.

Alternatives rejected:

• Option A (check finish_reason/stop_reason at parse time): Not feasible for the Anthropic streaming path — stop_reason arrives in message_delta, which fires after content_block_stop (where tool args are parsed). Would require buffering the entire turn.
• Option C (auto-retry with continuation): Requires re-injecting partial assistant messages with provider-specific "continue" semantics. Zhipu, OpenAI, and Anthropic all behave differently, and it risks corrupting multi-turn conversation state. Not worth the complexity for a user who just wants a clear error to act on.

Least confident about: The looks_truncated heuristic (!ends_with('}') && !ends_with(']')). It correctly catches mid-content truncation but also classifies some genuinely malformed (non-truncated) JSON as "truncated" — e.g. invalid json {. This is acceptable because the guidance (resend/break-up) is reasonable in both cases, but a more precise heuristic (e.g. tracking open/close brace depth) could distinguish them better. Happy to revisit if reviewers prefer.

Test plan

• CI passes
• test_streaming_truncated_tool_args_in_content_block_stop — truncated JSON in a closed tool block produces actionable error
• test_streaming_truncated_tool_args_no_content_block_stop — open tool block at stream end produces actionable error (the silent-drop case)
• test_streaming_complete_tool_call_unaffected — normal tool calls still work (regression guard)
• test_parse_tool_arguments_control_char_recovery — unescaped control chars still parse (regression guard)
• test_parse_tool_arguments_truncated_fails — truncated JSON is NOT silently repaired

Fixes #8272

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 27a1922a59

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[SynthLuvr]

Ah I think this will fix the issue reported in #8272

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 870f864a94

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Don't bypass truncation guard for empty streamed args

When an OpenAI-compatible stream ends with finish_reason: "length" before any argument deltas arrive, arguments is still empty and this branch turns the truncated call into a successful {} tool request. The collector above stops on any finish reason but doesn't retain that it was length, so a required-argument tool can still be executed with empty parameters instead of surfacing the new truncation error. Please track the final finish reason and treat empty arguments from a length-limited tool call as a parse error rather than a valid no-arg call.

Useful? React with 👍 / 👎.

[DOsinga]

Nice fix — surfacing an actionable error instead of silently dropping or repairing truncated tool-call arguments is a real improvement, and the looks_truncated delimiter-balance scan correctly handles the nested-closer case codex flagged.

I pushed a commit to your branch that:

• merges main to refresh Cargo.lock (the MSRV check was failing on a stale lock)
• fixes a clippy lint the agent.rs change introduced (is_ok() + unwrap_err() → if let Err)
• trims some of the more verbose comments down to the "why"

Both codex P1s look properly addressed. LGTM.

[vincenzopalazzo]

Thanks!