Skip to content

fix(agents): recover whitespace-prefixed tool-call JSON arguments - #5841

Merged
qbc2016 merged 1 commit into
agentscope-ai:mainfrom
VectorPeak:fix/tool-message-raw-decode-whitespace
Jul 9, 2026
Merged

qbc2016 merged 1 commit into
agentscope-ai:mainfrom
VectorPeak:fix/tool-message-raw-decode-whitespace

Conversation

@VectorPeak

@VectorPeak VectorPeak commented Jul 7, 2026 •

Copy link
Copy Markdown
Contributor

Description

Recover tool-call JSON arguments when stored input or raw_input starts with JSON whitespace and then contains a valid JSON object followed by trailing garbage.

What Problem This Solves

tool_message_utils already has two fallback paths for malformed-but-recoverable tool-call arguments:

  • _repair_empty_tool_inputs() repairs an empty input field from raw_input
  • _coerce_tool_inputs_to_json() recovers a leading JSON object before dropping malformed trailing bytes

Both paths first try strict json.loads(). That is the right first step for clean JSON, but it rejects inputs with extra trailing bytes. When that happens, the code falls back to JSONDecoder.raw_decode(...) so it can recover the first valid JSON object.

The missing edge case is leading JSON whitespace. json.loads() accepts leading whitespace, but raw_decode(...) starts parsing at the index it is given. Before this PR, both fallback paths passed index 0, so an otherwise recoverable payload like this was not recovered:

\n  {"path": "README.md"}trailing garbage

In practice, that means a recoverable tool-call argument can be left empty or dropped before the next provider request. For users, the visible effect is a tool call that should still have arguments, but is normalized as if the arguments were missing or unrecoverable.

Why This Change Was Made

This mirrors the JSON recovery boundary that was accepted in #5766 for safe_json_loads: strict parsing is tried first, then the fallback starts from the first JSON whitespace-adjusted document index before recovering the leading object.

The fix belongs in qwenpaw.agents.utils.tool_message_utils because this module owns the tool-call repair/coercion step before messages are submitted to model providers. Keeping the repair here makes historical sessions, streamed tool-call artifacts, and provider request normalization follow the same recovery rule.

Narrow Change

The implementation intentionally stays small and conservative:

  • it only changes the raw_decode start index in the two existing recovery paths
  • it does not accept arbitrary leading non-whitespace garbage
  • it does not convert recovered scalar/list values into tool arguments
  • unrecoverable malformed payloads are still dropped or left unchanged according to the existing path
  • normal JSON strings continue through the existing json.loads() path

Evidence

The regression tests cover both affected fallback paths:

_repair_empty_tool_inputs:
raw_input='\n  {"path": "README.md"}trailing garbage'
_coerce_tool_inputs_to_json:
input='\n  {"path": "README.md"}extra'

After this change, both paths recover the leading object as:

{"path": "README.md"}

Existing tests still cover the unchanged behavior for ordinary trailing-garbage recovery, completely invalid JSON, and non-dict recovered values.

Possible Call Chain / Impact

historical session or streamed tool-call artifact
  -> normalize_messages_for_model_request(...)
  -> _sanitize_tool_messages(...)
  -> _repair_empty_tool_inputs(...) / _coerce_tool_inputs_to_json(...)
  -> json.loads(...) fails on trailing garbage
  -> raw_decode fallback previously starts at index 0
  -> leading whitespace causes recovery to fail
  -> tool arguments remain empty or the tool-call block is dropped

This PR only changes the fallback start index for JSON whitespace. It does not broaden recovery to arbitrary leading garbage, scalar/list payloads, or otherwise unrecoverable malformed tool-call inputs.

Related Issue: Relates to #5766

Security Considerations: No new tool execution surface or permission behavior. This only normalizes already-recorded tool-call argument strings before provider submission; unrecoverable or non-object payloads continue to be rejected/dropped.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Refactoring

