fix: parse JSON string messages in send_message_to_user#7978
fix: parse JSON string messages in send_message_to_user#7978whatevertogo wants to merge 2 commits intoAstrBotDevs:masterfrom
Conversation
SendMessageToUser now recovers when providers deliver the messages array as a JSON string, which keeps multiline CronJob messages from failing validation before component construction. Constraint: Keep the compatibility fix local to send_message_to_user for this issue. Rejected: Global recursive tool argument parsing because it has wider schema and string-content risk. Directive: Preserve existing list validation after the JSON-string recovery path. Confidence: high Scope-risk: low Reversibility: Revert the local parsing block and its focused tests. Tested: uv run pytest tests/unit/test_message_tools.py Tested: uv run ruff check astrbot/core/tools/message_tools.py tests/unit/test_message_tools.py Tested: uv run ruff format --check astrbot/core/tools/message_tools.py tests/unit/test_message_tools.py Related: Fixes AstrBotDevs#7961 Co-authored-by: OmX <omx@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request implements a recovery mechanism for the messages parameter in message_tools.py, allowing it to handle JSON-serialized strings produced by certain LLMs. Corresponding unit tests were added to ensure robustness against valid and invalid JSON inputs. Feedback focuses on refining the exception handling by removing a redundant TypeError and improving code style by moving a local import in the test suite to the top of the file to comply with PEP 8.
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="astrbot/core/tools/message_tools.py" line_range="138-142" />
<code_context>
+ # Some LLMs (e.g. MiniMax) may serialize the array value as a JSON string
+ # when the text contains newlines. Try to recover.
+ # https://github.com/AstrBotDevs/AstrBot/issues/7961
+ if isinstance(messages, str):
+ try:
+ messages = json.loads(messages)
+ except (json.JSONDecodeError, TypeError):
+ pass
if not isinstance(messages, list) or not messages:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider simplifying the exception handling to only catch JSONDecodeError.
Because of the preceding `isinstance(messages, str)` check, `json.loads(messages)` should not legitimately raise `TypeError`; the realistic failure is `json.JSONDecodeError`. Restricting the `except` to `json.JSONDecodeError` clarifies the intent and avoids silently hiding unexpected `TypeError`s.
```suggestion
if isinstance(messages, str):
try:
messages = json.loads(messages)
except json.JSONDecodeError:
pass
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR fixes send_message_to_user handling when messages arrives as a JSON-encoded string (not a list) in CronJob-triggered tool calls, restoring compatibility with providers that serialize multiline arrays as strings.
Changes:
- Parse
messageswithjson.loads()whenmessagesis astr, then proceed with existing validation/construction logic. - Add unit tests covering JSON-string message arrays, multiline newline preservation, and invalid JSON-string inputs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
astrbot/core/tools/message_tools.py |
Adds JSON-string parsing fallback for messages before list validation. |
tests/unit/test_message_tools.py |
Adds regression tests for JSON-string messages behavior and newline preservation. |
The review feedback identified two small clarity issues: the TypeError handler was unreachable after the string guard, and the multiline regression test used a local json import. Narrow the exception handling and keep imports at module scope. Constraint: Keep this follow-up limited to PR review comments on AstrBotDevs#7978. Rejected: Broader tool argument normalization because it is unrelated to these review threads. Directive: Keep invalid JSON strings flowing into the existing messages validation error. Confidence: high Scope-risk: low Reversibility: Revert this cleanup commit without changing the original compatibility behavior. Tested: uv run pytest tests/unit/test_message_tools.py Tested: uv run ruff check astrbot/core/tools/message_tools.py tests/unit/test_message_tools.py Tested: uv run ruff format --check astrbot/core/tools/message_tools.py tests/unit/test_message_tools.py Related: PR AstrBotDevs#7978 review Co-authored-by: OmX <omx@users.noreply.github.com>
Fixes #7961
CronJob-triggered
send_message_to_usercalls can receivemessagesas a JSON string when some providers serialize a multiline message array inside the tool arguments. The tool previously rejected that value before building the message chain.Modifications / 改动点
Parse
messageswithjson.loads()when the value arrives as a string, then keep the existing list/object validation path.Add regression coverage for JSON-string message arrays, multiline text preservation, and invalid JSON strings.
This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txtandpyproject.toml. / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到requirements.txt和pyproject.toml文件相应位置。Summary by Sourcery
Handle JSON-string message payloads in send_message_to_user and add regression tests for this behavior.
Bug Fixes:
Tests: