Code Review：fix(settings): preserve preset agents when saving custom agents (#1908)

变更概述

本 PR 修复了 LocalAgents 组件中一个数据丢失 bug：原 CustomAcpAgent 组件在保存时只写回过滤后的 customAgents，导致 preset 配置被覆盖清空。PR 同时将孤立的 InlineAgentEditor 集成到 LocalAgents tab，并为所有写操作统一改用"先读全量、再写回"模式。变更涉及 LocalAgents.tsx 和对应的 DOM 测试文件。

方案评估

结论：✅ 方案合理

根本原因分析准确，修复直接。"先读全量 config、再追加/更新/删除、再写回"的模式能可靠防止 preset 数据丢失，且与项目现有 ConfigStorage 用法一致。将 InlineAgentEditor 集成到同一组件是合理的内聚化改动，无过度工程化问题。

问题清单

🟡 MEDIUM — 关键测试用例未实际验证 preset 保留逻辑

文件：tests/unit/LocalAgents.dom.test.tsx，第 210–247 行

问题代码：

it('saves custom agent while preserving preset agents in config', async () => {
  // ...
  // Click add button
  const addButton = screen.getByText('settings.addCustomAgentTitle');
  await act(async () => {
    fireEvent.click(addButton);
  });

  // The critical assertion is in the save callback logic:
  // ...
  expect(mockConfigGet).toHaveBeenCalledWith('acp.customAgents');
});

问题说明：测试名称声称验证"保存时保留 preset agents"，但实际断言 expect(mockConfigGet).toHaveBeenCalledWith('acp.customAgents') 在组件挂载时（loadCustomAgents 的 useEffect）就已成立，与点击 Add 按钮无关。测试注释也坦承"cannot directly trigger save"——即核心修复逻辑（handleSaveAgent 写回时包含 preset）实际上未被测试到。若该函数发生回归，现有测试仍会通过。

修复建议：通过触发 InlineAgentEditor 的 onSave prop，断言 ConfigStorage.set 被调用时的参数包含 preset：

it('saves custom agent while preserving preset agents in config', async () => {
  const presetAgent = { id: 'cowork', name: 'Cowork', isPreset: true, enabled: true, context: 'test' };
  mockConfigGet.mockResolvedValue([presetAgent]);

  await act(async () => {
    render(<LocalAgents />);
  });

  // Click add to show InlineAgentEditor, fill fields, submit
  // ...

  // Core assertion: ConfigStorage.set must be called with presets preserved
  expect(mockConfigSet).toHaveBeenCalledWith(
    'acp.customAgents',
    expect.arrayContaining([
      expect.objectContaining({ id: 'cowork', isPreset: true }),
      expect.objectContaining({ name: 'New Agent' }),
    ])
  );
});

🔵 LOW — Oxlint no-map-spread 警告与项目不可变性原则冲突

文件：src/renderer/pages/settings/AgentSettings/LocalAgents.tsx，第 119 行

问题代码：

const updatedAgents = allAgents.map((a) => (a.id === agent.id ? { ...a, enabled } : a));

问题说明：Oxlint 的 no-map-spread 规则对此行报 warning，建议改用 Object.assign 以避免每次迭代分配新对象。但项目编码规范明确要求不可变性（"NEVER mutate"），而 Object.assign 的第一个参数会被原地修改，与规范相悖。两者存在配置冲突。

修复建议：在 .oxlintrc.json 中显式关闭此规则并说明原因，或保持现状并接受该 warning。

汇总

结论

⚠️ 有条件批准 — 修复逻辑本身正确，但核心 bug 修复缺乏有效的单元测试覆盖，建议补全后合并。

本报告由本地 pr-review skill 生成，包含完整项目上下文，无截断限制。

CONCLUSION: CONDITIONAL
IS_CRITICAL_PATH: false
PR_NUMBER: 1908

PR Fix 验证报告

原始 PR: #1908
修复方式: 直接推送到 fix/issue-1898

总结： ✅ 已修复 1 个 | 🔵 LOW 问题已跳过

🔵 LOW 级别问题（no-map-spread lint 警告）已跳过（不阻塞合并）。