diff --git a/docs/bug-analysis-report.md b/docs/bug-analysis-report.md new file mode 100644 index 0000000..2d9f1ec --- /dev/null +++ b/docs/bug-analysis-report.md @@ -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 = 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, Vec), 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 | 用户体验小瑕疵 | diff --git a/e2e/mocks/ipc.ts b/e2e/mocks/ipc.ts index e553e99..1a20c58 100644 --- a/e2e/mocks/ipc.ts +++ b/e2e/mocks/ipc.ts @@ -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; diff --git a/src-tauri/src/commands/system.rs b/src-tauri/src/commands/system.rs index 0acd2c3..2524c59 100644 --- a/src-tauri/src/commands/system.rs +++ b/src-tauri/src/commands/system.rs @@ -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(); } diff --git a/src/core/undo-redo.ts b/src/core/undo-redo.ts index 15ecdb8..edc54e2 100644 --- a/src/core/undo-redo.ts +++ b/src/core/undo-redo.ts @@ -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]; diff --git a/src/hooks/use-app-actions.ts b/src/hooks/use-app-actions.ts index 5330797..5690c02 100644 --- a/src/hooks/use-app-actions.ts +++ b/src/hooks/use-app-actions.ts @@ -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]); diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index 6e3be2b..f7b551b 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -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)", diff --git a/src/i18n/locales/zh-CN.json b/src/i18n/locales/zh-CN.json index 0b3dce9..0033eca 100644 --- a/src/i18n/locales/zh-CN.json +++ b/src/i18n/locales/zh-CN.json @@ -38,6 +38,7 @@ "readonly": "只读模式 — 需要管理员权限才能编辑", "saving": "正在保存...", "saved": "保存成功", + "saved_without_backup": "保存成功（备份失败）", "error": "操作失败", "warning_backup": "备份创建失败，保存将继续但不生成备份", "deleted": "已删除 {{count}} 个路径", diff --git a/src/store/app-store.ts b/src/store/app-store.ts index e8ba4d0..83e9aa9 100644 --- a/src/store/app-store.ts +++ b/src/store/app-store.ts @@ -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((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((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((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((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((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 }); } }, diff --git a/tests/unit/app-store.test.ts b/tests/unit/app-store.test.ts index a15eb33..dc00856 100644 --- a/tests/unit/app-store.test.ts +++ b/tests/unit/app-store.test.ts @@ -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