Conversation
📝 WalkthroughWalkthroughThis PR implements a comprehensive multi-window cleanup that transitions the application architecture from single-window, multi-tab to multi-window, single-page. Key changes include: refactoring window and presenter APIs to be window-centric, removing tab-based shortcuts and UI, updating event systems from tab to window terminology, migrating session bindings to webContents-based routing, renaming shell renderer to browser, removing tooltip overlay, and updating build configuration and localization throughout. Changes
Estimated Code Review Effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly Related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/renderer/src/composables/usePageCapture.ts (1)
272-290:⚠️ Potential issue | 🟠 MajorThe window fallback is skipped when the first capture attempt throws.
The
try/catchwraps the wholefor ... of, so a rejectedcaptureTabArea()forwebContentsIdbreaks out beforewindowIdis ever tried. Catch per target, or normalize presenter failures tonull, so the advertisedwebContentsId -> windowIdfallback actually runs.🐛 Suggested change
- try { - let segmentData: string | null = null - for (const targetId of captureTargets) { - segmentData = await tabPresenter.captureTabArea(targetId, captureRect) - if (segmentData) { - break - } - } - - if (segmentData) { - imageDataList.push(segmentData) - } else { - console.error(`[CAPTURE_DEBUG] Iteration ${iteration}: 截图失败，未返回数据`) - break - } - } catch (captureError) { - console.error(`[CAPTURE_DEBUG] Iteration ${iteration}: 截图出错:`, captureError) - break - } + let segmentData: string | null = null + for (const targetId of captureTargets) { + try { + segmentData = await tabPresenter.captureTabArea(targetId, captureRect) + if (segmentData) { + break + } + } catch (captureError) { + console.error( + `[CAPTURE_DEBUG] Iteration ${iteration}: capture failed for target ${targetId}`, + captureError + ) + } + } + + if (segmentData) { + imageDataList.push(segmentData) + } else { + console.error(`[CAPTURE_DEBUG] Iteration ${iteration}: capture failed for all targets`) + break + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/composables/usePageCapture.ts` around lines 272 - 290, The try/catch currently wraps the entire loop so a thrown rejection from tabPresenter.captureTabArea aborts the whole fallback sequence; change the logic so each target is protected individually: inside the for (const targetId of captureTargets) loop, await tabPresenter.captureTabArea(targetId, captureRect) inside its own try/catch (or ensure captureTabArea failures are normalized to null) so on error you set segmentData = null (or continue) and allow the loop to try the next targetId; keep the outer behavior that pushes segmentData to imageDataList when non-null and logs/ breaks only when all targets return null.src/main/presenter/tabPresenter.ts (1)
928-948:⚠️ Potential issue | 🟠 MajorChoose the new window kind from the tab content, not the source window.
createTab()already allows non-local tabs in chat windows viaallowNonLocal. In that case this branch creates another chat window, andattachTab()immediately rejects the moved external tab, so the move always rolls back.🐛 Suggested fix
- const sourceWindowType = this.getWindowType(originalWindowId) + const isLocalTab = tabInfo.url?.startsWith('local://') const newWindowOptions: Record<string, any> = { forMovedTab: true, - windowType: sourceWindowType + windowType: isLocalTab ? 'chat' : 'browser' } @@ const newWindowId = - sourceWindowType === 'browser' - ? await this.windowPresenter.createBrowserWindow({ - x: newWindowOptions.x, - y: newWindowOptions.y - }) - : await this.windowPresenter.createAppWindow({ + isLocalTab + ? await this.windowPresenter.createAppWindow({ initialRoute: 'chat', x: newWindowOptions.x, y: newWindowOptions.y }) + : await this.windowPresenter.createBrowserWindow({ + x: newWindowOptions.x, + y: newWindowOptions.y + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/tabPresenter.ts` around lines 928 - 948, The code chooses the new window kind using getWindowType(originalWindowId) (sourceWindowType) which breaks moves for non-local tabs; instead determine the intended window type from the tab content being created (e.g., the createTab payload or the tab object that contains allowNonLocal / content.windowType) and use that value when deciding between createBrowserWindow and createAppWindow; update the branch that calls this.windowPresenter.createBrowserWindow / createAppWindow to use the tab-derived windowType (and preserve forMovedTab and x/y in newWindowOptions) so attachTab receives a window of the correct kind and moved external tabs are not rejected.src/main/presenter/floatingButtonPresenter/index.ts (1)
363-377:⚠️ Potential issue | 🔴 CriticalMake
openMainWindow()async and awaitcreateAppWindow()call.The
createAppWindow()method is async and returnsPromise<number | null>(line 503 in windowPresenter/index.ts), but line 376 calls it withoutawait. This causes the promise rejection to bypass the try/catch block (resulting in an unhandled promise rejection), and the success log runs immediately before the window creation completes. Other call sites (sessionPresenter, tabPresenter) correctly await this method.Suggested fix
- private openMainWindow(): void { + private async openMainWindow(): Promise<void> { try { const windowPresenter = presenter.windowPresenter if (windowPresenter) { const mainWindow = windowPresenter.mainWindow if (mainWindow && !mainWindow.isDestroyed()) { if (mainWindow.isMinimized()) { mainWindow.restore() } mainWindow.show() mainWindow.focus() console.log('Main window opened from floating button context menu') } else { - windowPresenter.createAppWindow({ initialRoute: 'chat' }) + const newWindowId = await windowPresenter.createAppWindow({ initialRoute: 'chat' }) + if (newWindowId == null) { + console.warn('Failed to create a new main window from floating button context menu') + return + } console.log('Created new main window from floating button context menu') } } } catch (error) { console.error('Failed to open main window from floating button:', error) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/floatingButtonPresenter/index.ts` around lines 363 - 377, openMainWindow currently calls the async method windowPresenter.createAppWindow(...) without awaiting it, causing unhandled promise rejections and log ordering issues; change openMainWindow to be async and await windowPresenter.createAppWindow({...}) inside the existing try/catch so errors are caught and the "Created new main window" log runs after creation completes; update any internal references if needed to handle the now-async openMainWindow and ensure you await its callers where appropriate.src/main/presenter/agentPresenter/streaming/streamUpdateScheduler.ts (1)
30-35:⚠️ Potential issue | 🟠 Major
webContentsIdis still ignored during dispatch.This rename stores per-window routing metadata, but
sendInit,flushRender, andflushAllstill emitSTREAM_EVENTS.RESPONSEtoSendTarget.ALL_WINDOWS. In multi-window mode every renderer will keep receiving every stream frame, which defeats the webContents migration.🛠️ Possible direction
export class StreamUpdateScheduler { + private sendResponse(state: SchedulerState, eventData: LLMAgentEventData): void { + if (state.webContentsId != null) { + void eventBus.sendToWebContents(state.webContentsId, STREAM_EVENTS.RESPONSE, eventData) + return + } + + eventBus.sendToRenderer(STREAM_EVENTS.RESPONSE, SendTarget.ALL_WINDOWS, eventData) + } + private sendInit(state: SchedulerState): void { state.hasSentInit = true @@ - eventBus.sendToRenderer(STREAM_EVENTS.RESPONSE, SendTarget.ALL_WINDOWS, eventData) + this.sendResponse(state, eventData) } @@ - eventBus.sendToRenderer(STREAM_EVENTS.RESPONSE, SendTarget.ALL_WINDOWS, eventData) + this.sendResponse(state, eventData) @@ - eventBus.sendToRenderer(STREAM_EVENTS.RESPONSE, SendTarget.ALL_WINDOWS, eventData) + this.sendResponse(state, eventData)Also applies to: 56-70, 82-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/agentPresenter/streaming/streamUpdateScheduler.ts` around lines 30 - 35, The scheduler currently ignores SchedulerState.webContentsId and always dispatches STREAM_EVENTS.RESPONSE to SendTarget.ALL_WINDOWS; update sendInit, flushRender, flushAll (and the other emit spots noted around lines 56-70 and 82-124) to check state.webContentsId and, when present, dispatch to SendTarget.WINDOW with that webContentsId (otherwise fall back to SendTarget.ALL_WINDOWS); locate dispatch/emitter calls in functions sendInit, flushRender, flushAll and replace the fixed SendTarget.ALL_WINDOWS with a helper or inline conditional that selects SendTarget.WINDOW + the numeric id from SchedulerState.webContentsId.src/main/presenter/sessionPresenter/managers/conversationManager.ts (1)
137-177:⚠️ Potential issue | 🟠 MajorEmit a deactivate transition before overwriting an existing binding.
Both paths replace the entry for
webContentsIdwithout deactivating the conversation that was previously active on that renderer. The old conversation can stay marked active in renderer state until unrelated cleanup runs.🛠️ Suggested fix
+ const previousConversationId = this.activeConversationBindings.get(webContentsId) + if (previousConversationId && previousConversationId !== conversationId) { + eventBus.sendToRenderer(CONVERSATION_EVENTS.DEACTIVATED, SendTarget.ALL_WINDOWS, { + webContentsId + }) + } this.activeConversationBindings.set(webContentsId, conversationId)Also applies to: 280-285
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/sessionPresenter/managers/conversationManager.ts` around lines 137 - 177, When assigning a new active conversation, always emit CONVERSATION_EVENTS.DEACTIVATED for any previous conversation bound to the same webContents before overwriting the mapping; in setActiveConversation inspect this.activeConversationBindings.get(webContentsId) (and the symmetric code at the 280-285 path) and if a different previousConversationId exists call eventBus.sendToRenderer(CONVERSATION_EVENTS.DEACTIVATED, SendTarget.ALL_WINDOWS, { webContentsId, /* optional: previousConversationId if payload expects it */ }) (or delete the old binding first) and then set the new binding and emit ACTIVATED; update setActiveConversation (and the other replacement site) to perform this deactivation step prior to calling this.activeConversationBindings.set(webContentsId, conversationId) or replacing entries.src/main/presenter/windowPresenter/index.ts (1)
716-725:⚠️ Potential issue | 🟡 MinorRemove
WINDOW_UNMAXIMIZEDfrom therestoreevent handler.Electron's
restoreevent fires when a window is restored from a minimized state, not when it exits maximize state (that's handled by the separateunmaximizeevent). SendingWINDOW_UNMAXIMIZEDwhenrestorefires incorrectly tells the renderer the window is unmaximized, even if it was restored in a maximized state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/windowPresenter/index.ts` around lines 716 - 725, The restore handler (handleRestore) is incorrectly sending WINDOW_EVENTS.WINDOW_UNMAXIMIZED; remove the appWindow.webContents.send(WINDOW_EVENTS.WINDOW_UNMAXIMIZED) call from the handleRestore function and leave only the restore-specific logic (call this.handleWindowRestore(windowId) and eventBus.sendToMain(WINDOW_EVENTS.WINDOW_RESTORED, windowId)); ensure that WINDOW_UNMAXIMIZED remains handled only by the unmaximize event listener so the renderer receives unmaximized messages only on actual unmaximize events.
🟡 Minor comments (19)
src/renderer/src/i18n/fa-IR/settings.json-944-944 (1)
944-944:⚠️ Potential issue | 🟡 MinorFix the Persian label to say “window,” not “tab”.
Line 944 still says
بستن برگه کنونی(“close current tab”), but this key is nowcloseWindowand the PR removes tab-based UI. That will show the wrong action to fa-IR users. Consider updating it toبستن پنجره کنونی(or your preferred Persian equivalent for “current window”).💬 Suggested change
- "closeWindow": "بستن برگه کنونی" + "closeWindow": "بستن پنجره کنونی"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/i18n/fa-IR/settings.json` at line 944, The Persian translation for the "closeWindow" key is still using the word for "tab" ("بستن برگه کنونی"); update the value for the closeWindow string (key: closeWindow) to use the correct word for "window", e.g., replace the current text with "بستن پنجره کنونی" (or another preferred Persian equivalent for "current window") so the label matches the new window-based UI.src/renderer/src/i18n/he-IL/settings.json-944-944 (1)
944-944:⚠️ Potential issue | 🟡 MinorFix the Hebrew label to say “window,” not “tab”.
closeWindowcurrently renders as "סגור את הכרטיסייה הנוכחית" ("Close the current tab"), which no longer matches the action name and will mislead users in the shortcuts UI. This should be translated as closing the current window instead. As per coding guidelines, "Add new translations to ALL language files (da-DK, en-US, fa-IR, fr-FR, he-IL, ja-JP, ko-KR, pt-BR, ru-RU, zh-CN, zh-HK, zh-TW) with consistent key names across all locales".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/i18n/he-IL/settings.json` at line 944, The translation for the i18n key "closeWindow" is incorrect in he-IL (currently reads "סגור את הכרטיסייה הנוכחית" = "Close the current tab"); update the he-IL value for the "closeWindow" entry to a correct Hebrew phrase meaning "Close the current window" (replace the tab wording), and ensure the same "closeWindow" key has consistent translations added/verified across all locale files (da-DK, en-US, fa-IR, fr-FR, he-IL, ja-JP, ko-KR, pt-BR, ru-RU, zh-CN, zh-HK, zh-TW) so the shortcuts UI shows the correct wording in every language.src/renderer/src/composables/usePageCapture.ts-186-195 (1)
186-195:⚠️ Potential issue | 🟡 MinorFail fast when no usable capture target is resolved.
typeof id === 'number'still admitsNaN, and if both lookups miss you only discover it later after the scroll loop has already mutated the page state. Validate these ids as finite integers and return an explicit error whencaptureTargetsends up empty.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/composables/usePageCapture.ts` around lines 186 - 195, Replace the loose typeof checks with strict finite-integer validation for webContentsId and windowId (e.g., use Number.isInteger or Number.isFinite+Number.isInteger) when building captureTargets in usePageCapture; ensure you only push ids that pass this validation and, if captureTargets is empty after both checks, fail fast by returning or throwing an explicit error describing that no valid capture target was resolved so the caller stops before the scroll/mutation loop.src/renderer/src/composables/usePageCapture.ts-186-186 (1)
186-186:⚠️ Potential issue | 🟡 MinorKeep inline comments in English.
These new comments are in Chinese inside a renderer TypeScript file. Please translate them to English to match the repository rule for logs/comments in this area.
As per coding guidelines,
src/renderer/src/**/*.{ts,tsx,vue}: Ensure all code comments are in English and all log messages are in English, with no non-English text in code comments or console statements.Also applies to: 272-272
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/composables/usePageCapture.ts` at line 186, The inline comment "// 优先使用当前 webContentsId；必要时回退到 windowId" (and the similar comment at the other occurrence) must be translated to English to comply with the renderer comments policy; locate the comments in src/renderer/src/composables/usePageCapture.ts (around the logic that prefers webContentsId over windowId, e.g., near variables or functions referencing webContentsId and windowId) and replace the Chinese text with an English comment such as "// Prefer the current webContentsId; fall back to windowId when necessary" (apply the same translation for the other occurrence at the mentioned location).src/renderer/src/i18n/pt-BR/settings.json-944-944 (1)
944-944:⚠️ Potential issue | 🟡 MinorFix the PT-BR label to refer to a window, not a tab.
closeWindowis translated asFechar a guia atual, which still says “current tab” and mislabels the new shortcut in Settings.📝 Suggested fix
- "closeWindow": "Fechar a guia atual" + "closeWindow": "Fechar a janela atual"Based on learnings, "Use English (en-US) as the reference for translation accuracy when adding new keys."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/i18n/pt-BR/settings.json` at line 944, The PT-BR translation for the key "closeWindow" is incorrect (reads "Fechar a guia atual" — "tab"); update the value for the "closeWindow" key to use the correct word for window (e.g., "Fechar a janela atual") to match the English reference and new Settings shortcut label; locate the "closeWindow" entry in the pt-BR settings.json and replace the string accordingly.src/renderer/src/composables/usePresenter.ts-4-4 (1)
4-4:⚠️ Potential issue | 🟡 MinorKeep source comments in English.
These new comments are in Chinese, which breaks the repo-wide rule for TS comments and makes this renderer code less consistent to maintain.
💬 Suggested wording
-// WebContentsId 缓存 +// Cached webContentsId @@ - // 获取当前 webContentsId + // Get the current webContentsId @@ - // 在调用中记录 webContentsId + // Log the webContentsId with the IPC callAs per coding guidelines, "
**/*.{js,ts,tsx,jsx,vue}: Use English for all logs and comments".Also applies to: 68-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/composables/usePresenter.ts` at line 4, Replace Chinese comments in this file with English equivalents to meet repo guidelines: change the comment "// WebContentsId 缓存" to something like "// WebContentsId cache" and update any other Chinese comments in usePresenter (including the block around lines 68-74) to clear English descriptions; search for occurrences of "WebContentsId" and the exported function/usePresenter scope to locate and replace all non-English comments.docs/specs/multi-window-cleanup/tasks.md-5-10 (1)
5-10:⚠️ Potential issue | 🟡 MinorUpdate the checklist to match work already landed in this PR.
At least Lines 5 and 9 are already done in the files shown here (
tsconfig.app.jsonand the locale shortcut updates). Leaving them unchecked makes the migration tracker misleading.📝 Suggested update
-- [ ] Update build aliases and inputs +- [x] Update build aliases and inputs @@ -- [ ] Remove tab shortcuts from main/settings +- [x] Remove tab shortcuts from main/settings🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/specs/multi-window-cleanup/tasks.md` around lines 5 - 10, The checklist in docs/specs/multi-window-cleanup/tasks.md is out of date; mark the items that were already completed in this PR as done (specifically check "[ ] Update build aliases and inputs" and "[ ] Remove tab shortcuts from main/settings" to reflect the tsconfig.app.json and locale shortcut updates), and update any remaining items as appropriate so the migration tracker accurately reflects current work (leave the other lines such as "Simplify browser renderer chrome", "Rewrite YoBrowser window model", and "Split app-window and browser-window creation" unchanged if still in progress).src/renderer/src/i18n/fr-FR/settings.json-944-944 (1)
944-944:⚠️ Potential issue | 🟡 MinorUse window wording in the FR shortcut label.
closeWindowis currently translated asFermez la page d'onglet actuelle, which still points to a tab-based action instead of closing the current window.📝 Suggested fix
- "closeWindow": "Fermez la page d'onglet actuelle" + "closeWindow": "Fermer la fenêtre actuelle"Based on learnings, "Use English (en-US) as the reference for translation accuracy when adding new keys."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/i18n/fr-FR/settings.json` at line 944, The French translation for the key "closeWindow" uses tab-specific wording; update the value of the "closeWindow" entry to reference a window instead of a tab (e.g., replace "Fermez la page d'onglet actuelle" with "Fermer la fenêtre actuelle" or the preferred imperative "Fermez la fenêtre actuelle") and ensure it matches the en-US source intent and existing project phrasing style.src/renderer/src/i18n/da-DK/settings.json-890-890 (1)
890-890:⚠️ Potential issue | 🟡 MinorTranslation mismatch: "fane" (tab) should be "vindue" (window).
The Danish translation "Luk aktiv fane" means "Close active tab", but the en-US reference is "Close the current window". The translation should use "vindue" instead of "fane" to accurately reflect the window-centric change.
Suggested fix
- "closeWindow": "Luk aktiv fane" + "closeWindow": "Luk det aktuelle vindue"Based on learnings: "Use English (en-US) as the reference for translation accuracy when adding new keys"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/i18n/da-DK/settings.json` at line 890, The translation for the key "closeWindow" is incorrect: replace the Danish string "Luk aktiv fane" (which means "Close active tab") with a window-centric phrase like "Luk det aktuelle vindue" (or "Luk aktivt vindue") so it matches the en-US source "Close the current window"; update the value for the "closeWindow" key accordingly.src/main/eventbus.ts-147-151 (1)
147-151:⚠️ Potential issue | 🟡 MinorFix forEach callback to not return a value.
The static analysis tool correctly flags that the forEach callback implicitly returns a value, which is ignored. Use a block body to make the intent clear.
🐛 Suggested fix
broadcastToWebContents(webContentsIds: number[], eventName: string, ...args: unknown[]) { - webContentsIds.forEach((webContentsId) => - this.sendToWebContents(webContentsId, eventName, ...args) - ) + webContentsIds.forEach((webContentsId) => { + this.sendToWebContents(webContentsId, eventName, ...args) + }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/eventbus.ts` around lines 147 - 151, The forEach callback in broadcastToWebContents currently uses a concise arrow expression which implicitly returns the result of this.sendToWebContents; change the callback to a block-bodied function so it does not return a value (e.g., replace webContentsIds.forEach((id) => this.sendToWebContents(...)) with webContentsIds.forEach((id) => { this.sendToWebContents(...); });), keeping the call to sendToWebContents intact and referencing the same webContentsId parameter.src/renderer/src/i18n/ja-JP/settings.json-944-944 (1)
944-944:⚠️ Potential issue | 🟡 MinorFix the Japanese
closeWindowlabel.This still says “close the current tab page”, so the shortcut text points back to the removed tab model. It should refer to the current window instead.
💬 Suggested translation
- "closeWindow": "現在のタブページを閉じます" + "closeWindow": "現在のウィンドウを閉じます"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/i18n/ja-JP/settings.json` at line 944, The "closeWindow" label currently reads "現在のタブページを閉じます" (close the current tab page); update the value for the "closeWindow" key in settings.json to reference the window instead, e.g. replace it with "現在のウィンドウを閉じます" so the shortcut text correctly says "close the current window" (key: closeWindow).src/renderer/src/i18n/ko-KR/settings.json-944-944 (1)
944-944:⚠️ Potential issue | 🟡 MinorTranslation inconsistent with window-centric terminology.
The key is
closeWindowbut the Korean translation "현재 탭 페이지를 닫습니다" means "Closes the current tab page". Given this PR removes tab concepts in favor of windows, the translation should reflect window terminology.🌐 Suggested translation fix
- "closeWindow": "현재 탭 페이지를 닫습니다" + "closeWindow": "현재 창을 닫습니다"The suggested translation means "Closes the current window" in Korean.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/i18n/ko-KR/settings.json` at line 944, The translation for the key closeWindow is window-centric but currently reads as "closes the current tab page"; update the value for closeWindow in settings.json to a window-focused Korean phrase (e.g., "현재 창을 닫습니다") so the string matches the key and the new window terminology used across the app.src/main/presenter/index.ts-384-401 (1)
384-401:⚠️ Potential issue | 🟡 MinorTranslate the updated IPC comments to English.
The new inline comments on Lines 384, 389, and 401 are still Chinese. Please keep this hunk consistent with the repo rule for English-only TS comments.
Suggested patch
-// IPC 主进程处理程序：动态调用 Presenter 的方法 (支持 window/webContents 上下文) +// Main-process IPC handler: dynamically invoke Presenter methods with window/webContents context ipcMain.handle( 'presenter:call', (event: IpcMainInvokeEvent, name: string, method: string, ...payloads: unknown[]) => { try { - // 构建调用上下文 + // Build the call context const webContentsId = event.sender.id const windowId = BrowserWindow.fromWebContents(event.sender)?.id @@ - // 记录调用日志 + // Log the IPC call if (import.meta.env.VITE_LOG_IPC_CALL === '1') {As per coding guidelines,
**/*.{js,ts,tsx,jsx,vue,mjs,cjs}: All logs and comments must be in English.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/index.ts` around lines 384 - 401, The inline comments in the IPC handler for ipcMain.handle are still in Chinese; update them to English to comply with repo rules. Locate the ipcMain.handle callback (the dynamic Presenter call) and replace the Chinese comments around the call description, context construction (near webContentsId and windowId assignments), and the log comment with concise English equivalents that describe: "IPC main handler: dynamically invoke Presenter methods (supports window/webContents context)", "build call context" and "log the call" while keeping references to IPCCallContext, webContentsId, windowId, presenterName and methodName intact.src/renderer/src/i18n/ru-RU/settings.json-944-944 (1)
944-944:⚠️ Potential issue | 🟡 MinorFix the Russian
closeWindowlabel.
Закройте текущую страницу вкладкиstill means “close the current tab page”. This shortcut is now window-scoped, so the RU copy should refer to the current window instead.Suggested patch
- "closeWindow": "Закройте текущую страницу вкладки" + "closeWindow": "Закрыть текущее окно"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/i18n/ru-RU/settings.json` at line 944, The Russian label for the i18n key closeWindow is inaccurate (it refers to a tab/page); update the value for "closeWindow" in src/renderer/src/i18n/ru-RU/settings.json to a phrase that references the current window (e.g., "Закрыть текущее окно" or similar), ensuring the translation is concise and matches the UI intent of a window-scoped shortcut.src/renderer/src/components/WindowSideBar.vue-242-246 (1)
242-246:⚠️ Potential issue | 🟡 MinorRemove the stale
windowIdgate before opening settings.
openOrFocusSettingsWindow()no longer consumes a caller window id. ThegetWindowId()check on Lines 243-245 can still turn this click into a silent no-op even though the presenter call itself is valid.Suggested patch
const openSettings = () => { - const windowId = window.api.getWindowId() - if (windowId != null) { - void windowPresenter.openOrFocusSettingsWindow() - } + void windowPresenter.openOrFocusSettingsWindow() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/components/WindowSideBar.vue` around lines 242 - 246, The click handler openSettings currently checks window.api.getWindowId() and returns early, which can silently prevent opening settings; remove the stale gate so openSettings simply calls windowPresenter.openOrFocusSettingsWindow() (keeping the void/async call pattern if needed) and delete the window.api.getWindowId() conditional and related null check to ensure the presenter is always invoked.src/shared/types/presenters/session.presenter.d.ts-109-112 (1)
109-112:⚠️ Potential issue | 🟡 Minor
preferredWindowTypeis missing'browser'.
SessionBindings.windowTypealready allows'browser', but the new lookup API narrows callers back to'main' | 'floating'. That makes the shared surface inconsistent and prevents browser-window callers from expressing the new binding type without casts.🛠️ Minimal fix
findWebContentsForSession( sessionId: string, - preferredWindowType?: 'main' | 'floating' + preferredWindowType?: 'main' | 'floating' | 'browser' ): Promise<number | null>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/types/presenters/session.presenter.d.ts` around lines 109 - 112, The API for findWebContentsForSession narrows preferredWindowType to 'main' | 'floating' even though SessionBindings.windowType permits 'browser'; update the signature of findWebContentsForSession(sessionId: string, preferredWindowType?: 'main' | 'floating') to accept the same union as SessionBindings.windowType (include 'browser') so callers can pass browser window types without casts, and ensure any call sites and the implementation of findWebContentsForSession handle the 'browser' case consistently.src/renderer/src/components/workspace/WorkspaceBrowserTabs.vue-47-52 (1)
47-52:⚠️ Potential issue | 🟡 MinorDon't permanently cache favicon load failures per window.
faviconErroris keyed only bybrowserWindow.id(line 75), so once a favicon fails to load, subsequent navigations in that window skip favicon rendering entirely, even if the new page has a different favicon URL. Key the cache by(windowId, faviconUrl)or clear the entry whenbrowserWindow.page.faviconchanges.Also, per Vue 3 Composition API guidelines, use
reactivefor objects instead ofref. Replace line 75 with:const faviconError = reactive<Record<number, string>>({})and track failed URLs per window to allow retry on different pages.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/components/workspace/WorkspaceBrowserTabs.vue` around lines 47 - 52, The current favicon failure cache (faviconError) is keyed only by browserWindow.id so a single failed load blocks all future favicons for that window; change faviconError to a reactive map that keys failures by windowId+faviconUrl (or store a per-window set of failed URLs) and update the template/check to use the current browserWindow.page.favicon when deciding whether to show the <img>; also replace the ref-based faviconError with a reactive object (per reviewer: const faviconError = reactive<Record<number, string>>({}) or a reactive<Record<string, true>>), set the failure entry in the `@error` handler for the specific favicon URL (e.g. key = `${browserWindow.id}:${browserWindow.page.favicon}`), and clear/remove the relevant key when browserWindow.page.favicon changes so new favicons can retry.src/renderer/browser/components/AppBar.vue-114-132 (1)
114-132:⚠️ Potential issue | 🟡 MinorUse named callback handlers and
removeListener()instead ofremoveAllListeners().The anonymous functions registered on lines 114–125 cannot be individually removed. Using
removeAllListeners()on lines 129–132 unregisters every subscriber on those channels. If additional listeners are added to the sameipcRendererevents in the future, this code will inadvertently break them. Store the callback in a variable and useremoveListener(channel, callback)in the cleanup hook instead, following the pattern used in other components likeyoBrowser.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/browser/components/AppBar.vue` around lines 114 - 132, The event handlers registered with ipcRenderer for WINDOW_EVENTS (WINDOW_MAXIMIZED, WINDOW_UNMAXIMIZED, WINDOW_ENTER_FULL_SCREEN, WINDOW_LEAVE_FULL_SCREEN) use anonymous callbacks and then call removeAllListeners in onBeforeUnmount, which removes every subscriber; fix by creating named callback functions (e.g. onWindowMaximized, onWindowUnmaximized, onWindowEnterFullScreen, onWindowLeaveFullScreen) that set isMaximized.value and isFullscreened.value, register those named handlers with ipcRenderer.on, and in onBeforeUnmount call ipcRenderer.removeListener(channel, correspondingNamedHandler) for each channel instead of removeAllListeners so only these listeners are removed.src/main/presenter/windowPresenter/index.ts-545-545 (1)
545-545:⚠️ Potential issue | 🟡 MinorKeep new comments in English.
The doc comment added here is not in English.
As per coding guidelines,
**/*.{js,ts,tsx,jsx,vue,mjs,cjs}: All logs and comments must be in English.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/windowPresenter/index.ts` at line 545, The doc comment "创建一个新的兼容窗口包装器。" must be replaced with an English comment; locate the comment above the function that creates a new compatible window wrapper (the comment text currently in Chinese) and update it to an English docstring such as "Create a new compatible window wrapper." ensuring all nearby comments and any TODOs are also in English to follow the repository coding guidelines.
🧹 Nitpick comments (17)
src/main/presenter/windowPresenter/FloatingChatWindow.ts (1)
8-8: Consider renamingTAB_EVENTSfor semantic consistency in window presenter context.
FloatingChatWindowusesTAB_EVENTS.RENDERER_TAB_READY(lines 197, 303), which is semantically odd since it's a window presenter, not a tab presenter. While the event works correctly, using tab-centric naming for window operations reduces naming clarity given the organized event system with separateWINDOW_EVENTSandTAB_EVENTScategories. Consider adding a window-specific renderer-ready event or using more semantically appropriate naming for this window presenter's lifecycle signals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/windowPresenter/FloatingChatWindow.ts` at line 8, The FloatingChatWindow is using TAB_EVENTS.RENDERER_TAB_READY which is semantically misleading for a window presenter; update the code to use a window-scoped event instead—either add/emit a new WINDOW_EVENTS.RENDERER_WINDOW_READY constant and replace uses of TAB_EVENTS.RENDERER_TAB_READY inside FloatingChatWindow, or import and use an existing WINDOW_EVENTS variant; make changes where FloatingChatWindow references TAB_EVENTS.RENDERER_TAB_READY (e.g., in event emission and listeners around the renderer-ready logic) and ensure the publisher/subscriber sides use the new WINDOW_EVENTS name consistently.src/main/presenter/shortcutPresenter.ts (1)
26-27: Consider removing or gating debug console.log statements.Multiple
console.logstatements appear to be debug artifacts. While logging is acceptable, these specific logs ('reg shortcuts','clean chat history','delete conversation','unreg shortcuts') seem like development-time debugging rather than production logging.Suggested approach
Either remove these debug logs or use a conditional/structured logging approach:
- console.log('reg shortcuts') + // Optionally use a debug flag or structured loggerAlso applies to: 107-107, 112-112, 126-126, 152-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/shortcutPresenter.ts` around lines 26 - 27, Remove or gate the stray debug console.log calls in shortcutPresenter (e.g., the "reg shortcuts", "clean chat history", "delete conversation", "unreg shortcuts" statements) — either delete them or replace them with a structured logger call (e.g., processLogger.debug/processLogger.info) or wrap them behind a dev-only flag (e.g., if (isDev) ...) so they don't pollute production output; update occurrences near the functions handling shortcut registration/cleanup to use the chosen logging strategy consistently.src/main/presenter/configPresenter/shortcutKeySettings.ts (1)
5-6: Outdated comment references removed tab functionality.The comment mentions "Register tab number shortcut keys (1-8)" but all tab-related shortcuts have been removed in this PR. Consider updating or removing this comment to avoid confusion.
Suggested fix
-// Register tab number shortcut keys (1-8) -> Fixed CommandKey+1 ~ CommandKey+8 to switch tabs -// Below are regular shortcut key definitions +// Shortcut key definitions for window and conversation management export const rendererShortcutKey = {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/configPresenter/shortcutKeySettings.ts` around lines 5 - 6, The header comment in shortcutKeySettings.ts wrongly references "Register tab number shortcut keys (1-8) -> Fixed CommandKey+1 ~ CommandKey+8 to switch tabs" which no longer exists; update or remove that sentence so the file top comment accurately describes the current regular shortcut key definitions in this module (e.g., change to "Register regular shortcut key definitions" or delete the obsolete tab-related line) and ensure any mention of tab shortcuts is removed from the comment block near the top of the file.src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts (4)
26-29: Error message should be in English.The validation message on line 28 is in Chinese.
♻️ Suggested fix
participants: z .array(ParticipantSchema) - .min(2, { message: '会议至少需要两位参会者。' }) + .min(2, { message: 'A meeting requires at least two participants.' }) .describe('参会者列表。'),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts` around lines 26 - 29, The validation message for the participants array is written in Chinese; update the schema definition that uses ParticipantSchema (the participants field in meetingServer.ts) to use an English error message (e.g., "Meeting requires at least two participants.") by replacing the current .min(2, { message: '会议至少需要两位参会者。' }) with an equivalent .min(2, { message: 'Meeting requires at least two participants.' }) so the validation message is in English.
67-69: Error message should be in English.♻️ Suggested fix
if (name !== 'start_meeting') { - throw new Error(`未知的工具: ${name}`) + throw new Error(`Unknown tool: ${name}`) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts` around lines 67 - 69, The thrown error message is in Chinese; change the Error thrown in the branch that checks name !== 'start_meeting' (the if condition comparing name to 'start_meeting') so it uses an English message like "Unknown tool: ${name}" instead of `未知的工具: ${name}` by updating the throw new Error(...) call where the invalid tool name is handled.
85-94: Error message should be in English.♻️ Suggested fix
} catch (error) { return { content: [ { type: 'text', - text: `会议启动失败: ${error instanceof Error ? error.message : String(error)}` + text: `Meeting startup failed: ${error instanceof Error ? error.message : String(error)}` } ], isError: true } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts` around lines 85 - 94, The catch block in meetingServer.ts returns an error message in Chinese; change the message to English (e.g., "Meeting failed to start: ...") and keep the existing error extraction logic (error instanceof Error ? error.message : String(error)) so the returned object (content array with type 'text' and isError: true) contains an English message; update the template string accordingly where the current catch returns `会议启动失败: ${...}`.
19-22: Error messages should be in English.The error message on line 21 is in Chinese. As per coding guidelines, all logs and comments must be in English.
♻️ Suggested fix
{ - message: - '错误：必须且只能通过 "tab_id" 或 "tab_title" 中的一个来指定参会者，两者不能同时提供，也不能都为空。' + message: + 'Error: A participant must be specified by exactly one of "tab_id" or "tab_title", not both and not neither.' }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts` around lines 19 - 22, Replace the Chinese error string in the object with an English message; locate the object with the "message" property in meetingServer.ts (the error object that currently reads '错误：必须且只能通过 "tab_id" 或 "tab_title" 中的一个来指定参会者，两者不能同时提供，也不能都为空。') and change it to a clear English message such as "Error: Attendees must be specified by exactly one of 'tab_id' or 'tab_title'; do not provide both or neither." to conform to the project's English-only logging/guidelines.src/renderer/browser/stores/window.ts (2)
60-65: Consider clarifying the reload condition logic.The condition
changedWindowId === null || changedWindowId === windowId.valuewill reload state when the windowId couldn't be resolved from the payload. This might cause unnecessary reloads for events with malformed payloads. Consider being more explicit:const handleWindowEvent = async (_event: unknown, payload: unknown) => { const changedWindowId = resolveWindowId(payload) - if (changedWindowId === null || changedWindowId === windowId.value) { + // Reload only when the event affects this window + if (changedWindowId !== null && changedWindowId === windowId.value) { await loadState() } }However, if the intent is to reload on all events where the target can't be determined (broadcast-style), keep the current logic but add a comment explaining the reasoning.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/browser/stores/window.ts` around lines 60 - 65, The reload logic in handleWindowEvent uses resolveWindowId(payload) and currently treats a null result as a trigger to call loadState, which causes reloads for malformed payloads; update the condition so loadState() is called only when changedWindowId === windowId.value (targeted event) and, if you really intend broadcast semantics, explicitly handle a well-defined broadcast marker (e.g., check for a specific payload flag) instead of null, or alternatively retain the existing null-behavior but add a clear comment above handleWindowEvent describing that null means "broadcast to all windows" and is intentionally used to force reloads; reference resolveWindowId, windowId.value, handleWindowEvent, and loadState when making the change.
67-80: Consider adding cleanup for IPC listeners.The store subscribes to IPC events but doesn't provide a way to unsubscribe. While this may be acceptable for a singleton store that lives for the app's lifetime, adding a
disposemethod would improve testability and prevent potential memory leaks if the store is ever used in a more dynamic context.♻️ Suggested addition
+ const dispose = () => { + if (!window?.electron?.ipcRenderer) return + WINDOW_EVENT_CHANNELS.forEach((channel) => { + window.electron.ipcRenderer.removeListener(channel, handleWindowEvent) + }) + } + return { windowId, browserWindow, page, isAboutBlank, init, - loadState + loadState, + dispose }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/browser/stores/window.ts` around lines 67 - 80, The init function registers IPC listeners for WINDOW_EVENT_CHANNELS with window.electron.ipcRenderer.on using handleWindowEvent but never removes them; add a dispose (or teardown) method on the same store that iterates WINDOW_EVENT_CHANNELS and calls window.electron.ipcRenderer.removeListener (or off) with handleWindowEvent (guarding that window?.electron?.ipcRenderer exists) and ensure tests or consumers can call dispose to unregister; keep init unchanged except optionally exposing dispose alongside initialized, windowId, and loadState so callers can clean up.src/renderer/src/components/chat-input/SkillsIndicator.vue (1)
79-84: Remove the now-unusedwindow.apidependency here too.This method no longer needs a renderer-side
windowId, so the preload check only adds another silent failure path before opening settings.♻️ Suggested cleanup
const openSettings = () => { - const windowId = window.api.getWindowId() - if (windowId != null) { - void windowPresenter.openOrFocusSettingsWindow() - } + void windowPresenter.openOrFocusSettingsWindow() panelOpen.value = false }Based on learnings: Use
usePresentercomposable for main process communication instead of direct IPC calls.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/components/chat-input/SkillsIndicator.vue` around lines 79 - 84, The openSettings function still calls window.api.getWindowId(), which is now unused and creates an unnecessary failure path; remove the window.api dependency and simplify openSettings to call windowPresenter.openOrFocusSettingsWindow() (via the usePresenter composable) unconditionally and then set panelOpen.value = false, ensuring you delete the windowId variable and the surrounding null-check so openSettings only references openOrFocusSettingsWindow and panelOpen.src/renderer/src/pages/WelcomePage.vue (1)
79-83: Drop the stalewindowIdgate.
openOrFocusSettingsWindow()is already window-scoped, so thiswindow.api.getWindowId()check only keeps an unnecessary preload dependency and can turn the button into a no-op whengetWindowId()is null. Call the presenter directly here.♻️ Suggested cleanup
const onAddProvider = () => { - const windowId = window.api.getWindowId() - if (windowId != null) { - void windowPresenter.openOrFocusSettingsWindow() - } + void windowPresenter.openOrFocusSettingsWindow() }Based on learnings: Use
usePresentercomposable for main process communication instead of direct IPC calls.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/pages/WelcomePage.vue` around lines 79 - 83, The onAddProvider handler currently checks window.api.getWindowId() before calling windowPresenter.openOrFocusSettingsWindow(), which creates an unnecessary preload dependency and can make the button a no-op when getWindowId() is null; remove the gate and call the presenter directly (replace the window.api.getWindowId() check with a direct call to windowPresenter.openOrFocusSettingsWindow()), and if the component is still using direct IPC access, switch to the usePresenter composable for main-process communication so the handler depends on the presenter API rather than window.api.vitest.config.renderer.ts (1)
28-28: Comment should be in English.As per coding guidelines, all comments must be in English.
♻️ Suggested change
- environment: 'jsdom', // 使用jsdom环境，适合renderer进程测试 + environment: 'jsdom', // Use jsdom environment, suitable for renderer process tests🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vitest.config.renderer.ts` at line 28, The inline comment on the vitest config line that reads "// 使用jsdom环境，适合renderer进程测试" must be translated to English to follow project guidelines; update the comment adjacent to the environment: 'jsdom' setting (in vitest.config.renderer.ts) to an English phrase such as "// use jsdom environment, suitable for renderer process tests" or equivalent, ensuring the meaning is preserved and no other code changes are made.src/main/presenter/sessionPresenter/types.ts (1)
54-66: Consider markingtabIdas deprecated in the types.Both
CreateSessionOptionsandCreateSessionParamsnow have bothwebContentsIdandtabIdfields. IftabIdis kept for backward compatibility during the migration, consider adding a JSDoc@deprecatedannotation to clarify which field should be used going forward.📝 Suggested documentation
export type CreateSessionOptions = { forceNewAndActivate?: boolean webContentsId?: number + /** `@deprecated` Use webContentsId instead */ tabId?: number } export type CreateSessionParams = { title: string settings?: Partial<SessionConfig> webContentsId?: number + /** `@deprecated` Use webContentsId instead */ tabId?: number options?: CreateSessionOptions }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/sessionPresenter/types.ts` around lines 54 - 66, Add JSDoc `@deprecated` annotations to the tabId properties in both CreateSessionOptions and CreateSessionParams to make it explicit they're legacy and that webContentsId should be used going forward; update the comment text to state "use webContentsId instead" and leave the properties in place for backward compatibility while clearly marking them deprecated in the type declarations (symbols: CreateSessionOptions.tabId, CreateSessionParams.tabId).test/renderer/components/WindowSideBar.test.ts (1)
44-59: Consider returningyoBrowserStorefrom the setup function.The new
yoBrowserStoremock is correctly added but not included in the return statement on line 100. If future tests need to assert onopenLatestOrCreatecalls, it won't be accessible.♻️ Suggested change
- return { wrapper, operations, agentStore, sessionStore } + return { wrapper, operations, agentStore, sessionStore, yoBrowserStore }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/renderer/components/WindowSideBar.test.ts` around lines 44 - 59, The test adds a mocked yoBrowserStore with openLatestOrCreate but never returns it from the setup function, so tests cannot access or assert on openLatestOrCreate; update the setup function to include yoBrowserStore in its returned object (alongside agentStore, sessionStore, themeStore, etc.) so tests can reference yoBrowserStore.openLatestOrCreate for assertions.src/main/utils/index.ts (1)
10-26: Comments should be in English.Multiple comments in this file are in Chinese, which violates coding guidelines. As per coding guidelines, all comments must be in English.
♻️ Suggested translations
- // 查找目标窗口 (焦点窗口或第一个窗口) + // Find target window (focused window or first window) const targetWindow = presenter.windowPresenter.getFocusedWindow() || allWindows[0] if (!targetWindow.isDestroyed()) { - // 逻辑: 如果窗口可见且不是从托盘点击触发，则隐藏；否则显示并置顶 + // Logic: If window is visible and not triggered from tray click, hide it; otherwise show and bring to front if (targetWindow.isVisible() && !mustShow) { presenter.windowPresenter.hide(targetWindow.id) } else { presenter.windowPresenter.show(targetWindow.id) - targetWindow.focus() // 确保窗口置顶 + targetWindow.focus() // Ensure window is brought to front } } else { - console.warn('Target window for SHOW_HIDDEN_WINDOW event is destroyed.') // 保持 warn - // 如果目标窗口已销毁，创建新窗口 + console.warn('Target window for SHOW_HIDDEN_WINDOW event is destroyed.') + // If target window is destroyed, create a new window presenter.windowPresenter.createAppWindow({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/utils/index.ts` around lines 10 - 26, The comments in this block are written in Chinese; update them to English while preserving the original intent and clarity. Replace the Chinese comment lines around the logic that selects targetWindow (presenter.windowPresenter.getFocusedWindow() || allWindows[0]), the isDestroyed() check, the visibility/mustShow logic (targetWindow.isVisible() && !mustShow), the hide/show calls (presenter.windowPresenter.hide(targetWindow.id) and presenter.windowPresenter.show(targetWindow.id)), the focus call (targetWindow.focus()), the console.warn message, and the fallback that calls presenter.windowPresenter.createAppWindow({ initialRoute: 'chat'}) with concise English comments describing each step (select target window, check destroyed state, hide if visible and not mustShow, otherwise show and focus, warn if destroyed and create new window).src/main/presenter/lifecyclePresenter/hooks/after-start/windowCreationHook.ts (1)
41-48: Make the final startup log branch-accurate.When windows already exist, Lines 42 and 48 together say both “skipping initial window creation” and “initial application window created successfully”. Reword the final log or keep the success message inside the creation branch only.
Suggested patch
- console.log('windowCreationHook: Initial application window created successfully') + console.log('windowCreationHook: Window creation hook completed successfully')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/lifecyclePresenter/hooks/after-start/windowCreationHook.ts` around lines 41 - 48, The final startup log is misleading because it prints "Initial application window created successfully" even when the else branch skips creation; update the logging so it’s branch-accurate: either move the success log into the branch that actually creates windows (the block that runs when windows are created) or change the final console.log after presenter.shortcutPresenter.registerShortcuts() to a neutral message (e.g., "Application startup complete" or "Window creation skipped, application ready") so the messages reflect the actual path taken; locate this behavior around the windowCreationHook and the console.log statements that mention "skipping initial window creation" and "Initial application window created successfully".src/renderer/src/stores/yoBrowser.ts (1)
18-18: UseshallowReffor the window snapshot.
BrowserWindowInfo[]is loaded as a whole snapshot and replaced wholesale inloadState(). Deep-proxying every nested window/page object here adds reactivity cost without buying finer-grained updates.♻️ Minimal refactor
-import { computed, onBeforeUnmount, onMounted, ref } from 'vue' +import { computed, onBeforeUnmount, onMounted, ref, shallowRef } from 'vue' @@ - const windows = ref<BrowserWindowInfo[]>([]) + const windows = shallowRef<BrowserWindowInfo[]>([])As per coding guidelines, "Use
shallowRefandshallowReactivefor optimizing reactivity with large objects".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/stores/yoBrowser.ts` at line 18, The windows state is currently a deep ref (const windows = ref<BrowserWindowInfo[]>([])) which needlessly deep-proxies the whole snapshot; change it to a shallowRef so replacing the entire array in loadState() doesn't create deep reactivity overhead: replace ref with shallowRef for the windows variable (import shallowRef from 'vue' if not already) and keep the BrowserWindowInfo[] type, ensuring loadState() assigns the new snapshot to windows.value.
| await this.ensureChatWindowReady(windowId) | ||
| eventBus.sendToRenderer(DEEPLINK_EVENTS.START, SendTarget.DEFAULT_WINDOW, { |
There was a problem hiding this comment.
Send the deeplink to the window you just prepared.
After resolving and waiting on a specific windowId, Line 270 still dispatches to SendTarget.DEFAULT_WINDOW. In multi-window mode that can deliver the payload to a different renderer than the one that was just shown/focused.
🛠️ Minimal fix
- eventBus.sendToRenderer(DEEPLINK_EVENTS.START, SendTarget.DEFAULT_WINDOW, {
+ eventBus.sendToWindow(DEEPLINK_EVENTS.START, windowId, {
msg,
modelId,
systemPrompt,
mentions,
autoSend
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/presenter/deeplinkPresenter/index.ts` around lines 269 - 270, The
deeplink is being sent to SendTarget.DEFAULT_WINDOW after awaiting
ensureChatWindowReady(windowId), which can route the payload to the wrong
renderer in multi-window mode; change the call to
eventBus.sendToRenderer(DEEPLINK_EVENTS.START, windowId, { ... }) (or use the
API variant that targets a specific window) so the deeplink is delivered to the
specific windowId you just prepared (refer to ensureChatWindowReady,
eventBus.sendToRenderer, DEEPLINK_EVENTS.START and SendTarget.DEFAULT_WINDOW).
| private focusBoundWebContents(webContentsId: number): void { | ||
| const targetContents = electronWebContents.fromId(webContentsId) | ||
| if (!targetContents || targetContents.isDestroyed()) { | ||
| return | ||
| } | ||
|
|
||
| if (existingTabId !== null && existingTabId !== tabId) { | ||
| const targetWindow = BrowserWindow.fromWebContents(targetContents) | ||
| if (!targetWindow || targetWindow.isDestroyed()) { | ||
| return | ||
| } | ||
|
|
||
| presenter.windowPresenter.show(targetWindow.id, true) | ||
| } |
There was a problem hiding this comment.
Handle the floating chat window separately when refocusing a bound conversation.
BrowserWindow.fromWebContents() can resolve to the floating chat window, but windowPresenter.show(targetWindow.id, true) only works for IDs stored in WindowPresenter.windows. In that case this path becomes a no-op instead of surfacing the existing conversation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/presenter/sessionPresenter/managers/conversationManager.ts` around
lines 123 - 135, The refocus path can hit a floating chat BrowserWindow that
isn't tracked in WindowPresenter.windows causing
presenter.windowPresenter.show(targetWindow.id, true) to be a no-op; update
focusBoundWebContents to detect whether targetWindow.id exists in
presenter.windowPresenter.windows (or equivalent registry) and if it is not
managed, fall back to directly making the floating window visible and focused
(e.g., call targetWindow.show() and targetWindow.focus() or use the
windowPresenter's floating-window API if one exists) instead of only calling
presenter.windowPresenter.show(targetWindow.id, true).
| public async createAppWindow(options?: { | ||
| initialRoute?: string | ||
| x?: number | ||
| y?: number | ||
| }): Promise<number | null> { | ||
| return await this.createManagedWindow({ | ||
| initialTab: { | ||
| url: | ||
| options?.initialRoute === 'chat' || !options?.initialRoute | ||
| ? 'local://chat' | ||
| : `local://${options.initialRoute}` | ||
| }, | ||
| windowType: 'chat', | ||
| x: options?.x, | ||
| y: options?.y | ||
| }) |
There was a problem hiding this comment.
Wire initialRoute through to the chat renderer load path.
createAppWindow() stores the requested route in initialTab.url, but the chat branch of createManagedWindow() always loads #/chat. Any caller passing initialRoute !== 'chat' still lands on chat, so the new option is effectively a no-op.
Also applies to: 845-857
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/presenter/windowPresenter/index.ts` around lines 503 - 518,
createAppWindow sets initialTab.url but createManagedWindow's chat branch always
loads '#/chat', so initialRoute is ignored; update createManagedWindow (the
chat/window loading code path) to parse/use the provided initialTab.url (or
fallback to 'local://chat') to construct the renderer load path (i.e., replace
hardcoded '#/chat' with a hash/route derived from initialTab.url) so callers
that pass initialRoute != 'chat' actually land on that route; apply the same
change to the other chat-load site referenced around the 845-857 area.
| const managedWindowState = windowStateManager({ | ||
| defaultWidth, | ||
| defaultHeight | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file in question and get context around the specified lines
wc -l src/main/presenter/windowPresenter/index.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 113
🏁 Script executed:
# Read the file with context around lines 571-574
sed -n '550,600p' src/main/presenter/windowPresenter/index.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 1507
🏁 Script executed:
# Search for windowStateManager definition and imports
rg "windowStateManager" -B 2 -A 5 src/main/presenter/windowPresenter/Repository: ThinkInAIXYZ/deepchat
Length of output: 2040
🏁 Script executed:
# Check what parameters electron-window-state accepts
rg "electron-window-state" src/main/presenter/windowPresenter/Repository: ThinkInAIXYZ/deepchat
Length of output: 187
Persist chat and browser window state in separate files.
Both window types currently use the default electron-window-state file. After one window saves its bounds, the other type restores those bounds too, so the 800x620 vs 600x620 defaults stop applying after the first saved window.
🛠️ Suggested fix
const managedWindowState = windowStateManager({
+ file: windowType === 'browser' ? 'browser-window-state.json' : 'app-window-state.json',
defaultWidth,
defaultHeight
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const managedWindowState = windowStateManager({ | |
| defaultWidth, | |
| defaultHeight | |
| }) | |
| const managedWindowState = windowStateManager({ | |
| file: windowType === 'browser' ? 'browser-window-state.json' : 'app-window-state.json', | |
| defaultWidth, | |
| defaultHeight | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/presenter/windowPresenter/index.ts` around lines 571 - 574, The
windowStateManager call (managedWindowState) is using the same default state
file for both chat and browser windows, causing one window's saved bounds to be
applied to the other; update the windowStateManager initialization to pass a
distinct state filename per window type (e.g., derive a unique file name or key
from the window type such as "chat-window-state" vs "browser-window-state") when
creating managedWindowState so each window type loads/saves its own bounds;
locate the calls to windowStateManager/managedWindowState (the place where
defaultWidth/defaultHeight are passed) and add the unique file option or key so
chat and browser windows persist separately.
|
|
||
| @source '../**/*.{vue,ts,tsx,js,jsx}'; | ||
| @source '../../shell/**/*.{vue,ts,tsx,js,jsx}'; | ||
| @source '../../browser/**/*.{vue,ts,tsx,js,jsx}'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Finding lint configuration files ==="
fd -HI 'biome.*\.json|\.stylelintrc.*|stylelint.*\.(js|cjs|mjs|json)' 2>/dev/null || echo "No pattern matches found"
echo ""
echo "=== Checking for `@source`, ignoreAtRules, or tailwindcss references ==="
rg -n '@source|ignoreAtRules|tailwindcss' \
--glob 'biome*.json' \
--glob '.stylelintrc*' \
--glob 'stylelint*.{js,cjs,mjs,json}' \
--glob 'package.json' \
2>/dev/null || echo "No matches found"
echo ""
echo "=== Checking the actual style.css file ==="
cat -n src/renderer/src/assets/style.css 2>/dev/null || echo "File not found"Repository: ThinkInAIXYZ/deepchat
Length of output: 40970
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== .stylelintrc.json ==="
cat .stylelintrc.json
echo ""
echo "=== biome.json ==="
cat biome.jsonRepository: ThinkInAIXYZ/deepchat
Length of output: 2891
Configure CSS tooling to recognize Tailwind v4 at-rules.
The .stylelintrc.json extends stylelint-config-standard-scss, which includes the at-rule-no-unknown rule that will flag @source, @plugin, and @theme directives as invalid. Similarly, Biome's CSS linter has no Tailwind v4 support and will reject these directives. Update the Stylelint config to ignore Tailwind-specific at-rules:
{
"rules": {
"at-rule-no-unknown": [
true,
{
"ignoreAtRules": [
"source",
"plugin",
"theme",
"layer",
"apply",
"custom-variant"
]
}
]
}
}Consider also extending or configuring Biome to suppress CSS linting errors for Tailwind directives, or disable CSS linting if Biome doesn't fully support Tailwind v4 yet.
🧰 Tools
🪛 Biome (2.4.4)
[error] 9-9: Tailwind-specific syntax is disabled.
(parse)
🪛 Stylelint (17.4.0)
[error] 9-9: Unexpected unknown at-rule "@source" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/src/assets/style.css` at line 9, The CSS linter is flagging
Tailwind v4 at-rules like `@source/`@plugin/@theme in
src/renderer/src/assets/style.css; update the Stylelint configuration (the
.stylelintrc.json that currently extends stylelint-config-standard-scss) to
allow Tailwind at-rules by adding an at-rule-no-unknown rule with ignoreAtRules
for "source", "plugin", "theme", "layer", "apply", and "custom-variant"; also
adjust Biome’s CSS lint settings (or disable CSS linting in Biome) to similarly
ignore or permit these Tailwind directives so the `@source` line in style.css is
not rejected.
| clearActiveConversationBinding(webContentsId: number): Promise<void> | ||
| /** @deprecated Use clearActiveConversationBinding instead. */ | ||
| clearActiveThread(webContentsId: number): Promise<void> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and locate the relevant files
find . -type f -name "*.presenter*" | head -20Repository: ThinkInAIXYZ/deepchat
Length of output: 594
🏁 Script executed:
# Read the thread presenter type definition file
cat -n src/shared/types/presenters/thread.presenter.d.ts | head -200Repository: ThinkInAIXYZ/deepchat
Length of output: 8160
🏁 Script executed:
# Also search for the implementation file
find . -type f -name "*thread.presenter*" ! -name "*.d.ts" 2>/dev/null | head -10Repository: ThinkInAIXYZ/deepchat
Length of output: 47
🏁 Script executed:
# Let's search for clearActiveThread and clearActiveConversationBinding to understand the migration
rg "clearActiveThread|clearActiveConversationBinding" --type ts --type tsx -B 2 -A 2Repository: ThinkInAIXYZ/deepchat
Length of output: 92
🏁 Script executed:
# Search for implementation files (not .d.ts)
find . -type f -path "*/presenters/*thread*" ! -name "*.d.ts" 2>/dev/nullRepository: ThinkInAIXYZ/deepchat
Length of output: 47
🏁 Script executed:
# Search for any references to tabId vs webContentsId to understand the relationship
rg "tabId|webContentsId" --type ts -B 1 -A 1 | head -100Repository: ThinkInAIXYZ/deepchat
Length of output: 7421
🏁 Script executed:
# Look for implementation of clearActiveThread or clearActiveConversationBinding
rg "clearActiveThread|clearActiveConversationBinding" --type ts -B 3 -A 5 2>/dev/null | head -150Repository: ThinkInAIXYZ/deepchat
Length of output: 7742
🏁 Script executed:
# Verify the legacy interface to confirm original parameter name
cat -n src/shared/types/presenters/legacy.presenters.d.ts | grep -A 2 -B 2 "clearActiveThread"Repository: ThinkInAIXYZ/deepchat
Length of output: 357
🏁 Script executed:
# Check how other deprecated aliases handle parameter naming in session.presenter
cat -n src/shared/types/presenters/session.presenter.d.ts | grep -B 3 -A 3 "tabId"Repository: ThinkInAIXYZ/deepchat
Length of output: 1194
Deprecated alias should preserve the original tabId parameter name.
The original clearActiveThread method accepted tabId (as shown in legacy.presenters.d.ts:1217), but the current deprecated version at line 164 changes the parameter to webContentsId. This breaks the deprecation pattern used elsewhere in the codebase (see session.presenter.d.ts where deprecated methods preserve old parameter names like tabId). Existing callers using the deprecated method become confused about parameter semantics. Either keep the deprecated signature as clearActiveThread(tabId: number) for full backward compatibility, or remove it in a breaking change.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/types/presenters/thread.presenter.d.ts` around lines 162 - 164,
The deprecated alias clearActiveThread currently uses parameter name
webContentsId which differs from the original API; update its signature to
preserve the original parameter name by changing
clearActiveThread(webContentsId: number) to clearActiveThread(tabId: number)
(matching the legacy.presenters.d.ts usage) so callers see the old parameter
name for backward compatibility; ensure the implementation still forwards to
clearActiveConversationBinding and retains deprecation comment.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Removed Features
UI/UX Updates