refactor(ipc): migrate presenter transport to typed route/event system#1765
Merged
Conversation
Contributor
|
Important Review skippedToo many files! This PR contains 300 files, which is 150 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (300)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
PR #1765 重构代码审查报告1. Guard 清理分析1.1 重构前已存在的 Guard（需要保留）
1.2 重构中新加的 Guard（需要保留）
1.3 建议清理的 Guard
2. 类型一致性检查2.1 Legacy 类型引用问题
建议：这些类型应该被提取到独立的类型文件中，逐步减少对 2.2 新增类型
3. 遗留代码检查3.1 已清理的遗留代码
3.2 仍存在的遗留代码
4. 潜在 Bug 检查4.1 路由注册完整性
4.2 Presenter 初始化顺序
4.3 事件发布/订阅
4.4 错误处理完整性
5. 测试完整性5.1 新测试覆盖
5.2 测试缺失
6. 其他问题6.1 缓存问题
6.2 类型导出冗余
6.3 文档和代码一致性
总结建议
|
Collaborator
Author
补充检查结果基于进一步深入代码审查，以下是针对 5 个关键问题的具体验证结论： 1. llmproviderPresenter 初始化引用 mcpPresenter 问题 ✅ 安全文件: this.llmproviderPresenter = new LLMProviderPresenter(
this.configPresenter,
this.sqlitePresenter,
{
getNpmRegistry: () => this.mcpPresenter.getNpmRegistry?.() ?? null,
getUvRegistry: () => this.mcpPresenter.getUvRegistry?.() ?? null
}
)结论: 当前实现安全。 建议: 考虑在 2. legacy.presenters.d.ts 类型引用分析文件: 仍被引用的类型：
结论: 3. 事件系统两套定义分析 ✅ 互补，非重复文件对比:
结论: 两套系统是互补的：
注意: 4. MIGRATED_RAW_CHANNEL_BASELINE 基线分析
|
| 问题 | 结论 | 优先级 |
|---|---|---|
| llmproviderPresenter 初始化 | 安全但脆弱 | 低 |
| legacy.presenters 类型引用 | 需逐步拆分 | 中 |
| 事件系统两套定义 | 互补但窗口事件有不一致 | 低 |
| MIGRATED_RAW_CHANNEL_BASELINE | 应收紧为 0 | 中 |
| cachedMainKernelRouteRuntime | 当前安全 | 低 |
Collaborator
Author
IPC Migration Code Review整体来看这个重构的迁移很完整，主线逻辑正确。以下是发现的问题和建议： 1. 潜在 Bug / 不合理之处无关键 bug 发现。 核心迁移链路验证通过：
2. Guard 是否还需要保留09eb09e (Guard background exec host from shared logger imports) — 需要保留。 这不是临时兼容代码，而是结构性 guard：
这些 guard 解决的是模块依赖隔离问题，与 IPC 迁移是正交的，迁移完成后仍需保留。 3. 未完成 / 不干净的地方3.1 过时注释 —
|
| 常量 | 行 | 说明 |
|---|---|---|
SETTINGS_EVENTS.CHECK_FOR_UPDATES |
L93 | 无引用 |
SETTINGS_EVENTS.PROVIDER_INSTALL |
L94 | 无引用 |
DEEPLINK_EVENTS.PROTOCOL_RECEIVED |
L118 | 无引用 |
注意：TRAY_EVENTS.CHECK_FOR_UPDATES（L150）仍有使用，不要误删。
3.3 Floating preload 死代码 — onConfigUpdate
floatingButtonAPI.onConfigUpdate 在 preload 中暴露、env.d.ts 中声明，但 floating renderer 中没有任何组件实际订阅它。建议移除或确认是否有计划中的消费者。
3.4 src/renderer/api/index.ts barrel export 不一致
OnboardingClient.ts、StartupClient.ts、ScheduledTasksClient.ts 存在但未从 barrel 文件 re-export，与其他 ~30 个 client 的 export 模式不一致。功能上不影响（调用方用直接路径），但作为清理项可以统一。
4. 总结
迁移本身是完整且正确的：
- 所有 legacy presenter reflection 调用已移除
- 所有
eventBus.sendToRenderer已替换为publishDeepchatEvent - Renderer 全面使用 typed client/event 订阅
- 二级渲染进程（floating/splash/browser-overlay）preload 已强化 payload 验证
- Architecture guard 已扩展覆盖 settings renderer
- Legacy 文件 (
api/legacy/**,ShortcutRuntime,RemoteControlRuntime等) 已完全删除
剩余清理量很小（~10 行），建议在 merge 前一并处理。
Collaborator
Author
PR #1765 重构代码审查报告整体评价本次 PR 是 presenter IPC 迁移计划的一部分，主要完成了以下工作：
整体架构方向正确，代码质量较高。但存在以下需要关注和修复的问题。 严重问题（需要修复）1.
|
| Guard | 状态 | 建议 |
|---|---|---|
architecture-guard.mjs |
需要更新 | MIGRATED_RAW_CHANNEL_BASELINE 基线需重新统计；MAIN_GUARD_PATHS 中 presenter 导入限制需确认是否仍适用 |
check-background-exec-utility-host.mjs |
保留 | 新增且有效，防止 utility process 依赖泄漏 |
eventBus.sendToRenderer / SendTarget |
可标记 deprecated | 仅 publishDeepchatEvent.ts 使用，完成迁移后可移除 |
ipcMain.handle/on in floatingButtonPresenter |
需要迁移 | 唯一未迁移的 presenter，需补全 route 或更新 guard |
ipcMain.on in windowPresenter |
部分迁移 | 生命周期通道建议保留，功能通道建议迁移 |
ipcMain.on in SplashWindowManager |
建议迁移 | 安全敏感流程应通过 route 验证 |
总结
本次重构整体方向正确，架构清晰，测试覆盖良好。主要风险点在于：
floatingButtonPresenter是唯一未迁移到 route 系统的 presenterwindowPresenter和SplashWindowManager中仍有少量直接 IPC 注册architecture-guard.mjs的基线数据需要同步更新以反映当前状态
建议优先修复严重问题 1-3，然后更新 guard 基线，最后考虑清理 eventBus 中的 deprecated API。
zerob13
changed the title
Collaborator
Author
Review follow-up 处理结果已参考几条模型 review 评论并做了两轮小范围修复，避免把启动期/窗口生命周期/floating 专用 IPC 迁移扩大成第二个大重构。 已修复
提交：
未在本轮修复的项及原因
验证
|
Collaborator
Author
Follow-up Review (post-fixup)上一轮提出的 4 项全部已修复（过时注释、死常量、floating 新发现：windowPresenter 中 4 个无发送方的
|
| 行 | Channel | 为什么是死代码 |
|---|---|---|
| 103-109 | 'browser:chrome-height' |
全项目无任何 ipcRenderer.send('browser:chrome-height', ...) 调用。tabPresenter.updateChromeHeight 唯一调用方就是这个死处理器，chrome height 始终走 DEFAULT_CHROME_HEIGHT 兜底。 |
| 111-120 | 'close-floating-window' |
全项目无任何发送方。Floating renderer 不发此事件。 |
| 138-144 | SHORTCUT_EVENTS.GO_SETTINGS |
无 renderer 发送方。shortcutPresenter 通过 eventBus.sendToMain 发送，由 line 129 的 eventBus.on 处理。此 ipcMain 注册完全多余。 |
| 146-148 | SETTINGS_EVENTS.READY |
已迁移到 typed route windowNotifySettingsReadyRoute。Settings renderer 现使用 windowClient.notifySettingsReady() 而非 raw IPC。 |
影响：
- 非功能性 bug，不会阻断任何现有功能
- architecture guard 的 baseline
4实际只需要2（get-window-id+get-web-contents-id） - 清理后 baseline 可收紧到
2
建议处理： 可在本 PR 清理或作为 follow-up。优先级低，属于收尾清扫。
确认：其他 deferred 项合理
Follow-up comment 中列出的 deferred 项（floating IPC、splash unlock IPC、publishDeepchatEvent 内部用 eventBus.sendToRenderer、eventBus.setTabPresenter）均已确认合理：
get-window-id/get-web-contents-id：preload 同步身份查询，无法迁移到 async route。- Floating renderer 专用 IPC：独立 preload channel 是合理设计。
- Splash database unlock：早于 route runtime 可用，保持 raw IPC 正确。
publishDeepchatEvent内部sendToRenderer：是 typed event 的单一出口实现，不是 legacy 残留。
总结
重构完整度很高。唯一残留是上述 4 个死处理器，风险为零但不干净。除此之外 LGTM。
Collaborator
Author
二次 review 死代码清理结果已核查最新 review 指出的 已删除
Guard 更新
这两个仍是 preload 同步 identity lookup，不能等价迁到 async route。 提交
验证
|
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
1 participant
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.
refactor(ipc): migrate presenter transport to typed route/event system
Summary
presenter:call,remoteControlPresenter:call)src/main/routes/legacyTypedEventBridge.tsandsrc/renderer/api/legacy/**quarantinesrc/shared/contracts/routes.ts) and event contracts (src/shared/contracts/events.ts)useLegacyPresenter()to typed API clients (src/renderer/api/*Client.ts)src/main/routes/index.tsdispatcher withdeepchat:route:invokechannel and runtime dependency injectioncreateBridge.tsvalidates route input/output and typed event envelopesscripts/check-background-exec-utility-host.mjsto prevent@electron-toolkit/utilsleaking into utility-process bundlesDevicePresenterfor explicit testable reset flowsarchitecture-guard.mjsto enforce zero legacy presenter imports in renderer business codeArchitecture Impact
src/main/presenter/index.tsno longer exposes generic dispatch; all IPC goes through typed route runtimeTesting
test/main/routes/dispatcher.test.ts), contract schemas (test/main/routes/contracts.test.ts), and renderer API clients (test/renderer/api/clients.test.ts)test/main/scripts/architectureGuard.test.ts)