Skip to content

fix: sanitize broken tool_call arguments before storing in message history - #14518

Closed
guigui0711 wants to merge 1 commit into
NousResearch:mainfrom
guigui0711:fix/sanitize-recovery-tool-call-args
Closed

fix: sanitize broken tool_call arguments before storing in message history#14518
guigui0711 wants to merge 1 commit into
NousResearch:mainfrom
guigui0711:fix/sanitize-recovery-tool-call-args

Conversation

@guigui0711

Copy link
Copy Markdown

Problem

When a model generates invalid JSON in tool call arguments 3 times in a row, the recovery path (run_agent.py:11186) appends the assistant message with its broken tool_calls directly into the message history:

# Before this fix:
recovery_assistant = self._build_assistant_message(assistant_message, finish_reason)
messages.append(recovery_assistant)  # broken JSON stored permanently

These poisoned entries then cause HTTP 400 "Invalid JSON format in tool call arguments" errors — especially after context compression brings them back into the API request. Strict APIs like GitHub Copilot (api.githubcopilot.com) validate ALL historical tool_calls and reject the entire request.

Root Cause

The existing _repair_tool_call_arguments() function (introduced recently) already sanitizes tool call arguments at read time (when building api_messages for the API call). However, the recovery code path writes broken arguments into the message history before that sanitization runs.

This means:

  1. Broken JSON persists in session state (.db files)
  2. Every subsequent API call must repair the same broken entries
  3. If _repair_tool_call_arguments() can't fix an edge case, the session is permanently poisoned

Fix

Reuse the existing _repair_tool_call_arguments() helper to sanitize arguments at write time — when storing the recovery message — so broken JSON never enters the message history.

recovery_assistant = self._build_assistant_message(assistant_message, finish_reason)
for tc_dict in recovery_assistant.get("tool_calls") or []:
    raw = tc_dict.get("function", {}).get("arguments", "")
    try:
        json.loads(raw)
    except (json.JSONDecodeError, TypeError):
        tc_dict["function"]["arguments"] = _repair_tool_call_arguments(
            raw, tc_dict.get("function", {}).get("name", "?"),
        )
messages.append(recovery_assistant)

Impact

  • 11 lines changed in run_agent.py
  • No new dependencies
  • Defense-in-depth: fixes data at the source rather than patching it on every read
  • Especially helps users on strict API providers (GitHub Copilot, Mistral, Fireworks)

🤖 Generated with Claude Code

…story

The invalid-JSON recovery path (after 3 retries) appends the assistant
message with its broken tool_calls directly into the message history.
Strict APIs like GitHub Copilot validate ALL historical tool_calls and
return HTTP 400 "Invalid JSON format in tool call arguments" when they
encounter these poisoned entries — especially after context compression
brings them back into the API request.

Reuse the existing _repair_tool_call_arguments() helper to fix malformed
arguments at write time (when stored) rather than only at read time
(when sent to the API), preventing the bad data from persisting in
session state.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels Apr 23, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #4662 (session poisoning from malformed tool calls) and #14455 (sanitize malformed tool_calls in outbound messages). This PR addresses the write-time gap in the recovery code path at run_agent.py:11186.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the thorough write-up and the clean fix, @guigui0711! This exact gap was addressed by PR #15348 (merged as 7a192b124, cross-referenced by @alt-glitch in the timeline).

This is an automated hermes-sweeper review.

Why this is covered:

  • AIAgent._sanitize_tool_call_arguments() was added at run_agent.py:7821 and is called at line 9548 on the persistent messages list (not just the ephemeral api_messages copy) at the top of every outer loop iteration.
  • When the recovery path at line 11712 appends a broken assistant message and calls continue, the very next loop iteration repairs it in-place before any API call — so broken JSON never reaches strict providers like GitHub Copilot.
  • The repair also persists into session state (.db files) because it mutates the same messages reference used for storage, addressing the poisoned-session concern.

One minor caveat (noted for future reference): the fix in main repairs broken entries on the next iteration rather than preventing them from entering messages at all. Your proposed approach of sanitizing at the write site is slightly cleaner as defense-in-depth. If you'd like to land that as a follow-up micro-improvement on top of the existing fix, a new PR narrowly scoped to the recovery append site at line 11711 would be a good candidate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P1 High — major feature broken, no workaround type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants