fix: 修复 5 个 bug + 备份警告丢失

- BUG 1: undo/redo 后持久化 disabled 状态到 disabled.json
- BUG 2: expand_env_vars 增加缓冲区不足检测(result > required)
- BUG 3: E2E mock load_disabled_state 返回格式从对象改为数组
- BUG 4: 双 hive 保存失败时同时显示两个错误原因
- BUG 5: 导入 both 合并为单条 undo 记录(新增 IMPORT_BOTH 操作类型)
- 备份失败后保存成功时显示"保存成功(备份失败)"而非覆盖警告

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
2026-05-28 00:38:32 +08:00
parent d7bc752b84
commit 3aed03f599
9 changed files with 296 additions and 12 deletions
+233
View File
@@ -0,0 +1,233 @@
# PathEditor v4.2 代码审查 — 隐含 Bug 分析报告
> 审查日期:2026-05-28 | 审查范围:全部前端核心模块 + Rust 后端 + E2E 测试
---
## BUG 1 [HIGH] — undo/redo TOGGLE 后不持久化 disabled 状态
### 位置
`src/store/app-store.ts``togglePath()` (L213-237) vs `undo()` (L239-248) / `redo()` (L251-260)
### 现象
1. 用户勾选复选框禁用一个路径 → `togglePath` 立即调用 `invoke('save_disabled_state', ...)` 写入 `disabled.json`
2. 用户按 Ctrl+Z 撤销 → `undo()` 恢复内存中的 `enabled` 状态,但**不调用** `save_disabled_state`
3. `disabled.json` 中该路径仍标记为 disabled
4. 下次启动 `loadPaths` 时,从 `disabled.json` 读到的是旧数据,路径又被标记为 disabled
### 根因
`undo()``redo()` 中处理 TOGGLE 时,只更新了 Zustand store 中的 `sysPaths`/`userPaths`,没有像 `togglePath` 那样同步持久化 disabled 状态。
```typescript
// togglePath — 有持久化
invoke('save_disabled_state', { system: sysDisabled, user: usrDisabled }).catch(() => {});
// undo/redo — 没有持久化!
set({
sysPaths: result[0], userPaths: result[1], selectedIndices: [],
isModified: !(arraysEqual(result[0], _savedSys) && arraysEqual(result[1], _savedUser)),
});
```
### 影响
- 撤销 toggle 后刷新/重启应用,disabled 状态恢复为撤销前的值
- 数据和 UI 展示不一致
### 修复方向
`undo()``redo()` 中,检测当前/上一条记录是否为 TOGGLE 类型,如果是则同步调用 `save_disabled_state`。或者更通用的方案:在 `set()` 之后统一检查 disabled 状态是否有变化并持久化。
---
## BUG 2 [MEDIUM] — `expand_env_vars` 未检测缓冲区截断
### 位置
`src-tauri/src/commands/system.rs:40-58``expand_env_vars()`
### 现象
Windows API `ExpandEnvironmentStringsW` 的行为:
- 第 1 次调用(`lpDst = NULL, nSize = 0`):返回所需缓冲区大小(TCHAR 数)
- 第 2 次调用(提供缓冲区):若缓冲区足够,返回写入的 TCHAR 数(≤ nSize);**若缓冲区不足,返回所需大小(> nSize),而非 0**
当前代码:
```rust
let required = unsafe {
ExpandEnvironmentStringsW(wide_path.as_ptr(), std::ptr::null_mut(), 0)
};
// ...
let mut buffer: Vec<u16> = vec![0; required as usize];
let result = unsafe {
ExpandEnvironmentStringsW(wide_path.as_ptr(), buffer.as_mut_ptr(), required)
};
if result == 0 {
// 仅处理了 API 失败
log::warn!("expand_env_vars: 展开失败, 返回原始路径: {path}");
return path.to_string();
}
// result > 0 但可能 > required(截断!)— 未检测
```
### 触发条件(低概率但真实存在)
环境变量在两次 `ExpandEnvironmentStringsW` 调用之间被修改:
1. 第 1 次调用查询到 `%VAR%` 展开后为 50 字符,`required = 51`(含 null
2. 在第 1 次和第 2 次调用之间,另一个进程修改了 `%VAR%` 使其变为 200 字符
3. 第 2 次调用时 51 大小的缓冲区不够,API 返回 201
4. `result = 201 > 51``result != 0`,代码未进入错误分支
5. 函数返回截断的不完整路径
### 修复方向
```rust
if result == 0 || result > required {
log::warn!("expand_env_vars: 展开失败或缓冲区不足, 返回原始路径: {path}");
return path.to_string();
}
```
---
## BUG 3 [MEDIUM] — E2E mock `load_disabled_state` 返回格式与 Rust 端不匹配
### 位置
- `e2e/mocks/ipc.ts:9`
- `src-tauri/src/commands/disabled.rs:44``load_disabled_state()`
- `src/store/app-store.ts:275``loadPaths()` 消费端
### 现象
**Rust 端返回类型**`Result<(Vec<String>, Vec<String>), String>`
Tauri 将元组序列化为 JSON 数组:`[["disabled_sys_1"], ["disabled_usr_1"]]`
**Mock 返回**
```javascript
case 'load_disabled_state': return { system: [], user: [] };
// ^^^^^^^^^^^^^^^^^^^^^^^^ 这是一个对象!
```
**前端消费**
```typescript
const result = await invoke<[string[], string[]]>('load_disabled_state');
sysDisabled = result[0]; // 从对象取 → undefined
usrDisabled = result[1]; // 从对象取 → undefined
new Set(sysDisabled); // → TypeError: undefined is not iterable
```
try/catch 捕获了 TypeError`sysDisabled`/`usrDisabled` 保持为初始空数组 `[]`
### 影响
- 生产环境无影响(Rust 端正确返回数组)
- E2E 测试中**disabled state 加载路径完全未被测试**(被 try/catch 静默跳过)
- 如果将来 E2E 测试要覆盖 disabled state 的加载-合并逻辑,会得到错误结果
### 修复方向
```javascript
case 'load_disabled_state': return []; // 返回空元组 [[], []]
// 或者如果要 mock 有禁用路径的场景:
case 'load_disabled_state': return [['C:\\disabled_path'], []];
```
---
## BUG 4 [LOW] — `savePaths` 双 hive 失败时丢失错误原因
### 位置
`src/store/app-store.ts:332-336`
### 现象
```typescript
const reason = (!sysOk && sysResult.status === 'rejected') ? String(sysResult.reason) :
(!userOk && userResult.status === 'rejected') ? String(userResult.reason) : '';
const msg = sysOk ? '用户 PATH 保存失败' :
userOk ? '系统 PATH 保存失败' :
`保存失败: ${reason}`;
```
当**两个 hive 都保存失败**时:
- `sysOk` = false,跳过第 1 个三元分支
- `userOk` = false,跳过第 2 个三元分支
- `reason` 取的是 `sysResult.reason`(第 1 行),`userResult.reason` 被丢弃
- 最终消息:`保存失败: <仅系统 PATH 的错误原因>`
### 修复方向
```typescript
const sysErr = (!sysOk && sysResult.status === 'rejected') ? String(sysResult.reason) : '';
const usrErr = (!userOk && userResult.status === 'rejected') ? String(userResult.reason) : '';
const parts = [sysErr, usrErr].filter(Boolean);
const msg = parts.length > 0 ? `保存失败: ${parts.join('; ')}` : '保存失败';
```
或者更清晰地分别显示两个 hive 的状态。
---
## BUG 5 [LOW] — `handleImportSelect` 导入 both 产生两条 undo 记录
### 位置
`src/hooks/use-app-actions.ts:160-165`
### 现象
```typescript
const handleImportSelect = useCallback((target: 'system' | 'user' | 'both') => {
const { system, user } = dialogs.importDialog;
const flat = flattenImportResult({ system, user }, target);
if (flat.system.length > 0)
useAppStore.getState().replacePaths(TargetType.SYSTEM, flat.system.map(e => e.path));
if (flat.user.length > 0)
useAppStore.getState().replacePaths(TargetType.USER, flat.user.map(e => e.path));
...
}, ...);
```
当用户选择「导入到两者」时:
1. 调用 `replacePaths(TargetType.SYSTEM, ...)` → 创建一条 IMPORT undo 记录
2. 调用 `replacePaths(TargetType.USER, ...)` → 创建另一条 IMPORT undo 记录
用户按一次 Ctrl+Z 只能撤销一个 hive 的导入,需要按两次才能完全撤销。
### 修复方向
`app-store.ts` 中新增一个 `replaceBothPaths` 方法,将 system + user 的替换合并为一条 undo 记录,或者把 `replacePaths` 扩展为支持同时替换两个 hive。
---
## 非 Bug 问题(值得关注但暂不紧急)
| # | 位置 | 描述 |
|---|------|------|
| 1 | `PathTable.tsx:35-36` | `validatedRef` / `expandedRef` 随会话持续增长,不清理。Set 中的 key 是路径字符串,正常使用下数量有限(几十到几百),无实际性能影响 |
| 2 | `use-keyboard.ts:39-54` | 非管理员模式下 Ctrl+Z/Y/N/S/Delete 静默忽略,用户无任何反馈 |
| 3 | `app-store.ts:317` | `backup_registry` 失败时 set statusMessage 为警告,但若后续保存成功,`statusMessage` 被覆盖为「保存成功」,用户看不到备份失败的警告 |
| 4 | `MergePreview.tsx:57` | `key={${source}-${displayIndex}}` — key 由翻译后的 source 字符串和 displayIndex 拼接,理论上在极端场景(同一秒内两次渲染翻译变化)可能重复,实际概率极低 |
| 5 | `system.rs:44-45` | `ExpandEnvironmentStringsW` 返回 0 时只 `log::warn`,可通过 `GetLastError` 获取具体错误码来改进日志 |
| 6 | `disabled.rs:54-56` | `load_disabled_state``content.trim().is_empty()` 检查在 `serde_json::from_str` 之前,空文件返回空数组。但如果文件包含有效 JSON 空对象 `{}`,反序列化为 `DisabledState::default()` 也正确。逻辑完整 |
---
## 修复优先级建议
| 优先级 | Bug | 理由 |
|--------|-----|------|
| **P0** | BUG 1 — undo/redo 不持久化 disabled | 用户可感知的数据不一致 |
| **P1** | BUG 2 — expand_env_vars 截断 | 概率低但后果是静默数据损坏 |
| **P2** | BUG 3 — E2E mock 格式 | 不影响生产,但阻碍 E2E 测试扩展 |
| **P3** | BUG 4 — 双失败错误消息 | 边缘场景的 UX 问题 |
| **P3** | BUG 5 — 导入双 undo | 用户体验小瑕疵 |
+1 -1
View File
@@ -6,7 +6,7 @@ export function createIpcMock() {
case 'check_admin': return true;
case 'load_system_paths': return ['C:\\\\Windows', 'C:\\\\Program Files'];
case 'load_user_paths': return ['C:\\\\Users\\\\me\\\\AppData'];
case 'load_disabled_state': return { system: [], user: [] };
case 'load_disabled_state': return [[], []];
case 'save_system_paths': return undefined;
case 'save_user_paths': return undefined;
case 'save_disabled_state': return undefined;
+2 -2
View File
@@ -53,8 +53,8 @@ pub fn expand_env_vars(path: &str) -> String {
ExpandEnvironmentStringsW(wide_path.as_ptr(), buffer.as_mut_ptr(), required)
};
if result == 0 {
log::warn!("expand_env_vars: 展开失败, 返回原始路径: {path}");
if result == 0 || result > required {
log::warn!("expand_env_vars: 展开失败或缓冲区不足, 返回原始路径: {path}");
return path.to_string();
}
+17 -1
View File
@@ -5,7 +5,7 @@
import type { PathEntry } from './path-entry';
export const OperationType = {
ADD: 0, DELETE: 1, EDIT: 2, MOVE_UP: 3, MOVE_DOWN: 4, CLEAN: 5, CLEAR: 6, IMPORT: 7, TOGGLE: 8,
ADD: 0, DELETE: 1, EDIT: 2, MOVE_UP: 3, MOVE_DOWN: 4, CLEAN: 5, CLEAR: 6, IMPORT: 7, TOGGLE: 8, IMPORT_BOTH: 9,
} as const;
export type OperationType = (typeof OperationType)[keyof typeof OperationType];
@@ -21,6 +21,10 @@ export interface OpRecord {
newPaths: PathEntry[];
/** DELETE 操作专用:被删除的各路径的原始 index(升序) */
indices?: number[];
/** IMPORT_BOTH 专用:用户 hive 的旧路径 */
oldPathsOther?: PathEntry[];
/** IMPORT_BOTH 专用:用户 hive 的新路径 */
newPathsOther?: PathEntry[];
}
const DEFAULT_MAX_SIZE = 50;
@@ -88,6 +92,12 @@ export class UndoRedoManager {
case OperationType.TOGGLE:
target[rec.index] = rec.oldPaths[0];
break;
case OperationType.IMPORT_BOTH:
sys.length = 0;
sys.push(...rec.oldPaths);
user.length = 0;
user.push(...(rec.oldPathsOther || []));
return [sys, user];
}
return [sys, user];
@@ -138,6 +148,12 @@ export class UndoRedoManager {
case OperationType.TOGGLE:
target[rec.index] = rec.newPaths[0];
break;
case OperationType.IMPORT_BOTH:
sys.length = 0;
sys.push(...rec.newPaths);
user.length = 0;
user.push(...(rec.newPathsOther || []));
return [sys, user];
}
return [sys, user];
+6 -2
View File
@@ -160,8 +160,12 @@ export function useAppActions(activeTab: TabId, dialogs: DialogState) {
const handleImportSelect = useCallback((target: 'system' | 'user' | 'both') => {
const { system, user } = dialogs.importDialog;
const flat = flattenImportResult({ system, user }, target);
if (flat.system.length > 0) useAppStore.getState().replacePaths(TargetType.SYSTEM, flat.system.map(e => e.path));
if (flat.user.length > 0) useAppStore.getState().replacePaths(TargetType.USER, flat.user.map(e => e.path));
if (target === 'both' && flat.system.length > 0 && flat.user.length > 0) {
useAppStore.getState().replaceBothPaths(flat.system.map(e => e.path), flat.user.map(e => e.path));
} else {
if (flat.system.length > 0) useAppStore.getState().replacePaths(TargetType.SYSTEM, flat.system.map(e => e.path));
if (flat.user.length > 0) useAppStore.getState().replacePaths(TargetType.USER, flat.user.map(e => e.path));
}
setImportDialog({ open: false, system: [], user: [] });
}, [dialogs.importDialog, setImportDialog]);
+1
View File
@@ -38,6 +38,7 @@
"readonly": "Read-only mode — Administrator privileges required for editing",
"saving": "Saving...",
"saved": "Saved successfully",
"saved_without_backup": "Saved (backup failed)",
"error": "Operation failed",
"warning_backup": "Backup creation failed, save will proceed without backup",
"deleted": "Deleted {{count}} path(s)",
+1
View File
@@ -38,6 +38,7 @@
"readonly": "只读模式 — 需要管理员权限才能编辑",
"saving": "正在保存...",
"saved": "保存成功",
"saved_without_backup": "保存成功(备份失败)",
"error": "操作失败",
"warning_backup": "备份创建失败,保存将继续但不生成备份",
"deleted": "已删除 {{count}} 个路径",
+34 -5
View File
@@ -36,6 +36,7 @@ interface AppState {
moveDown: (index: number, target: TargetType) => void;
cleanPaths: (target: TargetType, validateFn: (p: string) => boolean) => string[];
replacePaths: (target: TargetType, newPaths: string[]) => void;
replaceBothPaths: (sysPaths: string[], userPaths: string[]) => void;
clearPaths: (target: TargetType) => void;
togglePath: (index: number, target: TargetType) => void;
@@ -195,6 +196,20 @@ export const useAppStore = create<AppState>((set, get) => {
markDirty();
},
replaceBothPaths: (sysPaths, userPaths) => {
const state = get();
const sysEntries: PathEntry[] = sysPaths.map(p => ({ path: p, enabled: true }));
const usrEntries: PathEntry[] = userPaths.map(p => ({ path: p, enabled: true }));
state.undoRedo.push({
type: OperationType.IMPORT_BOTH, target: TargetType.SYSTEM, index: 0,
count: sysEntries.length + usrEntries.length,
oldPaths: [...state.sysPaths], newPaths: [...sysEntries],
oldPathsOther: [...state.userPaths], newPathsOther: [...usrEntries],
});
set({ sysPaths: [...sysEntries], userPaths: [...usrEntries], selectedIndices: [] });
markDirty();
},
clearPaths: (target) => {
const state = get();
const list = target === TargetType.SYSTEM ? state.sysPaths : state.userPaths;
@@ -245,6 +260,11 @@ export const useAppStore = create<AppState>((set, get) => {
// 内联计算 isModified 而非调用 markDirty(),避免两次 set() 导致额外渲染
isModified: !(arraysEqual(result[0], _savedSys) && arraysEqual(result[1], _savedUser)),
});
// 同步持久化 disabled 状态,与 togglePath 保持一致
invoke('save_disabled_state', {
system: result[0].filter(e => !e.enabled).map(e => e.path),
user: result[1].filter(e => !e.enabled).map(e => e.path),
}).catch(() => {});
}
},
@@ -257,6 +277,11 @@ export const useAppStore = create<AppState>((set, get) => {
// 内联计算 isModified 而非调用 markDirty(),避免两次 set() 导致额外渲染
isModified: !(arraysEqual(result[0], _savedSys) && arraysEqual(result[1], _savedUser)),
});
// 同步持久化 disabled 状态,与 togglePath 保持一致
invoke('save_disabled_state', {
system: result[0].filter(e => !e.enabled).map(e => e.path),
user: result[1].filter(e => !e.enabled).map(e => e.path),
}).catch(() => {});
}
},
@@ -314,8 +339,9 @@ export const useAppStore = create<AppState>((set, get) => {
}
// 备份当前注册表(保存前备份旧值,失败仅警告不中断)
let backupFailed = false;
await invoke('backup_registry', { customDir: null })
.catch(() => set({ statusMessage: i18n.t('status.warning_backup') }));
.catch(() => { backupFailed = true; });
const [sysResult, userResult] = await Promise.allSettled([
invoke('save_system_paths', { paths: sysPaths }),
@@ -328,11 +354,14 @@ export const useAppStore = create<AppState>((set, get) => {
if (sysOk && userOk) {
invoke('broadcast_env_change').catch(() => {});
const savedSys = [...state.sysPaths], savedUser = [...state.userPaths];
set({ isModified: false, isSaving: false, statusMessage: i18n.t('status.saved'), _savedSys: savedSys, _savedUser: savedUser });
set({ isModified: false, isSaving: false,
statusMessage: backupFailed ? i18n.t('status.saved_without_backup') : i18n.t('status.saved'),
_savedSys: savedSys, _savedUser: savedUser });
} else {
const reason = (!sysOk && sysResult.status === 'rejected') ? String(sysResult.reason) :
(!userOk && userResult.status === 'rejected') ? String(userResult.reason) : '';
const msg = sysOk ? '用户 PATH 保存失败' : userOk ? '系统 PATH 保存失败' : `保存失败: ${reason}`;
const sysErr = (!sysOk && sysResult.status === 'rejected') ? String(sysResult.reason) : '';
const usrErr = (!userOk && userResult.status === 'rejected') ? String(userResult.reason) : '';
const parts = [sysErr, usrErr].filter(Boolean);
const msg = sysOk ? '用户 PATH 保存失败' : userOk ? '系统 PATH 保存失败' : `保存失败: ${parts.join('; ')}`;
set({ isSaving: false, statusMessage: msg });
}
},
+1 -1
View File
@@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
// Mock @tauri-apps/api/core
vi.mock('@tauri-apps/api/core', () => ({
invoke: vi.fn(),
invoke: vi.fn().mockResolvedValue(undefined),
}));
// Mock i18n