Component(s) Affected

  • Core / Backend (app, agents, config, providers, utils, local_models)
  • Console (frontend web UI)
  • Channels (DingTalk, Lark, QQ, Discord, iMessage, etc.)
  • Skills
  • CLI
  • Documentation (website)
  • Tests
  • CI/CD
  • Scripts / Deploy

Checklist

  • I ran pre-commit run --all-files locally and it passes
  • If pre-commit auto-fixed files, I committed those changes and reran checks
  • I ran tests locally (pytest or as relevant) and they pass
  • Documentation updated (if needed)
  • Ready for review

For Channel Changes (DingTalk, Lark, QQ, Console, etc.)

  • I ran ./scripts/check-channels.sh (or ./scripts/check-channels.sh --changed) and it passes
  • Contract test exists in tests/contract/channels/test_<channel>_contract.py (REQUIRED)
  • Contract test implements create_instance() with proper channel initialization
  • All 19 contract verification points pass (see tests/contract/channels/__init__.py)
  • Optional: Unit tests in tests/unit/channels/test_<channel>.py for complex internal logic

Testing

The new regression tests cover both recovery paths:

  • _repair_empty_tool_inputs recovers raw_input='\n {"path": "README.md"}trailing garbage'
  • _coerce_tool_inputs_to_json recovers input='\n {"path": "README.md"}extra'

Existing tests continue to cover the unchanged behavior for normal trailing-garbage recovery, completely invalid JSON, and non-dict recovered values.

Local Verification Evidence

D:\ZXY\Dev\Miniforge3\condabin\mamba.bat run -n prw-qwenpaw-py3.11 pytest tests/unit/agents/utils/test_tool_message_utils.py tests/unit/agents/test_message_request_normalizer.py -q
# 72 passed

git diff --check
# passed

D:\ZXY\Dev\Miniforge3\condabin\mamba.bat run -n prw-qwenpaw-py3.11 pre-commit run --files src/qwenpaw/agents/utils/tool_message_utils.py tests/unit/agents/utils/test_tool_message_utils.py
# passed

pre-commit run --all-files has not been run locally for this PR.

Channel-specific checklist items are not applicable because this PR does not change any channel implementation or contract surface.

Additional Notes

Possible call chain / impact:

historical session or streamed tool-call artifact
  -> normalize_messages_for_model_request(...)
  -> _sanitize_tool_messages(...)
  -> _repair_empty_tool_inputs(...) / _coerce_tool_inputs_to_json(...)
  -> json.loads(...) fails on trailing garbage
  -> raw_decode fallback previously starts at index 0
  -> leading whitespace causes recovery to fail
  -> tool arguments remain empty or the tool-call block is dropped

This PR only changes the fallback start index for JSON whitespace. It does not broaden recovery to arbitrary leading garbage, scalar/list payloads, or otherwise unrecoverable malformed tool-call inputs.

Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Welcome to QwenPaw! 🐾

Hi @VectorPeak, this is your 3rd Pull Request.

🙌 Join Developer Community

Thanks so much for your contribution! We'd love to invite you to join the official QwenPaw developer group! You can find the Discord and DingTalk group links under the "Developer Community" section on our docs page:
https://qwenpaw.agentscope.io/docs/community

We truly appreciate your enthusiasm—and look forward to your future contributions! 😊

We'll review your PR soon.

@lalaliat
lalaliat requested a review from qbc2016 July 8, 2026 02:34

@qbc2016 qbc2016 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@qbc2016
qbc2016 merged commit 90d8cb8 into agentscope-ai:main Jul 9, 2026
22 of 23 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in QwenPaw Jul 9, 2026

This branch had an error being deployed

1 failed and 1 inactive deployments
maintainer-approved — 895d93ff Deployed Jul 7, 2026 by VectorPeak via Maintainer Approval #5892
ai-review-approved — 895d93ff Deployed Jul 7, 2026 by VectorPeak via AI Review Approval #66
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants