fix(oknte): 修复脚本配置目录含只读文件时备份与复原失败 - #456
Open
qiyinxi wants to merge 1 commit into
Open
Conversation
用户的 ok-nte 配置目录里带 .git，而 git 的 pack 对象在 Windows 上是只读的。 `shutil.rmtree(..., ignore_errors=True)` 删不掉只读文件且静默跳过，残留文件 让随后的 `copytree(..., dirs_exist_ok=True)` 覆盖时抛 PermissionError。 失败沿 prepare -> final_task -> on_crash 级联：后两者的收尾清理走同一个 `_restore_script_config_from_temp`，于是同一个错误连抛三次，最终裹进 ExceptionGroup 以「Task exception was never retrieved」落地，用户的脚本配置 既没复原，错误也没抛到界面上。 - 新增 `app/utils/io.force_rmtree`：遇到只读文件先清除只读位再重试，仍失败 的条目按原 ignore_errors 语义忽略 - OK-NTE 的备份与复原路径改用该函数 - `_restore_script_config_from_temp` 自身兜住异常并降级为告警，收尾清理失败 不再掩盖任务本身的异常，临时目录清理移入 finally Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
审查者指南本 PR 针对 Windows 上 OK-NTE 配置目录含只读 具备容错能力的 OK-NTE 配置备份与复原时序图sequenceDiagram
participant Task as OKNTE_Task
participant Temp as Temp_Config
participant IO as force_rmtree
participant Config as Script_Config
Task->>IO: force_rmtree(Temp)
IO->>IO: shutil.rmtree(path, onexc=_retry_without_readonly)
IO-->>Task: Temp directory removed
Task->>Temp: copytree(Script_Config, Temp)
Task->>Config: Run task
Task->>Config: _restore_script_config_from_temp()
alt Folder with original config
Task->>IO: force_rmtree(tmp_dst)
Task->>Temp: copytree(Temp, tmp_dst, dirs_exist_ok=True)
Task->>IO: force_rmtree(Script_Config)
Task->>Config: tmp_dst.rename(Script_Config)
else No original folder
Task->>IO: force_rmtree(Script_Config)
end
opt Restore fails
Task->>Task: logger.warning(...)
end
Task->>IO: force_rmtree(Temp)
OK-NTE 清理失败隔离流程图flowchart TD
A[prepare] --> B["force_rmtree(temp_path)"]
B --> C[Backup script configuration]
C --> D[Run OK-NTE task]
D --> E["_restore_script_config_from_temp"]
E --> F{Restore succeeds?}
F -->|Yes| G["force_rmtree(temp_path) in finally"]
F -->|No| H["logger.warning(...)"]
H --> G
G --> I[Continue task cleanup and state updates]
文件级变更
提示与命令使用 Sourcery
自定义使用体验访问你的控制面板以：
获取帮助Original review guide in EnglishReviewer's Guide本 PR 针对 Windows 上 OK-NTE 配置目录含只读 Sequence diagram for resilient OK-NTE configuration backup and restoresequenceDiagram
participant Task as OKNTE_Task
participant Temp as Temp_Config
participant IO as force_rmtree
participant Config as Script_Config
Task->>IO: force_rmtree(Temp)
IO->>IO: shutil.rmtree(path, onexc=_retry_without_readonly)
IO-->>Task: Temp directory removed
Task->>Temp: copytree(Script_Config, Temp)
Task->>Config: Run task
Task->>Config: _restore_script_config_from_temp()
alt Folder with original config
Task->>IO: force_rmtree(tmp_dst)
Task->>Temp: copytree(Temp, tmp_dst, dirs_exist_ok=True)
Task->>IO: force_rmtree(Script_Config)
Task->>Config: tmp_dst.rename(Script_Config)
else No original folder
Task->>IO: force_rmtree(Script_Config)
end
opt Restore fails
Task->>Task: logger.warning(...)
end
Task->>IO: force_rmtree(Temp)
Flow diagram for isolated OK-NTE cleanup failuresflowchart TD
A[prepare] --> B["force_rmtree(temp_path)"]
B --> C[Backup script configuration]
C --> D[Run OK-NTE task]
D --> E["_restore_script_config_from_temp"]
E --> F{Restore succeeds?}
F -->|Yes| G["force_rmtree(temp_path) in finally"]
F -->|No| H["logger.warning(...)"]
H --> G
G --> I[Continue task cleanup and state updates]
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.
嘿——我发现了 2 个问题
面向 AI Agent 的提示
请处理这次代码审查中的评论：
## 单独评论
### 评论 1
<location path="app/utils/io.py" line_range="104" />
<code_context>
+ func(target)
+
+ with suppress(FileNotFoundError):
+ shutil.rmtree(path, onexc=_retry_without_readonly)
+
+
</code_context>
<issue_to_address>
**issue (bug_risk)：** 在 Python 3.11 上，`shutil.rmtree(..., onexc=...)` 会引发 `TypeError`，因为 `onexc` 参数是在 Python 3.12 中才引入的。仓库的 CI 明确运行 Python 3.11，因此在该环境中，OK-NTE 准备工作会在创建临时目录之前失败。
**触发条件：** 应用或测试套件运行在 Python 3.11 或更早版本上时。
**建议修复：** 对 Python 3.11 使用版本兼容的 `onerror` 回调，或者仅在支持时有条件地选择 `onexc`。
```suggestion
shutil.rmtree(path, onerror=_retry_without_readonly)
```
</issue_to_address>
### 评论 2
<location path="tests/tools/test_io_force_rmtree.py" line_range="17-24" />
<code_context>
+ nested.mkdir(parents=True)
+ readonly = nested / "pack-0001.idx"
+ readonly.write_bytes(b"payload")
+ os.chmod(readonly, stat.S_IREAD)
+ return root, readonly
+
+ def test_removes_tree_containing_readonly_file(self):
+ root, readonly = self._make_tree_with_readonly_file()
+ self.addCleanup(self._cleanup, root, readonly)
+
+ force_rmtree(root)
+
+ self.assertFalse(root.exists())
</code_context>
<issue_to_address>
**issue (testing)：** 在 POSIX 系统上，只读文件测试并未触发只读重试路径：当父目录仍可写时，将文件设为只读并不会阻止其被删除。因此，即使 `_retry_without_readonly` 已损坏，该测试仍会通过，导致 Windows 特有的行为未得到验证。
**触发条件：** 测试套件在 Linux 或其他 POSIX 平台上运行时。
**建议修复：** 模拟失败的 `rmtree` 回调，或在 Windows 上运行该场景，并断言是否调用了 chmod-and-retry 路径。
</issue_to_address>Sourcery 评估
需要人工审查。 需要先处理 2 个发现；此外，新的清理路径会强制删除脚本配置目录，而恢复路径在复制或重命名失败且异常被吞掉时，可能导致线上配置被删除或仅部分恢复。恢复此 PR 可以阻止未来再次发生，但无法恢复已经被删除或覆盖的配置文件。
阻塞性发现：app/utils/io.py:104、tests/tools/test_io_force_rmtree.py:24
帮助我变得更有用！请在每条评论上点击 👍 或 👎，我会利用反馈来改进审查结果。
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/utils/io.py" line_range="104" />
<code_context>
+ func(target)
+
+ with suppress(FileNotFoundError):
+ shutil.rmtree(path, onexc=_retry_without_readonly)
+
+
</code_context>
<issue_to_address>
**issue (bug_risk):** `shutil.rmtree(..., onexc=...)` raises `TypeError` on Python 3.11 because the `onexc` parameter was introduced in Python 3.12. The repository's CI explicitly runs Python 3.11, so OK-NTE preparation fails before the temporary directory is created in that environment.
**Triggers:** When the application or test suite runs under Python 3.11 or earlier.
**Suggested fix:** Use the version-compatible `onerror` callback for Python 3.11, or conditionally select `onexc` only when supported.
```suggestion
shutil.rmtree(path, onerror=_retry_without_readonly)
```
</issue_to_address>
### Comment 2
<location path="tests/tools/test_io_force_rmtree.py" line_range="17-24" />
<code_context>
+ nested.mkdir(parents=True)
+ readonly = nested / "pack-0001.idx"
+ readonly.write_bytes(b"payload")
+ os.chmod(readonly, stat.S_IREAD)
+ return root, readonly
+
+ def test_removes_tree_containing_readonly_file(self):
+ root, readonly = self._make_tree_with_readonly_file()
+ self.addCleanup(self._cleanup, root, readonly)
+
+ force_rmtree(root)
+
+ self.assertFalse(root.exists())
</code_context>
<issue_to_address>
**issue (testing):** The read-only-file test does not exercise the read-only retry path on POSIX systems: making a file read-only does not prevent its removal when its parent directory remains writable. The test therefore passes even if `_retry_without_readonly` is broken, leaving the Windows-specific behavior unverified.
**Triggers:** When the test suite runs on Linux or another POSIX platform.
**Suggested fix:** Mock the failing `rmtree` callback or run the scenario on Windows, and assert that the chmod-and-retry path is invoked.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 2 findings to address first, and the new cleanup path force-deletes script configuration trees and the restore path can leave the live configuration removed or partially restored if copying or renaming fails, while the exception is swallowed. Reverting the PR would stop future occurrences but would not recover configuration files already deleted or overwritten.
Blocking findings: app/utils/io.py:104, tests/tools/test_io_force_rmtree.py:24
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| func(target) | ||
|
|
||
| with suppress(FileNotFoundError): | ||
| shutil.rmtree(path, onexc=_retry_without_readonly) |
There was a problem hiding this comment.
issue (bug_risk)： 在 Python 3.11 上，shutil.rmtree(..., onexc=...) 会引发 TypeError，因为 onexc 参数是在 Python 3.12 中才引入的。仓库的 CI 明确运行 Python 3.11，因此在该环境中，OK-NTE 准备工作会在创建临时目录之前失败。
触发条件： 应用或测试套件运行在 Python 3.11 或更早版本上时。
建议修复： 对 Python 3.11 使用版本兼容的 onerror 回调，或者仅在支持时有条件地选择 onexc。
Suggested change
| shutil.rmtree(path, onexc=_retry_without_readonly) | |
| shutil.rmtree(path, onerror=_retry_without_readonly) |
Original comment in English
issue (bug_risk): shutil.rmtree(..., onexc=...) raises TypeError on Python 3.11 because the onexc parameter was introduced in Python 3.12. The repository's CI explicitly runs Python 3.11, so OK-NTE preparation fails before the temporary directory is created in that environment.
Triggers: When the application or test suite runs under Python 3.11 or earlier.
Suggested fix: Use the version-compatible onerror callback for Python 3.11, or conditionally select onexc only when supported.
Suggested change
| shutil.rmtree(path, onexc=_retry_without_readonly) | |
| shutil.rmtree(path, onerror=_retry_without_readonly) |
Comment on lines
+17
to
+24
| os.chmod(readonly, stat.S_IREAD) | ||
| return root, readonly | ||
|
|
||
| def test_removes_tree_containing_readonly_file(self): | ||
| root, readonly = self._make_tree_with_readonly_file() | ||
| self.addCleanup(self._cleanup, root, readonly) | ||
|
|
||
| force_rmtree(root) |
There was a problem hiding this comment.
issue (testing)： 在 POSIX 系统上，只读文件测试并未触发只读重试路径：当父目录仍可写时，将文件设为只读并不会阻止其被删除。因此，即使 _retry_without_readonly 已损坏，该测试仍会通过，导致 Windows 特有的行为未得到验证。
触发条件： 测试套件在 Linux 或其他 POSIX 平台上运行时。
建议修复： 模拟失败的 rmtree 回调，或在 Windows 上运行该场景，并断言是否调用了 chmod-and-retry 路径。
Original comment in English
issue (testing): The read-only-file test does not exercise the read-only retry path on POSIX systems: making a file read-only does not prevent its removal when its parent directory remains writable. The test therefore passes even if _retry_without_readonly is broken, leaving the Windows-specific behavior unverified.
Triggers: When the test suite runs on Linux or another POSIX platform.
Suggested fix: Mock the failing rmtree callback or run the scenario on Windows, and assert that the chmod-and-retry path is invoked.
This was referenced Sep 4, 2026
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
2 participants
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.
摘要
.git，而 git 的 pack 对象在 Windows 上是只读的。shutil.rmtree(..., ignore_errors=True)删不掉只读文件且静默跳过，残留文件让随后的copytree(..., dirs_exist_ok=True)覆盖时抛PermissionError。prepare→final_task→on_crash级联：后两者的收尾清理走同一个_restore_script_config_from_temp，同一个错误连抛三次，最终裹进ExceptionGroup以「Task exception was never retrieved」落地——用户的脚本配置既没复原，错误也没抛到界面上。app/utils/io.force_rmtree（清除只读位后重试，仍失败的条目按原ignore_errors语义忽略）；_restore_script_config_from_temp自身兜住异常并降级为告警，临时目录清理移入finally。AUTO-MAS-BACKEND-2H，7 天 69 次，releaseauto-mas@v5.4.0-beta.8。检查
tests/tools/test_io_force_rmtree.py：2 个用例通过（含只读文件树的删除）python -m pytest tests --collect-only -q：收集 166 个，退出码 0ruff format无残留改动.git的 OK-NTE 配置目录跑一次 AutoProxy，确认备份、复原与异常路径都不再报错未一并处理
app/task/general/manager.py:149与app/task/general/AutoProxy.py:547是同一个rmtree(ignore_errors=True)+copytree组合，可能存在同类问题。本 PR 只修 OK-NTE 这条线上报的路径，通用脚本那条线是否要一并换成force_rmtree请维护者定夺。Sourcery 摘要
使 OK-NTE 配置备份和恢复能够适应只读文件，同时保留任务错误处理和清理逻辑。
错误修复：
增强功能：
测试：
Original summary in English
Summary by Sourcery
Make OK-NTE configuration backup and restoration resilient to read-only files while preserving task error handling and cleanup.
Bug Fixes:
Enhancements:
Tests: