feat: support nested forward node and configurable api_root for aiocqhttp#7438
Open
dongmingzjj wants to merge 1 commit intoAstrBotDevs:masterfrom
Open
feat: support nested forward node and configurable api_root for aiocqhttp#7438dongmingzjj wants to merge 1 commit intoAstrBotDevs:masterfrom
dongmingzjj wants to merge 1 commit intoAstrBotDevs:masterfrom
Conversation
…http - Add recursive node parsing in message_parts_helper to support nested forward messages (node within node content) - Add api_root config option to aiocqhttp platform adapter, allowing call_action (e.g. send_private_forward_msg) to use NapCat HTTP API instead of only reverse WebSocket
dosubot
bot
added
the
size:L
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
_parse_node_content, thestrictandverify_media_path_existsparameters are passed through but never actually used to validate or error on bad media paths or unsupported inner types, which may surprise callers expecting strict behavior; consider either enforcing them or removing the parameters from this helper. - Nested
nodecontent currently silently drops any part types other thanplain,image,record, andnode; if other types are possible in forward content, consider at least logging or surfacing an error instrictmode to make debugging easier. - The logic for handling
nodeparts in bothparse_webchat_message_partsandwebchat_message_parts_to_message_chainis nearly identical; you could factor this into a shared helper to reduce duplication and keep behavior consistent if it changes later.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_parse_node_content`, the `strict` and `verify_media_path_exists` parameters are passed through but never actually used to validate or error on bad media paths or unsupported inner types, which may surprise callers expecting strict behavior; consider either enforcing them or removing the parameters from this helper.
- Nested `node` content currently silently drops any part types other than `plain`, `image`, `record`, and `node`; if other types are possible in forward content, consider at least logging or surfacing an error in `strict` mode to make debugging easier.
- The logic for handling `node` parts in both `parse_webchat_message_parts` and `webchat_message_parts_to_message_chain` is nearly identical; you could factor this into a shared helper to reduce duplication and keep behavior consistent if it changes later.
## Individual Comments
### Comment 1
<location path="astrbot/core/platform/sources/webchat/message_parts_helper.py" line_range="46-55" />
<code_context>
+def _parse_node_content(
</code_context>
<issue_to_address>
**issue (bug_risk):** The `strict` and `verify_media_path_exists` parameters are unused here, so nested media inside nodes bypass strict validation and path checks.
In `_parse_node_content`, `strict` and `verify_media_path_exists` are threaded through but never used to control behavior, so media inside node content skips strict validation and path checks that top-level media gets. Please either apply the same validation helpers here or remove these parameters to avoid implying nested media is validated when it isn’t.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+46
to
+55
| def _parse_node_content( | ||
| content_parts: list[dict], | ||
| *, | ||
| strict: bool = False, | ||
| verify_media_path_exists: bool = True, | ||
| ) -> list: | ||
| """递归解析 node 的 content 为 AstrBot 消息组件列表。""" | ||
| inner_components = [] | ||
| for nc in content_parts: | ||
| nc_type = str(nc.get("type", "plain")) |
Contributor
There was a problem hiding this comment.
issue (bug_risk): The strict and verify_media_path_exists parameters are unused here, so nested media inside nodes bypass strict validation and path checks.
In _parse_node_content, strict and verify_media_path_exists are threaded through but never used to control behavior, so media inside node content skips strict validation and path checks that top-level media gets. Please either apply the same validation helpers here or remove these parameters to avoid implying nested media is validated when it isn’t.
dosubot
bot
added
the
area:platform
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds support for the NapCat HTTP API in the aiocqhttp adapter and implements handling for message nodes in the webchat platform, including recursive parsing and node merging. Review feedback identifies a NameError in webchat_message_parts_to_message_chain due to an undefined variable. Furthermore, the _parse_node_content function needs refinement to handle additional media types, implement strict validation, and use more specific type hints.
Comment on lines
+358
to
+362
| _append_node_to_components( | ||
| components, part, | ||
| strict=strict, | ||
| verify_media_path_exists=verify_media_path_exists, | ||
| ) |
Contributor
There was a problem hiding this comment.
The variable verify_media_path_exists is not defined in the scope of webchat_message_parts_to_message_chain. This will cause a NameError when a message part of type node is encountered. Since this function does not have a verify_media_path_exists parameter and generally expects files to exist, you should remove this argument to use the default value (True) of _append_node_to_components.
_append_node_to_components(
components, part,
strict=strict,
)
Comment on lines
+46
to
+84
| def _parse_node_content( | ||
| content_parts: list[dict], | ||
| *, | ||
| strict: bool = False, | ||
| verify_media_path_exists: bool = True, | ||
| ) -> list: | ||
| """递归解析 node 的 content 为 AstrBot 消息组件列表。""" | ||
| inner_components = [] | ||
| for nc in content_parts: | ||
| nc_type = str(nc.get("type", "plain")) | ||
| if nc_type == "plain": | ||
| inner_components.append(Plain(text=str(nc.get("text", "")))) | ||
| elif nc_type == "image": | ||
| nc_src = nc.get("path") or nc.get("url") or nc.get("file", "") | ||
| if nc_src.startswith("http"): | ||
| inner_components.append(Image.fromURL(nc_src)) | ||
| elif nc_src: | ||
| inner_components.append(Image.fromFileSystem(nc_src)) | ||
| elif nc_type == "record": | ||
| nc_src = nc.get("path") or nc.get("url") or nc.get("file", "") | ||
| if nc_src.startswith("http"): | ||
| inner_components.append(Record(url=nc_src)) | ||
| elif nc_src: | ||
| inner_components.append(Record.fromFileSystem(nc_src)) | ||
| elif nc_type == "node": | ||
| # 递归处理嵌套的 node | ||
| nested_content = _parse_node_content( | ||
| nc.get("content", []), | ||
| strict=strict, | ||
| verify_media_path_exists=verify_media_path_exists, | ||
| ) | ||
| inner_components.append( | ||
| Node( | ||
| content=nested_content, | ||
| uin=str(nc.get("uin", "0")), | ||
| name=str(nc.get("name", "")), | ||
| ) | ||
| ) | ||
| return inner_components |
Contributor
There was a problem hiding this comment.
The _parse_node_content function is incomplete and ignores some parameters:
- Missing validation: It accepts
verify_media_path_existsandstrictbut doesn't use them to check if local files exist forimageandrecordtypes. It should verify file existence if the flag is set. - Missing types: It does not handle
videoandfiletypes, which are part ofMEDIA_PART_TYPESand can be present in forward message nodes. - Strict mode: It should raise a
ValueErrorif a media part is missing its source (path/url/file) whenstrictis enabled, consistent with the logic inparse_webchat_message_parts. - Type Hinting: The return type hint
listis too broad; it should ideally belist[BaseMessageComponent].
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 AI-Assisted
This PR was created with the assistance of AI (Hermes Agent).
Summary
This PR adds support for nested forward message nodes and a configurable
api_rootoption for the aiocqhttp platform adapter.Changes
Nested forward node support (
message_parts_helper.py)nodetype messages, allowing nodes to contain other nodes in theircontentfieldnodeparts are automatically merged into a singleNodescomponentplain,image,record, and nestednodetypes within forward message contentConfigurable
api_rootfor aiocqhttp (aiocqhttp_platform_adapter.py)api_rootplatform config option, allowingcall_actionAPI calls (e.g.send_private_forward_msg) to route through NapCat's HTTP server instead of relying solely on reverse WebSocketApiNotAvailableerrors when NapCat only has reverse WS configuredMotivation
When using NapCat as the OneBot 11 implementation with AstrBot,
call_actionrequests sent via reverse WebSocket would fail withApiNotAvailable. Additionally, the message parser did not support nested forward nodes (a forward message containing another forward message), which is useful for chat history reconstruction and threaded conversations.Tested
POST /api/v1/im/messagewith NapCat HTTP API enabled