Repository navigation
fix(desktop): dismiss stuck tooltips and forward tab-switch hotkeys in in-workspace browser - #5494
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds shared keyboard-chord conversion, configurable browser-pane hotkey forwarding through tRPC, renderer-side replay for selected hotkeys, favicon fallback rendering, non-hoverable tooltip content, and URL-specific link defaults. ChangesKey forwarding pipeline
Browser tab presentation
Tooltip behavior
URL-link preferences
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BrowserPane
participant BrowserRouter
participant BrowserManager
participant Document
BrowserPane->>BrowserRouter: Configure forwardable chords
BrowserRouter->>BrowserManager: setForwardableChords(chords)
BrowserManager->>BrowserRouter: Emit key-forward:paneId
BrowserRouter->>BrowserPane: Deliver ForwardedKey
BrowserPane->>Document: Dispatch keydown and keyup
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Ready to review this PR? Stage has broken it down into 5 individual chapters for you: 
Chapters generated by Stage for commit 9a3cb48 on Jul 7, 2026 9:34pm UTC. |
Greptile SummaryThis PR fixes browser-pane tooltip and keyboard handling around native webview focus.
Confidence Score: 4/5The webview hotkey forwarding path has two contained issues that should be fixed before merging.
apps/desktop/src/main/lib/browser/browser-manager.ts
|
| Filename | Overview |
|---|---|
| apps/desktop/src/lib/trpc/routers/browser/browser.ts | Adds a pane-scoped onKeyForward subscription and cleans it up like the existing browser event subscriptions. |
| apps/desktop/src/main/lib/browser/browser-manager.ts | Extends webview before-input-event handling, but the forwarding branch can drop some configured tab-switch chords and can leave forwarded chords active in the guest page. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.ts | Subscribes to forwarded webview keys, resolves them through the central hotkey registry, and replays allowlisted tab-switch events. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/V2PresetsBar/components/V2PresetBarItem/V2PresetBarItem.tsx | Disables hoverable content for a static preset tooltip label. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/BrowserPane.tsx | Disables hoverable content for the static DevTools tooltip. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserToolbar/BrowserToolbar.tsx | Disables hoverable content for static back, forward, and reload tooltips. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Guest as Focused webview
participant Main as before-input-event
participant TRPC as browser.onKeyForward
participant Host as Host renderer
Guest->>Main: keyDown chord
Main->>TRPC: emit forwarded key
TRPC->>Host: deliver ForwardedKey
Host->>Host: resolveHotkeyFromEvent
alt Allowlisted tab hotkey
Host->>Host: dispatch synthetic keydown and keyup
else Not allowlisted
Host->>Host: ignore
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Guest as Focused webview
participant Main as before-input-event
participant TRPC as browser.onKeyForward
participant Host as Host renderer
Guest->>Main: keyDown chord
Main->>TRPC: emit forwarded key
TRPC->>Host: deliver ForwardedKey
Host->>Host: resolveHotkeyFromEvent
alt Allowlisted tab hotkey
Host->>Host: dispatch synthetic keydown and keyup
else Not allowlisted
Host->>Host: ignore
end
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/desktop/src/main/lib/browser/browser-manager.ts:264
**Alt-Only Bindings Are Dropped**
When a user configures a tab-switch binding that uses `Alt` without `Cmd` or `Ctrl`, this guard returns before the renderer can resolve the chord against the allowlist. The webview-focused shortcut then stays broken even though `resolveHotkeyFromEvent` would be able to accept that configured tab-switch hotkey.
### Issue 2 of 2
apps/desktop/src/main/lib/browser/browser-manager.ts:280-288
**Forwarded Chords Still Reach Guest**
The new forwarding path emits accepted tab-switch chords without stopping the original key event, unlike the close and reload branches above. A focused page can receive the same `Ctrl`/`Cmd` tab-switch chord while the host also replays it, so one keypress can both switch the workspace tab and trigger a guest-page shortcut.
Reviews (1): Last reviewed commit: "fix(desktop): dismiss stuck tooltips and..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| event.preventDefault(); | ||
| this.emit(`reload-pane:${paneId}`); | ||
| return; | ||
| if (!input.shift && !input.alt) { |
There was a problem hiding this comment.
When a user configures a tab-switch binding that uses Alt without Cmd or Ctrl, this guard returns before the renderer can resolve the chord against the allowlist. The webview-focused shortcut then stays broken even though resolveHotkeyFromEvent would be able to accept that configured tab-switch hotkey.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/lib/browser/browser-manager.ts
Line: 264
Comment:
**Alt-Only Bindings Are Dropped**
When a user configures a tab-switch binding that uses `Alt` without `Cmd` or `Ctrl`, this guard returns before the renderer can resolve the chord against the allowlist. The webview-focused shortcut then stays broken even though `resolveHotkeyFromEvent` would be able to accept that configured tab-switch hotkey.
How can I resolve this? If you propose a fix, please make it concise.| code: input.code, | ||
| meta: input.meta, | ||
| control: input.control, | ||
| alt: input.alt, | ||
| shift: input.shift, | ||
| } satisfies ForwardedKey); | ||
| }; | ||
|
|
||
| wc.on("before-input-event", handler); |
There was a problem hiding this comment.
The new forwarding path emits accepted tab-switch chords without stopping the original key event, unlike the close and reload branches above. A focused page can receive the same Ctrl/Cmd tab-switch chord while the host also replays it, so one keypress can both switch the workspace tab and trigger a guest-page shortcut.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/lib/browser/browser-manager.ts
Line: 280-288
Comment:
**Forwarded Chords Still Reach Guest**
The new forwarding path emits accepted tab-switch chords without stopping the original key event, unlike the close and reload branches above. A focused page can receive the same `Ctrl`/`Cmd` tab-switch chord while the host also replays it, so one keypress can both switch the workspace tab and trigger a guest-page shortcut.
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 6 files
Architecture diagram
sequenceDiagram
participant Guest as Guest Webview
participant Main as Electron Main Process (browser-manager)
participant Sub as browser.onKeyForward (tRPC)
participant Renderer as Renderer (usePersistentWebview)
participant Doc as Host Document
participant Hotkey as react-hotkeys-hook
Note over Guest,Hotkey: NEW: Keyboard forwarding for tab switch hotkeys
Note over Guest: User presses key in guest webview
Guest->>Main: before-input-event (keyDown)
alt Not meta/control
Main->>Main: return (ignored, guest handles normally)
else Meta/control + (W or R) WITHOUT shift/alt
Main->>Main: preventDefault
Main->>Main: emit close-pane / reload-pane (existing behavior)
else Other meta/control chords (including with shift/alt)
Main->>Main: emit key-forward event with ForwardedKey
Main->>Sub: ForwardedKey (paneId)
Sub->>Renderer: push ForwardedKey via subscription
Renderer->>Renderer: build KeyboardEventInit, create KeyboardEvent
Renderer->>Renderer: resolveHotkeyFromEvent(event)
alt HotkeyId in FORWARDABLE_HOTKEYS (tab-switch)
Renderer->>Doc: dispatch keydown event
Renderer->>Doc: dispatch keyup event (balance)
Doc->>Hotkey: react-hotkeys-hook processes chord
Hotkey->>Hotkey: Execute tab switch (e.g., NEXT_TAB)
else Non-tab-switch hotkey or no match
Renderer->>Renderer: ignore (do nothing)
end
end
Note over Guest,Hotkey: Tooltip fix: disableHoverableContent on four adjacent tooltips\n(static property, no runtime flow change)
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Thanks — addressed the key-forwarding findings in 9a3cb48. Both P1s (guest double-delivery + alt-only bindings) and the P2 (override-awareness) share one root cause: the old forward path forwarded all meta/ctrl chords and never Fix: the renderer now registers its override/layout-aware forwardable chord set with the main process, and
The chord canonicalization is now a single source of truth: pure helpers moved to |
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/main/lib/browser/browser-manager.ts">
<violation number="1" location="apps/desktop/src/main/lib/browser/browser-manager.ts:287">
P2: AltGr typing in the focused webview can be swallowed and replayed as a Ctrl+Alt tab-switch override, so non-US layout characters like AltGr+E may switch tabs instead of entering text. This new main-process gate uses `chordFromInput(input)`, which does not preserve the renderer hotkey system’s AltGraph suppression.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
| } | ||
|
|
||
| const chord = chordFromInput(input); |
There was a problem hiding this comment.
P2: AltGr typing in the focused webview can be swallowed and replayed as a Ctrl+Alt tab-switch override, so non-US layout characters like AltGr+E may switch tabs instead of entering text. This new main-process gate uses chordFromInput(input), which does not preserve the renderer hotkey system’s AltGraph suppression.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/main/lib/browser/browser-manager.ts, line 287:
<comment>AltGr typing in the focused webview can be swallowed and replayed as a Ctrl+Alt tab-switch override, so non-US layout characters like AltGr+E may switch tabs instead of entering text. This new main-process gate uses `chordFromInput(input)`, which does not preserve the renderer hotkey system’s AltGraph suppression.</comment>
<file context>
@@ -275,6 +284,9 @@ class BrowserManager extends EventEmitter {
}
}
+ const chord = chordFromInput(input);
+ if (!chord || !this.forwardableChords.has(chord)) return;
+ event.preventDefault();
</file context>
…n in-workspace browser
…om focused webview
9a3cb48 to
5e2b242
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/BrowserPane/BrowserPane.tsx:
- Around line 25-40: Move the BrowserTabFavicon component into a dedicated
BrowserTabFavicon/BrowserTabFavicon.tsx module, preserving its existing fallback
and image-error behavior, and export it through BrowserTabFavicon/index.ts.
Remove the inline definition from BrowserPane.tsx and update its import to use
the barrel export.
- Around line 42-47: Update renderBrowserTabIcon so BrowserTabFavicon is keyed
by both browser.data.url and faviconUrl, preserving the nullable favicon
fallback while ensuring navigation remounts the component even when the favicon
URL is unchanged.
In `@apps/desktop/src/shared/hotkey-chord.ts`:
- Around line 45-46: Update canonicalizeChord so each raw token is passed to
normalizeToken before lowercasing, allowing physical aliases such as MetaLeft
and ControlLeft to resolve consistently with eventToChord. Add coverage
verifying both aliases canonicalize to the same modifier names used by keyboard
events.
🪄 Autofix
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 Plus
Run ID: 981485b9-a415-4daa-9e9c-ec5b49d31f7d
📒 Files selected for processing (10)
apps/desktop/src/lib/trpc/routers/browser/browser.tsapps/desktop/src/main/lib/browser/browser-manager.tsapps/desktop/src/renderer/hotkeys/components/HotkeyTooltip/HotkeyTooltip.tsxapps/desktop/src/renderer/hotkeys/utils/binding.tsapps/desktop/src/renderer/hotkeys/utils/resolveHotkeyFromEvent.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/V2PresetsBar/V2PresetsBar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/BrowserPane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.tsapps/desktop/src/shared/hotkey-chord.test.tsapps/desktop/src/shared/hotkey-chord.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/desktop/src/renderer/hotkeys/utils/binding.ts
- apps/desktop/src/lib/trpc/routers/browser/browser.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.ts (1)
170-193: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRetry or report
setForwardableChordsfailures.The mutation currently calls
.catch(() => {}), andBrowserManager.forwardableChordsstarts empty if the initial sync fails. When the main process keeps an empty or stale chord set, tab-switch keys do not suppress/forward, or host replay falls back to guest-only behavior. Add bounded retry for transient failures and surface persistent failures instead of silencing them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.ts around lines 170 - 193, Update the forwardable-chord synchronization in the useEffect around setForwardableChords to retry transient mutation failures with a bounded limit and appropriate delay, while preventing overlapping retries and cleaning up pending work on unmount. After retries are exhausted, report the failure through the existing renderer error-reporting mechanism instead of swallowing it, while preserving the current subscription and chord computation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.ts:
- Around line 170-193: Update the forwardable-chord synchronization in the
useEffect around setForwardableChords to retry transient mutation failures with
a bounded limit and appropriate delay, while preventing overlapping retries and
cleaning up pending work on unmount. After retries are exhausted, report the
failure through the existing renderer error-reporting mechanism instead of
swallowing it, while preserving the current subscription and chord computation
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 21ce9b04-7404-4ca8-b7a4-bd26c30f8074
📒 Files selected for processing (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.ts
There was a problem hiding this comment.
3 issues found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/BrowserPane.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/BrowserPane.tsx:34">
P3: A failed favicon still cannot retry after hash/history-API navigation because this key reads pane data that is never persisted for `did-navigate-in-page`. Persist that navigation URL (or key from state updated by it) so the favicon component remounts on every intended navigation.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts:304">
P1: Existing users keep the old unbound Shift URL tier, so Shift-click still no-ops after upgrading; migrate the prior complete URL default in `healV2UserPreferences`, as done for legacy sidebar file links.</violation>
</file>
<file name="apps/desktop/src/shared/hotkey-chord.ts">
<violation number="1" location="apps/desktop/src/shared/hotkey-chord.ts:56">
P3: This comment now describes behavior the preceding case-insensitive alias map explicitly eliminates; remove or correct it so future changes are not guided by a false constraint.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| metaShift: "external", | ||
| }; | ||
|
|
||
| const DEFAULT_URL_LINKS: LinkTierMap = { |
There was a problem hiding this comment.
P1: Existing users keep the old unbound Shift URL tier, so Shift-click still no-ops after upgrading; migrate the prior complete URL default in healV2UserPreferences, as done for legacy sidebar file links.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts, line 304:
<comment>Existing users keep the old unbound Shift URL tier, so Shift-click still no-ops after upgrading; migrate the prior complete URL default in `healV2UserPreferences`, as done for legacy sidebar file links.</comment>
<file context>
@@ -301,6 +301,13 @@ const DEFAULT_LINK_TIER_MAP: LinkTierMap = {
metaShift: "external",
};
+const DEFAULT_URL_LINKS: LinkTierMap = {
+ plain: null,
+ shift: "newTab",
</file context>
| return ( | ||
| <img src={browser.data.faviconUrl} alt="" className="size-3.5 shrink-0" /> | ||
| <BrowserTabFavicon | ||
| key={`${browser.data.url}|${faviconUrl ?? "none"}`} |
There was a problem hiding this comment.
P3: A failed favicon still cannot retry after hash/history-API navigation because this key reads pane data that is never persisted for did-navigate-in-page. Persist that navigation URL (or key from state updated by it) so the favicon component remounts on every intended navigation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/BrowserPane.tsx, line 34:
<comment>A failed favicon still cannot retry after hash/history-API navigation because this key reads pane data that is never persisted for `did-navigate-in-page`. Persist that navigation URL (or key from state updated by it) so the favicon component remounts on every intended navigation.</comment>
<file context>
@@ -22,29 +23,18 @@ function getSingleBrowserPane(
+ // even when the favicon URL itself is unchanged.
+ return (
+ <BrowserTabFavicon
+ key={`${browser.data.url}|${faviconUrl ?? "none"}`}
+ src={faviconUrl}
+ />
</file context>
| * aliases: `meta+alt+up` ≡ `alt+meta+arrowup` ≡ `control+alt+arrowup`. | ||
| */ | ||
| export function canonicalizeChord(chord: string): string { | ||
| // normalizeToken lowercases after its alias lookup; lowercasing here first |
There was a problem hiding this comment.
P3: This comment now describes behavior the preceding case-insensitive alias map explicitly eliminates; remove or correct it so future changes are not guided by a false constraint.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/shared/hotkey-chord.ts, line 56:
<comment>This comment now describes behavior the preceding case-insensitive alias map explicitly eliminates; remove or correct it so future changes are not guided by a false constraint.</comment>
<file context>
@@ -43,7 +53,9 @@ export function isIgnorableKey(normalized: string): boolean {
*/
export function canonicalizeChord(chord: string): string {
- const parts = chord.toLowerCase().split("+").map(normalizeToken);
+ // normalizeToken lowercases after its alias lookup; lowercasing here first
+ // would break case-sensitive physical-code aliases like `MetaLeft` → meta.
+ const parts = chord.split("+").map(normalizeToken);
</file context>
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts:456">
P3: Reuse DEFAULT_LINK_TIER_MAP as the legacy urlLinks sentinel couples the urlLinks migration to the current fileLinks default: the same constant backs both the fileLinks default and the "retired" urlLinks default detection. If fileLinks' default is ever updated (new tier, changed action), this heal would silently re-baseline stored urlLinks maps it should treat as arbitrary customizations. Introduce a dedicated LEGACY_URL_LINKS constant mirroring the existing LEGACY_SIDEBAR_FILE_LINKS pattern and compare against that instead.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| const shouldMigrateLegacyUrlLinks = | ||
| r.urlLinks && | ||
| isCompleteLinkTierMap(r.urlLinks) && | ||
| isSameLinkTierMap(r.urlLinks, DEFAULT_LINK_TIER_MAP); |
There was a problem hiding this comment.
P3: Reuse DEFAULT_LINK_TIER_MAP as the legacy urlLinks sentinel couples the urlLinks migration to the current fileLinks default: the same constant backs both the fileLinks default and the "retired" urlLinks default detection. If fileLinks' default is ever updated (new tier, changed action), this heal would silently re-baseline stored urlLinks maps it should treat as arbitrary customizations. Introduce a dedicated LEGACY_URL_LINKS constant mirroring the existing LEGACY_SIDEBAR_FILE_LINKS pattern and compare against that instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts, line 456:
<comment>Reuse DEFAULT_LINK_TIER_MAP as the legacy urlLinks sentinel couples the urlLinks migration to the current fileLinks default: the same constant backs both the fileLinks default and the "retired" urlLinks default detection. If fileLinks' default is ever updated (new tier, changed action), this heal would silently re-baseline stored urlLinks maps it should treat as arbitrary customizations. Introduce a dedicated LEGACY_URL_LINKS constant mirroring the existing LEGACY_SIDEBAR_FILE_LINKS pattern and compare against that instead.</comment>
<file context>
@@ -448,11 +448,19 @@ export function healV2UserPreferences(raw: unknown): V2UserPreferencesRow {
+ const shouldMigrateLegacyUrlLinks =
+ r.urlLinks &&
+ isCompleteLinkTierMap(r.urlLinks) &&
+ isSameLinkTierMap(r.urlLinks, DEFAULT_LINK_TIER_MAP);
return {
...DEFAULT_V2_USER_PREFERENCES,
</file context>
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Fixes five UI bugs in the in-workspace browser (plus a favicon fallback picked up during verification). Most stem from the browser being a native Electron
<webview>composited on top of the host page, so it captures pointer/keyboard events the host renderer never sees.1. Popover/tooltip stays stuck over the URL bar
Radix keeps a tooltip open during its "hoverable content" grace period, waiting for more pointer events — but once the cursor crosses onto the native webview the host document stops receiving pointer events, so the close logic never runs and the tooltip hangs over the URL bar.
Fix:
disableHoverableContenton the webview-adjacent tooltips. Since #5870 restyled the preset tooltips intoHotkeyTooltip, the flag now lives there (its content is always a non-interactive hotkey chip, so there is no downside anywhere it's used), plus the Manage Presets and DevTools tooltips.2. Tab-switch keyboard shortcuts don't work when the browser is focused
With the guest webview focused, keystrokes route to the guest renderer, so host
react-hotkeys-hooknever sees them.browser-manager.tsalready intercepts viabefore-input-eventbut only forwardedCmd/Ctrl+Wand+R.Fix: the renderer computes the (override/layout-aware) chords for the tab-switch hotkeys (
PREV/NEXT_TAB,_ALTvariants,JUMP_TO_TAB_1..9) and syncs them to the main process viabrowser.setForwardableChords.before-input-eventsuppresses exactly those chords in the guest and forwards them over abrowser.onKeyForwardsubscription;usePersistentWebviewrebuilds theKeyboardEvent, re-resolves it throughresolveHotkeyFromEvent, and replays it ondocumentonly if it resolves to an allowlisted tab-switch hotkey. Chord canonicalization is shared between main and renderer (shared/hotkey-chord.ts, singleassembleChordhelper) and pinned by a parity unit test.3. Browser tab favicon fallback
Chromium guesses
/favicon.icofor pages that declare no favicon, sofaviconUrlcan point at a 404 and the tab shows a broken image (and pages with no favicon showed no icon at all). Tabs now fall back to a globe icon when the favicon is missing or fails to load, keyed by page + favicon URL so navigation retries.4. Opening a link into an existing browser pane never navigates
Clicking a link (e.g. a URL in a terminal) opens the in-workspace browser — but once a browser pane exists, every subsequent link click replaced the pane in the store without the page ever loading the new URL. Root cause:
usePersistentWebviewcaptured the pane URL in a ref at first mount; whenopenPane->replacePaneswaps in a new pane id on the same component instance, the attach effect re-ran with the stale first-mount URL, so the fresh webview loaded the old page (and the persist echo then reverted the pane data too).Fix: keep the attach-URL ref current on every render. Attach still keys on
paneIdalone, so tab switches and navigation echoes don't re-attach; only a pane replacement picks up the new URL. Verified live: alternating cmd+clicks between two terminal URLs navigate the pane correctly every time (previously only the first-ever link worked).5. Shift+click opens links in a new browser tab
The default URL-link tier map now assigns
shift-> "Open in new tab" (previously unbound): shift+clicking a URL in a terminal opens it in a new in-app browser tab instead of doing nothing. Existing profiles are migrated on read when their stored map deep-equals the retired default (same pattern as the sidebar file-link migration) — anyone who customized any tier keeps their configuration.Review-comment cleanups
setForwardableChordssync failures now log a warning instead of being swallowed (stale suppression is otherwise undiagnosable).BrowserTabFaviconextracted to its own component module per repo structure guidelines; its key now includes the page URL so a failed favicon retries on navigation even when the favicon URL is unchanged.normalizeToken's alias lookup is case-insensitive, socanonicalizeChord("MetaLeft+KeyK")===eventToChordoutput (meta+k); pinned by tests.preventDefaulton the forward path, alt-only chords dropped) were against the first commit only — the current code preventDefaults every forwarded chord (verified live: the guest page receives nothing) and has no meta/ctrl early-return.Regression safety
Cmd/Ctrl+W/+Rinterception is untouched (re-verified live).keyupis dispatched so react-hotkeys-hook's pressed-key set never gets a stuck key.Verification (live E2E on the dev app via CDP + OS-level keystrokes)
ctrl+Tabwas intercepted in main (guest page received nothing), forwarded, replayed on the host document (observed as an untrusted keydown), and the workspace switched tabs.⌘Wwith guest focus closed just the pane.HotkeyTooltip, hover a preset button → move toward the webview → tooltip sticks open indefinitely (bug reproduced); with this branch it closes as the pointer leaves the trigger.bun test src/shared/hotkey-chord.test.tspinseventToChord ≡ chordFromInputfor all forwarded chords plus alias-case canonicalization.origin/mainin the same environment (CI Typecheck green).