Skip to content

fix(agent): validate tool calls before persisting to prevent session poisoning (rebased)#14407

Closed
Bartok9 wants to merge 1 commit into
NousResearch:mainfrom
Bartok9:fix/validate-tool-calls-v2
Closed

fix(agent): validate tool calls before persisting to prevent session poisoning (rebased)#14407
Bartok9 wants to merge 1 commit into
NousResearch:mainfrom
Bartok9:fix/validate-tool-calls-v2

Conversation

@Bartok9

@Bartok9 Bartok9 commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Problem

Rebased version of #4724.

Malformed tool calls (empty function name, non-JSON arguments) silently enter session history and cause repeated HTTP 400 errors on every subsequent turn when that history is replayed to strict providers. The session becomes permanently broken — requiring /new to recover.

Root Cause

The tool-call persistence loop (_build_assistant_message) did not validate tool calls before appending them to session history.

Fix

Adds _is_valid_tool_call() static method and calls it in the persistence loop. Invalid calls are logged as warnings and skipped:

if not self._is_valid_tool_call(tool_call):
    logging.warning(f"Skipping malformed tool call: name={fn_name!r}...")
    continue

Validation checks:

  • function attribute exists
  • name is a non-empty string
  • arguments (when present) is a non-empty string that parses as a JSON object

Changes

File Change
run_agent.py _is_valid_tool_call() static method + validation in persistence loop

Supersedes #4724.

@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/agent Core agent loop, run_agent.py, prompt builder labels Apr 23, 2026
@Bartok9

Bartok9 commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

The test CI failure is pre-existing on main — the current upstream main commit faa13e49 also has a failing test check. This PR does not introduce that test failure.

…poisoning (v2, rebased)

Malformed tool calls (empty function name, non-JSON arguments) can poison
a session's history and cause repeated HTTP 400 errors on every subsequent
turn when the broken history is replayed to strict providers.

Adds `_is_valid_tool_call()` static method called in the tool-call
persistence loop. Invalid tool calls are skipped with a warning log and
never written to session history.

Fixes NousResearch#4714
@Bartok9

Bartok9 commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on current main (faa13e49) and resolved the assistant tool-call conflict by preserving upstream's assistant_tool_calls path while keeping the malformed-call validation before persistence. Ready for review.

@Bartok9 Bartok9 force-pushed the fix/validate-tool-calls-v2 branch from 4649275 to d35b9d8 Compare May 8, 2026 05:59
@Bartok9

Bartok9 commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

Test CI failure is pre-existing on main — verified at main faa13e49. This PR does not introduce the test failure.

@Bartok9

Bartok9 commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

The test CI failure is pre-existing on main — verified at main SHA 1997b3ba. Not introduced by this PR.

@Bartok9

Bartok9 commented May 9, 2026

Copy link
Copy Markdown
Contributor Author

The test CI failure is pre-existing on main — verified at main SHA 524cbabd. Not introduced by this PR.

@Bartok9

Bartok9 commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

Closing — this fix has landed on main via a subsequent PR or is superseded. Thanks for the review.

@Bartok9 Bartok9 closed this May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent loop, run_agent.py, prompt builder 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.

2 participants