perf: 自动清理运行日志支持删除更多日志 - #334
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
嘿——我发现了 3 个问题
面向 AI Agent 的提示
请处理本次代码审查中的评论：
## 单独评论
### 评论 1
<location path="src-tauri/src/commands/file_ops.rs" line_range="388-389" />
<code_context>
}
- Ok(deleted)
+ if exports_dir.is_dir() {
+ deleted = deleted.saturating_add(clear_dir_contents(exports_dir));
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk)：** `clear_log_dirs` 会删除 `debug_exports` 下的每个条目，但没有与 `export_logs_blocking` 进行协调。如果用户在导出正在写入时清除日志，`remove_dir_all` 会删除当前活动的导出目录，随后导出器在创建或完成 ZIP 时会失败，并可能返回错误或指向已删除文件的路径。
**触发条件：** 手动清除日志的操作与正在进行的日志导出重叠时。
**建议修复：** 使用共享锁串行化清除和导出操作，或者避免在导出完成前删除当前活动的导出目录。
</issue_to_address>
### 评论 2
<location path="src-tauri/src/commands/file_ops.rs" line_range="349-351" />
<code_context>
+ for entry in entries.flatten() {
let path = entry.path();
+
+ if path.is_dir() {
+ deleted =
+ deleted.saturating_add(remove_log_files_recursively(&path, exclude_file_name));
+ continue;
+ }
</code_context>
<issue_to_address>
**🚨 issue (security)：** `path.is_dir()` 会跟随目录符号链接，因此 `debug` 内部的符号链接会导致递归遍历进入符号链接的目标，并删除应用程序 debug 目录之外匹配的 `.log` 文件。
**触发条件：** `debug` 下存在意外的或用户创建的目录符号链接时。
**建议修复：** 使用支持符号链接识别的元数据检查条目，并跳过符号链接，而不是递归进入其中。
```suggestion
let Ok(metadata) = std::fs::symlink_metadata(&path) else {
continue;
};
if metadata.file_type().is_symlink() {
continue;
}
if metadata.is_dir() {
deleted =
deleted.saturating_add(remove_log_files_recursively(&path, exclude_file_name));
```
</issue_to_address>
### 评论 3
<location path="src-tauri/src/commands/file_ops.rs" line_range="363-365" />
<code_context>
- }
-
- if exclude_file_name.as_deref() == Some(name) {
+ if !name.ends_with(".log") || exclude_file_name == Some(name) {
continue;
}
</code_context>
<issue_to_address>
**nitpick (bug_risk)：** 排除逻辑仅基于文件名，因此与活动根日志文件同名的嵌套日志也会被跳过。因此，当两个目录包含同名日志文件时，递归清理会留下无关的过期日志。
**触发条件：** 嵌套的 debug 目录包含一个与当前会话日志同名但实际不同的日志文件时。
**建议修复：** 传递并比较活动日志的规范路径/完整路径，或者将基于文件名的排除限制在已知的活动日志位置。
</issue_to_address>帮助我变得更有用！请在每条评论上点击 👍 或 👎，我会利用反馈来改进审查结果。
Original comment in English
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src-tauri/src/commands/file_ops.rs" line_range="388-389" />
<code_context>
}
- Ok(deleted)
+ if exports_dir.is_dir() {
+ deleted = deleted.saturating_add(clear_dir_contents(exports_dir));
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** `clear_log_dirs` deletes every entry under `debug_exports` without coordinating with `export_logs_blocking`. If the user clears logs while an export is being written, `remove_dir_all` removes the active export directory and the exporter then fails while creating or finishing a ZIP, potentially returning an error or a path to a deleted file.
**Triggers:** When a manual log clear overlaps an in-progress log export.
**Suggested fix:** Serialize clearing and exporting with a shared lock, or avoid deleting the currently active export directory until the export completes.
</issue_to_address>
### Comment 2
<location path="src-tauri/src/commands/file_ops.rs" line_range="349-351" />
<code_context>
+ for entry in entries.flatten() {
let path = entry.path();
+
+ if path.is_dir() {
+ deleted =
+ deleted.saturating_add(remove_log_files_recursively(&path, exclude_file_name));
+ continue;
+ }
</code_context>
<issue_to_address>
**🚨 issue (security):** `path.is_dir()` follows directory symlinks, so a symlink inside `debug` causes the recursive traversal to enter the symlink target and delete matching `.log` files outside the application's debug directory.
**Triggers:** When an unexpected or user-created directory symlink exists below `debug`.
**Suggested fix:** Inspect the entry with symlink-aware metadata and skip symlinks instead of recursing into them.
```suggestion
let Ok(metadata) = std::fs::symlink_metadata(&path) else {
continue;
};
if metadata.file_type().is_symlink() {
continue;
}
if metadata.is_dir() {
deleted =
deleted.saturating_add(remove_log_files_recursively(&path, exclude_file_name));
```
</issue_to_address>
### Comment 3
<location path="src-tauri/src/commands/file_ops.rs" line_range="363-365" />
<code_context>
- }
-
- if exclude_file_name.as_deref() == Some(name) {
+ if !name.ends_with(".log") || exclude_file_name == Some(name) {
continue;
}
</code_context>
<issue_to_address>
**nitpick (bug_risk):** The exclusion is based only on the basename, so a nested log with the same filename as the active root log is skipped as well. Recursive cleanup therefore leaves unrelated stale logs whenever two directories contain the same log filename.
**Triggers:** When a nested debug directory contains a different log file with the same basename as the current session's log.
**Suggested fix:** Pass and compare the active log's canonical/full path, or restrict the basename exclusion to the known active-log location.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if exports_dir.is_dir() { | ||
| deleted = deleted.saturating_add(clear_dir_contents(exports_dir)); |
There was a problem hiding this comment.
issue (bug_risk)： clear_log_dirs 会删除 debug_exports 下的每个条目，但没有与 export_logs_blocking 进行协调。如果用户在导出正在写入时清除日志，remove_dir_all 会删除当前活动的导出目录，随后导出器在创建或完成 ZIP 时会失败，并可能返回错误或指向已删除文件的路径。
触发条件： 手动清除日志的操作与正在进行的日志导出重叠时。
建议修复： 使用共享锁串行化清除和导出操作，或者避免在导出完成前删除当前活动的导出目录。
Original comment in English
issue (bug_risk): clear_log_dirs deletes every entry under debug_exports without coordinating with export_logs_blocking. If the user clears logs while an export is being written, remove_dir_all removes the active export directory and the exporter then fails while creating or finishing a ZIP, potentially returning an error or a path to a deleted file.
Triggers: When a manual log clear overlaps an in-progress log export.
Suggested fix: Serialize clearing and exporting with a shared lock, or avoid deleting the currently active export directory until the export completes.
| if path.is_dir() { | ||
| deleted = | ||
| deleted.saturating_add(remove_log_files_recursively(&path, exclude_file_name)); |
There was a problem hiding this comment.
🚨 issue (security)： path.is_dir() 会跟随目录符号链接，因此 debug 内部的符号链接会导致递归遍历进入符号链接的目标，并删除应用程序 debug 目录之外匹配的 .log 文件。
触发条件： debug 下存在意外的或用户创建的目录符号链接时。
建议修复： 使用支持符号链接识别的元数据检查条目，并跳过符号链接，而不是递归进入其中。
| if path.is_dir() { | |
| deleted = | |
| deleted.saturating_add(remove_log_files_recursively(&path, exclude_file_name)); | |
| let Ok(metadata) = std::fs::symlink_metadata(&path) else { | |
| continue; | |
| }; | |
| if metadata.file_type().is_symlink() { | |
| continue; | |
| } | |
| if metadata.is_dir() { | |
| deleted = | |
| deleted.saturating_add(remove_log_files_recursively(&path, exclude_file_name)); |
Original comment in English
🚨 issue (security): path.is_dir() follows directory symlinks, so a symlink inside debug causes the recursive traversal to enter the symlink target and delete matching .log files outside the application's debug directory.
Triggers: When an unexpected or user-created directory symlink exists below debug.
Suggested fix: Inspect the entry with symlink-aware metadata and skip symlinks instead of recursing into them.
| if path.is_dir() { | |
| deleted = | |
| deleted.saturating_add(remove_log_files_recursively(&path, exclude_file_name)); | |
| let Ok(metadata) = std::fs::symlink_metadata(&path) else { | |
| continue; | |
| }; | |
| if metadata.file_type().is_symlink() { | |
| continue; | |
| } | |
| if metadata.is_dir() { | |
| deleted = | |
| deleted.saturating_add(remove_log_files_recursively(&path, exclude_file_name)); |
| if !name.ends_with(".log") || exclude_file_name == Some(name) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
nitpick (bug_risk)： 排除逻辑仅基于文件名，因此与活动根日志文件同名的嵌套日志也会被跳过。因此，当两个目录包含同名日志文件时，递归清理会留下无关的过期日志。
触发条件： 嵌套的 debug 目录包含一个与当前会话日志同名但实际不同的日志文件时。
建议修复： 传递并比较活动日志的规范路径/完整路径，或者将基于文件名的排除限制在已知的活动日志位置。
Original comment in English
nitpick (bug_risk): The exclusion is based only on the basename, so a nested log with the same filename as the active root log is skipped as well. Recursive cleanup therefore leaves unrelated stale logs whenever two directories contain the same log filename.
Triggers: When a nested debug directory contains a different log file with the same basename as the current session's log.
Suggested fix: Pass and compare the active log's canonical/full path, or restrict the basename exclusion to the known active-log location.
1 participant
解决问题，mxu开启自动删除日志选项后
1、cpp-algo中日志没有被删除
2、debug_exports中日志没有被删除
实现方式
1、debug目录下删除*.log改成递归子目录删除
2、删除debug_exports导出的*.zip日志
close MaaEnd/MaaEnd#5096
opus5.0