fix(desktop): preserve organization UI and fix auth provider architecture#654
Conversation
📝 WalkthroughWalkthroughThis PR consolidates the authentication infrastructure by replacing multiple discrete auth endpoints with a single Changes
Sequence DiagramssequenceDiagram
participant App as App
participant AuthProvider as AuthProvider
participant TRPC as TRPC Router
participant AuthService as Auth Service
participant Children as Child Components
App->>AuthProvider: Mount
AuthProvider->>TRPC: Subscribe to onAuthState
TRPC->>AuthService: Get current session + token
AuthService-->>TRPC: { session, token } or null
TRPC-->>AuthProvider: Emit initial state
AuthProvider->>AuthProvider: Store token & session in context
AuthProvider->>Children: Render with context
Note over AuthService: Session changes externally
AuthService->>TRPC: Emit state-changed event
TRPC-->>AuthProvider: New state
AuthProvider->>AuthProvider: Update context
AuthProvider->>Children: Re-render with new state
sequenceDiagram
participant UI as Team Settings UI
participant Collections as Collections Provider
participant TRPC as TRPC API
participant Org as Organization Service
UI->>Collections: useCollections hook
Collections->>TRPC: Query members with activeOrganizationId
TRPC->>Org: Fetch members & users
Org-->>TRPC: Member list with user data
TRPC-->>Collections: Joined member details
Collections-->>UI: Render members table
UI->>UI: User clicks remove member
UI->>TRPC: Call removeMember mutation
TRPC->>Org: Remove member from organization
Org-->>TRPC: Success
TRPC-->>UI: Show success toast
UI->>UI: Refresh member list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 5
🤖 Fix all issues with AI agents
In @apps/desktop/src/lib/trpc/routers/auth/index.ts:
- Around line 78-88: The emitCurrent function calls authService.getAccessToken()
synchronously but that method is async, so token becomes a Promise; change
emitCurrent into an async function and await authService.getAccessToken() before
calling emit.next so the emitted object contains the actual token string (or
null) rather than a Promise. Keep the early-return behavior when sessionData is
falsy, await the token only after confirming sessionData exists, and update any
callers/usage to handle emitCurrent returning a Promise if they invoke it
directly.
In
@apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsx:
- Around line 53-166: MemberActions is defined inside TeamSettings.tsx which
violates the one-component-per-file rule; extract the MemberActions function and
its props/types (MemberActionsProps, MemberDetails) into a new module and export
it (e.g., export function MemberActions(...)) so TeamSettings can import and use
it; move all local imports used by MemberActions (authClient, toast, Dialog,
DropdownMenu, Button, HiEllipsisVertical, HiOutlineTrash, useState) into the new
file, preserve the same implementation for handleRemove and dialog/menu logic,
and remove the nested definition from TeamSettings.tsx, replacing it with an
import and usage of the exported MemberActions component.
In @apps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsx:
- Around line 21-35: The four handlers (handleContactUs, handleReportIssue,
handleJoinDiscord, handleTwitter) call window.open without checking for failure;
update each to capture the return value from window.open, detect failures
(null/undefined) or catch thrown errors, and log a prefixed message (e.g.,
"[support/<operation>] ...") with the URL and error context; optionally surface
a user-friendly fallback (alert or in-app notification) when opening is blocked.
Ensure the logging includes the specific function name and URL to aid debugging.
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx:
- Around line 58-64: Both switchOrganization and handleSignOut perform mutations
without handling failures; wrap setActiveOrg.mutateAsync({ organizationId:
newOrgId }) in try/catch inside switchOrganization and call signOut.mutate
inside a try/catch in handleSignOut, using the mutation error to surface user
feedback (e.g., show a toast or set an error state) and optionally handle
loading state via setActiveOrg.isLoading / signOut.isLoading to disable UI while
pending; ensure you reference the setActiveOrg.mutateAsync and signOut.mutate
calls and pass the caught error message into the chosen user-notification
mechanism.
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx:
- Around line 129-132: The HiOutlineClipboardDocumentList component is being
passed a non-supported strokeWidth prop (STROKE_WIDTH); remove the
strokeWidth={STROKE_WIDTH} prop from the HiOutlineClipboardDocumentList JSX so
it uses className-based styling like the other Hi* icons, or alternatively
replace HiOutlineClipboardDocumentList with the Lucide equivalent
LuClipboardList if you need an icon that accepts strokeWidth.
🧹 Nitpick comments (8)
apps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsx (1)
21-23: Consider removing_blanktarget for mailto: links.Opening a
mailto:URL with_blanktarget is unusual since email clients don't open in browser tabs. The_blankparameter has no effect and can be omitted for clarity.♻️ Optional simplification
const handleContactUs = () => { - window.open(COMPANY.MAIL_TO, "_blank"); + window.open(COMPANY.MAIL_TO); };apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsx (1)
60-65: Consider extracting initials calculation to a shared utility.The initials calculation logic is duplicated in TeamSettings.tsx (lines 276-281). Extracting to a utility function would improve maintainability and consistency.
♻️ Proposed refactor to extract utility function
Create a utility file:
// utils/formatters.ts or similar export function getInitials(name: string | null): string { return name ?.split(" ") .map((n) => n[0]) .join("") .toUpperCase() .slice(0, 2) || "?"; }Then use it in both components:
- const initials = member.name - ?.split(" ") - .map((n) => n[0]) - .join("") - .toUpperCase() - .slice(0, 2); + const initials = getInitials(member.name);apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsx (3)
185-203: Optimize member-user join operation.The current implementation uses
array.find()insidemap(), resulting in O(n²) complexity. For larger teams, this could impact performance. Consider using a Map for O(n) lookup.⚡ Proposed optimization using Map
// Join members with users and create member details + const usersMap = usersData ? new Map(usersData.map(u => [u.id, u])) : new Map(); const memberDetails = membersData && usersData ? membersData.map((member) => { - const user = usersData.find((u) => u.id === member.userId); + const user = usersMap.get(member.userId); return { memberId: member.id, userId: member.userId, name: user?.name ?? null, email: user?.email ?? "", image: user?.image ?? null, role: member.role, joinedAt: member.createdAt instanceof Date ? member.createdAt.toISOString() : member.createdAt, organizationId: member.organizationId, }; }) : [];
246-246: Extract magic number to named constant.The value
3for skeleton rows should be extracted to a named constant at the module top for clarity and maintainability.Based on coding guidelines: "Extract magic numbers and hardcoded values to named constants at module top".
♻️ Proposed refactor
+const SKELETON_ROW_COUNT = 3; + interface MemberDetails { memberId: string;Then use it:
- {[1, 2, 3].map((i) => ( + {Array.from({ length: SKELETON_ROW_COUNT }, (_, i) => i + 1).map((i) => (
276-281: Code duplication: initials calculation.This initials calculation is identical to the one in MemberRow.tsx (lines 60-65). Consider extracting it to a shared utility function to maintain consistency and reduce duplication.
apps/desktop/src/lib/trpc/routers/auth/index.ts (1)
94-99: Consider consolidating identical handlers.
sessionHandlerandstateHandlerhave identical implementations. You could use a single handler for both events to reduce duplication.♻️ Optional simplification
- const sessionHandler = () => { - emitCurrent(); - }; - const stateHandler = () => { - emitCurrent(); - }; + const handler = () => { + void emitCurrent(); + }; - authService.on("session-changed", sessionHandler); - authService.on("state-changed", stateHandler); + authService.on("session-changed", handler); + authService.on("state-changed", handler); return () => { - authService.off("session-changed", sessionHandler); - authService.off("state-changed", stateHandler); + authService.off("session-changed", handler); + authService.off("state-changed", handler); };apps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsx (1)
27-48: Consider extractingConditionalProvidersto its own file.As per the coding guidelines for
.tsxfiles: "One component per file — do not create multi-component files." WhileConditionalProvidersis an internal helper, extracting it would maintain consistency.♻️ Optional extraction
Create
apps/desktop/src/renderer/contexts/AppProviders/ConditionalProviders.tsx:import type React from "react"; import { useAuth } from "../AuthProvider"; import { CollectionsProvider } from "../CollectionsProvider"; import { MonacoProvider } from "../MonacoProvider"; import { OrganizationsProvider } from "../OrganizationsProvider"; interface ConditionalProvidersProps { children: React.ReactNode; } export function ConditionalProviders({ children }: ConditionalProvidersProps) { const { session, token } = useAuth(); if (!token || !session?.user) { return <MonacoProvider>{children}</MonacoProvider>; } return ( <CollectionsProvider> <OrganizationsProvider> <MonacoProvider>{children}</MonacoProvider> </OrganizationsProvider> </CollectionsProvider> ); }Then import and use in
AppProviders.tsx.apps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsx (1)
16-30: Consider restructuring to avoid confusingsession.sessionaccess pattern.The current implementation passes the entire
authStateassession, which means consumers must usesession?.session?.activeOrganizationId. While the comment explains this, it creates a confusing API that contradicts the PR objective of flattening the session structure.♻️ Suggested restructuring
interface AuthContextValue { token: string | null; - session: RouterOutputs["auth"]["onAuthState"] | null; + user: RouterOutputs["auth"]["onAuthState"]["user"] | null; + activeOrganizationId: string | null; } export function AuthProvider({ children }: { children: ReactNode }) { const { data: authState } = trpc.auth.onAuthState.useSubscription(); const token = authState?.token ?? null; - // Pass the whole authState (which includes session + user) as "session" - // This way session.user.email, session.session.activeOrganizationId work - const session = authState ?? null; + const user = authState?.user ?? null; + const activeOrganizationId = authState?.session?.activeOrganizationId ?? null; const value: AuthContextValue = { token, - session, + user, + activeOrganizationId, }; return <AuthContext.Provider value={value}>{children}</AuthContext.Provider>; }This would allow consumers to use
activeOrganizationIddirectly instead ofsession?.session?.activeOrganizationId.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
apps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsxapps/desktop/src/renderer/contexts/AuthProvider/index.tsapps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar/GeneralSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/InviteMemberButton.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/stores/app-state.tsapps/desktop/tsconfig.json
💤 Files with no reviewable changes (2)
- apps/desktop/src/renderer/screens/main/components/AvatarDropdown/index.ts
- apps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.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/SettingsView/TeamSettings/components/InviteMemberButton/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsxapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar/GeneralSettings.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/contexts/AuthProvider/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/InviteMemberButton.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsxapps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.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/SettingsView/TeamSettings/components/InviteMemberButton/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsxapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar/GeneralSettings.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/contexts/AuthProvider/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/InviteMemberButton.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsxapps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.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/SettingsView/TeamSettings/components/InviteMemberButton/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsxapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar/GeneralSettings.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/contexts/AuthProvider/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/InviteMemberButton.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsxapps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.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/SettingsView/TeamSettings/components/InviteMemberButton/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsxapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar/GeneralSettings.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/contexts/AuthProvider/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/InviteMemberButton.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsxapps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.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/SettingsView/TeamSettings/components/InviteMemberButton/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsxapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar/GeneralSettings.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/contexts/AuthProvider/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/InviteMemberButton.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsxapps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.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/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsxapps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar/GeneralSettings.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/InviteMemberButton.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsxapps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsx
🧠 Learnings (10)
📚 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/lib/trpc/routers/auth/index.ts
📚 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/SettingsView/TeamSettings/components/MemberRow/index.ts
📚 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/contexts/AuthProvider/index.tsapps/desktop/tsconfig.json
📚 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/contexts/AuthProvider/index.tsapps/desktop/tsconfig.json
📚 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/main/index.ts : Load environment variables from monorepo root .env in desktop app with override: true before any imports in src/main/index.ts and electron.vite.config.ts
Applied to files:
apps/desktop/src/renderer/contexts/AuthProvider/index.ts
📚 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/tsconfig.json
📚 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} : Avoid using `any` type - maintain type safety in TypeScript code
Applied to files:
apps/desktop/tsconfig.json
📚 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/tsconfig.json
📚 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/tsconfig.json
📚 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,js,jsx} : Use Biome for formatting and linting - run at root level with `bun run lint:fix` or `biome check --write`
Applied to files:
apps/desktop/tsconfig.json
🧬 Code graph analysis (9)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx (6)
apps/desktop/src/renderer/stores/app-state.ts (1)
useOpenTasks(97-97)packages/shared/src/constants.ts (1)
FEATURE_FLAGS(45-48)apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (1)
OrganizationDropdown(29-168)packages/ui/src/components/ui/tooltip.tsx (3)
Tooltip(76-76)TooltipTrigger(76-76)TooltipContent(76-76)packages/ui/src/lib/utils.ts (1)
cn(4-6)apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/constants.ts (1)
STROKE_WIDTH(2-2)
apps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsx (2)
packages/shared/src/constants.ts (1)
COMPANY(12-24)packages/ui/src/components/ui/dropdown-menu.tsx (8)
DropdownMenu(249-249)DropdownMenuTrigger(251-251)DropdownMenuContent(252-252)DropdownMenuSub(261-261)DropdownMenuSubTrigger(262-262)DropdownMenuSubContent(263-263)DropdownMenuItem(255-255)DropdownMenuSeparator(259-259)
apps/desktop/src/renderer/screens/main/components/TopBar/index.tsx (1)
apps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsx (1)
SupportMenu(20-78)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (8)
apps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsx (1)
useAuth(32-38)apps/desktop/src/renderer/contexts/AuthProvider/index.ts (1)
useAuth(1-1)apps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsx (1)
useCollections(70-76)apps/desktop/src/main/lib/auth/auth.ts (1)
signOut(219-230)apps/desktop/src/renderer/stores/app-state.ts (1)
useOpenSettings(94-94)packages/ui/src/components/ui/tooltip.tsx (3)
Tooltip(76-76)TooltipTrigger(76-76)TooltipContent(76-76)packages/ui/src/components/ui/avatar.tsx (3)
Avatar(53-53)AvatarImage(53-53)AvatarFallback(53-53)packages/ui/src/components/ui/dropdown-menu.tsx (9)
DropdownMenu(249-249)DropdownMenuTrigger(251-251)DropdownMenuContent(252-252)DropdownMenuItem(255-255)DropdownMenuSeparator(259-259)DropdownMenuSub(261-261)DropdownMenuSubTrigger(262-262)DropdownMenuSubContent(263-263)DropdownMenuLabel(254-254)
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/InviteMemberButton.tsx (3)
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/index.ts (1)
InviteMemberButton(1-1)packages/ui/src/components/ui/tooltip.tsx (3)
Tooltip(76-76)TooltipTrigger(76-76)TooltipContent(76-76)packages/ui/src/components/ui/button.tsx (1)
Button(60-60)
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsx (6)
packages/auth/src/client.ts (1)
authClient(8-11)packages/ui/src/components/ui/sonner.tsx (1)
toast(40-40)packages/ui/src/components/ui/avatar.tsx (3)
Avatar(53-53)AvatarImage(53-53)AvatarFallback(53-53)packages/ui/src/components/ui/badge.tsx (1)
Badge(46-46)packages/ui/src/components/ui/button.tsx (1)
Button(60-60)packages/ui/src/components/ui/dialog.tsx (6)
Dialog(134-134)DialogContent(136-136)DialogHeader(139-139)DialogTitle(142-142)DialogDescription(137-137)DialogFooter(138-138)
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsx (9)
packages/auth/src/client.ts (1)
authClient(8-11)packages/ui/src/components/ui/sonner.tsx (1)
toast(40-40)packages/ui/src/components/ui/button.tsx (1)
Button(60-60)apps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsx (1)
useAuth(32-38)apps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsx (1)
useCollections(70-76)packages/db/src/schema/auth.ts (1)
members(105-122)apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/InviteMemberButton.tsx (1)
InviteMemberButton(5-19)packages/ui/src/components/ui/skeleton.tsx (1)
Skeleton(13-13)packages/ui/src/components/ui/badge.tsx (1)
Badge(46-46)
apps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsx (2)
apps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsx (2)
AuthProvider(16-30)useAuth(32-38)apps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsx (1)
CollectionsProvider(15-68)
apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsx (1)
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsx (1)
TeamSettings(168-348)
⏰ 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 (25)
apps/desktop/tsconfig.json (1)
22-23: Reconsider this exclusion—no .mjs files currently exist in the codebase.The
"**/*.mjs"pattern was added to exclude ES Module files, butfdfound zero .mjs files inapps/desktop/. The existing exclusions of"dist","dist-electron", and"release"already prevent TypeScript from processing generated files in build directories. Adding a blanket"**/*.mjs"exclusion across the entire workspace is overly broad and not justified without:
- Evidence of TypeScript errors caused by .mjs files
- Confirmation that .mjs files are expected in the source tree (not just in build output)
The trailing comma on line 22 is valid formatting.
apps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsx (1)
37-77: LGTM! Well-structured accessibility and UI.The dropdown menu structure is well-organized with proper ARIA labels, logical grouping of contact methods in a submenu, and appropriate visual hierarchy with icons and separators.
apps/desktop/src/renderer/screens/main/components/TopBar/index.tsx (1)
3-3: LGTM! Clean component replacement.The import and usage of
SupportMenucorrectly replaces the previousAvatarDropdown. The integration is straightforward and maintains the existing TopBar layout.Also applies to: 30-30
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/index.ts (1)
1-1: LGTM!The barrel export follows the established component folder structure pattern with ComponentName/ComponentName.tsx and index.ts for re-export.
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/index.ts (1)
1-1: LGTM!The barrel export correctly follows the established pattern for component organization.
apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsx (1)
9-9: LGTM!The TeamSettings import and conditional rendering follow the established pattern used for other settings sections. The integration is clean and consistent with the existing code structure.
Also applies to: 22-22
apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar/GeneralSettings.tsx (1)
9-9: LGTM!The new Team section is well-integrated into the settings sidebar:
- Icon import follows the existing pattern
- Section definition is consistent with other entries
- Logical placement between Account and Appearance sections
Also applies to: 28-32
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/index.ts (1)
1-1: LGTM!The barrel export follows the established component folder structure pattern consistently with the other component exports in this PR.
apps/desktop/src/renderer/stores/app-state.ts (1)
9-9: LGTM!The addition of "team" to the
SettingsSectionunion type correctly supports the new team management feature.apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/InviteMemberButton.tsx (1)
5-19: LGTM!Clean implementation of a placeholder invite button with appropriate disabled state and user feedback via tooltip.
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsx (1)
72-75: Verify the memberIdOrEmail parameter value.Similar to MemberRow.tsx, this passes
member.userIdasmemberIdOrEmail. Confirm the API signature expectsuserIdrather thanmemberId.apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsx (1)
44-47: The parameter value is correct. The API expectsuserIdwith the member's user ID, and the code correctly passesmember.userId. The interface contains bothmemberIdanduserIdas separate fields—usinguserIdis the appropriate choice for member removal.apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (3)
66-97: LGTM — trigger implementations are well-structured.The collapsed/expanded trigger variants provide appropriate UX with tooltips for collapsed state and full display for expanded. Good use of conditional rendering.
101-167: LGTM — dropdown menu structure is clean.Good defensive design: sign-out is always shown to prevent users from being trapped. Conditional rendering of org-specific items and the switch submenu based on context is appropriate.
38-38: No changes needed. Thesession?.session?.activeOrganizationIdaccess pattern is correct and intentional.AuthProvider explicitly documents this pattern: "Pass the whole authState (which includes session + user) as 'session'. This way session.user.email, session.session.activeOrganizationId work." The double nesting reflects the actual structure returned by better-auth's
onAuthStatesubscription, which includes nestedsessionanduserobjects. This pattern is used consistently across the codebase (including CollectionsProvider).Likely an incorrect or invalid review comment.
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx (4)
1-22: LGTM — imports are well-organized.Feature flag import, posthog hook, new icon, and OrganizationDropdown are properly imported with appropriate aliases.
37-42: LGTM — feature flag gating is properly implemented.Using
useFeatureFlagEnabledfrom posthog with the shared constantFEATURE_FLAGS.ELECTRIC_TASKS_ACCESSensures consistent flag checking.
93-95: LGTM — OrganizationDropdown integration.Correctly passes
isCollapsedprop in collapsed view and renders without prop (defaulting tofalse) in expanded view.Also applies to: 169-171
115-137: LGTM — Tasks button is properly feature-gated.The Tasks button follows the same pattern as the Workspaces button and is correctly hidden when the feature flag is disabled.
Also applies to: 189-209
apps/desktop/src/renderer/contexts/AuthProvider/index.ts (1)
1-1: LGTM — clean barrel export.Simple re-export pattern that provides a clean public API without circular dependency risk.
apps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsx (3)
16-20: Good consolidation, but depends on fixing the async bug inonAuthState.The switch from two separate subscriptions to the unified
onAuthStateis a clean simplification. However, the token value will be incorrect until the async bug inapps/desktop/src/lib/trpc/routers/auth/index.ts(line 80) is fixed —getAccessToken()is called synchronously when it's async.
22-42: LGTM — collections creation logic is well-guarded.The useMemo correctly guards against missing token/activeOrgId and handles creation errors. Logging provides good observability.
44-61: LGTM — loading and error states are appropriate.Clear user feedback for initialization failures and loading states.
apps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsx (1)
14-25: LGTM — provider composition is correct.AuthProvider correctly wraps ConditionalProviders, ensuring auth state is available for the conditional logic. This architecture fixes the infinite loading spinner for unauthenticated users.
apps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsx (1)
32-38: LGTM — useAuth hook follows proper context pattern.The guard throws a descriptive error if used outside AuthProvider, which is the correct pattern.
| const handleContactUs = () => { | ||
| window.open(COMPANY.MAIL_TO, "_blank"); | ||
| }; | ||
|
|
||
| const handleReportIssue = () => { | ||
| window.open(COMPANY.REPORT_ISSUE_URL, "_blank"); | ||
| }; | ||
|
|
||
| const handleJoinDiscord = () => { | ||
| window.open(COMPANY.DISCORD_URL, "_blank"); | ||
| }; | ||
|
|
||
| const handleTwitter = () => { | ||
| window.open(COMPANY.X_URL, "_blank"); | ||
| }; |
There was a problem hiding this comment.
Add error handling for window.open failures.
All handler functions call window.open without checking the return value or handling failures (e.g., popup blockers). This violates the coding guideline: "Never swallow errors silently - at minimum log them with context."
🛡️ Recommended fix with error handling and logging
+const openExternalUrl = (url: string, context: string) => {
+ const newWindow = window.open(url, "_blank");
+ if (!newWindow) {
+ console.error(`[SupportMenu/${context}] Failed to open URL: ${url}`);
+ }
+};
+
const handleContactUs = () => {
- window.open(COMPANY.MAIL_TO, "_blank");
+ openExternalUrl(COMPANY.MAIL_TO, "contact");
};
const handleReportIssue = () => {
- window.open(COMPANY.REPORT_ISSUE_URL, "_blank");
+ openExternalUrl(COMPANY.REPORT_ISSUE_URL, "report-issue");
};
const handleJoinDiscord = () => {
- window.open(COMPANY.DISCORD_URL, "_blank");
+ openExternalUrl(COMPANY.DISCORD_URL, "discord");
};
const handleTwitter = () => {
- window.open(COMPANY.X_URL, "_blank");
+ openExternalUrl(COMPANY.X_URL, "twitter");
};This also follows the guideline for prefixed console logging: [domain/operation] message.
Based on coding guidelines.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleContactUs = () => { | |
| window.open(COMPANY.MAIL_TO, "_blank"); | |
| }; | |
| const handleReportIssue = () => { | |
| window.open(COMPANY.REPORT_ISSUE_URL, "_blank"); | |
| }; | |
| const handleJoinDiscord = () => { | |
| window.open(COMPANY.DISCORD_URL, "_blank"); | |
| }; | |
| const handleTwitter = () => { | |
| window.open(COMPANY.X_URL, "_blank"); | |
| }; | |
| const openExternalUrl = (url: string, context: string) => { | |
| const newWindow = window.open(url, "_blank"); | |
| if (!newWindow) { | |
| console.error(`[SupportMenu/${context}] Failed to open URL: ${url}`); | |
| } | |
| }; | |
| const handleContactUs = () => { | |
| openExternalUrl(COMPANY.MAIL_TO, "contact"); | |
| }; | |
| const handleReportIssue = () => { | |
| openExternalUrl(COMPANY.REPORT_ISSUE_URL, "report-issue"); | |
| }; | |
| const handleJoinDiscord = () => { | |
| openExternalUrl(COMPANY.DISCORD_URL, "discord"); | |
| }; | |
| const handleTwitter = () => { | |
| openExternalUrl(COMPANY.X_URL, "twitter"); | |
| }; |
🤖 Prompt for AI Agents
In @apps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsx
around lines 21 - 35, The four handlers (handleContactUs, handleReportIssue,
handleJoinDiscord, handleTwitter) call window.open without checking for failure;
update each to capture the return value from window.open, detect failures
(null/undefined) or catch thrown errors, and log a prefixed message (e.g.,
"[support/<operation>] ...") with the URL and error context; optionally surface
a user-friendly fallback (alert or in-app notification) when opening is blocked.
Ensure the logging includes the specific function name and URL to aid debugging.
| <HiOutlineClipboardDocumentList | ||
| className="size-4" | ||
| strokeWidth={STROKE_WIDTH} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's examine the actual file mentioned
head -150 "apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx" | tail -30Repository: superset-sh/superset
Length of output: 874
🏁 Script executed:
#!/bin/bash
# Search for HeroIcons usage patterns across the codebase
rg "Hi[A-Z][a-zA-Z]*\(" --type tsx -B1 -A2 | head -100Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Check if strokeWidth is used with HeroIcons elsewhere
rg "Hi[A-Z].*strokeWidth" --type tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Search for HeroIcons usage without type restriction
rg "HiOutline|HiSolid" -A2 -B1 | head -80Repository: superset-sh/superset
Length of output: 8068
🏁 Script executed:
#!/bin/bash
# Look for strokeWidth usage with react-icons imports
rg "strokeWidth" -A1 -B1 | head -60Repository: superset-sh/superset
Length of output: 3368
🏁 Script executed:
#!/bin/bash
# Check imports from react-icons in the target file
cat "apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx" | head -50Repository: superset-sh/superset
Length of output: 1660
🏁 Script executed:
#!/bin/bash
# Search for STROKE_WIDTH constant definition
rg "STROKE_WIDTH\s*=" -B2 -A2Repository: superset-sh/superset
Length of output: 887
🏁 Script executed:
#!/bin/bash
# Check how Lucide icons (LuLayers, etc.) are used with props
rg "Lu[A-Z]" -A2 -B1 | head -100Repository: superset-sh/superset
Length of output: 9131
🏁 Script executed:
#!/bin/bash
# Look at the imports and see if strokeWidth is used with any Lucide icons
grep -n "strokeWidth\|strokeLinecap\|strokeLinejoin" "apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx"Repository: superset-sh/superset
Length of output: 582
Remove strokeWidth prop from HeroIcon — not supported by react-icons/hi2.
HiOutlineClipboardDocumentList from react-icons/hi2 (HeroIcons) does not support strokeWidth. This prop has no effect. HeroIcons use className for styling (matching all other Hi* icons in the codebase). Remove the strokeWidth prop or switch to a Lucide icon equivalent like LuClipboardList.
🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx
around lines 129 - 132, The HiOutlineClipboardDocumentList component is being
passed a non-supported strokeWidth prop (STROKE_WIDTH); remove the
strokeWidth={STROKE_WIDTH} prop from the HiOutlineClipboardDocumentList JSX so
it uses className-based styling like the other Hi* icons, or alternatively
replace HiOutlineClipboardDocumentList with the Lucide equivalent
LuClipboardList if you need an icon that accepts strokeWidth.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 6
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/index.tsx (1)
156-167: Refactor to use object parameters.This function has 3 positional parameters, which violates the coding guideline requiring object parameters for functions with 2+ parameters.
As per coding guidelines: "Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility"
♻️ Refactor to object parameters pattern
const resolveSplitTarget = useCallback( - (paneId: string, tabId: string, targetTab: Tab) => { + ({ paneId, tabId, targetTab }: { paneId: string; tabId: string; targetTab: Tab }) => { const path = findPanePath(targetTab.layout, paneId); if (path !== null) return { path, paneId }; const firstPaneId = getFirstPaneId(targetTab.layout); const firstPanePath = findPanePath(targetTab.layout, firstPaneId); setFocusedPane(tabId, firstPaneId); return { path: firstPanePath ?? [], paneId: firstPaneId }; }, [setFocusedPane], );Update call sites (lines 173, 200, 224):
-const target = resolveSplitTarget( - focusedPaneId, - activeTabId, - activeTab, -); +const target = resolveSplitTarget({ + paneId: focusedPaneId, + tabId: activeTabId, + targetTab: activeTab, +});
🤖 Fix all issues with AI agents
In @apps/desktop/src/lib/trpc/routers/auth/index.ts:
- Around line 14-24: emitCurrent calls authService.getAccessToken()
synchronously, so token is a Promise rather than a string; make emitCurrent
async, await authService.getAccessToken(), and then call emit.next with the
resolved token (or null) so consumers receive a string | null. Update
emitCurrent's signature to async, await authService.getAccessToken() into token,
keep the early return when authService.getSession() is falsy, and ensure any
callers that invoked emitCurrent synchronously are adjusted to handle the
returned Promise or invoked without awaiting if appropriate.
In
@apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsx:
- Around line 102-146: The confirmation dialog in MemberActions renders
member.name which is typed string | null and can display "null"; update the JSX
that shows <strong>{member.name}</strong> to use a safe fallback (e.g.,
member.name ?? member.email ?? "this user") so a readable value is shown when
name is null, and ensure any nearby uses of member.name in DialogDescription
follow the same fallback pattern.
In
@apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsx:
- Line 1: The MemberRow component currently calls
authClient.organization.removeMember directly; replace that IPC call with a tRPC
mutation by importing and using the trpc hook (e.g., create a
removeMemberMutation via trpc.organization.removeMember.useMutation()) and
invoke removeMemberMutation.mutate or mutateAsync instead of
authClient.organization.removeMember; also update the button disabled prop to
use removeMemberMutation.isPending (and handle success/error via
onSuccess/onError or await mutateAsync) and remove the authClient import. Ensure
you reference MemberRow and the authClient.organization.removeMember usage when
making the change.
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx:
- Around line 109-130: The HiOutlineClipboardDocumentList icon is being passed
an unsupported prop strokeWidth={STROKE_WIDTH}; remove that prop or replace the
icon with a Lucide equivalent that supports strokeWidth. Locate the usage of
HiOutlineClipboardDocumentList (and where STROKE_WIDTH is referenced) inside
WorkspaceSidebarHeader and either delete the strokeWidth attribute from the JSX
or swap the import/usage to LuClipboardList and ensure any strokeWidth usage
remains valid; apply the same change to the matching expanded-view usage
referenced in the review.
🧹 Nitpick comments (5)
apps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsx (2)
27-41: Extract ConditionalProviders to a separate file.The coding guidelines specify "One component per file - do not create multi-component files" for
**/*.tsx. WhileConditionalProvidersis a small helper component, extracting it to its own file would improve testability and maintainability.As per coding guidelines
📁 Suggested structure
Create a new file
apps/desktop/src/renderer/contexts/AppProviders/ConditionalProviders.tsx:import type React from "react"; import { useAuth } from "../AuthProvider"; import { CollectionsProvider } from "../CollectionsProvider"; import { MonacoProvider } from "../MonacoProvider"; import { OrganizationsProvider } from "../OrganizationsProvider"; interface ConditionalProvidersProps { children: React.ReactNode; } export function ConditionalProviders({ children }: ConditionalProvidersProps) { const { session, token } = useAuth(); if (!token || !session?.user) { return <MonacoProvider>{children}</MonacoProvider>; } return ( <CollectionsProvider> <OrganizationsProvider> <MonacoProvider>{children}</MonacoProvider> </OrganizationsProvider> </CollectionsProvider> ); }Then import it in
AppProviders.tsx.
30-32: Consider using a named boolean for authentication check.The condition
!token || !session?.userdetermines authentication state. Extracting this to a named constant likeconst isAuthenticated = Boolean(token && session?.user)would improve readability and make the intent clearer.♻️ Refactor for clarity
function ConditionalProviders({ children }: AppProvidersProps) { const { session, token } = useAuth(); + const isAuthenticated = Boolean(token && session?.user); - if (!token || !session?.user) { + if (!isAuthenticated) { return <MonacoProvider>{children}</MonacoProvider>; }apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsx (1)
16-16: Consider moving shared types to a common location.The
MemberDetailstype is imported from a sibling component (../MemberActions), which creates tight coupling between components. Consider defining shared types in a common types file or barrel export at the TeamSettings level for better separation of concerns.This would improve maintainability and make it easier to reuse types across multiple components.
Also applies to: 18-22
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsx (2)
69-75: Consider extracting date formatting configuration.The hardcoded
"en-US"locale and formatting options could be extracted to named constants at the module top for better maintainability and potential i18n support.♻️ Proposed refactor
+const DATE_LOCALE = "en-US"; +const DATE_FORMAT_OPTIONS: Intl.DateTimeFormatOptions = { + month: "short", + day: "numeric", +}; + export function TeamSettings() { // ... other code ... const formatDate = (dateString: string) => { const date = new Date(dateString); - return date.toLocaleDateString("en-US", { - month: "short", - day: "numeric", - }); + return date.toLocaleDateString(DATE_LOCALE, DATE_FORMAT_OPTIONS); };
95-108: Consider extracting skeleton count constant.The array
[1, 2, 3]on Line 97 is a magic value. Consider extracting it to a named constant for clarity.♻️ Proposed refactor
+const SKELETON_ROW_COUNT = 3; + export function TeamSettings() { // ... other code ... return ( // ... JSX ... {isLoading ? ( <div className="space-y-2 border rounded-lg"> - {[1, 2, 3].map((i) => ( + {Array.from({ length: SKELETON_ROW_COUNT }, (_, i) => i + 1).map((i) => ( <div key={i} className="flex items-center gap-4 p-4">
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/admin/src/app/(dashboard)/components/LeaderboardTable/LeaderboardTable.tsxapps/admin/src/app/(dashboard)/users/components/UsersTable/UsersTable.tsxapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/AccountSettings/AccountSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/tsconfig.json
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsx
- 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/contexts/AppProviders/AppProviders.tsxapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/AccountSettings/AccountSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.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/contexts/AppProviders/AppProviders.tsxapps/admin/src/app/(dashboard)/components/LeaderboardTable/LeaderboardTable.tsxapps/admin/src/app/(dashboard)/users/components/UsersTable/UsersTable.tsxapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/AccountSettings/AccountSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.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/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/AccountSettings/AccountSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/admin/src/app/(dashboard)/components/LeaderboardTable/LeaderboardTable.tsxapps/admin/src/app/(dashboard)/users/components/UsersTable/UsersTable.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/AccountSettings/AccountSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.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/contexts/AppProviders/AppProviders.tsxapps/admin/src/app/(dashboard)/components/LeaderboardTable/LeaderboardTable.tsxapps/admin/src/app/(dashboard)/users/components/UsersTable/UsersTable.tsxapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/AccountSettings/AccountSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.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/contexts/AppProviders/AppProviders.tsxapps/admin/src/app/(dashboard)/components/LeaderboardTable/LeaderboardTable.tsxapps/admin/src/app/(dashboard)/users/components/UsersTable/UsersTable.tsxapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/AccountSettings/AccountSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsx
🧠 Learnings (9)
📚 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/tsconfig.json
📚 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/tsconfig.json
📚 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/main/index.ts : Load environment variables from monorepo root .env in desktop app with override: true before any imports in src/main/index.ts and electron.vite.config.ts
Applied to files:
apps/desktop/tsconfig.json
📚 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/tsconfig.json
📚 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/tsconfig.jsonapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.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/tsconfig.jsonapps/desktop/src/renderer/screens/main/index.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/tsconfig.json
📚 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/lib/trpc/routers/auth/index.ts
📚 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/**/*.{ts,tsx} : Co-locate component dependencies: utils, hooks, constants, config, tests, stories in the same folder as the component using them
Applied to files:
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsx
🧬 Code graph analysis (5)
apps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsx (3)
apps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsx (2)
AuthProvider(12-24)useAuth(26-32)apps/desktop/src/renderer/contexts/AuthProvider/index.ts (2)
AuthProvider(1-1)useAuth(1-1)apps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsx (1)
CollectionsProvider(15-68)
apps/desktop/src/lib/trpc/routers/auth/index.ts (3)
packages/trpc/src/trpc.ts (1)
publicProcedure(31-31)packages/trpc/src/index.ts (1)
publicProcedure(12-12)apps/desktop/src/main/lib/auth/index.ts (1)
authService(2-2)
apps/desktop/src/renderer/screens/main/index.tsx (1)
apps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsx (1)
useAuth(26-32)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx (5)
apps/desktop/src/renderer/stores/app-state.ts (1)
useOpenTasks(97-97)packages/shared/src/constants.ts (1)
FEATURE_FLAGS(45-48)apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (1)
OrganizationDropdown(30-164)packages/ui/src/lib/utils.ts (1)
cn(4-6)apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/constants.ts (1)
STROKE_WIDTH(2-2)
apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx (2)
apps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsx (1)
useAuth(26-32)apps/desktop/src/renderer/lib/trpc.ts (1)
trpc(5-5)
⏰ 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 (22)
apps/desktop/tsconfig.json (1)
13-13: Explicit inclusion improves TypeScript reliability.The change from wildcard
electron.vite.config.*to explicitelectron.vite.config.tsaligns with the PR goal of excluding generated.mjsfiles from TypeScript checks. This is the correct approach—only oneelectron.vite.config.tsfile exists in the repository, and explicit inclusion reduces the risk of accidentally compiling unwanted file variants.apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx (4)
1-22: LGTM! Imports are well-organized and follow project conventions.The new imports for feature flags, Tasks functionality, and OrganizationDropdown are properly structured and use the appropriate path aliases.
34-42: LGTM! Feature flag integration follows best practices.The feature flag check and state derivation are implemented correctly, following the same pattern as the existing workspaces list logic.
161-162: LGTM! OrganizationDropdown integration is clean.The component is properly positioned in the header and correctly omits the
isCollapsedprop for the expanded state.
179-198: Tasks button implementation follows existing patterns.The expanded view Tasks button mirrors the structure of the Workspaces button and correctly uses the feature flag guard. Note that line 193 has the same
strokeWidthissue flagged in the collapsed view.apps/desktop/src/lib/trpc/routers/auth/index.ts (2)
45-56: LGTM!Both
setActiveOrganizationandsignInmutations follow coding guidelines: they use object parameters for named properties, employ proper Zod validation, and maintain type safety.
58-61: LGTM!The
signOutmutation is correctly implemented as an async operation with proper success indication.apps/desktop/src/renderer/screens/main/index.tsx (2)
11-11: Clean integration with centralized authentication.The migration to
useAuth()and centralized session management simplifies the authentication flow and eliminates the previous auth subscription logic. The conditional query enablement and guard clauses properly gate workspace operations on authentication state.Also applies to: 58-59, 96-96, 271-280
58-59: EnsureSKIP_ENV_VALIDATIONis only used in local development and test environments, never in production builds.The pattern at line 59 (
const isSignedIn = !!process.env.SKIP_ENV_VALIDATION || !!session?.user;) intentionally bypasses authentication when the environment variable is set. While documented as development-only inBUILDING.mdandREADME.md, this is a security-sensitive pattern that should be explicitly prevented from reaching production builds through CI/deployment safeguards.apps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsx (1)
14-25: No action needed.PostHogUserIdentifierobtains user data viatrpc.user.me.useQuery()and does not depend onAuthProvidercontext. It only requiresTRPCProvider, which is already a parent in the component tree. The current placement is correct.Likely an incorrect or invalid review comment.
apps/admin/src/app/(dashboard)/users/components/UsersTable/UsersTable.tsx (1)
3-3: LGTM! Good refactor to centralized utility.The change to use
getInitialsfrom the shared package improves consistency and maintainability across the codebase.Also applies to: 158-158
apps/desktop/src/renderer/screens/main/components/SettingsView/AccountSettings/AccountSettings.tsx (1)
1-1: LGTM! Proper handling of optional values.The refactor correctly uses optional chaining for potentially undefined user data and provides an appropriate fallback.
Also applies to: 16-16
apps/admin/src/app/(dashboard)/components/LeaderboardTable/LeaderboardTable.tsx (1)
3-3: LGTM! Consistent with the refactoring pattern.The change aligns with the broader effort to centralize initials generation logic.
Also applies to: 103-103
apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx (2)
35-35: VerifygetInitialsusage with single parameter.Unlike other files in this PR that call
getInitials(name, email)with both parameters, this code only passes the organization name. While this might be intentional (organizations may not have email addresses), it's inconsistent with the pattern elsewhere.Please confirm this is the intended usage and that
getInitialsproperly handles being called with a single argument.Also applies to: 70-70
20-20: No review comment was provided. Please share the review comment you would like me to rewrite, along with relevant context about the code being reviewed.apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsx (1)
32-49: Good error handling and user feedback.The error handling with try-catch, toast notifications for success/error states, and the comment documenting real-time sync behavior are well implemented.
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsx (3)
1-17: LGTM!Imports are well-organized, use aliases consistently, and import from concrete files as recommended by the coding guidelines.
114-188: LGTM!The table rendering logic is clean and handles edge cases appropriately with reasonable fallbacks for missing user data.
36-54: Empty email fallback works but produces awkward dialog text.Line 44 defaults
getInitials()handles empty strings safely and the removal confirmation dialog displays correctly—it produces awkward text showing()when the email is missing. Consider usingnullinstead of""for semantic clarity, or validate that users are always found inusersData.apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsx (3)
1-36: LGTM!Interfaces are well-defined. Exporting
MemberDetailsallows for type reuse across components, while keepingMemberActionsPropsprivate is appropriate for internal use.
73-100: LGTM!The dropdown menu logic is clear and handles all three cases appropriately (current user, removable member, and no actions available).
46-71: Verify which member identifier better-auth expects forremoveMember.Line 57 passes
member.userIdto thememberIdOrEmailparameter. However, theMemberDetailsinterface has bothmemberIdanduserIdproperties. The parameter namememberIdOrEmailsuggests it expects a member ID or email address, not a user ID. Confirm whetheruserIdis the correct value or ifmemberIdshould be passed instead. This applies to line 37 inMemberRow.tsxas well, which uses the same pattern.
| const emitCurrent = () => { | ||
| const sessionData = authService.getSession(); | ||
| const token = authService.getAccessToken(); | ||
|
|
||
| if (!sessionData) { | ||
| emit.next(null); | ||
| return; | ||
| } | ||
| }; | ||
|
|
||
| const handler = () => { | ||
| void emitToken(); | ||
| emit.next({ ...sessionData, token }); | ||
| }; |
There was a problem hiding this comment.
getAccessToken() is async but called synchronously — token will be a Promise, not a string.
As flagged in the previous review, authService.getAccessToken() is an async function that requires await. Here it's called synchronously on line 16, so token will be a Promise<string | null> instead of the actual token value, breaking downstream consumers who expect a string.
🐛 Proposed fix
- const emitCurrent = () => {
+ const emitCurrent = async () => {
const sessionData = authService.getSession();
- const token = authService.getAccessToken();
+ const token = await authService.getAccessToken();
if (!sessionData) {
emit.next(null);
return;
}
emit.next({ ...sessionData, token });
};
// Emit initial state
- emitCurrent();
+ void emitCurrent();
// Listen for changes to both session and auth state
const sessionHandler = () => {
- emitCurrent();
+ void emitCurrent();
};
const stateHandler = () => {
- emitCurrent();
+ void emitCurrent();
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @apps/desktop/src/lib/trpc/routers/auth/index.ts around lines 14 - 24,
emitCurrent calls authService.getAccessToken() synchronously, so token is a
Promise rather than a string; make emitCurrent async, await
authService.getAccessToken(), and then call emit.next with the resolved token
(or null) so consumers receive a string | null. Update emitCurrent's signature
to async, await authService.getAccessToken() into token, keep the early return
when authService.getSession() is falsy, and ensure any callers that invoked
emitCurrent synchronously are adjusted to handle the returned Promise or invoked
without awaiting if appropriate.
| import { AuthProvider, useAuth } from "../AuthProvider"; | ||
| import { CollectionsProvider } from "../CollectionsProvider"; | ||
| import { MonacoProvider } from "../MonacoProvider"; | ||
| import { OrganizationsProvider } from "../OrganizationsProvider"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if tsconfig.json exists for apps/desktop and inspect path aliases
if [ -f "apps/desktop/tsconfig.json" ]; then
echo "=== apps/desktop/tsconfig.json ==="
cat apps/desktop/tsconfig.json | jq '.compilerOptions.paths' 2>/dev/null || cat apps/desktop/tsconfig.json
else
echo "apps/desktop/tsconfig.json not found, checking root tsconfig"
cat tsconfig.json | jq '.compilerOptions.paths' 2>/dev/null || cat tsconfig.json | head -50
fiRepository: superset-sh/superset
Length of output: 153
🏁 Script executed:
# Get the full content of the file being reviewed
cat -n apps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxRepository: superset-sh/superset
Length of output: 1519
🏁 Script executed:
# Check if import paths are barrel files (index.ts/tsx)
for path in AuthProvider CollectionsProvider MonacoProvider OrganizationsProvider; do
echo "=== Checking $path ==="
if [ -d "apps/desktop/src/renderer/contexts/$path" ]; then
ls -la "apps/desktop/src/renderer/contexts/$path/" 2>/dev/null || echo "Directory does not exist"
elif [ -f "apps/desktop/src/renderer/contexts/$path.ts" ] || [ -f "apps/desktop/src/renderer/contexts/$path.tsx" ]; then
echo "File exists: apps/desktop/src/renderer/contexts/$path.ts(x)"
fi
doneRepository: superset-sh/superset
Length of output: 1220
🏁 Script executed:
# Check if "renderer" is actually an alias by looking for webpack/vite config or full tsconfig
fd -t f "tsconfig\|webpack\|vite" apps/desktop/ | head -10Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Let's see the complete tsconfig.json to understand the path aliases better
head -100 apps/desktop/tsconfig.jsonRepository: superset-sh/superset
Length of output: 440
🏁 Script executed:
# Verify the path alias for "renderer" by checking if it's defined
grep -r '"renderer"' apps/desktop/tsconfig.json || echo "No 'renderer' alias found in desktop tsconfig"Repository: superset-sh/superset
Length of output: 109
Fix import inconsistency and split multi-component file; import concrete files instead of barrel files.
Line 2 uses the alias pattern (renderer/components/PostHogUserIdentifier), but lines 3–8 use relative imports. Import paths should be consistent using the * alias mapping: renderer/contexts/AuthProvider instead of ../AuthProvider, and similarly for other providers.
Additionally, imports should be from concrete files directly, not barrel files. Import from renderer/contexts/AuthProvider/AuthProvider or renderer/contexts/CollectionsProvider/CollectionsProvider instead of the index.ts re-exports.
Finally, ConditionalProviders (lines 27–41) is a separate component and should be in its own file per the "one component per file" guideline for .tsx files.
| <Dialog open={showRemoveDialog} onOpenChange={setShowRemoveDialog}> | ||
| <DialogContent> | ||
| <DialogHeader> | ||
| <DialogTitle> | ||
| {isCurrentUser ? "Leave organization?" : "Remove team member?"} | ||
| </DialogTitle> | ||
| <DialogDescription> | ||
| {isCurrentUser ? ( | ||
| <> | ||
| Are you sure you want to leave this organization? You will | ||
| lose access immediately. | ||
| </> | ||
| ) : ( | ||
| <> | ||
| Are you sure you want to remove <strong>{member.name}</strong>{" "} | ||
| ({member.email}) from the organization? They will lose access | ||
| immediately. | ||
| </> | ||
| )} | ||
| </DialogDescription> | ||
| </DialogHeader> | ||
| <DialogFooter> | ||
| <Button | ||
| variant="outline" | ||
| onClick={() => setShowRemoveDialog(false)} | ||
| disabled={isRemoving} | ||
| > | ||
| Cancel | ||
| </Button> | ||
| <Button | ||
| variant="destructive" | ||
| onClick={handleRemove} | ||
| disabled={isRemoving} | ||
| > | ||
| {isRemoving | ||
| ? isCurrentUser | ||
| ? "Leaving..." | ||
| : "Removing..." | ||
| : isCurrentUser | ||
| ? "Leave Organization" | ||
| : "Remove Member"} | ||
| </Button> | ||
| </DialogFooter> | ||
| </DialogContent> | ||
| </Dialog> |
There was a problem hiding this comment.
Handle potential null name in removal confirmation.
Line 116 renders <strong>{member.name}</strong> in the confirmation dialog. Since MemberDetails.name is typed as string | null (Line 24), this could display "null" as text when the member's name is missing. Consider using a fallback like member.name ?? "this user" or member.email.
🐛 Proposed fix
) : (
<>
- Are you sure you want to remove <strong>{member.name}</strong>{" "}
- ({member.email}) from the organization? They will lose access
+ Are you sure you want to remove <strong>{member.name ?? member.email}</strong>
+ {member.name && ` (${member.email})`} from the organization? They will lose access
immediately.
</>
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Dialog open={showRemoveDialog} onOpenChange={setShowRemoveDialog}> | |
| <DialogContent> | |
| <DialogHeader> | |
| <DialogTitle> | |
| {isCurrentUser ? "Leave organization?" : "Remove team member?"} | |
| </DialogTitle> | |
| <DialogDescription> | |
| {isCurrentUser ? ( | |
| <> | |
| Are you sure you want to leave this organization? You will | |
| lose access immediately. | |
| </> | |
| ) : ( | |
| <> | |
| Are you sure you want to remove <strong>{member.name}</strong>{" "} | |
| ({member.email}) from the organization? They will lose access | |
| immediately. | |
| </> | |
| )} | |
| </DialogDescription> | |
| </DialogHeader> | |
| <DialogFooter> | |
| <Button | |
| variant="outline" | |
| onClick={() => setShowRemoveDialog(false)} | |
| disabled={isRemoving} | |
| > | |
| Cancel | |
| </Button> | |
| <Button | |
| variant="destructive" | |
| onClick={handleRemove} | |
| disabled={isRemoving} | |
| > | |
| {isRemoving | |
| ? isCurrentUser | |
| ? "Leaving..." | |
| : "Removing..." | |
| : isCurrentUser | |
| ? "Leave Organization" | |
| : "Remove Member"} | |
| </Button> | |
| </DialogFooter> | |
| </DialogContent> | |
| </Dialog> | |
| <Dialog open={showRemoveDialog} onOpenChange={setShowRemoveDialog}> | |
| <DialogContent> | |
| <DialogHeader> | |
| <DialogTitle> | |
| {isCurrentUser ? "Leave organization?" : "Remove team member?"} | |
| </DialogTitle> | |
| <DialogDescription> | |
| {isCurrentUser ? ( | |
| <> | |
| Are you sure you want to leave this organization? You will | |
| lose access immediately. | |
| </> | |
| ) : ( | |
| <> | |
| Are you sure you want to remove <strong>{member.name ?? member.email}</strong> | |
| {member.name && ` (${member.email})`} from the organization? They will lose access | |
| immediately. | |
| </> | |
| )} | |
| </DialogDescription> | |
| </DialogHeader> | |
| <DialogFooter> | |
| <Button | |
| variant="outline" | |
| onClick={() => setShowRemoveDialog(false)} | |
| disabled={isRemoving} | |
| > | |
| Cancel | |
| </Button> | |
| <Button | |
| variant="destructive" | |
| onClick={handleRemove} | |
| disabled={isRemoving} | |
| > | |
| {isRemoving | |
| ? isCurrentUser | |
| ? "Leaving..." | |
| : "Removing..." | |
| : isCurrentUser | |
| ? "Leave Organization" | |
| : "Remove Member"} | |
| </Button> | |
| </DialogFooter> | |
| </DialogContent> | |
| </Dialog> |
🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsx
around lines 102 - 146, The confirmation dialog in MemberActions renders
member.name which is typed string | null and can display "null"; update the JSX
that shows <strong>{member.name}</strong> to use a safe fallback (e.g.,
member.name ?? member.email ?? "this user") so a readable value is shown when
name is null, and ensure any nearby uses of member.name in DialogDescription
follow the same fallback pattern.
| @@ -0,0 +1,127 @@ | |||
| import { authClient } from "@superset/auth/client"; | |||
There was a problem hiding this comment.
Use tRPC instead of authClient for Electron IPC.
The coding guidelines specify: "For Electron interprocess communication, ALWAYS use tRPC as defined in src/lib/trpc". This component uses authClient.organization.removeMember directly instead of tRPC, which is inconsistent with other parts of the codebase (e.g., OrganizationSwitcher.tsx uses trpc.auth.setActiveOrganization.useMutation()).
♻️ Suggested refactor to use tRPC
-import { authClient } from "@superset/auth/client";
import { getInitials } from "@superset/shared/names";
// ... other imports
+import { trpc } from "renderer/lib/trpc";
export function MemberRow({
member,
isCurrentUser,
canRemove,
}: MemberRowProps) {
const [showRemoveDialog, setShowRemoveDialog] = useState(false);
- const [isRemoving, setIsRemoving] = useState(false);
+
+ const removeMemberMutation = trpc.organization.removeMember.useMutation({
+ onSuccess: () => {
+ toast.success("Member removed");
+ setShowRemoveDialog(false);
+ },
+ onError: (error) => {
+ toast.error(error.message || "Failed to remove member");
+ },
+ });
const handleRemove = async () => {
- setIsRemoving(true);
- try {
- await authClient.organization.removeMember({
- organizationId: member.organizationId,
- memberIdOrEmail: member.userId,
- });
- toast.success("Member removed");
- setShowRemoveDialog(false);
- // Electric collections will automatically update via real-time sync
- } catch (error) {
- toast.error(
- error instanceof Error ? error.message : "Failed to remove member",
- );
- } finally {
- setIsRemoving(false);
- }
+ removeMemberMutation.mutate({
+ organizationId: member.organizationId,
+ memberIdOrEmail: member.userId,
+ });
};Then update the button's disabled state to use removeMemberMutation.isPending.
Based on coding guidelines.
Also applies to: 32-49
🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsx
at line 1, The MemberRow component currently calls
authClient.organization.removeMember directly; replace that IPC call with a tRPC
mutation by importing and using the trpc hook (e.g., create a
removeMemberMutation via trpc.organization.removeMember.useMutation()) and
invoke removeMemberMutation.mutate or mutateAsync instead of
authClient.organization.removeMember; also update the button disabled prop to
use removeMemberMutation.isPending (and handle success/error via
onSuccess/onError or await mutateAsync) and remove the authClient import. Ensure
you reference MemberRow and the authClient.organization.removeMember usage when
making the change.
| {hasTasksAccess && ( | ||
| <Tooltip delayDuration={300}> | ||
| <TooltipTrigger asChild> | ||
| <button | ||
| type="button" | ||
| onClick={() => openTasks()} | ||
| className={cn( | ||
| "flex items-center justify-center size-8 rounded-md transition-colors", | ||
| isTasksOpen | ||
| ? "text-foreground bg-accent" | ||
| : "text-muted-foreground hover:text-foreground hover:bg-accent/50", | ||
| )} | ||
| > | ||
| <HiOutlineClipboardDocumentList | ||
| className="size-4" | ||
| strokeWidth={STROKE_WIDTH} | ||
| /> | ||
| </button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="right">Tasks</TooltipContent> | ||
| </Tooltip> | ||
| )} |
There was a problem hiding this comment.
Remove unsupported strokeWidth prop from HeroIcon.
Lines 122-125 still use strokeWidth={STROKE_WIDTH} on HiOutlineClipboardDocumentList, which is not supported by react-icons/hi2. This was flagged in a previous review. Either remove the prop or switch to a Lucide icon like LuClipboardList that supports this property.
♻️ Option 1: Remove the unsupported prop
<HiOutlineClipboardDocumentList
className="size-4"
- strokeWidth={STROKE_WIDTH}
/>♻️ Option 2: Switch to Lucide icon (if stroke control is needed)
Import change:
-import { HiOutlineClipboardDocumentList } from "react-icons/hi2";
+import { LuClipboardList } from "react-icons/lu";Usage (line 122):
-<HiOutlineClipboardDocumentList
+<LuClipboardList
className="size-4"
strokeWidth={STROKE_WIDTH}
/>Apply the same change at line 191 in the expanded view.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {hasTasksAccess && ( | |
| <Tooltip delayDuration={300}> | |
| <TooltipTrigger asChild> | |
| <button | |
| type="button" | |
| onClick={() => openTasks()} | |
| className={cn( | |
| "flex items-center justify-center size-8 rounded-md transition-colors", | |
| isTasksOpen | |
| ? "text-foreground bg-accent" | |
| : "text-muted-foreground hover:text-foreground hover:bg-accent/50", | |
| )} | |
| > | |
| <HiOutlineClipboardDocumentList | |
| className="size-4" | |
| strokeWidth={STROKE_WIDTH} | |
| /> | |
| </button> | |
| </TooltipTrigger> | |
| <TooltipContent side="right">Tasks</TooltipContent> | |
| </Tooltip> | |
| )} | |
| {hasTasksAccess && ( | |
| <Tooltip delayDuration={300}> | |
| <TooltipTrigger asChild> | |
| <button | |
| type="button" | |
| onClick={() => openTasks()} | |
| className={cn( | |
| "flex items-center justify-center size-8 rounded-md transition-colors", | |
| isTasksOpen | |
| ? "text-foreground bg-accent" | |
| : "text-muted-foreground hover:text-foreground hover:bg-accent/50", | |
| )} | |
| > | |
| <HiOutlineClipboardDocumentList | |
| className="size-4" | |
| /> | |
| </button> | |
| </TooltipTrigger> | |
| <TooltipContent side="right">Tasks</TooltipContent> | |
| </Tooltip> | |
| )} |
🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx
around lines 109 - 130, The HiOutlineClipboardDocumentList icon is being passed
an unsupported prop strokeWidth={STROKE_WIDTH}; remove that prop or replace the
icon with a Lucide equivalent that supports strokeWidth. Locate the usage of
HiOutlineClipboardDocumentList (and where STROKE_WIDTH is referenced) inside
WorkspaceSidebarHeader and either delete the strokeWidth attribute from the JSX
or swap the import/usage to LuClipboardList and ensure any strokeWidth usage
remains valid; apply the same change to the matching expanded-view usage
referenced in the review.
…ture This PR restores the organization management UI that was temporarily removed during auth system migration and fixes provider architecture issues. Key changes: - Created AuthProvider context to centralize auth state management - Fixed provider architecture by making CollectionsProvider conditional - Flattened auth state structure to avoid session.session stuttering - Added OrganizationDropdown component with org switching - Moved Tasks button to sidebar (feature flag gated) - Added TeamSettings component for member management - Fixed infinite loading spinner blocking sign-in screen Technical details: - onAuthState subscription now spreads session data instead of nesting - ConditionalProviders only renders collections/orgs when authenticated - OrganizationDropdown shows org switcher, settings, and team management - Tasks feature gated with FEATURE_FLAGS.ELECTRIC_TASKS_ACCESS - Excluded generated .mjs files from TypeScript checks Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…mponents - Simplified auth router by removing 5 unnecessary endpoints (-84 lines) - Removed: getState, getAccessToken, onStateChange, onAccessToken, onSessionChange - Kept: onAuthState, setActiveOrganization, signIn, signOut - Updated MainScreen and OrganizationSwitcher to use useAuth() hook - Replaced inline initials logic with shared getInitials() utility (7 instances) - Updated desktop app: OrganizationSwitcher, AccountSettings, OrganizationDropdown, TeamSettings, MemberRow - Updated admin app: UsersTable, LeaderboardTable - Extracted MemberActions component from TeamSettings (-160 lines from parent) - Created separate component file with proper exports - Exported shared MemberDetails type - Fixed TypeScript config to be more explicit - Changed include pattern from "electron.vite.config.*" to "electron.vite.config.ts" - Removed overly broad "**/*.mjs" exclusion - Removed unnecessary comments from multiple files - Auth router, AppProviders, AuthProvider, MainScreen, WorkspaceSidebarHeader Net change: -341 lines of code Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace non-null assertion with explicit null check for better type safety. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
saddlepaddle
force-pushed
the
preserve-org-ui
branch
from
4a02e16 to
0708e05
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @apps/desktop/src/renderer/contexts/CollectionsProvider/collections.ts:
- Around line 27-31: The collectionsCache Map and organizationsCollection
variable currently have no cleanup, which can leak memory and keep Electric
subscriptions alive; add a new exported clearCollections() function that clears
collectionsCache, nulls organizationsCollection, and disposes any Electric
subscriptions held in each OrgCollections entry (look up OrgCollections
structure and its dispose/close/unsubscribe methods) and ensure sign-out flow
calls clearCollections(); optionally implement an LRU eviction wrapper around
collectionsCache to call the same dispose logic when entries are evicted.
- Around line 168-192: The exported function getCollections currently accepts
two positional parameters (organizationId, token); change its signature to
accept a single params object (e.g., { organizationId, token }) and update the
internal references to use params.organizationId and params.token, while
preserving behavior (still check collectionsCache, call createOrgCollections and
getOrCreateOrganizationsCollection, and return the merged collections object).
Also update all call sites (e.g., in CollectionsProvider.tsx) to invoke
getCollections({ organizationId, token }) instead of
getCollections(organizationId, token) so callers match the new object-parameter
API.
- Around line 45-48: Change createOrgCollections to accept a single params
object (e.g., { organizationId, token }) instead of two positional args; update
its signature and any local type definitions (createOrgCollections params type)
and then update all callers of createOrgCollections to pass an object with named
properties (organizationId and token) rather than two separate arguments (ensure
the call site originally invoking createOrgCollections now uses the object
form).
In
@apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsx:
- Around line 32-49: The handleRemove function currently calls
authClient.organization.removeMember directly from the renderer (bypassing IPC);
change it to call the tRPC mutation exposed by the desktop router (or add an
organization/removeMember procedure to the settings or auth router if missing)
instead of authClient.organization.removeMember; invoke the tRPC mutation from
the same handler (preserving the setIsRemoving, toast.success/toast.error,
setShowRemoveDialog and finally block behavior) so the renderer uses
src/lib/trpc IPC, and update imports to use the trpc client hook/mutation for
the desktop router procedure you add.
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx:
- Around line 54-60: The mutations in switchOrganization and handleSignOut lack
error handling; wrap the await setActiveOrg.mutateAsync call in
switchOrganization and the signOut.mutate call in handleSignOut with try/catch,
import and use the same toast utility pattern used in MemberRow.tsx to show
success and error notifications, and ensure errors are logged (or passed to the
toast) so failures surface to the user instead of failing silently; keep
mutation calls inside the try block and show a descriptive toast on catch using
the error message.
🧹 Nitpick comments (8)
apps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsx (2)
21-35: Add error handling for window.open calls.The
window.opencalls can fail silently (e.g., due to popup blockers or browser security settings). Consider checking the return value and logging failures to aid debugging.Based on learnings, errors should never be swallowed silently.
♻️ Proposed refactor with error logging
+const openExternalUrl = (url: string, context: string) => { + const newWindow = window.open(url, "_blank"); + if (!newWindow) { + console.warn(`[SupportMenu/${context}] Failed to open URL: ${url}`); + } +}; + export function SupportMenu() { const handleContactUs = () => { - window.open(COMPANY.MAIL_TO, "_blank"); + openExternalUrl(COMPANY.MAIL_TO, "contact"); }; const handleReportIssue = () => { - window.open(COMPANY.REPORT_ISSUE_URL, "_blank"); + openExternalUrl(COMPANY.REPORT_ISSUE_URL, "report-issue"); }; const handleJoinDiscord = () => { - window.open(COMPANY.DISCORD_URL, "_blank"); + openExternalUrl(COMPANY.DISCORD_URL, "discord"); }; const handleTwitter = () => { - window.open(COMPANY.X_URL, "_blank"); + openExternalUrl(COMPANY.X_URL, "twitter"); };
60-62: Add space between icon and text for consistency.Line 61 is missing a space between the closing
/>and the text "X", which is inconsistent with other menu items.♻️ Formatting fix
<DropdownMenuItem onClick={handleTwitter}> - <FaXTwitter className="h-4 w-4" />X + <FaXTwitter className="h-4 w-4" /> X </DropdownMenuItem>apps/admin/src/app/(dashboard)/components/LeaderboardTable/LeaderboardTable.tsx (2)
95-118: Consider extracting a row component to reduce nesting.The nested structure exceeds 4 levels (map → TableRow → TableCell → div → Avatar → AvatarFallback). Per coding guidelines, extract the row rendering logic into a separate
LeaderboardRowcomponent.♻️ Suggested refactor
Create a new file for the extracted component:
// LeaderboardRow.tsx import { getInitials } from "@superset/shared/names"; import { Avatar, AvatarFallback, AvatarImage } from "@superset/ui/avatar"; import { TableCell, TableRow } from "@superset/ui/table"; interface LeaderboardRowProps { entry: { userId: string; name: string; email: string; image: string | null; count: number; }; index: number; } export function LeaderboardRow({ entry, index }: LeaderboardRowProps) { return ( <TableRow key={entry.userId}> <TableCell className="font-medium">{index + 1}</TableCell> <TableCell> <div className="flex items-center gap-3"> <Avatar className="h-8 w-8"> <AvatarImage src={entry.image ?? undefined} /> <AvatarFallback> {getInitials(entry.name, entry.email)} </AvatarFallback> </Avatar> <div> <p className="font-medium">{entry.name}</p> <p className="text-muted-foreground text-sm"> {entry.email} </p> </div> </div> </TableCell> <TableCell className="text-right font-mono"> {entry.count} </TableCell> </TableRow> ); }Then simplify the table body:
<TableBody> - {data.map((entry, index) => ( - <TableRow key={entry.userId}> - <TableCell className="font-medium">{index + 1}</TableCell> - <TableCell> - <div className="flex items-center gap-3"> - <Avatar className="h-8 w-8"> - <AvatarImage src={entry.image ?? undefined} /> - <AvatarFallback> - {getInitials(entry.name, entry.email)} - </AvatarFallback> - </Avatar> - <div> - <p className="font-medium">{entry.name}</p> - <p className="text-muted-foreground text-sm"> - {entry.email} - </p> - </div> - </div> - </TableCell> - <TableCell className="text-right font-mono"> - {entry.count} - </TableCell> - </TableRow> - ))} + {data.map((entry, index) => ( + <LeaderboardRow key={entry.userId} entry={entry} index={index} /> + ))} </TableBody>
62-62: Optional: Use numeric indices for skeleton keys.The string array
["a", "b", "c", "d", "e"]works but is arbitrary. Consider using numeric indices for clarity.♻️ Suggested refactor
+const SKELETON_ITEM_COUNT = 5; + export function LeaderboardTable({ // ... }: LeaderboardTableProps) { return ( <Card> {/* ... */} {isLoading ? ( <div className="space-y-3"> - {["a", "b", "c", "d", "e"].map((id) => ( - <div key={id} className="flex items-center gap-3"> + {Array.from({ length: SKELETON_ITEM_COUNT }, (_, i) => ( + <div key={i} className="flex items-center gap-3">apps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsx (1)
27-41: Consider extractingConditionalProvidersto its own file.Per coding guidelines, prefer one component per file. While
ConditionalProvidersis a small internal helper, extracting it toConditionalProviders.tsxwould align with the project conventions.apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx (2)
109-130: Minor: Simplify onClick handler.The arrow function wrapper is unnecessary since
openTaskstakes no arguments.♻️ Suggested simplification
<button type="button" - onClick={() => openTasks()} + onClick={openTasks} className={cn(
179-198: Same simplification applies to the expanded layout.♻️ Suggested simplification
{hasTasksAccess && ( <button type="button" - onClick={() => openTasks()} + onClick={openTasks} className={cn(apps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsx (1)
24-31: Consider adding context to the loading state.The loading spinner shows for both "waiting for auth state" and "no active organization" scenarios. If this becomes a UX concern, consider differentiating these states. However, since
CollectionsProvideris only rendered when authenticated (perConditionalProviders), this should primarily indicate initial data loading.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
apps/admin/src/app/(dashboard)/components/LeaderboardTable/LeaderboardTable.tsxapps/admin/src/app/(dashboard)/users/components/UsersTable/UsersTable.tsxapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsxapps/desktop/src/renderer/contexts/AuthProvider/index.tsapps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/collections.tsapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/AccountSettings/AccountSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar/GeneralSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/InviteMemberButton.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/index.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/stores/app-state.tsapps/desktop/tsconfig.json
💤 Files with no reviewable changes (4)
- apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/index.ts
- apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx
- apps/desktop/src/renderer/screens/main/components/AvatarDropdown/index.ts
- apps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsx
🚧 Files skipped from review as they are similar to previous changes (15)
- apps/desktop/src/renderer/contexts/AuthProvider/index.ts
- apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/index.ts
- apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/index.ts
- apps/admin/src/app/(dashboard)/users/components/UsersTable/UsersTable.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/TeamSettings.tsx
- apps/desktop/src/lib/trpc/routers/auth/index.ts
- apps/desktop/src/renderer/screens/main/index.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/AccountSettings/AccountSettings.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/InviteMemberButton.tsx
- apps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/index.ts
- apps/desktop/src/renderer/screens/main/components/TopBar/index.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar/GeneralSettings.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/InviteMemberButton/index.ts
🧰 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/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/collections.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.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/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/collections.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/stores/app-state.tsapps/admin/src/app/(dashboard)/components/LeaderboardTable/LeaderboardTable.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.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/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/collections.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.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/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/admin/src/app/(dashboard)/components/LeaderboardTable/LeaderboardTable.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.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/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/collections.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/stores/app-state.tsapps/admin/src/app/(dashboard)/components/LeaderboardTable/LeaderboardTable.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.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/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsxapps/desktop/src/renderer/contexts/CollectionsProvider/collections.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/stores/app-state.tsapps/admin/src/app/(dashboard)/components/LeaderboardTable/LeaderboardTable.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsx
🧠 Learnings (18)
📚 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/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsxapps/desktop/tsconfig.json
📚 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/SettingsView/TeamSettings/components/MemberRow/MemberRow.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/api/**/*.{ts,tsx} : Use TRPCError with appropriate error codes: NOT_FOUND, UNAUTHORIZED, FORBIDDEN, BAD_REQUEST, INTERNAL_SERVER_ERROR, NOT_IMPLEMENTED
Applied to files:
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.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/api/**/*.{ts,tsx} : tRPC procedures and API route handlers should validate and delegate; keep orchestrators thin
Applied to files:
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.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/api/**/*.{ts,tsx} : Extract business logic from tRPC procedures into utility functions when logic exceeds ~50 lines, is used by multiple procedures, or needs independent testing
Applied to files:
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.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/contexts/AppProviders/AppProviders.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/**/*.{ts,tsx} : Co-locate component dependencies: utils, hooks, constants, config, tests, stories in the same folder as the component using them
Applied to files:
apps/desktop/src/renderer/contexts/AppProviders/AppProviders.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/contexts/AppProviders/AppProviders.tsxapps/desktop/tsconfig.json
📚 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/contexts/AppProviders/AppProviders.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 **/*.tsx : One component per file - do not create multi-component files
Applied to files:
apps/desktop/src/renderer/contexts/AppProviders/AppProviders.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/contexts/AppProviders/AppProviders.tsxapps/desktop/tsconfig.json
📚 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} : Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Applied to files:
apps/desktop/src/renderer/contexts/AppProviders/AppProviders.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/{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/contexts/AppProviders/AppProviders.tsxapps/desktop/tsconfig.json
📚 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/contexts/AppProviders/AppProviders.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/contexts/AppProviders/AppProviders.tsxapps/desktop/tsconfig.json
📚 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/main/index.ts : Load environment variables from monorepo root .env in desktop app with override: true before any imports in src/main/index.ts and electron.vite.config.ts
Applied to files:
apps/desktop/tsconfig.json
📚 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/tsconfig.json
📚 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} : Never swallow errors silently - at minimum log them with context
Applied to files:
apps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsx
🧬 Code graph analysis (6)
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsx (7)
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsx (1)
MemberDetails(21-30)packages/auth/src/client.ts (1)
authClient(8-11)packages/ui/src/components/ui/sonner.tsx (1)
toast(40-40)packages/ui/src/components/ui/avatar.tsx (3)
Avatar(53-53)AvatarImage(53-53)AvatarFallback(53-53)packages/ui/src/components/ui/badge.tsx (1)
Badge(46-46)packages/ui/src/components/ui/button.tsx (1)
Button(60-60)packages/ui/src/components/ui/dialog.tsx (6)
Dialog(134-134)DialogContent(136-136)DialogHeader(139-139)DialogTitle(142-142)DialogDescription(137-137)DialogFooter(138-138)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (4)
apps/desktop/src/renderer/contexts/AuthProvider/AuthProvider.tsx (1)
useAuth(26-32)apps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsx (1)
useCollections(40-46)apps/desktop/src/main/lib/auth/auth.ts (1)
signOut(219-230)apps/desktop/src/renderer/stores/app-state.ts (1)
useOpenSettings(94-94)
apps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsx (2)
apps/desktop/src/renderer/contexts/CollectionsProvider/collections.ts (1)
getCollections(168-192)apps/api/src/trpc/context.ts (1)
createContext(4-14)
apps/desktop/src/renderer/contexts/CollectionsProvider/collections.ts (3)
packages/db/src/schema/schema.ts (1)
SelectRepository(56-56)packages/db/src/schema/auth.ts (1)
SelectMember(124-124)apps/desktop/src/main/lib/api-client.ts (1)
apiClient(11-42)
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (1)
packages/ui/src/components/ui/scroll-area.tsx (1)
ScrollArea(58-58)
apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsx (5)
packages/auth/src/client.ts (1)
authClient(8-11)packages/ui/src/components/ui/sonner.tsx (1)
toast(40-40)packages/ui/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(249-249)DropdownMenuTrigger(251-251)DropdownMenuContent(252-252)DropdownMenuItem(255-255)packages/ui/src/components/ui/button.tsx (1)
Button(60-60)packages/ui/src/components/ui/dialog.tsx (6)
Dialog(134-134)DialogContent(136-136)DialogHeader(139-139)DialogTitle(142-142)DialogDescription(137-137)DialogFooter(138-138)
⏰ 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). (6)
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Docs
- GitHub Check: Deploy API
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy Web
- GitHub Check: Build
🔇 Additional comments (14)
apps/desktop/tsconfig.json (1)
11-16: LGTM! Correctly excludes generated files from type checking.Narrowing from
electron.vite.config.*toelectron.vite.config.tsensures TypeScript only checks the source file and excludes generated.mjsor.jsbuild outputs, which aligns with the PR objective to fix TypeScript errors.apps/admin/src/app/(dashboard)/components/LeaderboardTable/LeaderboardTable.tsx (1)
3-3: Refactoring is solid —getInitialsrobustly handles edge cases.The function properly handles null/undefined values, whitespace normalization, and email fallback. It will work reliably with the required name/email fields in
LeaderboardEntry.apps/desktop/src/renderer/stores/app-state.ts (1)
5-14: LGTM!The addition of
"team"to theSettingsSectionunion type correctly enables the new team settings functionality. The type change is properly integrated with the existing settings section pattern.apps/desktop/src/renderer/contexts/AppProviders/AppProviders.tsx (1)
14-25: LGTM! Auth-gated provider composition resolves the infinite loading spinner.The architecture correctly ensures that
CollectionsProviderandOrganizationsProvideronly mount when authenticated, allowing unauthenticated screens (sign-in) to render without blocking.apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsx (2)
55-95: LGTM! Clean member row UI with proper accessibility.The component correctly handles edge cases (missing name, initials), uses semantic button elements, and provides clear visual feedback with badges for the current user and role indicators.
97-125: LGTM! Confirmation dialog follows best practices.The removal confirmation dialog properly:
- Disables buttons during the async operation
- Shows loading state in the destructive action button
- Provides clear messaging about the impact of removal
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx (1)
37-42: LGTM! Feature flag gating is properly implemented.The Tasks UI is correctly gated behind
FEATURE_FLAGS.ELECTRIC_TASKS_ACCESS, ensuring the feature is only accessible to enabled users.apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (1)
306-313: LGTM! Simplified TasksView aligns with the new provider architecture.The removal of provider wrappers is correct since
CollectionsProviderandOrganizationsProviderare now handled at theAppProviderslevel viaConditionalProviders. This makesTasksViewa pure presentation component.apps/desktop/src/renderer/contexts/CollectionsProvider/CollectionsProvider.tsx (1)
9-22: LGTM! Clean consolidation to auth-state-driven collections.The switch from separate
onSessionChange/onAccessTokensubscriptions to a singleonAuthStatesubscription simplifies the data flow. TheuseMemocorrectly recreates collections when auth state changes, andgetCollectionshandles per-organization caching internally.apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (2)
97-163: LGTM! Robust dropdown with defensive rendering.The dropdown correctly:
- Always renders sign-out to prevent trapping users
- Conditionally shows org-specific items only when an active organization exists
- Shows org switcher only when multiple organizations are available
- Provides clear visual indication of the active organization with checkmark
39-39: No issue - the session structure is correct.The code properly accesses
session?.session?.activeOrganizationIdbecause the Better Auth response object has this nested structure:{ session: { activeOrganizationId: ... }, user: { ... } }. This pattern is consistent withCollectionsProvider, which also accessesauthState?.session?.activeOrganizationIddirectly from the subscription. The PR description's mention of "flattening" is either aspirational or applies to a different aspect of the change.apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberActions/MemberActions.tsx (2)
38-149: Well-structured component with proper error handling and loading states.The component correctly:
- Differentiates between the current user leaving vs removing another member
- Provides clear confirmation dialogs with contextual messaging
- Tracks loading state to prevent duplicate operations
- Handles errors with user-friendly toast notifications
- Disables actions during operations
21-30: The code correctly usesuserIdfor theremoveMemberAPI call. The two identifiers serve distinct purposes:memberIdis the membership record ID (used for React keys), whileuserIdis the user account ID (used for user-related API operations). The removeMember API explicitly expectsuserIdas its parameter, making the current implementation semantically correct with no ambiguity.Likely an incorrect or invalid review comment.
apps/desktop/src/renderer/contexts/CollectionsProvider/collections.ts (1)
171-174: Good use of prefixed console logging pattern.The logging follows the coding guideline to "use prefixed console logging with context pattern: [domain/operation] message". This makes debugging easier by clearly identifying the source of log messages.
As per coding guidelines.
| // Per-org collections cache | ||
| const collectionsCache = new Map<string, OrgCollections>(); | ||
|
|
||
| // Shared organizations collection (same for all orgs) | ||
| let organizationsCollection: Collection<SelectOrganization> | null = null; |
There was a problem hiding this comment.
Consider adding cache cleanup to prevent memory leaks in long-running sessions.
The collectionsCache and organizationsCollection grow unbounded without any cleanup mechanism. In a long-running Electron app, this could lead to:
- Memory accumulation: Each organization creates Electric shape subscriptions that hold resources
- Stale data after sign-out: No mechanism to clear collections when user signs out
- Connection leaks: Electric subscriptions may remain active even for unused organizations
Consider adding:
- A
clearCollections()export to call on sign-out - An LRU eviction policy if cache size becomes an issue
- A way to dispose/cleanup Electric subscriptions for evicted entries
💡 Suggested cleanup function
/**
* Clear all cached collections (call on sign-out)
*/
export function clearCollections() {
console.log('[collections] Clearing all cached collections');
collectionsCache.clear();
organizationsCollection = null;
}Then call this from the auth sign-out flow to prevent leaks.
🤖 Prompt for AI Agents
In @apps/desktop/src/renderer/contexts/CollectionsProvider/collections.ts around
lines 27 - 31, The collectionsCache Map and organizationsCollection variable
currently have no cleanup, which can leak memory and keep Electric subscriptions
alive; add a new exported clearCollections() function that clears
collectionsCache, nulls organizationsCollection, and disposes any Electric
subscriptions held in each OrgCollections entry (look up OrgCollections
structure and its dispose/close/unsubscribe methods) and ensure sign-out flow
calls clearCollections(); optionally implement an LRU eviction wrapper around
collectionsCache to call the same dispose logic when entries are evicted.
| function createOrgCollections( | ||
| organizationId: string, | ||
| token: string, | ||
| ): OrgCollections { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use object parameters for functions with 2+ parameters.
Per coding guidelines, functions with 2 or more parameters should accept a single params object with named properties for self-documentation and extensibility.
♻️ Refactor to use object parameters
-function createOrgCollections(
- organizationId: string,
- token: string,
-): OrgCollections {
+function createOrgCollections({
+ organizationId,
+ token,
+}: {
+ organizationId: string;
+ token: string;
+}): OrgCollections {Update the call site on line 177:
collectionsCache.set(
organizationId,
- createOrgCollections(organizationId, token),
+ createOrgCollections({ organizationId, token }),
);As per coding guidelines: "Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility."
🤖 Prompt for AI Agents
In @apps/desktop/src/renderer/contexts/CollectionsProvider/collections.ts around
lines 45 - 48, Change createOrgCollections to accept a single params object
(e.g., { organizationId, token }) instead of two positional args; update its
signature and any local type definitions (createOrgCollections params type) and
then update all callers of createOrgCollections to pass an object with named
properties (organizationId and token) rather than two separate arguments (ensure
the call site originally invoking createOrgCollections now uses the object
form).
| export function getCollections(organizationId: string, token: string) { | ||
| // Get or create org-specific collections | ||
| if (!collectionsCache.has(organizationId)) { | ||
| console.log( | ||
| "[collections] Creating new collections for org:", | ||
| organizationId, | ||
| ); | ||
| collectionsCache.set( | ||
| organizationId, | ||
| createOrgCollections(organizationId, token), | ||
| ); | ||
| } | ||
|
|
||
| const orgCollections = collectionsCache.get(organizationId); | ||
| if (!orgCollections) { | ||
| throw new Error(`Collections not found for org: ${organizationId}`); | ||
| } | ||
|
|
||
| const organizations = getOrCreateOrganizationsCollection(token); | ||
|
|
||
| return { tasks, repositories, members, users, organizations }; | ||
| return { | ||
| ...orgCollections, | ||
| organizations, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use object parameters for the public API function.
The exported getCollections function has 2 parameters and should follow the coding guideline to use object parameters for better self-documentation and extensibility.
♻️ Refactor to use object parameters
-export function getCollections(organizationId: string, token: string) {
+export function getCollections({
+ organizationId,
+ token,
+}: {
+ organizationId: string;
+ token: string;
+}) {Update all call sites (likely in CollectionsProvider.tsx) to use the object syntax:
// Before:
getCollections(organizationId, token)
// After:
getCollections({ organizationId, token })As per coding guidelines: "Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility."
🤖 Prompt for AI Agents
In @apps/desktop/src/renderer/contexts/CollectionsProvider/collections.ts around
lines 168 - 192, The exported function getCollections currently accepts two
positional parameters (organizationId, token); change its signature to accept a
single params object (e.g., { organizationId, token }) and update the internal
references to use params.organizationId and params.token, while preserving
behavior (still check collectionsCache, call createOrgCollections and
getOrCreateOrganizationsCollection, and return the merged collections object).
Also update all call sites (e.g., in CollectionsProvider.tsx) to invoke
getCollections({ organizationId, token }) instead of
getCollections(organizationId, token) so callers match the new object-parameter
API.
| const handleRemove = async () => { | ||
| setIsRemoving(true); | ||
| try { | ||
| await authClient.organization.removeMember({ | ||
| organizationId: member.organizationId, | ||
| memberIdOrEmail: member.userId, | ||
| }); | ||
| toast.success("Member removed"); | ||
| setShowRemoveDialog(false); | ||
| // Electric collections will automatically update via real-time sync | ||
| } catch (error) { | ||
| toast.error( | ||
| error instanceof Error ? error.message : "Failed to remove member", | ||
| ); | ||
| } finally { | ||
| setIsRemoving(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for tRPC procedures related to removing members
rg -n "removeMember" --type ts -C 3 | head -50Repository: superset-sh/superset
Length of output: 2648
🏁 Script executed:
#!/bin/bash
# Check how authClient is imported and used in renderer files
rg -n "authClient\." apps/desktop/src/renderer --type ts -C 2 | head -100Repository: superset-sh/superset
Length of output: 2322
🏁 Script executed:
#!/bin/bash
# Check if authClient is using Node.js modules (which would be invalid in renderer)
find . -name "*authClient*" -o -name "*auth*client*" | grep -E "\.(ts|tsx)$" | head -10Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check the src/lib/trpc structure for organization/member procedures
find . -path "*/src/lib/trpc*" -type f -name "*.ts" | head -20Repository: superset-sh/superset
Length of output: 1253
🏁 Script executed:
#!/bin/bash
# Check the imports in MemberRow.tsx to see where authClient comes from
head -30 apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsx | grep -E "^import|^from"Repository: superset-sh/superset
Length of output: 477
🏁 Script executed:
#!/bin/bash
# Search for authClient definition/export
rg -n "export.*authClient|const authClient" --type ts | head -20Repository: superset-sh/superset
Length of output: 138
🏁 Script executed:
#!/bin/bash
# Check the desktop tRPC auth router
cat apps/desktop/src/lib/trpc/routers/auth/index.tsRepository: superset-sh/superset
Length of output: 1785
🏁 Script executed:
#!/antml/bash
# Check what's in the main packages/trpc organization router
head -150 packages/trpc/src/router/organization/organization.ts | tail -50Repository: superset-sh/superset
Length of output: 1127
🏁 Script executed:
#!/bin/bash
# Check authClient implementation and dependencies
cat packages/auth/src/client.ts | head -50Repository: superset-sh/superset
Length of output: 457
🏁 Script executed:
#!/an/bin/bash
# Search for Node.js module imports in packages/auth that authClient might use
rg "import.*from.*['\"](fs|path|os|net|child_process)" packages/auth --type tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check what routers exist in desktop tRPC
ls -la apps/desktop/src/lib/trpc/routers/Repository: superset-sh/superset
Length of output: 1353
🏁 Script executed:
#!/bin/bash
# Check if organization router exists in desktop tRPC
find apps/desktop/src/lib/trpc -name "*organization*" -o -name "*org*"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check the main tRPC index to see all available routers
cat apps/desktop/src/lib/trpc/index.tsRepository: superset-sh/superset
Length of output: 1292
🏁 Script executed:
#!/bin/bash
# Check the routers index to see what routers are merged
cat apps/desktop/src/lib/trpc/routers/index.tsRepository: superset-sh/superset
Length of output: 1924
🏁 Script executed:
#!/bin/bash
# Check if there's an organization router anywhere in desktop
find apps/desktop -name "*organization*" -o -name "*organization.ts"Repository: superset-sh/superset
Length of output: 46
Use tRPC for organization member operations instead of direct authClient calls.
The coding guideline requires: "For Electron interprocess communication, ALWAYS use tRPC as defined in src/lib/trpc". The current authClient.organization.removeMember call bypasses the IPC layer by making direct HTTP requests from the renderer process.
Replace with a tRPC mutation through the appropriate desktop router (consider adding organization procedures to the settings or auth router if not already present).
🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsx
around lines 32 - 49, The handleRemove function currently calls
authClient.organization.removeMember directly from the renderer (bypassing IPC);
change it to call the tRPC mutation exposed by the desktop router (or add an
organization/removeMember procedure to the settings or auth router if missing)
instead of authClient.organization.removeMember; invoke the tRPC mutation from
the same handler (preserving the setIsRemoving, toast.success/toast.error,
setShowRemoveDialog and finally block behavior) so the renderer uses
src/lib/trpc IPC, and update imports to use the trpc client hook/mutation for
the desktop router procedure you add.
| const switchOrganization = async (newOrgId: string) => { | ||
| await setActiveOrg.mutateAsync({ organizationId: newOrgId }); | ||
| }; | ||
|
|
||
| const handleSignOut = () => { | ||
| signOut.mutate(); | ||
| }; |
There was a problem hiding this comment.
Add error handling to organization switch and sign-out mutations.
Both switchOrganization and handleSignOut call mutations without error handling. Failed operations will silently fail without user feedback. Consider adding try/catch with toast notifications consistent with the pattern in MemberRow.tsx.
🛠️ Suggested error handling
const switchOrganization = async (newOrgId: string) => {
- await setActiveOrg.mutateAsync({ organizationId: newOrgId });
+ try {
+ await setActiveOrg.mutateAsync({ organizationId: newOrgId });
+ } catch (error) {
+ toast.error(
+ error instanceof Error ? error.message : "Failed to switch organization",
+ );
+ }
};
const handleSignOut = () => {
- signOut.mutate();
+ signOut.mutate(undefined, {
+ onError: (error) => {
+ toast.error(
+ error instanceof Error ? error.message : "Failed to sign out",
+ );
+ },
+ });
};You'll also need to add the toast import:
+import { toast } from "@superset/ui/sonner";🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx
around lines 54 - 60, The mutations in switchOrganization and handleSignOut lack
error handling; wrap the await setActiveOrg.mutateAsync call in
switchOrganization and the signOut.mutate call in handleSignOut with try/catch,
import and use the same toast utility pattern used in MemberRow.tsx to show
success and error notifications, and ensure errors are logged (or passed to the
toast) so failures surface to the user instead of failing silently; keep
mutation calls inside the try block and show a descriptive toast on catch using
the error message.
1 participant
Summary
Restores organization management UI that was temporarily removed during auth system migration and fixes critical provider architecture issues that caused infinite loading.
Changes
Core Architecture
session.sessionto spreading session data inonAuthStatesubscriptionUI Components
FEATURE_FLAGS.ELECTRIC_TASKS_ACCESSBug Fixes
.mjsfiles from checksTechnical Details
Auth State Structure
Provider Architecture
Testing
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.