fix(hsr): 兼容 Config V2 宿主契约 - #4
Merged
Merged
Conversation
Reviewer's GuideHSR 脚本的通知处理已更新，以支持 Config V2 合约，同时保留旧版回退机制；基于 websocket 的错误通知从已移除的 通过 Publisher/task_notice 发送 HSR 错误通知的序列图sequenceDiagram
participant HSRManager as HSRManager
participant Publisher as Publisher
participant Protocol as protocol
participant Schema as WSTaskNoticeData
HSRManager->>Schema: WSTaskNoticeData(level="error", message)
HSRManager->>Publisher: send(id, Protocol.TASK_NOTICE, WSTaskNoticeData)
note over HSRManager,Publisher: Replaces Config.send_websocket_message for error notifications
文件级变更
Tips and commands与 Sourcery 交互
自定义你的体验访问你的 dashboard 以：
获取帮助Original review guide in EnglishReviewer's GuideHSR script notification handling is updated to support Config V2 contracts while retaining legacy fallbacks, and websocket-based error notices are migrated from the removed Config.send_websocket_message helper to the new Publisher/task.notice pipeline, alongside a minor version bump to 0.1.9 for the HSR package and metadata/tests alignment. Sequence diagram for HSR error notices via Publisher/task_noticesequenceDiagram
participant HSRManager as HSRManager
participant Publisher as Publisher
participant Protocol as protocol
participant Schema as WSTaskNoticeData
HSRManager->>Schema: WSTaskNoticeData(level="error", message)
HSRManager->>Publisher: send(id, Protocol.TASK_NOTICE, WSTaskNoticeData)
note over HSRManager,Publisher: Replaces Config.send_websocket_message for error notifications
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并留下了一些总体反馈：
- 建议将新的 Config V2 辅助函数（
_global_notify_value,_global_custom_webhooks）移动到一个共享的 config/notify 工具模块中，这样其他脚本可以复用它们，而不是以后再次添加类似逻辑。 - 在多个异步方法中重复进行内联导入
Publisher、protocol和WSTaskNoticeData，可以考虑提升到模块级别，这样可读性更好，也能避免重复的导入逻辑，除非有充足理由必须保持惰性导入。 - 在
_global_custom_webhooks中，你可能需要返回一个元组或更具体的类型化集合，而不是list[Any]，以更好地反映预期的 webhook 对象结构，并帮助下游的静态分析。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider moving the new Config V2 helper functions (`_global_notify_value`, `_global_custom_webhooks`) into a shared config/notify utility module so other scripts can reuse them instead of re-adding similar logic later.
- The repeated inline imports of `Publisher`, `protocol`, and `WSTaskNoticeData` inside multiple async methods could be hoisted to the module level for readability and to avoid duplicated import logic unless there is a strong reason to keep them lazy.
- In `_global_custom_webhooks`, you might want to return a tuple or a more specific typed collection instead of `list[Any]` to better reflect the expected webhook object shape and help downstream static analysis.
## Individual Comments
### Comment 1
<location path="packages/automas_script_hsr/src/automas_script_hsr/runtime/notify.py" line_range="138" />
<code_context>
+ if _global_notify_value("IfSendMail"):
await Notify.send_mail(
- "网页", title, message_html, Config.get("Notify", "ToAddress")
+ "网页", title, message_html, _global_notify_value("ToAddress", "")
)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using an empty string as default for `ToAddress`/`ServerChanKey` changes behavior when config is missing.
The previous behavior relied on `Config.get` returning `None` (or another sentinel) when a key was missing; this change now substitutes `""`. If notifications are enabled but `ToAddress`/`ServerChanKey` are unset, the code may attempt to send using empty values instead of failing. Please either keep a `None` default and let downstream logic handle it, or add validation to ensure these values are non-empty before sending.
Suggested implementation:
```python
to_address = _global_notify_value("ToAddress")
if _global_notify_value("IfSendMail") and to_address:
await Notify.send_mail(
```
```python
"网页", title, message_html, to_address
```
1. Apply the same pattern for ServerChan (and any other notification channels): fetch the value once, check that it is truthy/non-empty before sending, and pass the validated value to the send function.
2. If `_global_notify_value` currently uses `""` as a default for missing keys, consider changing its default to `None` so downstream code can reliably distinguish between “unset” and “empty string”.
3. Review other call sites where `_global_notify_value("ToAddress", "")` or `_global_notify_value("ServerChanKey", "")` are used and update them to avoid substituting empty strings when configuration values are missing.
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- Consider moving the new Config V2 helper functions (
_global_notify_value,_global_custom_webhooks) into a shared config/notify utility module so other scripts can reuse them instead of re-adding similar logic later. - The repeated inline imports of
Publisher,protocol, andWSTaskNoticeDatainside multiple async methods could be hoisted to the module level for readability and to avoid duplicated import logic unless there is a strong reason to keep them lazy. - In
_global_custom_webhooks, you might want to return a tuple or a more specific typed collection instead oflist[Any]to better reflect the expected webhook object shape and help downstream static analysis.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider moving the new Config V2 helper functions (`_global_notify_value`, `_global_custom_webhooks`) into a shared config/notify utility module so other scripts can reuse them instead of re-adding similar logic later.
- The repeated inline imports of `Publisher`, `protocol`, and `WSTaskNoticeData` inside multiple async methods could be hoisted to the module level for readability and to avoid duplicated import logic unless there is a strong reason to keep them lazy.
- In `_global_custom_webhooks`, you might want to return a tuple or a more specific typed collection instead of `list[Any]` to better reflect the expected webhook object shape and help downstream static analysis.
## Individual Comments
### Comment 1
<location path="packages/automas_script_hsr/src/automas_script_hsr/runtime/notify.py" line_range="138" />
<code_context>
+ if _global_notify_value("IfSendMail"):
await Notify.send_mail(
- "网页", title, message_html, Config.get("Notify", "ToAddress")
+ "网页", title, message_html, _global_notify_value("ToAddress", "")
)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using an empty string as default for `ToAddress`/`ServerChanKey` changes behavior when config is missing.
The previous behavior relied on `Config.get` returning `None` (or another sentinel) when a key was missing; this change now substitutes `""`. If notifications are enabled but `ToAddress`/`ServerChanKey` are unset, the code may attempt to send using empty values instead of failing. Please either keep a `None` default and let downstream logic handle it, or add validation to ensure these values are non-empty before sending.
Suggested implementation:
```python
to_address = _global_notify_value("ToAddress")
if _global_notify_value("IfSendMail") and to_address:
await Notify.send_mail(
```
```python
"网页", title, message_html, to_address
```
1. Apply the same pattern for ServerChan (and any other notification channels): fetch the value once, check that it is truthy/non-empty before sending, and pass the validated value to the send function.
2. If `_global_notify_value` currently uses `""` as a default for missing keys, consider changing its default to `None` so downstream code can reliably distinguish between “unset” and “empty string”.
3. Review other call sites where `_global_notify_value("ToAddress", "")` or `_global_notify_value("ServerChanKey", "")` are used and update them to avoid substituting empty strings when configuration values are missing.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
将 HSR 错误通知从已移除的 Config.send_websocket_message 切到 Publisher/task.notice；通知配置优先读取 Config V2，保留旧宿主回退。核心包版本升级到 0.1.9，适配器与 aggregate 保持 0.1.9，最低依赖仍兼容 core >=0.1.8。验证：全量 unittest 268/268；metadata 6/6；changed modules compileall；git diff --check。构建上传由合并后的官方发布工作流执行。
Summary by Sourcery
适配 HSR 通知处理逻辑以支持 Config V2 主机契约以及新的任务通知 WebSocket 渠道。
新功能：
缺陷修复：
TASK_NOTICEWebSocket 渠道上报，而不是使用已移除的Config.send_websocket_messageAPI。改进：
automas-script-hsr包版本提升至0.1.9，同时保持适配器的核心最低依赖与core 0.1.8兼容。文档：
automas-script-hsr 0.1.9的发布以及相关适配器/核心版本信息。测试：
automas-script-hsr版本，并明确适配器与核心的兼容性预期。Original summary in English
Summary by Sourcery
Adapt HSR notification handling to Config V2 host contracts and new task notice WebSocket channel.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: