feat: 支持自更新 & 使用目录打包形式#39
Open
ShadowLemoon wants to merge 21 commits intomainfrom
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough此PR重组PyInstaller打包结构，将资源统一迁移到 ChangesPyInstaller 构建与运行时组织
YAML 配置框架
下载与更新基础设施
集成与UI
Sequence DiagramssequenceDiagram
participant User as 用户界面
participant Checker as GithubUpdateChecker<br/>(QThread)
participant Service as GithubUpdateService
participant GitHub as GitHub API
participant Card as GithubUpdateCard
User->>Card: 点击检查更新
Card->>Checker: 启动线程
activate Checker
Checker->>Service: get_latest_tags()
Service->>GitHub: GET /releases/latest
GitHub-->>Service: stable tag
Service->>GitHub: GET /releases?per_page=30
GitHub-->>Service: beta tag (or empty)
Service-->>Checker: (stable_tag, beta_tag)
Checker-->>Card: 发射 tags_updated 信号
deactivate Checker
Card->>Card: 更新版本显示<br/>(无版本/已最新/可更新)
Card-->>User: 刷新按钮状态
sequenceDiagram
participant User as 用户界面
participant Runner as GithubUpdateRunner<br/>(QThread)
participant Service as GithubUpdateService
participant Downloader as ZipDownloader
participant Http as http_utils
participant GitHub as GitHub<br/>Release
participant PS as PowerShell
User->>Card: 点击更新按钮
Card->>Runner: 启动线程 (with progress_signal)
activate Runner
Runner->>Service: download_and_restart(target_tag, progress_callback)
Service->>Downloader: download(skip_if_existed=False)
activate Downloader
Downloader->>Http: download_file(release_url)
Http->>GitHub: GET release.zip
GitHub-->>Http: 流式传输 (with progress)
Http->>Http: 本地保存并验证
Http-->>Downloader: True/False
Downloader->>Downloader: unzip() 至 staging
Downloader-->>Service: True/False
deactivate Downloader
Service->>Service: 生成 apply_update.ps1
Service->>PS: 执行 PowerShell 脚本<br/>(进程替换、依赖更新)
PS->>PS: 等待当前进程 → 备份旧文件<br/> → 移动新文件 → 重启Python
Runner->>Card: 发射 update_finished
deactivate Runner
Card->>User: 应用退出或显示完成
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 此PR涉及多个独立但相互关联的功能层：
变化异质性高，各文件需独立推理，但在资源路径和缓存策略上存在一致性主题。 Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/one_dragon_qt/app/installer.py (1)
28-30: 这里的图标设置已经和父类重复了。
src/one_dragon_qt/windows/app_window_base.py:27-29已经处理过app_icon。继续在子类保留这段，会让安装器窗口的初始化逻辑继续双轨维护，后面很容易出现父子类行为不一致。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/one_dragon_qt/app/installer.py` around lines 28 - 30, The installer subclass duplicates icon setup already done in the parent (see app_window_base.py handling of app_icon); remove the duplicated block in src/one_dragon_qt/app/installer.py that calls os_utils.get_resource_path(...) and self.setWindowIcon(QIcon(...)) so the child no longer sets app_icon itself and instead inherits the parent's behavior (leave any other installer init intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/one_dragon/utils/yolo_config_utils.py`:
- Around line 8-9: get_model_category_dir currently returns a resource
(read-only) path via get_resource_path which is then passed to OnnxModelLoader
as the download/write directory; change the API to separate read vs write paths:
keep a read helper (e.g. get_builtin_model_category_dir using get_resource_path)
for loading bundled models and add/keep a write helper (e.g.
get_user_model_category_dir using get_path_under_work_dir) for
downloads/updates, and update calls in yolo_model_card.py that pass
get_model_category_dir to OnnxModelLoader to use the user/write helper instead
while retaining the built-in/read helper where models are only read.
---
Nitpick comments:
In `@src/one_dragon_qt/app/installer.py`:
- Around line 28-30: The installer subclass duplicates icon setup already done
in the parent (see app_window_base.py handling of app_icon); remove the
duplicated block in src/one_dragon_qt/app/installer.py that calls
os_utils.get_resource_path(...) and self.setWindowIcon(QIcon(...)) so the child
no longer sets app_icon itself and instead inherits the parent's behavior (leave
any other installer init intact).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bc89d64a-7000-461c-a309-21ef58d9a7a2
📒 Files selected for processing (7)
deploy/OneDragon ScriptChainer.specsrc/one_dragon/utils/i18_utils.pysrc/one_dragon/utils/os_utils.pysrc/one_dragon/utils/yolo_config_utils.pysrc/one_dragon_qt/app/installer.pysrc/one_dragon_qt/view/like_interface.pysrc/one_dragon_qt/windows/app_window_base.py
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/build-release.yml (2)
123-123: 建议在复制 Runner 前增加显式存在性校验。Line 123 当前逻辑可用，但在路径变化时报错上下文不够直观。建议在复制前用
Test-Path做前置校验并给出明确错误信息，方便排查构建失败。可选修改示例
- Copy-Item "dist/OneDragon ScriptChainer Runner.exe" -Destination "dist/OneDragon ScriptChainer/" -Force + $runnerExe = "dist/OneDragon ScriptChainer Runner.exe" + $editorDir = "dist/OneDragon ScriptChainer/" + if (-not (Test-Path $runnerExe)) { + throw "未找到 Runner 可执行文件：$runnerExe" + } + if (-not (Test-Path $editorDir)) { + throw "未找到 Editor 目录：$editorDir" + } + Copy-Item $runnerExe -Destination $editorDir -Force🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-release.yml at line 123, 在执行 Copy-Item "dist/OneDragon ScriptChainer Runner.exe" -Destination "dist/OneDragon ScriptChainer/" -Force 之前增加显式文件存在性校验：使用 Test-Path 检查 "dist/OneDragon ScriptChainer Runner.exe" 是否存在，若不存在则输出明确的错误信息（包含被查找的路径）并终止流程（例如用 Write-Error/throw 并返回非零退出码），这样在路径变更或构建产物缺失时能快速定位问题；保留原有 Copy-Item 调用用于实际复制。
130-145: 建议统一为所有上传步骤开启缺失文件即失败。Line 145 的
Upload Dist已设置if-no-files-found: error，但Upload Editor/Runner（Line 125-138）未设置。建议统一开启，避免产物丢失时“上传成功但内容为空/缺失”。可选修改示例
- name: Upload Editor if: ${{ env.CREATE_RELEASE == 'false' }} uses: actions/upload-artifact@v4 with: name: Editor path: deploy/dist/OneDragon ScriptChainer/ + if-no-files-found: error - name: Upload Runner if: ${{ env.CREATE_RELEASE == 'false' }} uses: actions/upload-artifact@v4 with: name: Runner path: deploy/dist/OneDragon ScriptChainer Runner.exe + if-no-files-found: error🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-release.yml around lines 130 - 145, Add the missing if-no-files-found: error to all upload-artifact steps so uploads fail when artifacts are missing: update the "Upload Runner" step (and any other upload steps like "Upload Editor") to include with: if-no-files-found: error alongside their name/path entries, matching the existing "Upload Dist" behavior; ensure the steps using uses: actions/upload-artifact@v4 include that key to prevent silent empty uploads.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/build-release.yml:
- Line 123: 在执行 Copy-Item "dist/OneDragon ScriptChainer Runner.exe" -Destination
"dist/OneDragon ScriptChainer/" -Force 之前增加显式文件存在性校验：使用 Test-Path 检查
"dist/OneDragon ScriptChainer Runner.exe" 是否存在，若不存在则输出明确的错误信息（包含被查找的路径）并终止流程（例如用
Write-Error/throw 并返回非零退出码），这样在路径变更或构建产物缺失时能快速定位问题；保留原有 Copy-Item 调用用于实际复制。
- Around line 130-145: Add the missing if-no-files-found: error to all
upload-artifact steps so uploads fail when artifacts are missing: update the
"Upload Runner" step (and any other upload steps like "Upload Editor") to
include with: if-no-files-found: error alongside their name/path entries,
matching the existing "Upload Dist" behavior; ensure the steps using uses:
actions/upload-artifact@v4 include that key to prevent silent empty uploads.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e5576cac-f38a-4e4d-a537-da9727f44e83
📒 Files selected for processing (1)
.github/workflows/build-release.yml
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/one_dragon/utils/os_utils.py (1)
34-46: 资源路径解析逻辑清晰，实现正确。函数正确处理了开发模式和 PyInstaller 打包模式下的资源查找优先级。有一个小问题：
文档字符串中使用了全角逗号
，，建议改为半角逗号,以保持一致性。📝 建议修复
def get_resource_path(*sub_paths: str) -> str: """获取资源文件路径。 - 优先查找工作目录下的路径，不存在时回退到 PyInstaller _MEIPASS。 + 优先查找工作目录下的路径,不存在时回退到 PyInstaller _MEIPASS。 """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/one_dragon/utils/os_utils.py` around lines 34 - 46, The docstring in get_resource_path uses a full-width comma '，' — update the docstring to replace the full-width comma with a standard ASCII comma ',' so punctuation is consistent (e.g., change "优先查找工作目录下的路径，不存在时回退到 PyInstaller _MEIPASS。" to use a half-width comma).src/one_dragon/base/config/yaml_operator.py (1)
9-21: YAML 缓存机制实现合理。基于文件 mtime 的缓存失效策略是正确的做法。需要注意：
如果文件在
os.path.exists()检查和os.path.getmtime()调用之间被删除，会抛出FileNotFoundError。当前设计依赖调用方（__read_from_file）在调用前检查文件存在性，但存在 TOCTOU（检查时间到使用时间）竞态条件。模块级缓存
cached_yaml_data是全局状态，在多线程环境下可能存在并发访问问题。如果应用是单线程的，这不是问题。💡 可选：增加防御性处理
def read_cache_or_load(file_path: str): cached = cached_yaml_data.get(file_path) - last_modify = os.path.getmtime(file_path) + try: + last_modify = os.path.getmtime(file_path) + except OSError: + return None if cached is not None and cached[0] == last_modify: return cached[1]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/one_dragon/base/config/yaml_operator.py` around lines 9 - 21, read_cache_or_load currently reads os.path.getmtime and accesses module-level cached_yaml_data without defenses against the TOCTOU FileNotFoundError and concurrent access; wrap the getmtime/open/cache-read/write sequence in a try/except that catches FileNotFoundError (and optionally OSError) and returns None (or propagates a clear error) to avoid crashing if the file is removed between exists/check and use, and add a thread lock (e.g., cached_yaml_lock = threading.Lock()) to serialize accesses to cached_yaml_data so reads and writes around cached_yaml_data[file_path] and cached_yaml_data.get(file_path) are done under the lock; update references to cached_yaml_data and read_cache_or_load accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/one_dragon/base/config/yaml_operator.py`:
- Around line 9-21: read_cache_or_load currently reads os.path.getmtime and
accesses module-level cached_yaml_data without defenses against the TOCTOU
FileNotFoundError and concurrent access; wrap the getmtime/open/cache-read/write
sequence in a try/except that catches FileNotFoundError (and optionally OSError)
and returns None (or propagates a clear error) to avoid crashing if the file is
removed between exists/check and use, and add a thread lock (e.g.,
cached_yaml_lock = threading.Lock()) to serialize accesses to cached_yaml_data
so reads and writes around cached_yaml_data[file_path] and
cached_yaml_data.get(file_path) are done under the lock; update references to
cached_yaml_data and read_cache_or_load accordingly.
In `@src/one_dragon/utils/os_utils.py`:
- Around line 34-46: The docstring in get_resource_path uses a full-width comma
'，' — update the docstring to replace the full-width comma with a standard ASCII
comma ',' so punctuation is consistent (e.g., change "优先查找工作目录下的路径，不存在时回退到
PyInstaller _MEIPASS。" to use a half-width comma).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0c6d9a61-92b9-48f4-bb7c-945005a4a8ba
📒 Files selected for processing (6)
deploy/OneDragon ScriptChainer Runner.specdeploy/OneDragon ScriptChainer.specsrc/one_dragon/base/config/yaml_operator.pysrc/one_dragon/utils/i18_utils.pysrc/one_dragon/utils/os_utils.pysrc/script_chainer/win_exe/script_runner.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/one_dragon/utils/i18_utils.py
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/one_dragon/base/config/yaml_operator.py (1)
8-8: 全局 YAML 缓存缺少淘汰策略，长期运行可能持续占用内存。建议给
cached_yaml_data增加上限（LRU/FIFO 均可），避免路径数量增长导致常驻内存不可控。Also applies to: 10-20
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/one_dragon/base/config/yaml_operator.py` at line 8, cached_yaml_data is an unbounded global dict and needs a size limit to prevent unbounded memory growth; introduce a configurable max (e.g., YAML_CACHE_MAX) and use an eviction policy (LRU recommended) when inserting into cached_yaml_data — implement this by replacing the plain dict with an ordered mapping (collections.OrderedDict or a small LRU wrapper) or by adding logic in the function that populates cached_yaml_data (look for functions that set cached_yaml_data entries) to pop the least-recently-used/oldest key once len(cached_yaml_data) > YAML_CACHE_MAX; ensure access updates recency if using LRU and add a default max constant and a short comment explaining the policy.src/one_dragon/utils/os_utils.py (1)
142-151:get_monday_dt的文档与变量名建议同步到“周一”语义。当前返回说明与局部变量名仍是“星期天”，易误导维护者。
建议修复
def get_monday_dt(dt: str) -> str: @@ - :return: 星期天日期 yyyyMMdd 格式 + :return: 星期一日期 yyyyMMdd 格式 @@ - sunday_date = date + datetime.timedelta(days=-weekday) - return sunday_date.strftime("%Y%m%d") + monday_date = date + datetime.timedelta(days=-weekday) + return monday_date.strftime("%Y%m%d")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/one_dragon/utils/os_utils.py` around lines 142 - 151, The get_monday_dt function's docstring and local variable names incorrectly reference "Sunday" while the logic returns Monday; rename the misleading symbol(s) and update the docstring to reflect "Monday" semantics (e.g., change sunday_date to monday_date and update the return description and parameter doc to say 星期一/周一), and ensure any inline comment about weekday mapping consistently states 0 = Monday; keep the existing computation (date + datetime.timedelta(days=-weekday)) which correctly yields Monday.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/one_dragon/base/config/yaml_config.py`:
- Line 42: The docstring line """是否只读取sample文件（即使.yml文件存在也只读sample）""" contains
full-width parentheses which trigger Ruff; update that string to use
ASCII/half-width parentheses like """是否只读取sample文件(即使.yml文件存在也只读sample)"""
(locate the exact docstring in yaml_config.py), save and run ruff/linters to
confirm the warning is resolved.
- Around line 82-84: 当前实现：在判断 self._copy_from_sample 为 True 后执行
shutil.copyfile(sample_yml_path, yml_path) 但仍返回
sample_yml_path，导致后续写入错误。修改该方法使其在执行复制后返回目标路径 yml_path（而非 sample_yml_path）；即保留
shutil.copyfile(sample_yml_path, yml_path) 调用不变，但在返回值处根据 self._copy_from_sample
或复制结果返回 yml_path，确保后续保存落在正确的 <module>.yml
文件（引用符号：self._copy_from_sample、sample_yml_path、yml_path）。
In `@src/one_dragon/base/config/yaml_operator.py`:
- Around line 10-20: The cache currently returns the same mutable object to
callers and doesn't enforce the YAML root to be a dict, causing shared-mutation
bugs and type-contract violations; in read_cache_or_load you should (1) validate
the result of yaml_utils.safe_load(file) is a dict (raise a clear TypeError or
convert to an empty dict if your contract requires) and (2) when returning
cached_yaml_data[file_path][1] return a defensive copy (e.g., a deep copy) so
callers do not share the same dict instance; update any callers that assume
YamlOperator.data is always a dict to rely on the validated/defensively-copied
value.
In `@src/one_dragon/utils/os_utils.py`:
- Around line 64-66: get_work_dir() currently returns os.getcwd() when
run_in_exe() is true which yields an unstable working directory; instead detect
the frozen/exe context and return the directory of the running executable/bundle
so relative paths remain stable. Modify get_work_dir()/run_in_exe() branch to
use the executable location (e.g., os.path.dirname(sys.executable) or the frozen
bundle path such as getattr(sys, "frozen", False) and sys._MEIPASS when
applicable) rather than os.getcwd(); ensure callers like
src/one_dragon/utils/cmd_utils.py that rely on get_work_dir() (e.g.,
subprocess.Popen default cwd) receive the executable directory.
---
Nitpick comments:
In `@src/one_dragon/base/config/yaml_operator.py`:
- Line 8: cached_yaml_data is an unbounded global dict and needs a size limit to
prevent unbounded memory growth; introduce a configurable max (e.g.,
YAML_CACHE_MAX) and use an eviction policy (LRU recommended) when inserting into
cached_yaml_data — implement this by replacing the plain dict with an ordered
mapping (collections.OrderedDict or a small LRU wrapper) or by adding logic in
the function that populates cached_yaml_data (look for functions that set
cached_yaml_data entries) to pop the least-recently-used/oldest key once
len(cached_yaml_data) > YAML_CACHE_MAX; ensure access updates recency if using
LRU and add a default max constant and a short comment explaining the policy.
In `@src/one_dragon/utils/os_utils.py`:
- Around line 142-151: The get_monday_dt function's docstring and local variable
names incorrectly reference "Sunday" while the logic returns Monday; rename the
misleading symbol(s) and update the docstring to reflect "Monday" semantics
(e.g., change sunday_date to monday_date and update the return description and
parameter doc to say 星期一/周一), and ensure any inline comment about weekday
mapping consistently states 0 = Monday; keep the existing computation (date +
datetime.timedelta(days=-weekday)) which correctly yields Monday.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dcf8c15f-4d0f-4761-b9b0-08602f6f84cb
📒 Files selected for processing (3)
src/one_dragon/base/config/yaml_config.pysrc/one_dragon/base/config/yaml_operator.pysrc/one_dragon/utils/os_utils.py
ShadowLemoon
force-pushed
the
refactor/onedir
branch
from
64f387c to
83688c2
Compare
ShadowLemoon
force-pushed
the
refactor/onedir
branch
from
83688c2 to
60ede42
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/one_dragon/base/config/yaml_config.py (1)
80-84:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
copy_from_sample=True时返回了错误的路径，会导致后续写入覆盖 sample 文件。当
_copy_from_sample=True时，代码正确地将 sample 文件复制到yml_path，但随后仍返回sample_yml_path。这导致YamlOperator的file_path指向 sample 文件，后续所有保存操作都会写入 sample 文件而非用户配置文件。此问题在之前的 review 中已被标记但未修复。
🐛 建议修复
# 最后看是否有示例文件 if self._sample and os.path.exists(sample_yml_path): if self._copy_from_sample: shutil.copyfile(sample_yml_path, yml_path) - return sample_yml_path + return yml_path + return sample_yml_path🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/one_dragon/base/config/yaml_config.py` around lines 80 - 84, The method currently returns sample_yml_path even after copying the sample into yml_path when self._copy_from_sample is True, causing YamlOperator.file_path to point at the sample; change the return so that after shutil.copyfile(sample_yml_path, yml_path) the function returns yml_path (while keeping the existing return of sample_yml_path when not copying), and ensure this logic is applied where variables sample_yml_path, yml_path, and flag self._copy_from_sample are used so subsequent saves write to the copied user's config rather than the sample.
🧹 Nitpick comments (3)
src/script_chainer/gui/page/editor_setting_interface.py (3)
62-67: ⚡ Quick win捕获宽泛异常是合理的，但建议记录日志。
静态分析工具标记了
except Exception，但在这种网络请求场景下捕获所有异常是合理的。不过建议添加日志记录以便排查问题。♻️ 建议添加日志
+from one_dragon.utils.log_utils import log + class GithubUpdateChecker(QThread): # ... def run(self) -> None: try: latest_stable, latest_beta = self.ctx.github_update_service.get_latest_tags() self.check_finished.emit(True, latest_stable, latest_beta, '') except Exception as e: + log.error('检查 GitHub 更新失败', exc_info=True) self.check_finished.emit(False, '', '', str(e))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/script_chainer/gui/page/editor_setting_interface.py` around lines 62 - 67, The except block in run currently swallows all exceptions when calling self.ctx.github_update_service.get_latest_tags(); update it to log the error before emitting the failure signal—use an available logger (e.g. self.logger.exception(...) if self has a logger, or logging.exception(...)/logging.getLogger(__name__).exception(...)) to record the exception and stacktrace, then call self.check_finished.emit(False, '', '', str(e)) as before.
91-96: 💤 Low valueQThread 复用可能导致信号重复连接。
version_checker和update_runner在构造函数中创建并连接信号。如果按照上述建议每次创建新实例，需要注意断开旧连接或在新实例上重新连接信号。当前代码如果不修改，则version_checker也存在类似的复用问题。Also applies to: 179-189
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/script_chainer/gui/page/editor_setting_interface.py` around lines 91 - 96, The constructor currently creates and connects signals on version_checker (GithubUpdateChecker) and update_runner (GithubUpdateRunner) which can be recreated later and cause duplicate signal handlers; before replacing or recreating these instances, explicitly disconnect the old signals (e.g., disconnect version_checker.check_finished from _on_version_check_finished and update_runner.progress_changed/update_finished from _on_update_progress/_on_update_finished), and ensure the old QThread/QObject is stopped/quit and deleted (or use deleteLater()) before assigning and connecting the new instance so callbacks aren’t invoked multiple times.
163-163: 💤 Low value全角逗号会触发 Ruff 告警。
第163行和第263行包含全角逗号
，，建议替换为半角逗号,以消除静态分析警告。Also applies to: 263-263
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/script_chainer/gui/page/editor_setting_interface.py` at line 163, Replace the fullwidth comma (，) in the f-strings that trigger Ruff warnings with the ASCII comma (,) — locate the f-string containing "当前不是发布版，无法自动更新" in the EditorSettingInterface (or the module-level f-string usages) and the similar f-string around the other occurrence, and change the punctuation from the fullwidth comma to a normal comma so the strings use "," instead of "，".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/script_chainer/context/script_chainer_context.py`:
- Around line 37-38: init() currently calls
self.gh_proxy_service.update_proxy_url() synchronously when
self.env_config.is_gh_proxy is true, which blocks startup on a network call;
remove that direct call from ScriptChainerContext.init() and instead either (a)
expose a public method on GhProxyService (e.g., refresh_proxy) that the settings
UI invokes explicitly, or (b) kick off an asynchronous background refresh
(non-blocking task/future) that uses the last-known proxy value until a
successful refresh completes; ensure references to self.env_config.is_gh_proxy
and self.gh_proxy_service.update_proxy_url() are updated accordingly so init()
no longer performs a blocking network call.
In `@src/script_chainer/gui/page/editor_setting_interface.py`:
- Around line 36-52: The GithubUpdateRunner QThread is being reused and its
mutable attribute target_version is set outside the thread causing a race; fix
by either instantiating a fresh GithubUpdateRunner for each update request
(create new runner and connect its progress_changed/update_finished before
calling start) or modify GithubUpdateRunner.run() to capture a local copy of
target_version at the very start (e.g., local_target = self.target_version) so
the thread uses an immutable snapshot; ensure the UI code replaces the reused
instance instead of reusing the same GithubUpdateRunner and that any isRunning()
checks remain intact.
- Around line 197-207: The failure path in _on_update_finished can leave
update_btn disabled because it sets enabled state to bool(self.target_version)
and os_utils.run_in_exe(); change the failure branch so that after setting the
message and resetting texts you explicitly enable the retry controls: call
self.channel_combo.setEnabled(True), self.check_btn.setEnabled(True) (already
present) and set self.update_btn.setEnabled(True) so the user can retry
regardless of target_version or os_utils.run_in_exe() on failure; keep the
success path behavior unchanged.
In `@src/script_chainer/services/github_update_service.py`:
- Around line 109-111: get_latest_beta_tag() currently calls _open_request()
directly against api.github.com (releases_url) and bypasses the GH proxy; wrap
this call with the existing _with_gh_proxy() helper so the releases_url request
uses the same proxy path as stable queries and downloads. Modify the logic that
builds/releases the request in get_latest_beta_tag() to call
self._with_gh_proxy(releases_url, headers=...) (or pass the request callable
through _with_gh_proxy) instead of calling self._open_request(releases_url, ...
) directly, ensuring behavior is consistent with get_latest_tags() and other
proxy-aware methods.
---
Duplicate comments:
In `@src/one_dragon/base/config/yaml_config.py`:
- Around line 80-84: The method currently returns sample_yml_path even after
copying the sample into yml_path when self._copy_from_sample is True, causing
YamlOperator.file_path to point at the sample; change the return so that after
shutil.copyfile(sample_yml_path, yml_path) the function returns yml_path (while
keeping the existing return of sample_yml_path when not copying), and ensure
this logic is applied where variables sample_yml_path, yml_path, and flag
self._copy_from_sample are used so subsequent saves write to the copied user's
config rather than the sample.
---
Nitpick comments:
In `@src/script_chainer/gui/page/editor_setting_interface.py`:
- Around line 62-67: The except block in run currently swallows all exceptions
when calling self.ctx.github_update_service.get_latest_tags(); update it to log
the error before emitting the failure signal—use an available logger (e.g.
self.logger.exception(...) if self has a logger, or
logging.exception(...)/logging.getLogger(__name__).exception(...)) to record the
exception and stacktrace, then call self.check_finished.emit(False, '', '',
str(e)) as before.
- Around line 91-96: The constructor currently creates and connects signals on
version_checker (GithubUpdateChecker) and update_runner (GithubUpdateRunner)
which can be recreated later and cause duplicate signal handlers; before
replacing or recreating these instances, explicitly disconnect the old signals
(e.g., disconnect version_checker.check_finished from _on_version_check_finished
and update_runner.progress_changed/update_finished from
_on_update_progress/_on_update_finished), and ensure the old QThread/QObject is
stopped/quit and deleted (or use deleteLater()) before assigning and connecting
the new instance so callbacks aren’t invoked multiple times.
- Line 163: Replace the fullwidth comma (，) in the f-strings that trigger Ruff
warnings with the ASCII comma (,) — locate the f-string containing
"当前不是发布版，无法自动更新" in the EditorSettingInterface (or the module-level f-string
usages) and the similar f-string around the other occurrence, and change the
punctuation from the fullwidth comma to a normal comma so the strings use ","
instead of "，".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb45bf6f-47b4-48d2-9f78-eebc21a17da4
📒 Files selected for processing (13)
.github/workflows/build-release.yml.gitignoredeploy/OneDragon ScriptChainer Runner.specdeploy/OneDragon ScriptChainer.specsrc/one_dragon/base/config/yaml_config.pysrc/one_dragon/base/config/yaml_operator.pysrc/one_dragon/utils/i18_utils.pysrc/one_dragon/utils/os_utils.pysrc/one_dragon_qt/view/like_interface.pysrc/script_chainer/context/script_chainer_context.pysrc/script_chainer/gui/page/editor_setting_interface.pysrc/script_chainer/services/github_update_service.pysrc/script_chainer/win_exe/script_runner.py
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- src/one_dragon_qt/view/like_interface.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/one_dragon/utils/i18_utils.py
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/one_dragon/base/config/yaml_config.py (1)
81-84:⚠️ Potential issue | 🟠 Major | ⚡ Quick win复制 sample 后应该返回目标
.yml。
_copy_from_sample=True时这里已经把sample_yml_path复制到了yml_path，但返回值仍是sample_yml_path。这样YamlOperator后续的save()/update()还是会写回 sample 文件，迁移结果不会落到用户配置。这里在复制成功后应直接return yml_path。建议修复
if self._sample and os.path.exists(sample_yml_path): if self._copy_from_sample: shutil.copyfile(sample_yml_path, yml_path) - return sample_yml_path + return yml_path + return sample_yml_path🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/one_dragon/base/config/yaml_config.py` around lines 81 - 84, The function that checks self._sample currently returns sample_yml_path even when it copies the sample to the target path; change the logic in the block that handles self._sample and self._copy_from_sample so that after shutil.copyfile(sample_yml_path, yml_path) the function returns yml_path (the target file) instead of sample_yml_path, ensuring downstream code in YamlOperator (save()/update()) operates on the user's config file rather than the original sample.src/script_chainer/services/github_update_service.py (1)
88-103:⚠️ Potential issue | 🟠 Major | ⚡ Quick win版本查询也要走 GH 代理。
现在只有 ZIP 下载走了
_with_gh_proxy()，而稳定版/测试版标签查询还是直接打api.github.com。只开启免费 GH 代理时，检查更新以及target_tag=None的自动更新都会先卡在这里。建议这两个 API URL 也统一包一层_with_gh_proxy()。建议修复
def get_latest_tag(self) -> str: repo_path = self._get_github_repo_path() - release_url = f'https://api.github.com/repos/{repo_path}/releases/latest' + release_url = self._with_gh_proxy( + f'https://api.github.com/repos/{repo_path}/releases/latest' + ) with self._open_request(release_url, headers={'User-Agent': 'OneDragon-ScriptChainer'}) as response: release = json.loads(response.read().decode('utf-8')) def get_latest_beta_tag(self) -> str: repo_path = self._get_github_repo_path() - releases_url = f'https://api.github.com/repos/{repo_path}/releases?per_page=30' + releases_url = self._with_gh_proxy( + f'https://api.github.com/repos/{repo_path}/releases?per_page=30' + ) with self._open_request(releases_url, headers={'User-Agent': 'OneDragon-ScriptChainer'}) as response: releases = json.loads(response.read().decode('utf-8'))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/script_chainer/services/github_update_service.py` around lines 88 - 103, Both get_latest_tag and get_latest_beta_tag call api.github.com directly so they bypass the GH proxy; wrap their API URLs with the existing _with_gh_proxy() helper before opening the request. Concretely, update get_latest_tag to pass the release_url through self._with_gh_proxy(release_url) (or call _with_gh_proxy when building the URL) and do the same in get_latest_beta_tag for releases_url so the subsequent self._open_request(...) uses the proxied URL; keep existing headers/User-Agent and error handling unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/one_dragon/base/config/yaml_operator.py`:
- Around line 22-32: The YAML cache only checks mtime in read_cache_or_load but
write paths (save, save_diy, delete) don't invalidate cached_yaml_data, so rapid
writes can leave stale reads; update the write/delete methods (e.g., save,
save_diy, any delete_file method) to explicitly invalidate the cache for that
path by calling cached_yaml_data.pop(file_path, None) (or update the cached
entry with a new version/token) immediately after successfully writing/deleting,
and ensure read_cache_or_load still verifies mtime/version as a safety net.
In `@src/script_chainer/gui/page/editor_setting_interface.py`:
- Around line 78-80: The run() worker method can throw inside
ctx.gh_proxy_service.update_proxy_url(), preventing update_finished.emit(...)
from running and leaving the UI button disabled; wrap the network call in a
try/finally (or catch exceptions and rethrow) and ensure
update_finished.emit(self.ctx.env_config.gh_proxy_url) is always executed in the
finally block so _on_gh_proxy_update_finished() runs and the "Get" button is
re-enabled; alternatively mention using QThread.finished to restore state, but
implement the try/finally around update_proxy_url() in run() to guarantee
emission.
---
Duplicate comments:
In `@src/one_dragon/base/config/yaml_config.py`:
- Around line 81-84: The function that checks self._sample currently returns
sample_yml_path even when it copies the sample to the target path; change the
logic in the block that handles self._sample and self._copy_from_sample so that
after shutil.copyfile(sample_yml_path, yml_path) the function returns yml_path
(the target file) instead of sample_yml_path, ensuring downstream code in
YamlOperator (save()/update()) operates on the user's config file rather than
the original sample.
In `@src/script_chainer/services/github_update_service.py`:
- Around line 88-103: Both get_latest_tag and get_latest_beta_tag call
api.github.com directly so they bypass the GH proxy; wrap their API URLs with
the existing _with_gh_proxy() helper before opening the request. Concretely,
update get_latest_tag to pass the release_url through
self._with_gh_proxy(release_url) (or call _with_gh_proxy when building the URL)
and do the same in get_latest_beta_tag for releases_url so the subsequent
self._open_request(...) uses the proxied URL; keep existing headers/User-Agent
and error handling unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fed12a5c-662f-464c-abdf-9e431436de86
📒 Files selected for processing (13)
.github/workflows/build-release.yml.gitignoredeploy/OneDragon ScriptChainer Runner.specdeploy/OneDragon ScriptChainer.specsrc/one_dragon/base/config/yaml_config.pysrc/one_dragon/base/config/yaml_operator.pysrc/one_dragon/utils/i18_utils.pysrc/one_dragon/utils/os_utils.pysrc/one_dragon_qt/view/like_interface.pysrc/script_chainer/context/script_chainer_context.pysrc/script_chainer/gui/page/editor_setting_interface.pysrc/script_chainer/services/github_update_service.pysrc/script_chainer/win_exe/script_runner.py
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (4)
- src/script_chainer/win_exe/script_runner.py
- src/one_dragon/utils/i18_utils.py
- src/one_dragon_qt/view/like_interface.py
- .github/workflows/build-release.yml
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
src/script_chainer/services/github_update_service.py (1)
220-229: 💤 Low value
_get_update_items会无差别采纳 stage 根目录下的所有*.exe。第 226-228 行兜底将
stage_dir下任何额外*.exe也加入更新清单。如果未来 release 包结构调整带入了无关的 exe（例如调试工具、第三方依赖单文件），它们也会被搬到work_dir并替换/新增到运行目录。建议要么仅信任APP_EXE_NAMES，要么在加入前做一次白名单/签名校验。如果当前 release 内容完全可控，则可以暂忽略。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/script_chainer/services/github_update_service.py` around lines 220 - 229, The helper _get_update_items currently appends every *.exe found under stage_dir (fall-through loop using stage_dir.glob('*.exe')), which can unintentionally include unrelated executables; change this to either restrict additions to the known APP_EXE_NAMES set or apply an explicit whitelist/signature check before appending: in _get_update_items, replace the unconditional glob append with logic that filters by APP_EXE_NAMES (or verifies a digital signature/checksum) using known_names/APP_EXE_NAMES and only then add to items; ensure RUNTIME_DIR_NAME handling is unchanged and keep stage_dir, items, APP_EXE_NAMES, and _get_update_items as the places to modify.src/script_chainer/gui/page/editor_setting_interface.py (2)
86-88: ⚡ Quick win
GhProxyUpdateRunner.run()建议加try/finally兜底。目前
update_proxy_url()内部已经把requests异常都吞掉了，所以不会真的崩。但这里耦合了对下游实现“永远不抛”的隐含假设——一旦ghproxy_service.py后续改动让异常透出（例如改用raise_for_status之外的逻辑），update_finished就不会emit，UI 上的fetch_gh_proxy_url_btn会卡在 disabled。🛡️ 建议加防御
def run(self) -> None: - self.ctx.gh_proxy_service.update_proxy_url() - self.update_finished.emit(self.ctx.env_config.gh_proxy_url) + try: + self.ctx.gh_proxy_service.update_proxy_url() + finally: + self.update_finished.emit(self.ctx.env_config.gh_proxy_url)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/script_chainer/gui/page/editor_setting_interface.py` around lines 86 - 88, GhProxyUpdateRunner.run currently calls self.ctx.gh_proxy_service.update_proxy_url() and then self.update_finished.emit(...), but it assumes update_proxy_url never raises; wrap the call in a try/finally so update_finished.emit(self.ctx.env_config.gh_proxy_url) is always executed (in the finally block) to ensure the UI button (fetch_gh_proxy_url_btn) is re-enabled even if update_proxy_url raises; reference GhProxyUpdateRunner.run, update_proxy_url, update_finished.emit and ctx.env_config.gh_proxy_url and keep any exception handling/logging for the raised error before re-emitting.
411-413: 💤 Low value参数命名建议去掉前导下划线。
_proxy_url的下划线前缀通常表示“未使用”，但这里第 412 行就把它写进了 UI。改成proxy_url更贴近实际语义，便于阅读。♻️ 建议调整
- def _on_gh_proxy_update_finished(self, _proxy_url: str) -> None: - self.gh_proxy_url_opt.setValue(_proxy_url, emit_signal=False) + def _on_gh_proxy_update_finished(self, proxy_url: str) -> None: + self.gh_proxy_url_opt.setValue(proxy_url, emit_signal=False) self.fetch_gh_proxy_url_btn.setEnabled(True)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/script_chainer/gui/page/editor_setting_interface.py` around lines 411 - 413, The parameter name _proxy_url in the method _on_gh_proxy_update_finished incorrectly uses a leading underscore (which suggests unused) but is actually used; rename the parameter to proxy_url and update all references inside _on_gh_proxy_update_finished (e.g., the gh_proxy_url_opt.setValue call) to use proxy_url so the name matches its usage and improves readability.src/one_dragon/base/web/common_downloader.py (1)
70-78: ⚖️ Poor tradeoff下载源选择逻辑建议补充优先级说明并允许多源回退。
当前
if/elif/elif只会尝试一个下载源；如果调用方同时设了download_by_github=True与download_by_gitee=True，gitee 永远不会被尝试。即使实际场景里只有一个布尔为True，从 API 设计角度也容易误用。如果允许多源回退是预期行为，可改为按优先级顺序尝试，失败则回退到下一个；如果仅允许单源，建议在文档/类型上更明确（例如改成
source: Literal['github','gitee','mirror_chan']）。该优化影响接口面，可在后续迭代中处理。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/one_dragon/base/web/common_downloader.py` around lines 70 - 78, Change the current if/elif chain that picks a single download source (conditions using download_by_github, download_by_gitee, download_by_mirror_chan and self.param.*_download_url) to an ordered-priority attempt loop: build a list of candidate sources in priority order (e.g., github, gitee, mirror_chan) and for each candidate, construct the candidate download_url (apply ghproxy_url when candidate is github), attempt to fetch/head/verify the URL inside a try/except, and on success set download_url and break; on failure log and continue to next candidate; if none succeed raise/return an error. Update references in the surrounding code to use the final download_url and ensure errors are handled consistently.src/one_dragon/utils/http_utils.py (2)
81-97: ⚡ Quick win
DownloadCancelledError定义建议放到模块顶部。虽然运行时不会出问题（模块加载完毕后才会调用
download_file），但当前写法是“函数体里raise一个写在该函数后面的异常类”，阅读时容易让人误以为存在循环引用或定义错误。建议把异常类提到import区块之后、download_file之前。♻️ 建议调整
from one_dragon.utils.i18_utils import gt from one_dragon.utils.log_utils import log +class DownloadCancelledError(Exception): + pass + + def download_file(download_url: str, save_file_path: str, ... @@ log.error(msg, exc_info=True) return False - -class DownloadCancelledError(Exception): - pass🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/one_dragon/utils/http_utils.py` around lines 81 - 97, Move the DownloadCancelledError class definition up to the top of the module (immediately after the imports) so it is defined before download_file; update the file so DownloadCancelledError is declared before any references (e.g., the raise inside download_file and the except DownloadCancelledError block) to improve readability and avoid confusion when scanning the module.
64-74: 💤 Low value取消信号判断的粒度建议补充说明。
每次循环只在读取下一个 chunk 之前检查
progress_signal['signal']。当文件较大或网络较慢时，单次response.read(chunk_size)可能阻塞数秒甚至更久，期间无法响应取消。opener.open(...)已使用timeout=60，所以最坏情况下仍会有 60 秒级别的延迟。如果对取消的实时性有要求，可考虑：
- 减小
chunk_size（当前 64KB，已经较小，主要受网络延迟影响）；- 或将
opener.open的 socket 设置更短的读超时 + 重试。如果当前响应时延可以接受，则忽略此评论。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/one_dragon/utils/http_utils.py` around lines 64 - 74, The current download loop checks progress_signal only once per chunk and can block during response.read(chunk_size); to improve cancel responsiveness update the read logic in the same function (the loop that uses response.read, progress_signal, log_download_progress and raises DownloadCancelledError) to check progress_signal more frequently: either reduce chunk_size or implement an inner loop that reads the chunk in smaller sub-chunks and checks progress_signal between sub-reads, or immediately set a shorter socket read timeout on the opened response (e.g., adjust opener.open/read socket timeout or response.fp.raw._sock.settimeout) so response.read doesn't block for long; ensure you still write bytes, update downloaded_bytes, call log_download_progress, and raise DownloadCancelledError when progress_signal['signal']=='cancel'.src/one_dragon/base/web/zip_downloader.py (1)
40-40: 💤 Low value重试次数建议提取为常量。
for i in range(2)中的2是硬编码的重试次数，建议提到模块级常量（例如MAX_DOWNLOAD_ATTEMPTS = 2），便于以后调整以及让阅读者一眼明白意图。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/one_dragon/base/web/zip_downloader.py` at line 40, The hardcoded retry count in the loop using the expression "for i in range(2)" should be extracted to a module-level constant (e.g. MAX_DOWNLOAD_ATTEMPTS = 2) so it’s self-documenting and easy to change; replace the literal 2 with MAX_DOWNLOAD_ATTEMPTS wherever the retry loop appears (the loop currently written as "for i in range(2)"), and update any related comments/docstrings to reference the constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/one_dragon/base/config/yaml_config.py`:
- Around line 87-90: Currently the code returns the bundled frozen_path (from
os_utils.get_resource_path) which later becomes self.file_path and causes saves
(yaml_operator.save/save_diy) to write into package resources; instead, detect
the bundled source (frozen_path), compute the intended writable yml_path (the
existing yml_path variable or config location used elsewhere), ensure its parent
directory exists, copy the bundled frozen_path to yml_path (e.g.,
shutil.copyfile) and then return yml_path so subsequent saves go to the writable
location; reference frozen_path, yml_path, get_resource_path, and
self.file_path/save()/save_diy() when making the change.
In `@src/one_dragon/base/web/common_downloader.py`:
- Around line 91-103: The is_file_existed method currently returns True when
check_existed_list is empty, causing callers that use skip_if_existed=True to
wrongly skip downloads; change is_file_existed (in class containing
param.check_existed_list) to explicitly treat an empty check_existed_list as
“not all files exist” (return False) or raise a clear error—i.e., at the start
of is_file_existed check if not self.param.check_existed_list and return False
(or raise ValueError), so download(skip_if_existed=True) will not silently skip
when there are no files to verify; update any callers/tests accordingly (notably
download and GithubUpdateService usage).
In `@src/one_dragon/base/web/zip_downloader.py`:
- Around line 56-64: The download() loop may return True after two failed
unzip() attempts because self.is_file_existed() checks the zip itself; change
the post-loop existence check to call CommonDownloader.is_file_existed(self) to
require the extracted artifacts exist (use that explicit method in
ZipDownloader.download()). Also fix CommonDownloader.is_file_existed() so it
does not treat an empty param.check_existed_list as a successful existence check
(make it return False when check_existed_list is empty or validate list before
claiming success) to prevent false positives.
In `@src/script_chainer/gui/page/editor_setting_interface.py`:
- Around line 353-363: on_interface_shown currently always calls
refresh_gh_proxy_url_by_config, causing an external fetch on every page entry;
add a session-level boolean flag (e.g., self._gh_proxy_refreshed_this_session)
to the class and change on_interface_shown to call
refresh_gh_proxy_url_by_config only if the flag is False and the auto-fetch
option (ctx.env_config.get_prop_adapter('auto_fetch_gh_proxy_url') /
auto_fetch_gh_proxy_url_opt) is enabled; inside refresh_gh_proxy_url_by_config
set the flag to True only after a successful fetch and clear/reset it on failure
so subsequent entries or an explicit “Fetch” button (the existing manual
trigger) can retry.
---
Nitpick comments:
In `@src/one_dragon/base/web/common_downloader.py`:
- Around line 70-78: Change the current if/elif chain that picks a single
download source (conditions using download_by_github, download_by_gitee,
download_by_mirror_chan and self.param.*_download_url) to an ordered-priority
attempt loop: build a list of candidate sources in priority order (e.g., github,
gitee, mirror_chan) and for each candidate, construct the candidate download_url
(apply ghproxy_url when candidate is github), attempt to fetch/head/verify the
URL inside a try/except, and on success set download_url and break; on failure
log and continue to next candidate; if none succeed raise/return an error.
Update references in the surrounding code to use the final download_url and
ensure errors are handled consistently.
In `@src/one_dragon/base/web/zip_downloader.py`:
- Line 40: The hardcoded retry count in the loop using the expression "for i in
range(2)" should be extracted to a module-level constant (e.g.
MAX_DOWNLOAD_ATTEMPTS = 2) so it’s self-documenting and easy to change; replace
the literal 2 with MAX_DOWNLOAD_ATTEMPTS wherever the retry loop appears (the
loop currently written as "for i in range(2)"), and update any related
comments/docstrings to reference the constant.
In `@src/one_dragon/utils/http_utils.py`:
- Around line 81-97: Move the DownloadCancelledError class definition up to the
top of the module (immediately after the imports) so it is defined before
download_file; update the file so DownloadCancelledError is declared before any
references (e.g., the raise inside download_file and the except
DownloadCancelledError block) to improve readability and avoid confusion when
scanning the module.
- Around line 64-74: The current download loop checks progress_signal only once
per chunk and can block during response.read(chunk_size); to improve cancel
responsiveness update the read logic in the same function (the loop that uses
response.read, progress_signal, log_download_progress and raises
DownloadCancelledError) to check progress_signal more frequently: either reduce
chunk_size or implement an inner loop that reads the chunk in smaller sub-chunks
and checks progress_signal between sub-reads, or immediately set a shorter
socket read timeout on the opened response (e.g., adjust opener.open/read socket
timeout or response.fp.raw._sock.settimeout) so response.read doesn't block for
long; ensure you still write bytes, update downloaded_bytes, call
log_download_progress, and raise DownloadCancelledError when
progress_signal['signal']=='cancel'.
In `@src/script_chainer/gui/page/editor_setting_interface.py`:
- Around line 86-88: GhProxyUpdateRunner.run currently calls
self.ctx.gh_proxy_service.update_proxy_url() and then
self.update_finished.emit(...), but it assumes update_proxy_url never raises;
wrap the call in a try/finally so
update_finished.emit(self.ctx.env_config.gh_proxy_url) is always executed (in
the finally block) to ensure the UI button (fetch_gh_proxy_url_btn) is
re-enabled even if update_proxy_url raises; reference GhProxyUpdateRunner.run,
update_proxy_url, update_finished.emit and ctx.env_config.gh_proxy_url and keep
any exception handling/logging for the raised error before re-emitting.
- Around line 411-413: The parameter name _proxy_url in the method
_on_gh_proxy_update_finished incorrectly uses a leading underscore (which
suggests unused) but is actually used; rename the parameter to proxy_url and
update all references inside _on_gh_proxy_update_finished (e.g., the
gh_proxy_url_opt.setValue call) to use proxy_url so the name matches its usage
and improves readability.
In `@src/script_chainer/services/github_update_service.py`:
- Around line 220-229: The helper _get_update_items currently appends every
*.exe found under stage_dir (fall-through loop using stage_dir.glob('*.exe')),
which can unintentionally include unrelated executables; change this to either
restrict additions to the known APP_EXE_NAMES set or apply an explicit
whitelist/signature check before appending: in _get_update_items, replace the
unconditional glob append with logic that filters by APP_EXE_NAMES (or verifies
a digital signature/checksum) using known_names/APP_EXE_NAMES and only then add
to items; ensure RUNTIME_DIR_NAME handling is unchanged and keep stage_dir,
items, APP_EXE_NAMES, and _get_update_items as the places to modify.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 49f852f4-a72e-47ff-91bb-25aa3ed63dae
📒 Files selected for processing (18)
.github/workflows/build-release.yml.gitignoredeploy/OneDragon ScriptChainer Runner.specdeploy/OneDragon ScriptChainer.specsrc/one_dragon/base/config/yaml_config.pysrc/one_dragon/base/config/yaml_operator.pysrc/one_dragon/base/web/__init__.pysrc/one_dragon/base/web/common_downloader.pysrc/one_dragon/base/web/zip_downloader.pysrc/one_dragon/envs/ghproxy_service.pysrc/one_dragon/utils/http_utils.pysrc/one_dragon/utils/i18_utils.pysrc/one_dragon/utils/os_utils.pysrc/one_dragon_qt/view/like_interface.pysrc/script_chainer/context/script_chainer_context.pysrc/script_chainer/gui/page/editor_setting_interface.pysrc/script_chainer/services/github_update_service.pysrc/script_chainer/win_exe/script_runner.py
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (4)
- src/one_dragon_qt/view/like_interface.py
- src/script_chainer/win_exe/script_runner.py
- .github/workflows/build-release.yml
- src/one_dragon/utils/i18_utils.py
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.
Summary by CodeRabbit
发布说明
新功能
改进