feat (desktop): Scroll to bottom + org at bottom#709
Conversation
📝 WalkthroughWalkthroughMoves OrganizationDropdown from the sidebar header to the footer and enhances its menu; makes the terminal's ScrollToBottomButton hover-aware by adding hover tracking, changing visibility logic, alignment, tooltip delay, and styling. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Terminal as Terminal Component
participant ScrollBtn as ScrollToBottomButton
participant XTerm as XTerm API
User->>Terminal: mouseEnter
Terminal->>Terminal: set isHovered = true
Terminal->>ScrollBtn: render(terminal, isHovered=true)
ScrollBtn->>ScrollBtn: compute isNotAtBottom && isHovered -> isVisible
alt isVisible
ScrollBtn->>User: show button (right-aligned)
else hidden
ScrollBtn-->>User: remain hidden
end
User->>ScrollBtn: click
ScrollBtn->>XTerm: scrollToBottom()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/ScrollToBottomButton/ScrollToBottomButton.tsx:
- Around line 9-18: The ScrollToBottomButton currently stays keyboard-focusable
when hidden; update the ScrollToBottomButton component (the function
ScrollToBottomButton and its rendered <button>) to remove it from the tab order
when not visible by adding tabIndex={-1} and aria-hidden="true" (or
alternatively conditionally render the button only when isVisible is true) while
preserving the existing animation/opacity classes; apply the same change to the
other hidden-button instance referenced around lines 53-77 so hidden buttons
cannot receive focus.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarFooter.tsx (1)
8-8: Consider relocating OrganizationDropdown to reflect its new usage.The component is now used exclusively in the footer but still resides in the
WorkspaceSidebarHeaderdirectory. This creates architectural coupling where the footer depends on the header's subdirectory structure.Consider moving
OrganizationDropdown.tsxto either:
- A shared components directory (e.g.,
WorkspaceSidebar/shared/)- The footer's subdirectory
This would improve module cohesion and make the codebase easier to navigate.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (1)
47-48: Hover-state plumbing looks correct; consider CSS-only hover to avoid state if desired.This won’t retrigger the expensive xterm setup effect (good), and the prop wiring into
ScrollToBottomButtonis clear. If you want to avoid a component re-render on hover entirely, you could also implement this via Tailwindgroup/group-hoverclasses instead ofisHoveredstate.Also applies to: 617-618, 625-626
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarFooter.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/ScrollToBottomButton/ScrollToBottomButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarFooter.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/ScrollToBottomButton/ScrollToBottomButton.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarFooter.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/ScrollToBottomButton/ScrollToBottomButton.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarFooter.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/ScrollToBottomButton/ScrollToBottomButton.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarFooter.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/ScrollToBottomButton/ScrollToBottomButton.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarFooter.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/ScrollToBottomButton/ScrollToBottomButton.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarFooter.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/ScrollToBottomButton/ScrollToBottomButton.tsx
🧠 Learnings (1)
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/*/src/components/{ui,ai-elements,react-flow}/*.tsx : Use kebab-case single files for shadcn/ui components (e.g., button.tsx, base-node.tsx) in src/components/ui/, src/components/ai-elements, and src/components/react-flow/
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/ScrollToBottomButton/ScrollToBottomButton.tsx
🧬 Code graph analysis (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/ScrollToBottomButton/ScrollToBottomButton.tsx (1)
ScrollToBottomButton(14-80)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarFooter.tsx (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (1)
OrganizationDropdown(30-164)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarFooter.tsx (2)
59-75: LGTM! Collapsed state layout updated correctly.The layout changes from horizontal to vertical column (
flex-col) with appropriate spacing (gap-2) to accommodate the newOrganizationDropdowncomponent. TheisCollapsedprop is correctly passed to the dropdown.
80-93: LGTM! Expanded state layout updated correctly.The layout changes add flexbox column with
gap-1spacing to accommodate theOrganizationDropdown. The component correctly uses the defaultisCollapsed={false}for the expanded state.Note: The collapsed state uses
gap-2while the expanded state usesgap-1. This appears intentional for visual balance but could be verified for consistency.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/ScrollToBottomButton/ScrollToBottomButton.tsx (1)
9-17: Prop contract change verified; all call sites updated.
The component has exactly one usage inTerminal.tsx, which correctly provides bothterminalandisHoveredprops:<ScrollToBottomButton terminal={xtermInstance} isHovered={isHovered} />.
There was a problem hiding this comment.
Actionable comments posted: 2
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/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (1)
62-64: Handle async org-switch errors (avoid unhandled promise rejection).
switchOrganization()awaitsmutateAsync, but the caller (onSelect={() => switchOrganization(...)}) doesn’t await or catch. If the mutation fails, this can surface as an unhandled rejection and gives users no feedback.Proposed fix (catch + prefixed logging; optional pending guard)
-const switchOrganization = async (newOrgId: string) => { - await setActiveOrg.mutateAsync({ organizationId: newOrgId }); -}; +const switchOrganization = (newOrgId: string) => { + void setActiveOrg + .mutateAsync({ organizationId: newOrgId }) + .catch((err) => console.error("[auth/switch-organization] failed", err)); +}; @@ - onSelect={() => switchOrganization(organization.id)} + onSelect={() => { + if (setActiveOrg.isPending) return; + switchOrganization(organization.id); + }}Also applies to: 158-161
🤖 Fix all issues with AI agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx:
- Line 31: Change the import of useHotkeyText in OrganizationDropdown.tsx to
import from the concrete module instead of the barrel; replace the current
`import { useHotkeyText } from "renderer/stores/hotkeys"` with `import {
useHotkeyText } from "renderer/stores/hotkeys/store"` so OrganizationDropdown
uses the concrete store export and avoids barrel file abuse.
- Around line 181-214: In OrganizationDropdown (the DropdownMenuItem onSelect
handlers inside the Contact us submenu) replace direct window.open(...) calls
with the tRPC call trpc.external.openUrl.mutate(url) so external navigation goes
through the main process; update each onSelect for Discord, Email founders, and
X (Twitter) to call trpc.external.openUrl.mutate with the same URL string
instead of window.open and keep the className/children intact.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (1)
45-46: Don’t key UI logic off magic string"Unassigned".
IfuseHotkeyText()ever changes its “unassigned” display text, this condition breaks. Prefer a constant, or (better) have the hook exposeundefined/nullfor unassigned and format at render time.Proposed tweak (local constant)
+const UNASSIGNED_HOTKEY_TEXT = "Unassigned"; + export function OrganizationDropdown({ isCollapsed = false, }: OrganizationDropdownProps) { @@ const openSettings = useOpenSettings(); const hotkeysShortcut = useHotkeyText("SHOW_HOTKEYS"); @@ - {hotkeysShortcut !== "Unassigned" && ( + {hotkeysShortcut !== UNASSIGNED_HOTKEY_TEXT && ( <DropdownMenuShortcut>{hotkeysShortcut}</DropdownMenuShortcut> )}Also applies to: 128-137
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (2)
apps/desktop/src/renderer/stores/hotkeys/store.ts (1)
useHotkeyText(206-215)packages/ui/src/components/ui/dropdown-menu.tsx (6)
DropdownMenuItem(255-255)DropdownMenuShortcut(260-260)DropdownMenuSub(261-261)DropdownMenuSubTrigger(262-262)DropdownMenuSubContent(263-263)DropdownMenuSeparator(259-259)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (1)
37-40: DefaultisCollapsed = false+ expanded menu structure look good.
This makes the component safer to use from new call sites and the added items/shortcut UI are coherent with the dropdown patterns.Also applies to: 113-137
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (2)
46-46: Avoid stringly-typed sentinel"Unassigned"; prefer a typed helper/constant.
IfformatHotkeyTextever changes its “unassigned” label, this UI will silently regress.Low-risk improvement
+const UNASSIGNED_HOTKEY_LABEL = "Unassigned"; @@ - {hotkeysShortcut !== "Unassigned" && ( + {hotkeysShortcut !== UNASSIGNED_HOTKEY_LABEL && ( <DropdownMenuShortcut>{hotkeysShortcut}</DropdownMenuShortcut> )}Based on coding guidelines (extract hardcoded values).
Also applies to: 129-138
216-219: Text change to “Log out” is fine—just ensure consistency across the app (“Sign out” vs “Log out”).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
🧠 Learnings (10)
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/*/src/components/{ui,ai-elements,react-flow}/*.tsx : Use kebab-case single files for shadcn/ui components (e.g., button.tsx, base-node.tsx) in src/components/ui/, src/components/ai-elements, and src/components/react-flow/
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to **/*.{ts,tsx} : Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/lib/*.ts : Never import Node.js modules in shared code like electron-router-dom.ts - it runs in both main and renderer processes
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/lib/electron-router-dom.ts : Do not import Node.js modules like node:path or dotenv in electron-router-dom.ts and similar shared files - they run in both main and renderer processes
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/*/src/components/**/[A-Z]*.tsx : Create component folder structure with one folder per component: ComponentName/ComponentName.tsx with barrel export index.ts
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from `trpc/server/observable` instead of async generators, as the library explicitly checks `isObservable(result)` and throws an error otherwise
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (3)
apps/desktop/src/renderer/stores/app-state.ts (1)
useOpenSettings(94-94)apps/desktop/src/renderer/stores/hotkeys/store.ts (1)
useHotkeyText(206-215)packages/ui/src/components/ui/dropdown-menu.tsx (6)
DropdownMenuItem(255-255)DropdownMenuShortcut(260-260)DropdownMenuSub(261-261)DropdownMenuSubTrigger(262-262)DropdownMenuSubContent(263-263)DropdownMenuSeparator(259-259)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (2)
37-39: Nice: defaultingisCollapsed = falsemakes callsites simpler.
114-138: Both"team"and"keyboard"are valid parameters.The
openSettingsfunction accepts an optionalsection?: SettingsSectionparameter, whereSettingsSectionis a union type that explicitly includes both "team" and "keyboard". TypeScript's type system guarantees that only valid section values can be passed to this function, preventing silent failures or "no-ops with no obvious error."
| const openUrl = trpc.external.openUrl.useMutation(); | ||
| const openSettings = useOpenSettings(); | ||
| const hotkeysShortcut = useHotkeyText("SHOW_HOTKEYS"); | ||
|
|
There was a problem hiding this comment.
Don’t silently ignore openUrl failures; add pending/disabled + contextual error logging. (Guideline: “Never swallow errors silently”)
Right now openUrl.mutate(...) has no local handling, so failures can be invisible and users can spam-click items while pending.
Proposed patch
export function OrganizationDropdown({
isCollapsed = false,
}: OrganizationDropdownProps) {
@@
const signOut = trpc.auth.signOut.useMutation();
const openUrl = trpc.external.openUrl.useMutation();
@@
const hotkeysShortcut = useHotkeyText("SHOW_HOTKEYS");
+
+ type OpenUrlInput = Parameters<typeof openUrl.mutateAsync>[0];
+ const handleOpenUrl = async (input: OpenUrlInput) => {
+ try {
+ await openUrl.mutateAsync(input);
+ } catch (err) {
+ console.error("[external/openUrl] failed to open url", err);
+ }
+ };
@@
<DropdownMenuItem
- onSelect={() => openUrl.mutate("https://discord.gg/superset")}
+ onSelect={() => handleOpenUrl("https://discord.gg/superset" as OpenUrlInput)}
className="gap-2"
+ disabled={openUrl.isPending}
>
@@
<DropdownMenuItem
- onSelect={() => openUrl.mutate("mailto:founders@superset.sh")}
+ onSelect={() => handleOpenUrl("mailto:founders@superset.sh" as OpenUrlInput)}
className="gap-2"
+ disabled={openUrl.isPending}
>
@@
<DropdownMenuItem
- onSelect={() => openUrl.mutate("https://x.com/supersetsh")}
+ onSelect={() => handleOpenUrl("https://x.com/supersetsh" as OpenUrlInput)}
className="gap-2"
+ disabled={openUrl.isPending}
>Also applies to: 189-209
| {/* Contact Us */} | ||
| <DropdownMenuSub> | ||
| <DropdownMenuSubTrigger className="gap-2"> | ||
| <HiOutlineEnvelope className="h-4 w-4" /> | ||
| <span>Contact us</span> | ||
| </DropdownMenuSubTrigger> | ||
| <DropdownMenuSubContent> | ||
| <DropdownMenuItem | ||
| onSelect={() => openUrl.mutate("https://discord.gg/superset")} | ||
| className="gap-2" | ||
| > | ||
| <FaDiscord className="h-4 w-4" /> | ||
| <span>Discord</span> | ||
| </DropdownMenuItem> | ||
| <DropdownMenuItem | ||
| onSelect={() => openUrl.mutate("mailto:founders@superset.sh")} | ||
| className="gap-2" | ||
| > | ||
| <HiOutlineEnvelope className="h-4 w-4" /> | ||
| <span>Email founders</span> | ||
| </DropdownMenuItem> | ||
| <DropdownMenuItem | ||
| onSelect={() => openUrl.mutate("https://x.com/supersetsh")} | ||
| className="gap-2" | ||
| > | ||
| <FaXTwitter className="h-4 w-4" /> | ||
| <span>X (Twitter)</span> | ||
| </DropdownMenuItem> | ||
| </DropdownMenuSubContent> | ||
| </DropdownMenuSub> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the target file to understand the context
cat -n "apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx" | head -250Repository: superset-sh/superset
Length of output: 8770
🏁 Script executed:
# Find where openUrl is defined as a tRPC handler
fd . apps/desktop --type f --name "*.ts" --o --name "*.tsx" | xargs rg -l "openUrl" | head -20Repository: superset-sh/superset
Length of output: 863
🏁 Script executed:
# Search for tRPC external handlers/setup
rg "trpc\.external\." apps/desktop/src --type ts --type tsx -A 5 -B 5Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Check for URL validation or allowlist patterns in the codebase
rg "allowlist|allow.*list|validateUrl|scheme.*check" apps/desktop/src --type ts --type tsx -A 3Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
cat -n "apps/desktop/src/lib/trpc/routers/external/index.ts"Repository: superset-sh/superset
Length of output: 3270
🏁 Script executed:
# Search for shell.openExternal usage and any validation
rg "shell\.openExternal|openExternal" apps/desktop/src --type ts -B 5 -A 10Repository: superset-sh/superset
Length of output: 7788
🏁 Script executed:
# Check if there are any URL validation utilities or patterns
rg "url|URL|scheme|protocol" apps/desktop/src/lib/trpc/routers/external -B 2 -A 5Repository: superset-sh/superset
Length of output: 5370
🏁 Script executed:
rg "COMPANY\." apps/desktop/src --type ts -B 2 -A 2 | head -50Repository: superset-sh/superset
Length of output: 4103
🏁 Script executed:
# Find where COMPANY is defined
fd . apps/desktop/src -name "constants*" -o -name "config*" | xargs cat 2>/dev/null | grep -A 10 -B 2 "COMPANY\|DISCORD\|MAIL"Repository: superset-sh/superset
Length of output: 294
🏁 Script executed:
# Search for COMPANY definition
rg "^const COMPANY|export.*COMPANY" apps/desktop/src --type ts -A 10Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
rg "export const COMPANY\s*=" apps/desktop --type ts -A 20Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Search more broadly for COMPANY definition
rg "COMPANY\s*=" apps/desktop --type ts | head -5Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Find config or constants files in main and renderer
find apps/desktop/src -type f \( -name "config.ts" -o -name "constants.ts" -o -name "company.ts" \)Repository: superset-sh/superset
Length of output: 476
🏁 Script executed:
cat -n apps/desktop/src/shared/constants.tsRepository: superset-sh/superset
Length of output: 2126
🏁 Script executed:
# Also check if COMPANY is imported from somewhere
rg "import.*COMPANY|from.*constants" apps/desktop/src/main/lib/menu.ts apps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsxRepository: superset-sh/superset
Length of output: 278
🏁 Script executed:
cat -n $(find apps -name "constants.ts" -path "*/shared/*")Repository: superset-sh/superset
Length of output: 2126
🏁 Script executed:
# Check the shared constants more directly
find . -path "*/shared/constants*" -type fRepository: superset-sh/superset
Length of output: 156
🏁 Script executed:
rg "COMPANY" apps/desktop/src/shared/constants.tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check the entire shared/constants directory
ls -la apps/desktop/src/shared/constants/Repository: superset-sh/superset
Length of output: 238
🏁 Script executed:
# If it's in a different file, search across shared
rg "export.*COMPANY" apps/desktop/src/shared --type ts -A 15Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check tsconfig to understand the @superset/shared alias
rg "@superset/shared" apps/desktop/tsconfig.jsonRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Find where COMPANY is actually defined by searching all files
rg "COMPANY\s*:" apps/desktop/src --type ts | head -20Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Try a broader search in apps folder
rg "COMPANY.*DISCORD\|DISCORD.*superset" apps --type ts -B 5 | head -40Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look at tsconfig to understand path aliases
cat apps/desktop/tsconfig.json | grep -A 5 '"@superset'Repository: superset-sh/superset
Length of output: 211
🏁 Script executed:
# Search in packages folder for COMPANY
rg "COMPANY" packages --type ts | head -10Repository: superset-sh/superset
Length of output: 524
🏁 Script executed:
# Check if COMPANY is defined inline in the importing files
cat -n apps/desktop/src/main/lib/menu.ts | head -100Repository: superset-sh/superset
Length of output: 3186
🏁 Script executed:
cat -n packages/shared/src/constants.ts | grep -A 30 "export const COMPANY"Repository: superset-sh/superset
Length of output: 1319
🏁 Script executed:
# Verify the specific URLs being used in OrganizationDropdown vs COMPANY constants
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx | grep -E "discord|founders|x\.com"Repository: superset-sh/superset
Length of output: 333
🏁 Script executed:
# Check if there are other usages of these inconsistent URLs
rg "discord\.gg/superset" apps/desktop/src --type tsRepository: superset-sh/superset
Length of output: 248
🏁 Script executed:
# Verify the Electron security practices - check what Electron docs recommend
web_search
Electron shell.openExternal security best practices URL validation allowlistRepository: superset-sh/superset
Length of output: 161
🌐 Web query:
Electron shell.openExternal security best practices URL validation allowlist
💡 Result:
Key practices to safely use shell.openExternal in Electron
-
Restrict protocols — only allow safe schemes (http, https). Block or reject file:, javascript:, data:, vbscript:, and app-specific protocols unless explicitly needed. Enforce this in main process code. [1]
-
Use a domain / host allowlist (preferred) or a robust denylist:
- If possible maintain an explicit allowlist of trusted domains (exact host or hostname+path rules).
- If allowlist isn’t feasible, require that the URL matches a safe pattern, then show user confirmation. [1][2]
-
Canonicalize and parse URLs with a standard parser (WHATWG URL) before checking — do not do string-only checks (slashes, %-encoding, userinfo, IDN homographs). Example:
- let u = new URL(input);
- if (!['http:','https:'].includes(u.protocol)) reject;
- if (!allowlist.has(u.hostname)) prompt or reject.
-
Validate caller / origin — only permit openExternal when the request originates from a trusted renderer or a validated IPC channel. Authenticate sender in main process. [1][2]
-
Show the full destination and require explicit user consent for non-allowlisted hosts (display hostname, port, and scheme). Avoid silent opens. [1][3]
-
Sanitize inputs — percent-decode/normalize and reject suspicious userinfo (user:pass@host) or embedded credentials. Block URLs containing newlines, control chars, or CR/LF injection. [2]
-
Log and rate-limit external-open attempts for auditing and detection of abuse. Monitor telemetry for unexpected hosts/protocols. [2][3]
-
Keep Electron up to date and follow upstream security guidance — many published CVEs stem from using openExternal without validation. Review Electron’s security checklist and vendor advisories when designing your checks. [1][3]
Minimal example (main process, pseudo-JS)
- Note: run these checks in main before calling shell.openExternal.
const allowlist = new Set(['example.com','docs.example.com']);
function safeOpenExternal(raw) {
try {
const u = new URL(raw);
if (!['http:','https:'].includes(u.protocol)) throw new Error('bad-protocol');
if (!allowlist.has(u.hostname)) return promptUserAndMaybeOpen(u); // or reject
require('electron').shell.openExternal(u.toString());
} catch (e) {
// reject / log
}
}
Citations
- Electron security checklist (do not use shell.openExternal with untrusted content). [1]
- Real-world advisories/patches where lack of URL checks led to RCE/abuse (Wire, WebCatalog, Bruno examples). [2][3]
Add URL allowlist validation to trpc.external.openUrl and extract hardcoded URLs to constants.
The openUrl handler accepts any string without validating scheme or host, creating a security vulnerability. Per Electron best practices, validate URLs against a whitelist before calling shell.openExternal(). Additionally, extract the three hardcoded URLs/emails (lines 190, 197, 204) to constants referencing COMPANY from @superset/shared/constants. Note the Discord URL differs from the centralized constant (https://discord.gg/superset vs https://discord.gg/cZeD9WYcV7) and the X URL differs from COMPANY.X_URL (supersetsh vs superset_sh).
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
UI Improvements
New Features
✏️ Tip: You can customize this high-level summary in your review settings.