fix(general): 通用脚本配置缺失时下发与回写改为跳过,不再抛 FileNotFoundError - #829
Open
beichen24a1 wants to merge 5 commits into
Open
beichen24a1 wants to merge 5 commits into
beichen24a1 wants to merge 5 commits into
Conversation
同一套「按 ConfigPathMode 在两个目录间拷贝」的逻辑在通用脚本里写了 4 份,只有 ScriptConfig.set_general 做了存在性判断,另外三处源路径不存在时直接抛异常: - AutoProxy.set_general(下发):新建用户后没跑过「脚本设置」,data/<脚本>/<用户>/ ConfigFile 还不存在,自动代理一启动就崩、任务判成异常(Sentry AUTO-MAS-BACKEND-4Q)。 ScriptConfig.set_general 在同样情况下会跳过拷贝、让脚本用自己的配置 —— 两条路径 对同一件事行为不一致,有判断的那份是预期行为。 - AutoProxy.update_config(回写):脚本配置路径不存在时抛异常;File 分支还缺目标目录, copy 不像 copytree 会自建目录,会再抛一次。 - ScriptConfig.final_task(回写):先 rmtree 掉 MAS 侧副本再拷,源不存在时异常发生在 清空之后,用户会以为配置丢了(4Y / 4W)。 三处统一成「先判源存在,不存在就跳过并给一条可读 warning」,与 ScriptConfig.set_general 同口径。 验证: - 三处源不存在路径的前后走查:修复前分别是 swap_in_dir 的 RuntimeError 与 shutil 的 FileNotFoundError,修复后都是 warning + 跳过 - python -m pytest tests/task -q:804 passed / 17 failed,与改前基线逐条一致(既有失败) - python -m pytest tests --collect-only -q:1406 collected,exit 0 - ruff check / ruff format --check:通过
审查者指南将此前不安全的三个通用脚本配置交付/回写路径与现有的缺失时跳过行为保持一致:先验证源路径,在源路径不存在时发出警告并返回,同时确保文件模式的回写操作具有目标目录。 安全的通用脚本配置交付时序图sequenceDiagram
participant Task as AutoProxy
participant MAS as MASConfig
participant Script as GeneralScript
Task->>MAS: set_general()
alt Folder mode
Task->>MAS: exists(ConfigFile directory)
alt source exists
MAS-->>Task: swap_in_dir()
Task->>Script: start with delivered config
else source missing
MAS-->>Task: warning and return
Task->>Script: start with script-owned config
end
else File mode
Task->>MAS: exists(ConfigFile/file)
alt source exists
MAS-->>Task: copy()
Task->>Script: start with delivered config
else source missing
MAS-->>Task: warning and return
Task->>Script: start with script-owned config
end
end
安全的通用脚本配置回写时序图sequenceDiagram
participant Task as ConfigTask
participant Script as GeneralScript
participant MAS as MASConfig
Task->>Script: update_config() or final_task()
Task->>Script: exists(script_config_path)
alt source missing
Script-->>Task: warning and return
Note over MAS: Existing MAS copy remains unchanged
else source exists
alt Folder mode
Task->>MAS: rmtree and copytree
else File mode
Task->>MAS: mkdir destination
Task->>MAS: copy configuration file
end
end
文件级变更
可能相关的问题
提示和命令与 Sourcery 交互
自定义体验访问你的控制面板以:
获取帮助Original review guide in EnglishReviewer's GuideAlign the three previously unsafe general-script configuration delivery/write-back paths with the existing skip-on-missing behavior: validate sources first, warn and return when absent, and ensure file-mode write-back has a destination directory. Sequence diagram for safe general-script configuration deliverysequenceDiagram
participant Task as AutoProxy
participant MAS as MASConfig
participant Script as GeneralScript
Task->>MAS: set_general()
alt Folder mode
Task->>MAS: exists(ConfigFile directory)
alt source exists
MAS-->>Task: swap_in_dir()
Task->>Script: start with delivered config
else source missing
MAS-->>Task: warning and return
Task->>Script: start with script-owned config
end
else File mode
Task->>MAS: exists(ConfigFile/file)
alt source exists
MAS-->>Task: copy()
Task->>Script: start with delivered config
else source missing
MAS-->>Task: warning and return
Task->>Script: start with script-owned config
end
end
Sequence diagram for safe general-script configuration write-backsequenceDiagram
participant Task as ConfigTask
participant Script as GeneralScript
participant MAS as MASConfig
Task->>Script: update_config() or final_task()
Task->>Script: exists(script_config_path)
alt source missing
Script-->>Task: warning and return
Note over MAS: Existing MAS copy remains unchanged
else source exists
alt Folder mode
Task->>MAS: rmtree and copytree
else File mode
Task->>MAS: mkdir destination
Task->>MAS: copy configuration file
end
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.
嘿——我发现了 2 个问题
面向 AI Agent 的提示
请处理本次代码审查中的评论:
## 单独评论
### 评论 1
<location path="app/task/general/AutoProxy.py" line_range="659-664" />
<code_context>
+ # 跳过下发、让脚本用它自己的配置 —— 与 ScriptConfig.set_general 同口径,
+ # 否则 swap_in_dir / copy 会直接抛异常,把任务判成失败。
if self.script_config.get("Script", "ConfigPathMode") == "Folder":
+ if not (
+ Path.cwd()
+ / f"data/{self.script_info.script_id}/{self.cur_user_uid}/ConfigFile"
+ ).exists():
+ logger.warning("MAS 中尚无该用户的配置, 跳过配置下发")
+ return
swap_in_dir(
Path.cwd()
</code_context>
<issue_to_address>
**问题(更广泛的影响):** 当非直连用户在 MAS 端没有 `ConfigFile` 时,`AutoProxyTask.main_task` 会在 `prepare()` 和 `set_general()` 执行之前从 `check()` 返回异常任务,因此这些新的“警告并跳过”分支永远不会被执行。任务仍然会失败,而不是使用脚本自身的配置启动。
**触发条件:** `use_mas_config` 为 true 且缺少该用户对应的 MAS 配置目录时。
**建议修复:** 修改之前的 `check()` 行为,允许此路径缺少 MAS 配置;或者将源文件缺失处理移入检查流程,使其返回 `Pass`,并让 `set_general()` 跳过复制。
</issue_to_address>
### 评论 2
<location path="app/task/general/AutoProxy.py" line_range="576" />
<code_context>
+ # 源是用户填的脚本配置位置,可能不存在(路径填错、脚本还没生成过配置)。
+ # 跳过并说明即可,不能先清掉 MAS 侧副本再抛 FileNotFoundError。
+ if not self.script_config_path.exists():
+ logger.warning(
+ f"跳过配置回写: 脚本配置路径不存在 {self.script_config_path}"
</code_context>
<issue_to_address>
**问题(错误风险):** 这些保护措施只验证路径是否存在,没有验证其类型是否符合 `ConfigPathMode` 的要求:将普通文件用作 Folder 路径时,仍会执行 `rmtree`/`copytree`;将目录用作 File 路径时,仍会执行 `shutil.copy`,从而引发 `NotADirectoryError`、`IsADirectoryError` 或 `FileNotFoundError`。因此,对于已存在但格式错误的配置路径,新的检查仍无法避免失败。
**触发条件:** `ConfigPathMode` 与已配置源路径的文件系统类型不一致时。
**建议修复:** Folder 模式使用 `is_dir()`,File 模式使用 `is_file()`;对于类型错误的情况,也采用相同的警告并跳过行为。
```suggestion
if (
(
self.script_config.get("Script", "ConfigPathMode") == "Folder"
and not self.script_config_path.is_dir()
)
or (
self.script_config.get("Script", "ConfigPathMode") == "File"
and not self.script_config_path.is_file()
)
):
```
</issue_to_address>Sourcery 评估
等待批准。 请先处理 2 个发现的问题。
阻塞性发现:app/task/general/AutoProxy.py:664、app/task/general/AutoProxy.py:576
Original comment in English
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="app/task/general/AutoProxy.py" line_range="659-664" />
<code_context>
+ # 跳过下发、让脚本用它自己的配置 —— 与 ScriptConfig.set_general 同口径,
+ # 否则 swap_in_dir / copy 会直接抛异常,把任务判成失败。
if self.script_config.get("Script", "ConfigPathMode") == "Folder":
+ if not (
+ Path.cwd()
+ / f"data/{self.script_info.script_id}/{self.cur_user_uid}/ConfigFile"
+ ).exists():
+ logger.warning("MAS 中尚无该用户的配置, 跳过配置下发")
+ return
swap_in_dir(
Path.cwd()
</code_context>
<issue_to_address>
**issue (broader_impact):** When a non-direct user has no MAS-side `ConfigFile`, `AutoProxyTask.main_task` returns from `check()` as an abnormal task before `prepare()` and `set_general()` run, so these new warning-and-skip branches are never reached. The task still fails instead of starting the script with its own configuration.
**Triggers:** When `use_mas_config` is true and the per-user MAS configuration directory is absent.
**Suggested fix:** Change the earlier `check()` behavior to allow a missing MAS configuration for this path, or move the missing-source handling into the check flow so it returns `Pass` and lets `set_general()` skip the copy.
</issue_to_address>
### Comment 2
<location path="app/task/general/AutoProxy.py" line_range="576" />
<code_context>
+ # 源是用户填的脚本配置位置,可能不存在(路径填错、脚本还没生成过配置)。
+ # 跳过并说明即可,不能先清掉 MAS 侧副本再抛 FileNotFoundError。
+ if not self.script_config_path.exists():
+ logger.warning(
+ f"跳过配置回写: 脚本配置路径不存在 {self.script_config_path}"
</code_context>
<issue_to_address>
**issue (bug_risk):** The guards only verify that the path exists, not that it has the type required by `ConfigPathMode`: a regular file used as a Folder path still reaches `rmtree`/`copytree`, and a directory used as a File path still reaches `shutil.copy`, raising `NotADirectoryError`, `IsADirectoryError`, or `FileNotFoundError`. The new checks therefore do not prevent failures for an existing but malformed configuration path.
**Triggers:** When `ConfigPathMode` disagrees with the filesystem type of the configured source path.
**Suggested fix:** Use `is_dir()` for Folder mode and `is_file()` for File mode, with the same warning-and-skip behavior for the wrong type.
```suggestion
if (
(
self.script_config.get("Script", "ConfigPathMode") == "Folder"
and not self.script_config_path.is_dir()
)
or (
self.script_config.get("Script", "ConfigPathMode") == "File"
and not self.script_config_path.is_file()
)
):
```
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: app/task/general/AutoProxy.py:664, app/task/general/AutoProxy.py:576
- check(): MAS 侧尚无该用户 ConfigFile 时只提示、不再置「异常」并 return。 原判断在 main_task() 里早于 prepare()/set_general(),一旦命中就整单结束, 后面新加的跳过逻辑永远走不到;现在任务继续,下发环节跳过、让脚本用它 自己已有的配置(与 ScriptConfig.set_general 同口径)。 - update_config()/set_general(): 按 ConfigPathMode 校验路径类型 (Folder 要 is_dir、File 要 is_file),类型不符只 warning 跳过,不再让 rmtree/copytree/swap_in_dir/copy 抛异常;update_config 的类型判断放在 清空 MAS 侧副本之前,避免先删掉唯一副本再发现源不可用。
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Closes #533同一套「按
ConfigPathMode在两个目录间拷贝」的逻辑在通用脚本里写了 4 份,只有ScriptConfig.set_general做了存在性判断,另外三处源路径不存在时直接抛异常:AutoProxy.set_general(下发):新建用户后没跑过「脚本设置」,data/<脚本>/<用户>/ConfigFile还不存在,自动代理一启动就崩、任务判成异常(SentryAUTO-MAS-BACKEND-4Q)。ScriptConfig.set_general在同样情况下会跳过拷贝、让脚本用自己的配置——两条路径对同一件事行为不一致,有判断的那份是预期行为。AutoProxy.update_config(回写):脚本配置路径不存在时抛异常;File 分支还缺目标目录,copy不像copytree会自建目录,会再抛一次。ScriptConfig.final_task(回写):先rmtree掉 MAS 侧副本再拷,源不存在时异常发生在清空之后,用户会以为配置丢了(4Y / 4W)。三处统一成「先判源存在,不存在就跳过并给一条可读 warning」,与
ScriptConfig.set_general同口径。验证
swap_in_dir的RuntimeError与shutil的FileNotFoundError,修复后都是 warning + 跳过python -m pytest tests/task -q:804 passed / 17 failed,与改前基线逐条一致(既有失败,与本次无关)python -m pytest tests --collect-only -q:1406 collected,exit 0ruff check/ruff format --check:通过tests/AGENTS.md的 bug 边界规则不入库Sourcery 摘要
通过跳过配置同步而不是引发文件错误,优雅地处理缺失的通用脚本配置路径。
错误修复:
Original summary in English
Sourcery 摘要
通过跳过同步并保留任务执行,优雅地处理缺失的 general-script 配置路径。
错误修复:
杂项:
Original summary in English
Sourcery 总结
通过跳过同步并允许任务继续执行,同时提供清晰的警告,优雅地处理缺失的通用脚本配置路径。
错误修复:
增强功能:
维护工作:
Original summary in English
Sourcery 摘要
优雅地处理缺失的通用脚本配置路径,使任务在出现警告的情况下跳过配置同步,而不是失败。
错误修复:
改进:
杂项:
Original summary in English
Summary by Sourcery
Handle missing general-script configuration paths gracefully so tasks skip configuration synchronization with a warning instead of failing.
Bug Fixes:
Enhancements:
Chores: