fix(SRC): 增强SRC的日志匹配要求并调用SRC内置模块进行进程清理, Fixes #337 - #346
Open
HarcoChen wants to merge 3 commits into
Open
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's Guide此 PR 收紧了 SRC 日志匹配用于检测人工接管,并在杀死进程后增加了一个清理步骤,调用 SRC 自带的 Python 工具包来清理残留的 SRC Python 进程。 set_src 中 SRC 进程清理的时序图sequenceDiagram
participant AutoProxy
participant SrcProcessManager as src_process_manager
participant System
participant ProcessRunner
AutoProxy->>SrcProcessManager: kill
AutoProxy->>System: kill_process(src_exe_path)
AutoProxy->>ProcessRunner: run_process(toolkit/python.exe, -m, deploy.Windows.alas, cwd=src_root_path)
check_log 中收紧日志匹配的时序图sequenceDiagram
participant AutoProxy
AutoProxy->>AutoProxy: check_log(log_content, latest_time)
alt [any " | CRITICAL | Request human takeover" in log_content]
AutoProxy->>AutoProxy: set cur_user_log.status = "SRC 无法继续执行任务, 需要用户接管"
else ["Close game during wait" in log]
AutoProxy->>AutoProxy: set cur_user_log.status = "Success!"
end
文件级变更
可能关联的问题
Tips and commands与 Sourcery 交互
自定义你的使用体验访问你的仪表盘以:
获取帮助Original review guide in EnglishReviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR tightens SRC log matching for human takeover detection and adds a post-kill cleanup step that invokes SRC’s bundled Python toolkit to clean residual SRC Python processes. Sequence diagram for SRC process cleanup in set_srcsequenceDiagram
participant AutoProxy
participant SrcProcessManager as src_process_manager
participant System
participant ProcessRunner
AutoProxy->>SrcProcessManager: kill
AutoProxy->>System: kill_process(src_exe_path)
AutoProxy->>ProcessRunner: run_process(toolkit/python.exe, -m, deploy.Windows.alas, cwd=src_root_path)
Sequence diagram for tightened log matching in check_logsequenceDiagram
participant AutoProxy
AutoProxy->>AutoProxy: check_log(log_content, latest_time)
alt [any " | CRITICAL | Request human takeover" in log_content]
AutoProxy->>AutoProxy: set cur_user_log.status = "SRC 无法继续执行任务, 需要用户接管"
else ["Close game during wait" in log]
AutoProxy->>AutoProxy: set cur_user_log.status = "Success!"
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - 我在这里给出了一些宏观层面的反馈:
- 新的
ProcessRunner.run_process调用假设toolkit/python.exe和deploy.Windows.alas模块总是存在;建议添加错误处理或预检查,这样当这个清理步骤失败时,可以清晰地暴露问题,而不是在不知不觉中破坏整个安装流程。 - 更新后的人工接管日志匹配条件依赖于对
" | CRITICAL | Request human takeover"的硬编码子串搜索;建议使用更结构化的方式(例如分别解析日志级别和消息,或者使用正则表达式进行匹配),以避免在日志格式稍有变化时变得脆弱。
给 AI 代理的提示
请根据这次代码评审的评论进行修改:
## 整体评论
- 新的 `ProcessRunner.run_process` 调用假设 `toolkit/python.exe` 和 `deploy.Windows.alas` 模块总是存在;建议添加错误处理或预检查,这样当这个清理步骤失败时,可以清晰地暴露问题,而不是在不知不觉中破坏整个安装流程。
- 更新后的人工接管日志匹配条件依赖于对 `" | CRITICAL | Request human takeover"` 的硬编码子串搜索;建议使用更结构化的方式(例如分别解析日志级别和消息,或者使用正则表达式进行匹配),以避免在日志格式稍有变化时变得脆弱。帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've left some high level feedback:
- The new
ProcessRunner.run_processinvocation assumestoolkit/python.exeand thedeploy.Windows.alasmodule always exist; consider adding error handling or a pre-check so failures in this cleanup step are surfaced clearly rather than silently breaking the setup flow. - The updated log matching condition for human takeover relies on a hardcoded substring search for
" | CRITICAL | Request human takeover"; consider using a more structured approach (e.g., parsing log level and message separately or matching against a regex) to avoid brittleness if the log format changes slightly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `ProcessRunner.run_process` invocation assumes `toolkit/python.exe` and the `deploy.Windows.alas` module always exist; consider adding error handling or a pre-check so failures in this cleanup step are surfaced clearly rather than silently breaking the setup flow.
- The updated log matching condition for human takeover relies on a hardcoded substring search for `" | CRITICAL | Request human takeover"`; consider using a more structured approach (e.g., parsing log level and message separately or matching against a regex) to avoid brittleness if the log format changes slightly.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
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.
Fixes #337
| CRITICAL | Request human takeover以避免traceback错误命中Summary by Sourcery
收紧 SRC 日志匹配规则用于人工接管检测,并在终止 SRC 进程后调用 SRC 内置清理模块。
Bug Fixes:
Enhancements:
src.exe和相关脚本进程后,运行 SRC 工具包的 Python 模块,以清理残留的 SRC Python 进程。Original summary in English
Summary by Sourcery
Tighten SRC log matching for human takeover detection and invoke SRC’s built-in cleanup module after terminating SRC processes.
Bug Fixes:
Enhancements: