feat: 同步 next_monday、任务结束关机、单次领奖卡住与道馆建馆修复 (#2)#1613
Conversation
* feat(Scheduler): 添加 next_monday 运行支持 Co-authored-by: huiyuhang1 <huiyuhang1@users.noreply.github.com> * feat(Wait): 添加任务完毕关闭电脑选项 Co-authored-by: huiyuhang1 <huiyuhang1@users.noreply.github.com> * fix(CollectiveMissions): 修复单次领奖时循环等待卡住 Co-authored-by: huiyuhang1 <huiyuhang1@users.noreply.github.com> * fix(Dokan): 优化道馆建立逻辑,移除周一限制 Co-authored-by: huiyuhang1 <huiyuhang1@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: huiyuhang1 <huiyuhang1@users.noreply.github.com>
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并留下了一些高层次的反馈:
- 在
_wait_close_computer中,调用os.system('shutdown /s /t 1')是特定于 Windows 的,而且在执行该命令后立刻等待到next_run很可能是多余的,或者在关机后根本不会被执行;建议让这一逻辑具备平台感知能力,并在安排关机后立刻返回。 task_delay中关于next_monday的调度逻辑(通过计算today = next_run - timedelta(days=1)然后再计算days_until_monday)有点晦涩,而且可能比较容易出错;建议将这段逻辑重构成一个语义更清晰的小工具函数(例如显式地从next_run计算下一个周一),以提升可读性和可维护性。
面向 AI 代理的提示
请根据这次代码审查中的评论进行修改:
## 总体评论
- 在 `_wait_close_computer` 中,调用 `os.system('shutdown /s /t 1')` 是特定于 Windows 的,而且在执行该命令后立刻等待到 `next_run` 很可能是多余的,或者在关机后根本不会被执行;建议让这一逻辑具备平台感知能力,并在安排关机后立刻返回。
- `task_delay` 中关于 `next_monday` 的调度逻辑(通过计算 `today = next_run - timedelta(days=1)` 然后再计算 `days_until_monday`)有点晦涩,而且可能比较容易出错;建议将这段逻辑重构成一个语义更清晰的小工具函数(例如显式地从 `next_run` 计算下一个周一),以提升可读性和可维护性。
## 单独评论
### 评论 1
<location path="script.py" line_range="441-443" />
<code_context>
self.device.release_during_wait()
return self.wait_until(next_run)
+ def _wait_close_computer(self, next_run: datetime) -> bool:
+ logger.info("Close computer during wait")
+ os.system('shutdown /s /t 1')
+ return self.wait_until(next_run)
+
</code_context>
<issue_to_address>
**issue (bug_risk):** 直接调用 `os.system('shutdown /s /t 1')` 在不同环境和平台上存在风险。
这条命令会在所有运行该代码的平台上执行,包括本地开发环境、CI 或其他非 Windows / 配置错误的环境,并可能强制关闭这些系统。请至少通过平台检查(例如使用 `sys.platform`)对其进行保护,更理想的方式是通过配置或抽象进行控制,使其可以在非生产环境中被禁用或模拟。此外,如果关机命令执行成功,后续的 `wait_until` 调用将永远不会执行,因此在这里依赖它的返回值是有误导性的。
</issue_to_address>帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈改进审查质量。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
_wait_close_computer, callingos.system('shutdown /s /t 1')is Windows-specific and then immediately waiting untilnext_runis likely unnecessary or never reached after shutdown; consider making this platform-aware and returning immediately after scheduling shutdown. - The
next_mondayscheduling logic intask_delay(computingtoday = next_run - timedelta(days=1)and thendays_until_monday) is a bit opaque and could be error-prone; refactoring this into a small helper with clearer semantics (e.g., explicitly computing the next Monday fromnext_run) would improve readability and maintainability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_wait_close_computer`, calling `os.system('shutdown /s /t 1')` is Windows-specific and then immediately waiting until `next_run` is likely unnecessary or never reached after shutdown; consider making this platform-aware and returning immediately after scheduling shutdown.
- The `next_monday` scheduling logic in `task_delay` (computing `today = next_run - timedelta(days=1)` and then `days_until_monday`) is a bit opaque and could be error-prone; refactoring this into a small helper with clearer semantics (e.g., explicitly computing the next Monday from `next_run`) would improve readability and maintainability.
## Individual Comments
### Comment 1
<location path="script.py" line_range="441-443" />
<code_context>
self.device.release_during_wait()
return self.wait_until(next_run)
+ def _wait_close_computer(self, next_run: datetime) -> bool:
+ logger.info("Close computer during wait")
+ os.system('shutdown /s /t 1')
+ return self.wait_until(next_run)
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Calling `os.system('shutdown /s /t 1')` directly is risky across environments and platforms.
This command will run on any platform where this code executes, including local dev, CI, or other non-Windows/misconfigured environments, and may forcibly shut them down. Please at least guard it with a platform check (e.g. via `sys.platform`) and ideally gate it behind configuration/abstraction so it can be disabled or mocked outside production. Also, if the shutdown succeeds, the subsequent `wait_until` call will never execute, so relying on its return value here is misleading.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| def _wait_close_computer(self, next_run: datetime) -> bool: | ||
| logger.info("Close computer during wait") | ||
| os.system('shutdown /s /t 1') |
There was a problem hiding this comment.
issue (bug_risk): 直接调用 os.system('shutdown /s /t 1') 在不同环境和平台上存在风险。
这条命令会在所有运行该代码的平台上执行,包括本地开发环境、CI 或其他非 Windows / 配置错误的环境,并可能强制关闭这些系统。请至少通过平台检查(例如使用 sys.platform)对其进行保护,更理想的方式是通过配置或抽象进行控制,使其可以在非生产环境中被禁用或模拟。此外,如果关机命令执行成功,后续的 wait_until 调用将永远不会执行,因此在这里依赖它的返回值是有误导性的。
Original comment in English
issue (bug_risk): Calling os.system('shutdown /s /t 1') directly is risky across environments and platforms.
This command will run on any platform where this code executes, including local dev, CI, or other non-Windows/misconfigured environments, and may forcibly shut them down. Please at least guard it with a platform check (e.g. via sys.platform) and ideally gate it behind configuration/abstraction so it can be disabled or mocked outside production. Also, if the shutdown succeeds, the subsequent wait_until call will never execute, so relying on its return value here is misleading.
Findings
Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
💡 Tip: tools:
github:
mode: gh-proxySee GitHub Tools for more information on To allow these domains, add them to the network:
allowed:
- defaults
- "api.github.com"
- "github.com"See Network Configuration for more information.
|
Change Summary
Reconstructed Intent点击此处展开- 这次改动最可能是在补齐三类行为:一是让部分任务可以被同步到“下一个周一”再运行;二是让脚本在空闲期支持“任务跑完后直接关机”;三是修复集体任务在只出现单次领奖时的卡死。 - `Dokan` 这部分更像是在放宽既有流程约束:不再把“建馆”限定在周一,而是交给当前场景与权限判断决定是否尝试。Observed Constraints点击此处展开- `next_monday` 依赖现有调度入口最终都会走 `Config.task_delay()`;同时它是在已有 `next_run` 计算完成后再二次改写日期,因此会继承原有 `success_interval` / `target` / `server_update` 的结果。 - 这次为了支持 `next_monday`,顺带把 `target is not None` 纳入了 `server=True` 的“直接追加随机浮动”分支;这会改变现有 `sync_next_run` 这类传入明确目标时间的调用语义,不再走 `parse_tomorrow_server(...)`。 - `close_computer` 当前直接执行 `shutdown /s /t 1`,实现上明显依赖宿主机接受这一命令格式;如果运行环境不是 Windows,行为是否失败、阻塞或静默无效,这里都没有额外兜底。 - 集体任务的领奖修复依赖一个隐含前提:正常双倍场景下第二个奖励弹窗会在 10 秒内出现,否则流程会按“只有一次奖励”提前结束。 - `Dokan` 放开周一限制后,是否应该建馆就完全依赖 `try_start_dokan`、当前场景识别以及 `creat_dokan()` 自身幂等性;外层不再提供日期边界保护。Intent Alignment
Release Risk
Validation Gaps点击此处展开- 需要确认 `sync_next_run` / 手动同步下次运行时间时,带 `server=True` 的目标时间偏移是否是有意设计,而不是为了 `next_monday` 顺手改坏了既有语义。 - 需要至少覆盖一次非双倍与双倍两种 `CollectiveMissions` 场景,确认 10 秒窗口不会把慢弹出的第二个奖励误判为单次奖励。 - 需要验证 `close_computer` 在项目实际支持的宿主环境上的行为,尤其是非 Windows 或模拟器托管场景下的失败模式。 - 需要验证非周一进入 `Dokan` 时,`creat_dokan()` 在“已有馆 / 无权限 / 不可建馆”几类场景下是否会安全退出,而不是反复触发无效尝试。Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
💡 Tip: tools:
github:
mode: gh-proxySee GitHub Tools for more information on To allow these domains, add them to the network:
allowed:
- defaults
- "api.github.com"
- "github.com"See Network Configuration for more information.
|
|
feat(Scheduler): 添加 next_monday 运行支持
feat(Wait): 添加任务完毕关闭电脑选项
fix(CollectiveMissions): 修复单次领奖时循环等待卡住
fix(Dokan): 优化道馆建立逻辑,移除周一限制
由 Sourcery 生成的摘要
新增可选的下周一调度功能、支持任务完成后关闭电脑、修复单次领取任务奖励导致卡住的问题,并放宽 Dokan 创建对工作日的限制。
新功能:
错误修复:
Original summary in English
Summary by Sourcery
Add optional next-Monday scheduling, support shutting down the computer after tasks complete, prevent single-claim mission rewards from hanging, and relax Dokan creation weekday constraints.
New Features:
Bug Fixes: