Skip to content

fix(desktop): prevent browser webview reload on tab/workspace switch#2

Merged
MocA-Love merged 8 commits intomainfrom
fix/desktop-browser-tab-reload
Mar 27, 2026
Merged

fix(desktop): prevent browser webview reload on tab/workspace switch#2
MocA-Love merged 8 commits intomainfrom
fix/desktop-browser-tab-reload

Conversation

@MocA-Love
Copy link
Copy Markdown
Owner

@MocA-Love MocA-Love commented Mar 27, 2026

概要

Electronの<webview>タグはDOMの親要素が変わる(reparent)と、コンテンツが完全にリロードされる。従来はタブ/ワークスペース切り替え時にBrowserPaneがアンマウントされ、webviewが隠しコンテナに移動(reparent)されていたため、毎回ハードリロードが発生していた。

このPRでは、webviewを含むタブとワークスペースページをマウントしたまま維持し、DOMのreparentを完全に排除することでリロードを防止する。

また、webview内でマウスの戻る/進むボタンが効かない問題も修正。

変更内容

タブ切り替えのリロード防止(PersistentTabRenderer)

  • webviewペインを含むタブだけ、非アクティブ時もマウントを維持(オフスクリーン配置)
  • webviewを含まないタブ(ターミナル、チャット、ファイル)は従来通りアンマウント(メモリ節約)
  • 同時マウントされるMosaicインスタンスのドラッグ&ドロップ競合を防ぐため、タブごとにユニークなMosaic IDを使用

ワークスペース切り替えのリロード防止(KeepAliveWorkspaces)

  • 訪問済みワークスペースページをDashboardLayoutレベル(ルーターのOutletの上)で保持
  • WorkspacePageworkspaceIdOverrideプロップを受け取り、ルーターのマッチ外でも動作可能に
  • workspaceIduseParams()ではなくprops経由でWorkspaceLayout → ContentView → TabsContentに伝搬。非アクティブなワークスペースが誤ったIDを読む問題を防止
  • 削除されたワークスペースはDB上のワークスペース一覧と照合して自動的にkeep-aliveリストから除去

コンテキスト分離(WorkspaceIdContext)

  • useParams()でworkspaceIdを取得していた9コンポーネントを、WorkspacePageが提供するReact Contextから読むように変更
  • 非アクティブなワークスペースのコンポーネントがアクティブなワークスペースのデータにアクセスしてしまう問題を防止

ホットキー分離

  • WorkspacePage内の全useAppHotkey呼び出しに{ enabled: isActive }を追加
  • アクティブなワークスペースのみがキーボードショートカットに応答
  • usePresetHotkeysも同様にenabledオプションに対応

マウスの戻る/進むボタン対応(webview内)

  • macOS: dom-ready時にexecuteJavaScriptでwebviewのゲストページにマウスイベントリスナーを注入
  • Windows/Linux: メインプロセスのBrowserWindowapp-commandイベントをハンドル

永続webviewラッパー

  • 各webviewを永続的な<div>ラッパーで包み、webviewのparentNodeが変わらないようにする
  • アンマウント時はラッパー(webviewではなく)を隠しコンテナに退避
  • 再マウント時はラッパーを回収 — webviewの親は常に同じラッパー、reparentなし、リロードなし

備考

このブランチにはリロード修正とは無関係のコミットa7816ee3(Excel/スプレッドシートファイルビューア)が含まれています。

テスト項目

  • ブラウザタブでページを開き、別タブに切り替えて戻る → リロードされないこと
  • サイドバーから別のワークスペースに切り替えて戻る → リロードされないこと
  • ワークスペースを削除 → keep-aliveリストから除去されること(メモリリークなし)
  • キーボードショートカット(Cmd+T、Cmd+Wなど) → アクティブなワークスペースのみ反応すること
  • マウスの戻る/進むボタン → webview内でカーソルがある時も動作すること
  • タブのドラッグ&ドロップ → 正常に動作すること
  • ファイルビューア、変更サイドバー、右サイドバー → ワークスペース切り替え後に正しいデータが表示されること
  • ターミナル/チャットタブ → タブ切り替え時に通常通りアンマウント/再マウントされること(keep-aliveされないこと)

Summary by CodeRabbit

  • New Features

    • Mouse back/forward support for embedded webviews.
    • Persistent workspace and tab keeping so workspace and webview tabs stay loaded when switching.
    • Better persistence of webviews to preserve state across navigations.
  • Improvements

    • Hotkeys now respect active/inactive workspace or tab state.
    • Workspace scoping updated so workspace context is consistently used across views.

Electron's <webview> tag reloads its content whenever the element is
reparented in the DOM. The previous approach rendered only the active
tab, causing BrowserPane to unmount on every tab switch and park the
webview in a hidden container (DOM reparent) — triggering a hard reload.

- Add PersistentTabRenderer that keeps all workspace tabs mounted and
  hides inactive ones via off-screen positioning (not display:none, which
  stops Electron's compositor)
- Wrap each webview in a persistent wrapper div so the webview's
  parentNode never changes during park/reclaim cycles
- Use unique Mosaic IDs per tab to prevent drag-drop conflicts between
  simultaneously mounted Mosaic instances
Keep workspace pages mounted across workspace switches so that webview
elements are never removed from the DOM.

- Add KeepAliveWorkspaces component that replaces <Outlet /> for
  workspace routes, rendering all visited workspaces simultaneously and
  hiding inactive ones off-screen
- Make WorkspacePage accept workspaceId as a prop override so it works
  outside the router's matched-route context
- Thread workspaceId through WorkspaceLayout → ContentView → TabsContent
  via props instead of useParams(), preventing hidden workspaces from
  reading the wrong workspace ID from the active route
…p-alive

When multiple WorkspacePage instances are mounted simultaneously
(KeepAliveWorkspaces), several issues occurred:

1. Components using useParams() read the ACTIVE workspace's ID instead
   of their own — causing wrong files, wrong sidebar data, and wrong
   tab lists. Fixed by introducing WorkspaceIdContext and replacing
   useParams() with useWorkspaceId() in 9 affected components.

2. Mosaic drag-drop was broken because GroupItem used a static mosaicId
   that no longer matched the per-tab dynamic ID. Fixed by computing
   the active tab's mosaicId dynamically.

3. All workspace hotkeys fired for every mounted workspace. Fixed by
   passing `enabled: isActive` to all useAppHotkey calls so only the
   active workspace responds to keyboard shortcuts.
Electron's <webview> tags consume mouse events in their guest process,
so mouse back/forward buttons (button 3/4) never reach the host
renderer's event listeners. Handle the `app-command` event on the main
BrowserWindow to intercept `browser-backward` and `browser-forward`
commands, forwarding them to the focused webview's navigation history.

Also update usePresetHotkeys to accept an options parameter for
consistency with the enabled-flag pattern used across workspace hotkeys.
The `app-command` event is Windows/Linux only — it never fires on macOS.
Instead, inject a mouse event listener into the webview guest page via
executeJavaScript on every `dom-ready`. The injected script calls
`history.back()` / `history.forward()` directly inside the guest when
mouse buttons 3/4 are pressed.

The existing `app-command` handler in the main process is kept as a
fallback for Windows/Linux.
…spaces

PersistentTabRenderer now only keeps tabs mounted when they contain a
webview pane. Tabs with only terminals, chat, or file viewers unmount
normally on tab switch, reducing memory usage.

KeepAliveWorkspaces now watches the workspace list from the database
and automatically removes deleted workspaces from the keep-alive set,
preventing stale WorkspacePage instances from lingering in memory.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 27, 2026

Warning

Rate limit exceeded

@MocA-Love has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 44 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 11 minutes and 44 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2af3aa67-803a-4458-9ca0-b2211c230d03

📥 Commits

Reviewing files that changed from the base of the PR and between d261b13 and 0ef6124.

📒 Files selected for processing (22)
  • apps/desktop/src/main/windows/main.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/KeepAliveWorkspaces.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/hooks/usePresetHotkeys.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/ChangesContent.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/FileDiffSection/FileDiffSection.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/PersistentTabRenderer.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/PresetsBar/PresetsBar.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceIdContext.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceLayout/WorkspaceLayout.tsx
📝 Walkthrough

Walkthrough

Adds a WorkspaceId React context, converts many workspace-related components to consume workspaceId from context, implements keep-alive mounting for workspace pages and tabs that host webviews, persists webview DOM wrappers for reparenting, and forwards Electron app back/forward commands to the focused guest webContents.

Changes

Cohort / File(s) Summary
Electron main
apps/desktop/src/main/windows/main.ts
Imports webContents and adds window.on("app-command", ...) handler to forward browser-backward/browser-forward to the focused guest's navigationHistory.goBack()/goForward().
Workspace ID context
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceIdContext.tsx
New WorkspaceIdProvider and useWorkspaceId() hook (throws when no provider).
Keep-alive workspaces
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/KeepAliveWorkspaces.tsx, apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx
New KeepAliveWorkspaces component; DashboardLayout now renders it instead of Outlet. Tracks visited workspace IDs and mounts multiple WorkspacePage instances, hiding inactive ones off-screen.
WorkspacePage & routing changes
apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx
WorkspacePage exported and accepts workspaceIdOverride?: string and isActive?: boolean; uses generic useParams/useSearch hooks; hotkeys now receive enabled based on isActive.
Persistent tab rendering
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/PersistentTabRenderer.tsx, .../TabsContent/index.tsx
Added PersistentTabRenderer to keep webview-containing tabs mounted (inactive ones hidden off-screen); TabsContent now takes workspaceId prop and uses PersistentTabRenderer.
Persistent webview lifecycle
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.ts
Introduces wrapperRegistry to park a persistent wrapper <div> for each pane; mount/unmount and destroy logic updated to move wrappers instead of raw webviews; injects guest JS on macOS to map mouse buttons to history navigation.
WorkspaceId prop threading
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceLayout/WorkspaceLayout.tsx, .../ContentView/index.tsx
WorkspaceLayout and ContentView require and forward workspaceId prop throughout the layout.
Context migration: components
apps/desktop/src/renderer/screens/main/components/...
Multiple components replaced useParams() with useWorkspaceId() (e.g., ChangesContent, FileDiffSection, EmptyTabView, GroupStrip, FileViewerPane, PresetsBar, RightSidebar and its children) to source workspaceId from context.
Tab/pane identity & drag payloads
apps/desktop/src/renderer/screens/main/components/.../TabView/index.tsx, .../GroupStrip/GroupItem.tsx
Mosaic mosaicId now scoped per tab (${MOSAIC_ID}-${tab.id}); drag payload appends -${activeTabId} when dropping onto active tab.
Hotkeys
apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/hooks/usePresetHotkeys.ts
usePresetHotkeys signature now accepts options?: { enabled?: boolean } and forwards it to useAppHotkey registrations.

Sequence Diagram(s)

sequenceDiagram
    participant Router as TanStack Router
    participant DashLayout as DashboardLayout
    participant KeepAlive as KeepAliveWorkspaces
    participant WSPage as WorkspacePage
    participant WSContext as WorkspaceIdContext
    participant Child as Child Components

    Router->>DashLayout: route activates dashboard
    DashLayout->>KeepAlive: render KeepAliveWorkspaces (replaces Outlet)
    KeepAlive->>KeepAlive: detect /workspace/$id, add to visitedIds
    KeepAlive->>WSPage: render WorkspacePage for each visitedId
    WSPage->>WSContext: provide workspaceId
    Child->>WSContext: call useWorkspaceId()
    Child->>Child: fetch data & register hotkeys using workspaceId
Loading
sequenceDiagram
    participant Electron as Electron Main
    participant FocusFinder as Focus Finder
    participant Guest as Guest WebContents
    participant History as Guest NavigationHistory

    Electron->>Electron: receives app-command(browser-backward/forward)
    Electron->>FocusFinder: enumerate all webContents to find focused guest
    FocusFinder->>Guest: identify focused guest webContents
    Electron->>History: call guest.navigationHistory.goBack()/goForward()
    History->>Guest: guest navigates back/forward
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Possibly related PRs

Poem

🐰 I hopped through layouts, IDs in tow,
Kept tabs alive where webviews go.
Back-buttons whisper, guests answer all,
Context roots steady — no route-based fall.
Hooray — persistent workspaces, let's brawl! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.54% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(desktop): prevent browser webview reload on tab/workspace switch' directly and clearly describes the main problem being solved (webview reload prevention during navigation), which aligns with the core objective of this large refactor.
Description check ✅ Passed The PR description is comprehensive and well-structured in Japanese, covering changes across multiple subsystems (tabs, workspaces, context, hotkeys, webview, mouse navigation) with rationale and test items. However, it deviates significantly from the repository's English description template.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/desktop-browser-tab-reload

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Keep both our useWorkspaceId import and main's new githubQueryPolicy
imports. The useParams import was already removed by our branch.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (5)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx (1)

144-148: Dead fallback code after migration to useWorkspaceId().

Since useWorkspaceId() throws when called outside a provider (never returns undefined), the ?? "" fallback on line 146 and the enabled: !!workspaceId guard on line 147 are now effectively dead code. The same applies to lines 371 and 420.

This isn't a bug, but the defensive checks are misleading. Consider removing them for clarity, or if you want to keep defensive coding for potential future refactoring, add a comment explaining the intent.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx`
around lines 144 - 148, The code uses useWorkspaceId() which never returns
undefined (it throws outside a provider), so remove the dead fallback and
redundant enabled guard around the query: update the
electronTrpc.workspaces.get.useQuery call to pass workspaceId directly (no "??
''") and drop the { enabled: !!workspaceId } option; alternatively, if you
intentionally want to keep defensive checks, add a short comment next to
useWorkspaceId(), workspaceId, or the electronTrpc.workspaces.get.useQuery
invocation explaining that the fallback and enabled guard are retained
intentionally for future refactors.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx (1)

35-35: Dead guards after migration to useWorkspaceId().

Similar to other migrated components, the null/undefined guards for activeWorkspaceId are now dead code since useWorkspaceId() throws rather than returning undefined. This includes:

  • Line 54: activeWorkspaceId ?? ""
  • Line 55: enabled: !!activeWorkspaceId
  • Lines 113, 120, 222, 227, 232, 238, 249, 272: Various if (!activeWorkspaceId) guards

Not a bug, but these guards are now misleading.

Also applies to: 53-56

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx`
at line 35, Remove the now-dead null/undefined guards added for
activeWorkspaceId (which is returned by useWorkspaceId() and will throw instead
of returning undefined); specifically, eliminate usages like "activeWorkspaceId
?? ''", boolean coercions "enabled: !!activeWorkspaceId", and all "if
(!activeWorkspaceId)" branches in GroupStrip (references: activeWorkspaceId,
useWorkspaceId, GroupStrip component) and either simplify to use
activeWorkspaceId directly or replace with explicit non-null
assumptions/assertions where necessary so the logic is not misleading.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx (1)

130-131: Dead fallback after migration to useWorkspaceId().

Since useWorkspaceId() throws when called outside a provider rather than returning undefined, the ?? worktreePath fallback on line 131 is now dead code. The same applies to other null-coalescing operators and boolean guards for workspaceId throughout this file (e.g., lines 360, 409, 444).

The previous behavior with useParams({ strict: false }) allowed workspaceId to be undefined, making the fallback meaningful. Now, the component simply crashes if not wrapped in WorkspaceIdProvider.

♻️ Suggested simplification
 	const workspaceId = useWorkspaceId();
-	const normalizedWorkspaceId = workspaceId ?? worktreePath;
+	const normalizedWorkspaceId = workspaceId;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx`
around lines 130 - 131, The null-coalescing fallback using worktreePath is dead
because useWorkspaceId() now throws when no provider exists; remove the
unnecessary fallback and guards: change the normalizedWorkspaceId declaration to
simply use the provider value (keep "const workspaceId = useWorkspaceId();" and
remove "const normalizedWorkspaceId = workspaceId ?? worktreePath;"), replace
any uses of "normalizedWorkspaceId" or expressions like "workspaceId ??
worktreePath" with "workspaceId", and remove boolean checks that branch on
workspaceId being undefined (e.g., any conditional guards or ternaries expecting
undefined), relying on the provider guarantee instead.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/PersistentTabRenderer.tsx (1)

28-37: Consider defensive null-check for tab.layout.

The extractPaneIdsFromLayout function (per context snippet) doesn't handle null/undefined layouts. While Tab.layout is documented as "Always defined," a defensive check would prevent runtime errors if this invariant is ever violated.

🛡️ Optional defensive check
 const tabsWithWebview = useMemo(() => {
   const ids = new Set<string>();
   for (const tab of tabs) {
+    if (!tab.layout) continue;
     const paneIds = extractPaneIdsFromLayout(tab.layout);
     if (paneIds.some((id) => panes[id]?.type === "webview")) {
       ids.add(tab.id);
     }
   }
   return ids;
 }, [tabs, panes]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/PersistentTabRenderer.tsx`
around lines 28 - 37, tabsWithWebview calls extractPaneIdsFromLayout(tab.layout)
without guarding against tab.layout being null/undefined; add a defensive check
in the useMemo loop (in PersistentTabRenderer) so you only call
extractPaneIdsFromLayout when tab.layout is non-null/undefined (or compute an
empty array/skip that tab otherwise), and continue using panes[id]?.type to
detect "webview" — reference the tabsWithWebview computation, the tab.layout
property, and extractPaneIdsFromLayout to locate the change.
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/KeepAliveWorkspaces.tsx (1)

27-35: Consider bounding the number of kept-alive workspaces.

The visitedIds list grows unboundedly as users navigate between workspaces. While deleted workspaces are evicted, frequently switching between many existing workspaces could accumulate significant memory usage (each mounted WorkspacePage includes its own state, queries, and potentially webviews).

A maximum keep-alive count (e.g., 5-10 most recent) could limit memory footprint while preserving the core benefit.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/KeepAliveWorkspaces.tsx`
around lines 27 - 35, The visitedIds/visitedSetRef list grows unboundedly; add a
MAX_KEEP_ALIVE (e.g., const MAX_KEEP_ALIVE = 5) and trim oldest entries when
adding a new activeWorkspaceId. In the useEffect that watches activeWorkspaceId,
when a new id is added to visitedSetRef and setVisitedIds, enforce: push the new
id to the end of the ordered visitedIds array, and if length > MAX_KEEP_ALIVE
remove the earliest id(s) from both the array and visitedSetRef before calling
setVisitedIds(Array.from(...)) so visitedIds (state) preserves recency order
while visitedSetRef (ref) stays in sync. Ensure you update both visitedIds and
visitedSetRef in the same effect to avoid drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/`$workspaceId/page.tsx:
- Around line 117-118: Remove the redundant declaration const genericNavigate =
useNavigate(); and consolidate to a single useNavigate() call (const navigate =
useNavigate()); then replace all references to genericNavigate with navigate
(e.g., in the navigation calls around the workspace page handlers) so there is
only one navigate variable used throughout the component.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.ts`:
- Around line 201-205: The guest mouse-nav shim is being injected
unconditionally inside handleDomReady via webview.executeJavaScript; change that
so the executeJavaScript call that injects the shim only runs when
PLATFORM.IS_MAC is true (gate the injection with PLATFORM.IS_MAC), leaving the
existing cleanup logic as-is; locate the injection by searching for
handleDomReady and the executeJavaScript call that loads the guest mouse-nav
shim and wrap/guard that call with PLATFORM.IS_MAC so Windows/Linux no longer
run the shim.

---

Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/KeepAliveWorkspaces.tsx`:
- Around line 27-35: The visitedIds/visitedSetRef list grows unboundedly; add a
MAX_KEEP_ALIVE (e.g., const MAX_KEEP_ALIVE = 5) and trim oldest entries when
adding a new activeWorkspaceId. In the useEffect that watches activeWorkspaceId,
when a new id is added to visitedSetRef and setVisitedIds, enforce: push the new
id to the end of the ordered visitedIds array, and if length > MAX_KEEP_ALIVE
remove the earliest id(s) from both the array and visitedSetRef before calling
setVisitedIds(Array.from(...)) so visitedIds (state) preserves recency order
while visitedSetRef (ref) stays in sync. Ensure you update both visitedIds and
visitedSetRef in the same effect to avoid drift.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx`:
- Line 35: Remove the now-dead null/undefined guards added for activeWorkspaceId
(which is returned by useWorkspaceId() and will throw instead of returning
undefined); specifically, eliminate usages like "activeWorkspaceId ?? ''",
boolean coercions "enabled: !!activeWorkspaceId", and all "if
(!activeWorkspaceId)" branches in GroupStrip (references: activeWorkspaceId,
useWorkspaceId, GroupStrip component) and either simplify to use
activeWorkspaceId directly or replace with explicit non-null
assumptions/assertions where necessary so the logic is not misleading.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/PersistentTabRenderer.tsx`:
- Around line 28-37: tabsWithWebview calls extractPaneIdsFromLayout(tab.layout)
without guarding against tab.layout being null/undefined; add a defensive check
in the useMemo loop (in PersistentTabRenderer) so you only call
extractPaneIdsFromLayout when tab.layout is non-null/undefined (or compute an
empty array/skip that tab otherwise), and continue using panes[id]?.type to
detect "webview" — reference the tabsWithWebview computation, the tab.layout
property, and extractPaneIdsFromLayout to locate the change.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx`:
- Around line 130-131: The null-coalescing fallback using worktreePath is dead
because useWorkspaceId() now throws when no provider exists; remove the
unnecessary fallback and guards: change the normalizedWorkspaceId declaration to
simply use the provider value (keep "const workspaceId = useWorkspaceId();" and
remove "const normalizedWorkspaceId = workspaceId ?? worktreePath;"), replace
any uses of "normalizedWorkspaceId" or expressions like "workspaceId ??
worktreePath" with "workspaceId", and remove boolean checks that branch on
workspaceId being undefined (e.g., any conditional guards or ternaries expecting
undefined), relying on the provider guarantee instead.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx`:
- Around line 144-148: The code uses useWorkspaceId() which never returns
undefined (it throws outside a provider), so remove the dead fallback and
redundant enabled guard around the query: update the
electronTrpc.workspaces.get.useQuery call to pass workspaceId directly (no "??
''") and drop the { enabled: !!workspaceId } option; alternatively, if you
intentionally want to keep defensive checks, add a short comment next to
useWorkspaceId(), workspaceId, or the electronTrpc.workspaces.get.useQuery
invocation explaining that the fallback and enabled guard are retained
intentionally for future refactors.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0f446cd0-c646-448a-8e66-c676151aeca5

📥 Commits

Reviewing files that changed from the base of the PR and between d261b13 and 6d73a23.

📒 Files selected for processing (22)
  • apps/desktop/src/main/windows/main.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/KeepAliveWorkspaces.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/hooks/usePresetHotkeys.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/ChangesContent.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/FileDiffSection/FileDiffSection.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/PersistentTabRenderer.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/PresetsBar/PresetsBar.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceIdContext.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceLayout/WorkspaceLayout.tsx

@MocA-Love MocA-Love self-assigned this Mar 27, 2026
- Remove redundant `genericNavigate` declaration; reuse existing
  `navigate` from useNavigate()
- Gate mouse-nav shim injection to macOS only (PLATFORM.IS_MAC);
  Windows/Linux already handle side buttons via `app-command`
@MocA-Love
Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 27, 2026

✅ Actions performed

Full review triggered.

@MocA-Love MocA-Love merged commit 5d39b15 into main Mar 27, 2026
1 check passed
MocA-Love added a commit that referenced this pull request Apr 4, 2026
#1 ダッシュボード戻り時のポーリング永続: activeWorkspaceId=null時に
   deactivateAll()を呼ぶ。setActiveWorkspace(null)対応追加

#2 deactivateされたWSのMapエントリ蓄積: 問題の根本はregisterが増え続ける
   ことではなく、全WSがisActive:trueで起動していたこと。#3で解消

#3 起動直後の全WSポーリング並走: registerWorkspaceをisActive:falseに変更。
   タイマーはactivateWorkspace/setActiveWorkspaceでのみ起動

#4 WS切替時の初回表示5s遅延: activateWorkspace/setActiveWorkspace時に
   即時sync(syncPRStatus+syncPRComments)を実行

追加改善:
- startTimersに防衛的stopTimers追加(二重タイマー防止)
- onWindowFocus()をデッドコードとして削除
- deactivateAll()メソッド追加
MocA-Love added a commit that referenced this pull request Apr 11, 2026
8 additional upstream commits cherry-picked via PR #149.

=== Cherry-picked (8 commits) ===
PR8 (#149): 23b53ed, 87b6fff, d7eed78, 1bdd700, bf07410,
            ec1bacc, a44108a, 2c67364
MocA-Love pushed a commit that referenced this pull request Apr 18, 2026
…uperset-sh#3517)

* remove 7 day rule

* Upgrade mastra

* upgrade ai

* Ad mastra

* refactor(desktop): remove dead provider-diagnostics plumbing

The provider-diagnostics store was fed by callSmallModel's per-attempt
reporting, which was removed when small-model tasks moved to direct AI-SDK
+ mastracode's AuthStorage. Nothing writes to the issue map anymore, so the
clearIssue mutation, getStatuses query, and diagnosticStatus plumbing in
ModelsSettings were all no-ops.

Settings still surfaces "Session expired / Reconnect" via auth-status alone.
ProviderIssue type collapsed from 8 codes to just "expired" to match.

* fix(auth): auto-refresh expired Anthropic OAuth tokens

Anthropic credentials were read via authStorage.get() everywhere, so
mastracode's built-in refresh flow never ran. Once the 1-hour access
token expired, status flipped to "Reconnect" and users had to do a
full PKCE re-auth, even though a valid refresh token was already
stored.

Resolvers now call authStorage.getApiKey() for oauth creds on expiry,
which triggers refreshToken() and persists the refreshed credential.
getAnthropicAuthStatus does the same before declaring issue: "expired".
Mirrors the pattern already used for OpenAI small-model auth.

* review: address PR feedback from cubic + coderabbit + greptile

- host-service ai-branch-name: run trailing-trim after slice so a
  100-char truncation can't re-introduce a bare "." or "-" that git
  rejects as an invalid ref (coderabbit / cubic #2, #7).
- host-service workspace-creation.generateBranchName: reuse the
  existing listBranchNames helper instead of the inline git walk,
  which classified off the short refname and could conflate a local
  "origin/foo" with refs/remotes/origin/foo (coderabbit #3).
- packages/chat shared/small-model: drop the unused
  hasSmallModelCredentials export; only a test mock consumed it
  (greptile #4).
- resolveAnthropicCredential: on refresh failure, return null instead
  of kind:"oauth" with a stale expiresAt so callers fall back cleanly
  (cubic #8).
- chat-service.getAnthropicAuthStatus: log context when refresh throws
  instead of silently swallowing (cubic #9).

* fix(chat): read auth.json directly instead of importing mastracode

Importing createAuthStorage from mastracode loads the entire CLI tree
(fastembed → onnxruntime-node's 208 MB native binary) via eager
top-level requires in mastracode's CJS entry. This crashed
electron-vite bundling and bloated the get-small-model chunk.

getSmallModel now reads mastracode's auth.json file directly using
the same path resolution logic (~/Library/Application Support/mastracode/
on macOS). Zero mastracode import, zero bundle impact. The chunk stays
at 1.2 MB (just @ai-sdk/anthropic + @ai-sdk/openai).

Production build verified: compile:app succeeds, Electron main process
boots with no onnxruntime error.

* docs(desktop): add manual testing plan for PR superset-sh#3517

* fix api key storage slot

* fix(auth): store API keys in dedicated slot so OAuth doesn't clobber them

setApiKeyForProvider and setStoredAnthropicApiKeyFromEnvVariables now
use authStorage.setStoredApiKey() (writes to "apikey:<provider>")
instead of authStorage.set() (writes to the main "<provider>" slot
shared with OAuth). This way connecting/disconnecting OAuth doesn't
overwrite or delete a stored API key.

resolveAuthMethodForProvider falls back to hasStoredApiKey() after
checking the main slot, so status correctly reports authenticated
when only an API key is stored.

* fix(auth): backup/restore API keys across OAuth connect/disconnect

mastracode's resolveModel only reads API keys from the main
authStorage slot (authStorage.get("anthropic")). OAuth login
overwrites this slot, and disconnect removes it — losing any
previously saved API key.

Fix: backup the API key to the dedicated apikey: slot before OAuth
connect, restore it after disconnect. setApiKeyForProvider now writes
to both slots (main for resolveModel compatibility, apikey: for
backup). resolveAuthMethodForProvider checks both.

Applies to both Anthropic and OpenAI providers.

* chore: add upstream PR reference to auth workaround

Point to mastra-ai/mastra#15483 so the backup/restore code can be
removed once upstream lands and we bump mastracode.

* refactor(desktop): derive settings provider action from status

Replace the cascade of if/else + canDisconnect flag with a single
getProviderAction(status) → connect | reconnect | logout | null.
Fixes "Active" badge + "Connect" button showing simultaneously
when authenticated via API key.

* fix(desktop): always show Logout when provider is active

Active providers now always show a Logout button. Clears OAuth or
API key depending on authMethod — no more "Active" badge with no
way to disconnect.

* fix(desktop): simplify OpenAI OAuth dialog + auto-open browser

Match Anthropic dialog's layout: remove the raw OAuth URL display
and "Tip" block, auto-open the browser on OAuth start. Change
"Back" to "Cancel" for consistency.

* refactor(desktop): unify OAuth dialogs into shared OAuthDialog

Extract shared OAuthDialog component with provider config object.
AnthropicOAuthDialog and OpenAIOAuthDialog become thin wrappers
that pass provider-specific labels and options.

* fix(desktop): show 'Copied!' feedback on Copy URL button

* refactor(desktop): merge provider account + API key into single card

Each provider section now renders AccountCard + ConfigRow inside
one rounded card with a divider, instead of two separate cards.
Removes the standalone "API Keys" collapsible section.

* refactor(desktop): compact OAuth row in provider settings card

OAuth row is now a single inline row (label + status + action)
instead of a stacked AccountCard. Both providers share the same
2-row card layout: OAuth row + API key row with divider.

* fix(desktop): contextual buttons in provider settings

Connect is now primary (filled). Save only shows when there's input.
Clear only shows when a key is saved. Removes visual noise from
empty-state provider cards.

* ui(desktop): add provider icons to settings section headers

* ui(desktop): show 'Not connected' badge instead of subtitle for disconnected providers

* ui: remove redundant disconnected subtitle

* ui: remove subtitle text from OAuth rows

* chore: remove dead AccountCard + getProviderSubtitle

* docs: update test plan to match current UI

* chore: move shipped plans to done/

---------

Co-authored-by: AviPeltz <aj.peltz@gmail.com>
MocA-Love pushed a commit that referenced this pull request Apr 18, 2026
…uperset-sh#3517)

* remove 7 day rule

* Upgrade mastra

* upgrade ai

* Ad mastra

* refactor(desktop): remove dead provider-diagnostics plumbing

The provider-diagnostics store was fed by callSmallModel's per-attempt
reporting, which was removed when small-model tasks moved to direct AI-SDK
+ mastracode's AuthStorage. Nothing writes to the issue map anymore, so the
clearIssue mutation, getStatuses query, and diagnosticStatus plumbing in
ModelsSettings were all no-ops.

Settings still surfaces "Session expired / Reconnect" via auth-status alone.
ProviderIssue type collapsed from 8 codes to just "expired" to match.

* fix(auth): auto-refresh expired Anthropic OAuth tokens

Anthropic credentials were read via authStorage.get() everywhere, so
mastracode's built-in refresh flow never ran. Once the 1-hour access
token expired, status flipped to "Reconnect" and users had to do a
full PKCE re-auth, even though a valid refresh token was already
stored.

Resolvers now call authStorage.getApiKey() for oauth creds on expiry,
which triggers refreshToken() and persists the refreshed credential.
getAnthropicAuthStatus does the same before declaring issue: "expired".
Mirrors the pattern already used for OpenAI small-model auth.

* review: address PR feedback from cubic + coderabbit + greptile

- host-service ai-branch-name: run trailing-trim after slice so a
  100-char truncation can't re-introduce a bare "." or "-" that git
  rejects as an invalid ref (coderabbit / cubic #2, #7).
- host-service workspace-creation.generateBranchName: reuse the
  existing listBranchNames helper instead of the inline git walk,
  which classified off the short refname and could conflate a local
  "origin/foo" with refs/remotes/origin/foo (coderabbit #3).
- packages/chat shared/small-model: drop the unused
  hasSmallModelCredentials export; only a test mock consumed it
  (greptile #4).
- resolveAnthropicCredential: on refresh failure, return null instead
  of kind:"oauth" with a stale expiresAt so callers fall back cleanly
  (cubic #8).
- chat-service.getAnthropicAuthStatus: log context when refresh throws
  instead of silently swallowing (cubic #9).

* fix(chat): read auth.json directly instead of importing mastracode

Importing createAuthStorage from mastracode loads the entire CLI tree
(fastembed → onnxruntime-node's 208 MB native binary) via eager
top-level requires in mastracode's CJS entry. This crashed
electron-vite bundling and bloated the get-small-model chunk.

getSmallModel now reads mastracode's auth.json file directly using
the same path resolution logic (~/Library/Application Support/mastracode/
on macOS). Zero mastracode import, zero bundle impact. The chunk stays
at 1.2 MB (just @ai-sdk/anthropic + @ai-sdk/openai).

Production build verified: compile:app succeeds, Electron main process
boots with no onnxruntime error.

* docs(desktop): add manual testing plan for PR superset-sh#3517

* fix api key storage slot

* fix(auth): store API keys in dedicated slot so OAuth doesn't clobber them

setApiKeyForProvider and setStoredAnthropicApiKeyFromEnvVariables now
use authStorage.setStoredApiKey() (writes to "apikey:<provider>")
instead of authStorage.set() (writes to the main "<provider>" slot
shared with OAuth). This way connecting/disconnecting OAuth doesn't
overwrite or delete a stored API key.

resolveAuthMethodForProvider falls back to hasStoredApiKey() after
checking the main slot, so status correctly reports authenticated
when only an API key is stored.

* fix(auth): backup/restore API keys across OAuth connect/disconnect

mastracode's resolveModel only reads API keys from the main
authStorage slot (authStorage.get("anthropic")). OAuth login
overwrites this slot, and disconnect removes it — losing any
previously saved API key.

Fix: backup the API key to the dedicated apikey: slot before OAuth
connect, restore it after disconnect. setApiKeyForProvider now writes
to both slots (main for resolveModel compatibility, apikey: for
backup). resolveAuthMethodForProvider checks both.

Applies to both Anthropic and OpenAI providers.

* chore: add upstream PR reference to auth workaround

Point to mastra-ai/mastra#15483 so the backup/restore code can be
removed once upstream lands and we bump mastracode.

* refactor(desktop): derive settings provider action from status

Replace the cascade of if/else + canDisconnect flag with a single
getProviderAction(status) → connect | reconnect | logout | null.
Fixes "Active" badge + "Connect" button showing simultaneously
when authenticated via API key.

* fix(desktop): always show Logout when provider is active

Active providers now always show a Logout button. Clears OAuth or
API key depending on authMethod — no more "Active" badge with no
way to disconnect.

* fix(desktop): simplify OpenAI OAuth dialog + auto-open browser

Match Anthropic dialog's layout: remove the raw OAuth URL display
and "Tip" block, auto-open the browser on OAuth start. Change
"Back" to "Cancel" for consistency.

* refactor(desktop): unify OAuth dialogs into shared OAuthDialog

Extract shared OAuthDialog component with provider config object.
AnthropicOAuthDialog and OpenAIOAuthDialog become thin wrappers
that pass provider-specific labels and options.

* fix(desktop): show 'Copied!' feedback on Copy URL button

* refactor(desktop): merge provider account + API key into single card

Each provider section now renders AccountCard + ConfigRow inside
one rounded card with a divider, instead of two separate cards.
Removes the standalone "API Keys" collapsible section.

* refactor(desktop): compact OAuth row in provider settings card

OAuth row is now a single inline row (label + status + action)
instead of a stacked AccountCard. Both providers share the same
2-row card layout: OAuth row + API key row with divider.

* fix(desktop): contextual buttons in provider settings

Connect is now primary (filled). Save only shows when there's input.
Clear only shows when a key is saved. Removes visual noise from
empty-state provider cards.

* ui(desktop): add provider icons to settings section headers

* ui(desktop): show 'Not connected' badge instead of subtitle for disconnected providers

* ui: remove redundant disconnected subtitle

* ui: remove subtitle text from OAuth rows

* chore: remove dead AccountCard + getProviderSubtitle

* docs: update test plan to match current UI

* chore: move shipped plans to done/

---------

Co-authored-by: AviPeltz <aj.peltz@gmail.com>
MocA-Love added a commit that referenced this pull request Apr 21, 2026
…sions

#1 Target.closeTarget UI integrity
  v1/v2 secondary tab registry が webview "close" イベントを購読。
  MCP の Target.closeTarget で Chromium が webContents を破棄すると
  guest 側が close を発火し、registry が closeTab → unregisterTab
  経由で paneTabTargetIds を整理。tab バー UI と CDP allowedTargetIds
  が同期した状態を保つ。

#2 target="_blank" / window.open を MCP 可視に
  windowOpenHandler の非 new-window 分岐で new-window event を emit
  していた箇所を create-tab-requested:${paneId} に置換。同じペイン
  内の secondary tab として生成されるので paneTabTargetIds に入り、
  MCP が list_pages / select_page で扱える。Chrome の target="_blank"
  デフォルト (新タブ) 挙動に揃う。split-pane / workspace-tab が
  欲しいケースは既存の "Open in Split" コンテキストメニューでカバー。

#3 非 media 権限の UI prompt 化
  SITE_PERMISSION_KINDS に geolocation / notifications / clipboard-read
  を追加。browser-site-permission-manager が Electron の
  setPermissionRequestHandler で media 同様の consent flow に乗せる。
  既存の permissionRequested イベント経路はそのまま再利用。
  認識しない permission は従来通り permissive で許可。
MocA-Love added a commit that referenced this pull request Apr 23, 2026
upstream 取り込み PR #2: v2 UI 小粒 / marketing / 9 commits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant