Router refactor 2: Settings page cleanup#732
Conversation
📝 WalkthroughWalkthroughThis PR migrates the desktop app's settings architecture from a modal state-based system to a file-based routing approach using TanStack Router. It removes legacy settings state management, creates new route-based settings pages under Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts:
- Around line 35-74: The duplicated baseBranch detection/persistence logic
should be extracted into a shared helper (e.g., detectAndPersistBaseBranch) and
called from both locations (the current block in query.ts and the getActive
function) to avoid duplication; implement a helper (in ../utils/git.ts or a new
helper file) that accepts the worktree and project objects, returns
worktree.baseBranch if already defined, otherwise runs
hasOriginRemote(project.mainRepoPath), calls detectBaseBranch(worktree.path,
worktree.branch, project.defaultBranch || "main") when remote exists, converts
undefined detection to null, persists the result via
localDb.update(worktrees).set({ baseBranch }).where(eq(worktrees.id,
worktree.id)).run(), and returns the final baseBranch (string | null |
undefined), then replace the duplicated blocks with calls to
detectAndPersistBaseBranch and use its return value.
In @apps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsx:
- Around line 14-18: The sign-out mutation only handles success; update the
trpc.auth.signOut.useMutation call (signOutMutation) to include an onError
handler that surfaces failures (e.g., toast.error with the error message or a
generic "Sign out failed") and optionally logs the error, and ensure the signOut
wrapper (signOut function) still calls signOutMutation.mutate() so failures
trigger the new onError path; reference trpc.auth.signOut.useMutation,
signOutMutation, and signOut when applying the change.
In
@apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/ProjectsSettings.tsx:
- Around line 14-19: The effect watching groups is re-running on every query
refetch because groups is a new array reference; change it to only perform the
initial expansion once by adding a ref flag (e.g., initialExpansionDoneRef)
checked inside the same useEffect so that when groups.length > 0 you call
setExpandedProjects(new Set(groups.map(g => g.project.id))) only if the ref is
false, then set the ref to true; keep the dependency on groups but guard with
the ref to prevent resetting expandedProjects on subsequent refetches.
In
@apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/page.tsx:
- Around line 13-22: The component ProjectSettingsPage currently only
destructures data and isLoading from trpc.projects.get.useQuery (and similarly
for trpc.config.getConfigFilePath.useQuery) which swallows query errors and
causes "Project not found" to show on failures; update both hook usages to also
destructure error (e.g., { data: project, isLoading, error }) and handle error
cases explicitly by rendering a clear error UI/message (using error.message) or
returning early, and avoid treating undefined project as "not found" when an
error exists; also optionally log the error for debugging.
🧹 Nitpick comments (15)
apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/page.tsx (3)
25-151: Consider extractingRingtoneCardto a separate file.Per coding guidelines, each file should contain only one component.
RingtoneCardcould be moved to a separate file likeringtone-card.tsxin a shared components directory or a local_componentsfolder.
193-194: Console logging should follow the prefixed context pattern.The guideline specifies using
[domain/operation] messageformat for console logging.Suggested change
if (playingId === ringtone.id) { try { await trpcClient.ringtone.stop.mutate(); } catch (error) { - console.error("Failed to stop ringtone:", error); + console.error("[ringtone/stop] Failed to stop ringtone:", error); } setPlayingId(null); return; } // Stop any currently playing sound first try { await trpcClient.ringtone.stop.mutate(); } catch (error) { - console.error("Failed to stop ringtone:", error); + console.error("[ringtone/stop] Failed to stop ringtone:", error); } // Play the new sound setPlayingId(ringtone.id); try { await trpcClient.ringtone.preview.mutate({ filename: ringtone.filename, }); } catch (error) { - console.error("Failed to play ringtone:", error); + console.error("[ringtone/preview] Failed to play ringtone:", error); setPlayingId(null); }Also applies to: 203-204, 214-215
219-223: Extract magic numbers to named constants.The buffer time and default duration values should be named constants for clarity.
Suggested change
Add at module top:
const PREVIEW_BUFFER_SECONDS = 0.5; const DEFAULT_RINGTONE_DURATION_SECONDS = 5; const MS_PER_SECOND = 1000;Then update line 219:
- const durationMs = ((ringtone.duration ?? 5) + 0.5) * 1000; + const durationMs = ((ringtone.duration ?? DEFAULT_RINGTONE_DURATION_SECONDS) + PREVIEW_BUFFER_SECONDS) * MS_PER_SECOND;apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx (3)
42-89: ExtractHotkeyRowto its own file.Per coding guidelines, each file should contain only one component. Consider moving
HotkeyRowto a separate file likehotkey-row.tsxin a sibling directory or a shared components location.Additionally, using the key value as the React
keyprop (line 78) may cause collisions if the display array ever contains duplicate strings.Suggested fix for the key prop
<KbdGroup> - {display.map((key) => ( - <Kbd key={key}>{key}</Kbd> + {display.map((key, index) => ( + <Kbd key={index}>{key}</Kbd> ))} </KbdGroup>
258-265: Same React key collision risk.Similar to line 78, using the key value directly as the React
keyprop could cause issues if the display contains duplicates.Suggested fix
<KbdGroup> - {showHotkeysDisplay.map((key) => ( - <Kbd key={key}>{key}</Kbd> + {showHotkeysDisplay.map((key, index) => ( + <Kbd key={index}>{key}</Kbd> ))} </KbdGroup>
310-310: Extract magic number to a named constant.The
320pxvalue used in the max-height calculation should be extracted to a named constant at the module top for clarity and maintainability. As per coding guidelines, hardcoded values should be named constants.Suggested refactor
Add at module top (after line 40):
const HEADER_OFFSET_PX = 320;Then update line 310:
-<div className="max-h-[calc(100vh-320px)] overflow-y-auto divide-y divide-border"> +<div className={`max-h-[calc(100vh-${HEADER_OFFSET_PX}px)] overflow-y-auto divide-y divide-border`}>apps/desktop/src/renderer/routes/_authenticated/settings/presets/components/PresetRow/PresetRow.tsx (1)
24-56: Consider extractingPresetCellto its own file.Per coding guidelines, prefer one component per file. While
PresetCellis a private helper tightly coupled toPresetRow, extracting it to a separate file (e.g.,PresetCell.tsx) would improve adherence to the guideline and make the component easier to test independently.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx (1)
54-56: Consider extracting the hover delay to a named constant.The
150ms timeout value is used in both hover handlers. Per coding guidelines, extract magic numbers to named constants at module top for better readability and maintainability.Suggested change
+const HOVER_DELAY_MS = 150; + export function GroupStrip() { // ... const handleDropdownMouseEnter = useCallback(() => { if (hoverTimeoutRef.current) { clearTimeout(hoverTimeoutRef.current); } hoverTimeoutRef.current = setTimeout(() => { setDropdownOpen(true); - }, 150); + }, HOVER_DELAY_MS); }, []); const handleDropdownMouseLeave = useCallback(() => { if (hoverTimeoutRef.current) { clearTimeout(hoverTimeoutRef.current); } hoverTimeoutRef.current = setTimeout(() => { setDropdownOpen(false); - }, 150); + }, HOVER_DELAY_MS); }, []);Also applies to: 63-65
apps/desktop/src/renderer/screens/main/index.tsx (1)
104-110: Type assertion may mask invalid route navigation.The cast
as "/settings/account"silences TypeScript but allows anysectionvalue through. Ifevent.data.sectioncontains an invalid route name, navigation will fail silently at runtime.Consider defining a union type for valid sections and validating before navigation:
♻️ Suggested improvement
+const VALID_SETTINGS_SECTIONS = [ + "account", "team", "appearance", "ringtones", + "keyboard", "presets", "behavior" +] as const; +type SettingsSection = (typeof VALID_SETTINGS_SECTIONS)[number]; trpc.menu.subscribe.useSubscription(undefined, { onData: (event) => { if (event.type === "open-settings") { const section = event.data.section || "account"; - navigate({ to: `/settings/${section}` as "/settings/account" }); + if (VALID_SETTINGS_SECTIONS.includes(section as SettingsSection)) { + navigate({ to: `/settings/${section}` as `/settings/${SettingsSection}` }); + } else { + navigate({ to: "/settings/account" }); + } } }, });apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts (1)
76-95: Consider extracting the enriched workspace response builder.This return structure is duplicated in
getActive(Lines 273-292). A shared helper could reduce duplication and ensure consistent response shapes across both procedures.apps/desktop/src/renderer/routes/_authenticated/settings/presets/page.tsx (2)
91-97: Index-based sync may cause stale data issues.Using array indices to map between
localPresetsandserverPresets(e.g., inhandleCellBlur,handleCommandsBlur) assumes both arrays maintain identical ordering. If the server reorders presets or concurrent mutations occur, the indices could mismatch, causing updates to the wrong preset.Consider keying comparisons by
preset.idinstead of array index:♻️ Suggested approach
const handleCellBlur = (rowIndex: number, column: PresetColumnKey) => { const preset = localPresets[rowIndex]; - const serverPreset = serverPresets[rowIndex]; + const serverPreset = serverPresets.find((p) => p.id === preset?.id); if (!preset || !serverPreset) return; if (preset[column] === serverPreset[column]) return; // ... };Apply the same pattern to
handleCommandsBlur.
150-156: Consider validating or auto-focusing new preset name.Creating a preset with an empty name may lead to confusing UX if the user doesn't immediately rename it. Consider either auto-focusing the name input after creation or generating a default name like "Untitled Preset".
apps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsx (1)
62-64: Minor: Unnecessary arrow function wrapper.The
onClick={() => signOut()}can be simplified toonClick={signOut}sincesignOuttakes no arguments.♻️ Suggested simplification
-<Button variant="outline" onClick={() => signOut()}> +<Button variant="outline" onClick={signOut}> Sign Out </Button>apps/desktop/src/renderer/routes/_authenticated/settings/appearance/page.tsx (1)
63-74: Consider deriving SelectItem values from the MarkdownStyle type to prevent drift.The SelectItem values ("default", "tufte") currently match the
MarkdownStyletype definition, but hardcoding them in two places creates maintenance risk. To ensure future changes to the type are reflected in the UI, consider deriving the items from a shared constant.♻️ Recommended approach
const MARKDOWN_STYLES = ["default", "tufte"] as const; type MarkdownStyle = (typeof MARKDOWN_STYLES)[number]; // Then in JSX: {MARKDOWN_STYLES.map((style) => ( <SelectItem key={style} value={style}> {style === "default" ? "Default" : "Tufte"} </SelectItem> ))}apps/desktop/src/renderer/routes/_authenticated/settings/behavior/page.tsx (1)
70-74: Consider deriving SelectItem values from the type definition for maintainability.The type assertion
value as TerminalLinkBehavioris currently safe because the SelectItem values ("external-editor", "file-viewer") match theTERMINAL_LINK_BEHAVIORSconstant exactly, and the tRPC endpoint validates input usingz.enum(TERMINAL_LINK_BEHAVIORS). However, to prevent accidental drift between the UI options and type definition in the future, extract the SelectItem values directly from theTERMINAL_LINK_BEHAVIORSconstant.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (54)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/appearance/components/ThemeCard/ThemeCard.tsxapps/desktop/src/renderer/routes/_authenticated/settings/appearance/components/ThemeCard/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/appearance/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/behavior/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/GeneralSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/ProjectsSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/components/CommandsEditor/CommandsEditor.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/components/CommandsEditor/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/presets/components/PresetRow/PresetRow.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/components/PresetRow/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/presets/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/types.tsapps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/ringtones/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/team/components/InviteMemberButton/InviteMemberButton.tsxapps/desktop/src/renderer/routes/_authenticated/settings/team/components/InviteMemberButton/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/team/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/routes/_authenticated/settings/team/components/MemberActions/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/team/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/workspace/page.tsxapps/desktop/src/renderer/routes/sign-in/page.tsxapps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/AccountSettings/AccountSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/AccountSettings/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/BehaviorSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/KeyboardShortcutsSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/PresetsSettings/PresetsSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/PresetsSettings/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/ProjectSettings/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/RingtonesSettings/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar/ProjectsSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar/SettingsSidebar.tsxapps/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/SettingsView/WorkspaceSettings/index.tsapps/desktop/src/renderer/screens/main/components/SettingsView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/stores/app-state.ts
💤 Files with no reviewable changes (20)
- apps/desktop/src/renderer/screens/main/components/SettingsView/WorkspaceSettings/index.ts
- apps/desktop/src/renderer/screens/main/components/SettingsView/PresetsSettings/index.ts
- apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar/SettingsSidebar.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/BehaviorSettings.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/PresetsSettings/PresetsSettings.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar/ProjectsSettings.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/AccountSettings/AccountSettings.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/MemberRow.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/ProjectSettings/index.ts
- apps/desktop/src/renderer/screens/main/components/SettingsView/KeyboardShortcutsSettings.tsx
- apps/desktop/src/renderer/routes/_authenticated/settings/page.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/index.ts
- apps/desktop/src/renderer/screens/main/components/SettingsView/index.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/AccountSettings/index.ts
- apps/desktop/src/renderer/routes/_authenticated/settings/workspace/page.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/RingtonesSettings/index.ts
- apps/desktop/src/renderer/routes/sign-in/page.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsx
- apps/desktop/src/renderer/screens/main/components/SettingsView/TeamSettings/components/MemberRow/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/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/appearance/components/ThemeCard/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/ProjectsSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/GeneralSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/routes/_authenticated/settings/appearance/page.tsxapps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/renderer/routes/_authenticated/settings/ringtones/page.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/routes/_authenticated/settings/workspace/$workspaceId/page.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/settings/behavior/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/team/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/components/PresetRow/PresetRow.tsxapps/desktop/src/renderer/stores/app-state.ts
**/*.{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/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/appearance/components/ThemeCard/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/ProjectsSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/GeneralSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/routes/_authenticated/settings/appearance/page.tsxapps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/renderer/routes/_authenticated/settings/ringtones/page.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/routes/_authenticated/settings/workspace/$workspaceId/page.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/settings/behavior/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/team/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/components/PresetRow/PresetRow.tsxapps/desktop/src/renderer/stores/app-state.ts
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/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/appearance/components/ThemeCard/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/ProjectsSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/GeneralSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/routes/_authenticated/settings/appearance/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/ringtones/page.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/routes/_authenticated/settings/workspace/$workspaceId/page.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/settings/behavior/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/team/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/components/PresetRow/PresetRow.tsxapps/desktop/src/renderer/stores/app-state.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/ProjectsSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/GeneralSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/routes/_authenticated/settings/appearance/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/ringtones/page.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/routes/_authenticated/settings/workspace/$workspaceId/page.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/settings/behavior/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/team/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/components/PresetRow/PresetRow.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/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/appearance/components/ThemeCard/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/ProjectsSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/GeneralSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/routes/_authenticated/settings/appearance/page.tsxapps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/renderer/routes/_authenticated/settings/ringtones/page.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/routes/_authenticated/settings/workspace/$workspaceId/page.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/settings/behavior/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/team/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/components/PresetRow/PresetRow.tsxapps/desktop/src/renderer/stores/app-state.ts
**/*.{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/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/appearance/components/ThemeCard/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/ProjectsSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/GeneralSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/routes/_authenticated/settings/appearance/page.tsxapps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/renderer/routes/_authenticated/settings/ringtones/page.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/routes/_authenticated/settings/workspace/$workspaceId/page.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/settings/behavior/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/team/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/presets/components/PresetRow/PresetRow.tsxapps/desktop/src/renderer/stores/app-state.ts
🧠 Learnings (4)
📚 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/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/GeneralSettings.tsxapps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/*/src/components/**/[A-Z]*.tsx : Create component folder structure with one folder per component: ComponentName/ComponentName.tsx with barrel export index.ts
Applied to files:
apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/ProjectsSettings.tsx
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Applied to files:
apps/desktop/src/renderer/screens/main/index.tsx
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
Applied to files:
apps/desktop/src/renderer/stores/app-state.ts
🧬 Code graph analysis (8)
apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.tsx (5)
apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/index.ts (1)
SettingsSidebar(1-1)apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/GeneralSettings.tsx (1)
GeneralSettings(64-95)apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/index.ts (1)
GeneralSettings(1-1)apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/ProjectsSettings.tsx (1)
ProjectsSettings(7-124)apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/index.ts (1)
ProjectsSettings(1-1)
apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/ProjectsSettings.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/page.tsx (11)
apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx (1)
Route(5-7)apps/desktop/src/renderer/routes/_authenticated/settings/workspace/$workspaceId/page.tsx (1)
Route(3-7)apps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsx (1)
Route(8-10)apps/desktop/src/renderer/routes/_authenticated/settings/appearance/page.tsx (1)
Route(20-22)apps/desktop/src/renderer/routes/_authenticated/settings/behavior/page.tsx (1)
Route(14-16)apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx (1)
Route(91-93)apps/desktop/src/renderer/routes/_authenticated/settings/presets/page.tsx (1)
Route(78-80)apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/page.tsx (1)
Route(3-7)apps/desktop/src/renderer/routes/_authenticated/settings/team/page.tsx (1)
Route(20-22)apps/desktop/src/renderer/routes/_authenticated/settings/page.tsx (1)
Route(3-5)apps/desktop/src/renderer/routes/sign-in/page.tsx (1)
Route(12-14)
apps/desktop/src/renderer/routes/_authenticated/settings/workspace/$workspaceId/page.tsx (11)
apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx (1)
Route(5-7)apps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsx (1)
Route(8-10)apps/desktop/src/renderer/routes/_authenticated/settings/appearance/page.tsx (1)
Route(20-22)apps/desktop/src/renderer/routes/_authenticated/settings/behavior/page.tsx (1)
Route(14-16)apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx (1)
Route(91-93)apps/desktop/src/renderer/routes/_authenticated/settings/presets/page.tsx (1)
Route(78-80)apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/page.tsx (1)
Route(3-7)apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/page.tsx (1)
Route(153-155)apps/desktop/src/renderer/routes/_authenticated/settings/team/page.tsx (1)
Route(20-22)apps/desktop/src/renderer/routes/_authenticated/settings/page.tsx (1)
Route(3-5)apps/desktop/src/renderer/routes/sign-in/page.tsx (1)
Route(12-14)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (1)
packages/ui/src/components/ui/dropdown-menu.tsx (1)
DropdownMenuItem(255-255)
apps/desktop/src/renderer/routes/_authenticated/settings/behavior/page.tsx (4)
packages/local-db/src/schema/zod.ts (1)
TerminalLinkBehavior(109-109)packages/ui/src/components/ui/label.tsx (1)
Label(24-24)packages/ui/src/components/ui/switch.tsx (1)
Switch(31-31)packages/ui/src/components/ui/select.tsx (5)
Select(177-177)SelectTrigger(185-185)SelectValue(186-186)SelectContent(178-178)SelectItem(180-180)
apps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsx (4)
apps/desktop/src/renderer/routes/sign-in/page.tsx (1)
Route(12-14)packages/ui/src/components/ui/sonner.tsx (1)
toast(40-40)apps/desktop/src/main/lib/auth/auth.ts (1)
signOut(222-233)packages/ui/src/components/ui/skeleton.tsx (1)
Skeleton(13-13)
apps/desktop/src/renderer/routes/_authenticated/settings/team/page.tsx (11)
apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx (1)
Route(5-7)apps/desktop/src/renderer/routes/_authenticated/settings/workspace/$workspaceId/page.tsx (1)
Route(3-7)apps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsx (1)
Route(8-10)apps/desktop/src/renderer/routes/_authenticated/settings/appearance/page.tsx (1)
Route(20-22)apps/desktop/src/renderer/routes/_authenticated/settings/behavior/page.tsx (1)
Route(14-16)apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx (1)
Route(91-93)apps/desktop/src/renderer/routes/_authenticated/settings/presets/page.tsx (1)
Route(78-80)apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/page.tsx (1)
Route(3-7)apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/page.tsx (1)
Route(153-155)apps/desktop/src/renderer/routes/_authenticated/settings/page.tsx (1)
Route(3-5)apps/desktop/src/renderer/routes/sign-in/page.tsx (1)
Route(12-14)
⏰ 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 (39)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsx (3)
14-14: LGTM!Clean import addition for the router-based navigation migration.
54-54: LGTM!Hook is correctly initialized at the component's top level.
82-84: LGTM!Clean migration to router-based navigation using TanStack Router's typed params pattern. The object parameter style aligns with the coding guidelines.
apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/page.tsx (1)
153-155: Route setup follows established pattern.The route configuration is consistent with other settings pages in the codebase.
apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/page.tsx (3)
1-11: LGTM!The route definition using
createFileRouteis correctly structured for TanStack Router's file-based routing. The function hoisting ofProjectSettingsPageallows the Route to reference it before its definition.
24-46: LGTM!The loading skeleton and not-found states are well-structured. The UI is consistent and the loading indicator provides good visual feedback.
48-88: LGTM!The component renders project settings clearly with proper semantic structure. The data flow to
ConfigFilePreviewis correct, and the nullish coalescing onconfigFilePathproperly convertsnulltoundefined.apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx (5)
1-40: LGTM!Imports are well-organized, correctly using tRPC from the designated location, and the
CATEGORY_ORDERconstant is properly extracted at module top. No Node.js modules imported in the renderer process.
95-128: LGTM!State management is well-structured with individual zustand selectors to minimize re-renders. The tRPC mutations are properly declared, and the memoization of
allHotkeysis appropriate.
138-184: LGTM!The keyboard recording effect is well-implemented with:
- Proper cleanup on unmount/dependency change
- Capture phase to intercept keys before other handlers
- Correct handling hierarchy (Escape → Backspace/Delete → terminal reserved → conflict → OS reserved → assign)
- Appropriate user feedback via toasts
186-249: LGTM!Event handlers are well-structured with proper error handling, user feedback via toasts, and efficient state updates. The
setHotkeysBatchcall for conflict reassignment is a good pattern for atomic updates.
331-413: LGTM!The AlertDialog implementations for conflict resolution and import confirmation are well-structured with clear messaging, proper state management, and appropriate action buttons.
apps/desktop/src/renderer/routes/_authenticated/settings/appearance/components/ThemeCard/index.ts (1)
1-1: LGTM!Simple re-export for the ThemeCard component. This is a reasonable pattern for component organization, enabling cleaner imports from the component folder.
apps/desktop/src/renderer/routes/_authenticated/settings/presets/components/PresetRow/PresetRow.tsx (1)
11-12: LGTM!Import path adjustments are correct for the updated directory structure.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx (1)
11-11: LGTM!Clean migration from
openSettings("presets")to route-based navigation usinguseNavigate. This aligns well with the PR's goal of moving to file-based routing for settings.Also applies to: 46-46, 114-114
apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/index.ts (1)
1-1: LGTM!Standard re-export pattern for the GeneralSettings component, enabling cleaner imports.
apps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsx (1)
3-3: LGTM!Clean migration from
openSettings()to route-based navigation. The default navigation to/settings/accountis appropriate for the main settings entry point.Also applies to: 8-8, 16-16
apps/desktop/src/renderer/routes/_authenticated/settings/team/page.tsx (1)
14-24: LGTM! Route setup follows established patterns.The file-based routing implementation is consistent with other settings pages in the codebase (e.g.,
account/page.tsx,presets/page.tsx).apps/desktop/src/renderer/screens/main/index.tsx (2)
120-125: LGTM!Clean migration from imperative settings opening to route-based navigation for the keyboard shortcuts hotkey.
251-251: LGTM!The simplified condition correctly reflects that settings is now handled via routing rather than as a view state.
apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/ProjectsSettings.tsx (1)
37-123: LGTM!Well-structured hierarchical navigation with proper accessibility attributes (
type="button"), consistent styling patterns, and appropriate use of route matching for active states.apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/index.ts (1)
1-1: LGTM!Clean barrel export following the established component folder structure pattern.
apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/SettingsSidebar.tsx (1)
6-25: LGTM!Clean sidebar composition with proper separation of concerns. The child components manage their own routing/state, keeping this component focused on layout.
apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/GeneralSettings/GeneralSettings.tsx (1)
13-20: TheSettingsRoutetype andLinkpaths are correctly formatted. In TanStack Router, route names prefixed with an underscore (like_authenticated) are pathless layout groups that don't add a segment to the URL. The route structure/_authenticated/settings/account/correctly resolves to the path/settings/account, which matches theLinkanduseMatchRouteusage in this component. Path resolution works correctly.Likely an incorrect or invalid review comment.
apps/desktop/src/renderer/routes/_authenticated/settings/workspace/$workspaceId/page.tsx (3)
23-45: LGTM!Good use of early returns for loading and not-found states. The skeleton UI provides appropriate feedback during data fetching.
91-106: LGTM!Branch section is properly conditionally rendered and git status is safely accessed with optional chaining.
15-21: No changes needed - pattern is sound.
useWorkspaceRenameis designed to handle empty initial values gracefully. The hook initializesrenameValuestate and includes auseEffect(lines 18-20) that syncs the value whenever theworkspaceNameparameter changes. Additionally, thesubmitRenamefunction includes a guard (if (trimmedValue && trimmedValue !== workspaceName)) that prevents mutations with empty or unchanged values. Calling the hook before data loads is a valid pattern that ensures rename handlers are available immediately while synchronizing with incoming data.apps/desktop/src/renderer/routes/_authenticated/settings/presets/page.tsx (3)
99-105: LGTM!Good use of
useMemofor computing existing preset names, and the duplicate prevention logic is clean.
210-241: LGTM!The quick-add template UI is well-implemented with proper disabled states, tooltips, and visual feedback for already-added templates.
261-274: LGTM!PresetRow correctly uses
preset.idas the key for stable React reconciliation, and all necessary handlers are passed down.apps/desktop/src/renderer/stores/app-state.ts (1)
1-54: LGTM!Clean removal of settings-related state in favor of route-based navigation. The store is now simpler and focused. Good use of Zustand per coding guidelines.
apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx (1)
9-31: LGTM!Clean layout implementation with proper platform detection for Mac-specific spacing. The flex layout correctly hosts the sidebar and routed content via
<Outlet />.apps/desktop/src/renderer/routes/_authenticated/settings/appearance/page.tsx (1)
44-56: LGTM!Theme grid correctly renders all themes with proper key usage and selection state management.
apps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsx (1)
31-54: LGTM!Profile section handles loading, success, and error states appropriately with good UX feedback.
apps/desktop/src/renderer/routes/_authenticated/settings/behavior/page.tsx (2)
24-39: LGTM!Well-implemented optimistic update pattern with proper cache cancellation, rollback on error, and invalidation on settle. This provides responsive UX while maintaining data consistency.
96-102: LGTM!Good use of disabled state combining both loading and pending states to prevent interaction during async operations.
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx (3)
15-15: LGTM!The import of
useNavigatefrom TanStack Router is appropriate for the migration from modal-based settings to route-based navigation.
36-36: LGTM!Hook is correctly placed at the component's top level, following React's rules of hooks.
103-114: Routes/settings/accountand/settings/teamare properly defined.Both settings routes are confirmed to exist in the router configuration at:
apps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/team/page.tsxThe navigation calls in the code use the correct paths and object parameter syntax.
| // Detect and persist base branch for existing worktrees that don't have it | ||
| // We use undefined to mean "not yet attempted" and null to mean "attempted but not found" | ||
| let baseBranch = worktree?.baseBranch; | ||
| if (worktree && baseBranch === undefined && project) { | ||
| // Only attempt detection if there's a remote origin | ||
| const hasRemote = await hasOriginRemote(project.mainRepoPath); | ||
| if (hasRemote) { | ||
| try { | ||
| const defaultBranch = project.defaultBranch || "main"; | ||
| const detected = await detectBaseBranch( | ||
| worktree.path, | ||
| worktree.branch, | ||
| defaultBranch, | ||
| ); | ||
| if (detected) { | ||
| baseBranch = detected; | ||
| } | ||
| // Persist the result (detected branch or null sentinel) | ||
| localDb | ||
| .update(worktrees) | ||
| .set({ baseBranch: detected ?? null }) | ||
| .where(eq(worktrees.id, worktree.id)) | ||
| .run(); | ||
| } catch { | ||
| // Detection failed, persist null to avoid retrying | ||
| localDb | ||
| .update(worktrees) | ||
| .set({ baseBranch: null }) | ||
| .where(eq(worktrees.id, worktree.id)) | ||
| .run(); | ||
| } | ||
| } else { | ||
| // No remote - persist null to avoid retrying | ||
| localDb | ||
| .update(worktrees) | ||
| .set({ baseBranch: null }) | ||
| .where(eq(worktrees.id, worktree.id)) | ||
| .run(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated baseBranch detection logic.
This entire block (Lines 35-74) is duplicated nearly verbatim in getActive (Lines 232-271). Extract to a shared helper to improve maintainability:
♻️ Proposed refactor
// In ../utils/git.ts or a new helper file
async function detectAndPersistBaseBranch(params: {
worktree: { id: string; path: string; branch: string; baseBranch?: string | null };
project: { mainRepoPath: string; defaultBranch?: string | null } | null;
}): Promise<string | null | undefined> {
const { worktree, project } = params;
if (worktree.baseBranch !== undefined || !project) {
return worktree.baseBranch;
}
const hasRemote = await hasOriginRemote(project.mainRepoPath);
let baseBranch: string | null = null;
if (hasRemote) {
try {
const defaultBranch = project.defaultBranch || "main";
const detected = await detectBaseBranch(worktree.path, worktree.branch, defaultBranch);
baseBranch = detected ?? null;
} catch {
baseBranch = null;
}
}
localDb
.update(worktrees)
.set({ baseBranch })
.where(eq(worktrees.id, worktree.id))
.run();
return baseBranch;
}🤖 Prompt for AI Agents
In @apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts around
lines 35 - 74, The duplicated baseBranch detection/persistence logic should be
extracted into a shared helper (e.g., detectAndPersistBaseBranch) and called
from both locations (the current block in query.ts and the getActive function)
to avoid duplication; implement a helper (in ../utils/git.ts or a new helper
file) that accepts the worktree and project objects, returns worktree.baseBranch
if already defined, otherwise runs hasOriginRemote(project.mainRepoPath), calls
detectBaseBranch(worktree.path, worktree.branch, project.defaultBranch ||
"main") when remote exists, converts undefined detection to null, persists the
result via localDb.update(worktrees).set({ baseBranch }).where(eq(worktrees.id,
worktree.id)).run(), and returns the final baseBranch (string | null |
undefined), then replace the duplicated blocks with calls to
detectAndPersistBaseBranch and use its return value.
| const signOutMutation = trpc.auth.signOut.useMutation({ | ||
| onSuccess: () => toast.success("Signed out"), | ||
| }); | ||
|
|
||
| const signOut = () => signOutMutation.mutate(); |
There was a problem hiding this comment.
Add error handling for sign-out mutation.
The sign-out mutation only handles onSuccess. If the sign-out fails (e.g., network error), the user won't receive any feedback.
🐛 Proposed fix
const signOutMutation = trpc.auth.signOut.useMutation({
onSuccess: () => toast.success("Signed out"),
+ onError: (error) => toast.error(`Sign out failed: ${error.message}`),
});📝 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 signOutMutation = trpc.auth.signOut.useMutation({ | |
| onSuccess: () => toast.success("Signed out"), | |
| }); | |
| const signOut = () => signOutMutation.mutate(); | |
| const signOutMutation = trpc.auth.signOut.useMutation({ | |
| onSuccess: () => toast.success("Signed out"), | |
| onError: (error) => toast.error(`Sign out failed: ${error.message}`), | |
| }); | |
| const signOut = () => signOutMutation.mutate(); |
🤖 Prompt for AI Agents
In @apps/desktop/src/renderer/routes/_authenticated/settings/account/page.tsx
around lines 14 - 18, The sign-out mutation only handles success; update the
trpc.auth.signOut.useMutation call (signOutMutation) to include an onError
handler that surfaces failures (e.g., toast.error with the error message or a
generic "Sign out failed") and optionally logs the error, and ensure the signOut
wrapper (signOut function) still calls signOutMutation.mutate() so failures
trigger the new onError path; reference trpc.auth.signOut.useMutation,
signOutMutation, and signOut when applying the change.
| // Expand all projects by default when groups are loaded | ||
| useEffect(() => { | ||
| if (groups.length > 0) { | ||
| setExpandedProjects(new Set(groups.map((g) => g.project.id))); | ||
| } | ||
| }, [groups]); |
There was a problem hiding this comment.
useEffect may re-run unnecessarily on each query refetch.
The groups dependency is a new array reference on every query result, causing this effect to run repeatedly and reset expandedProjects even when data hasn't meaningfully changed. This could cause unexpected collapse/expand behavior during background refetches.
🐛 Proposed fix
- // Expand all projects by default when groups are loaded
- useEffect(() => {
- if (groups.length > 0) {
- setExpandedProjects(new Set(groups.map((g) => g.project.id)));
- }
- }, [groups]);
+ // Expand all projects by default when groups are first loaded
+ const groupIds = groups.map((g) => g.project.id).join(",");
+ useEffect(() => {
+ if (groups.length > 0) {
+ setExpandedProjects(new Set(groups.map((g) => g.project.id)));
+ }
+ }, [groupIds]); // Only re-run when project IDs actually changeAlternatively, use a ref to track if initial expansion has been done:
+ const hasInitializedRef = useRef(false);
+
useEffect(() => {
- if (groups.length > 0) {
+ if (groups.length > 0 && !hasInitializedRef.current) {
+ hasInitializedRef.current = true;
setExpandedProjects(new Set(groups.map((g) => g.project.id)));
}
}, [groups]);📝 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.
| // Expand all projects by default when groups are loaded | |
| useEffect(() => { | |
| if (groups.length > 0) { | |
| setExpandedProjects(new Set(groups.map((g) => g.project.id))); | |
| } | |
| }, [groups]); | |
| const hasInitializedRef = useRef(false); | |
| // Expand all projects by default when groups are loaded | |
| useEffect(() => { | |
| if (groups.length > 0 && !hasInitializedRef.current) { | |
| hasInitializedRef.current = true; | |
| setExpandedProjects(new Set(groups.map((g) => g.project.id))); | |
| } | |
| }, [groups]); |
🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/components/ProjectsSettings/ProjectsSettings.tsx
around lines 14 - 19, The effect watching groups is re-running on every query
refetch because groups is a new array reference; change it to only perform the
initial expansion once by adding a ref flag (e.g., initialExpansionDoneRef)
checked inside the same useEffect so that when groups.length > 0 you call
setExpandedProjects(new Set(groups.map(g => g.project.id))) only if the ref is
false, then set the ref to true; keep the dependency on groups but guard with
the ref to prevent resetting expandedProjects on subsequent refetches.
| function ProjectSettingsPage() { | ||
| const { projectId } = Route.useParams(); | ||
| const { data: project, isLoading } = trpc.projects.get.useQuery({ | ||
| id: projectId, | ||
| }); | ||
|
|
||
| const { data: configFilePath } = trpc.config.getConfigFilePath.useQuery( | ||
| { projectId: activeWorkspace?.projectId ?? "" }, | ||
| { enabled: !!activeWorkspace?.projectId }, | ||
| { projectId }, | ||
| { enabled: !!projectId }, | ||
| ); |
There was a problem hiding this comment.
Missing error state handling could mislead users.
The query destructures only data and isLoading, but doesn't handle the error state. If the query fails (e.g., network error), isLoading becomes false and project is undefined, causing the component to display "Project not found" instead of a meaningful error message.
Per coding guidelines, errors should not be swallowed silently.
Proposed fix
function ProjectSettingsPage() {
const { projectId } = Route.useParams();
- const { data: project, isLoading } = trpc.projects.get.useQuery({
+ const { data: project, isLoading, error } = trpc.projects.get.useQuery({
id: projectId,
});
const { data: configFilePath } = trpc.config.getConfigFilePath.useQuery(
{ projectId },
{ enabled: !!projectId },
);
if (isLoading) {
return (
<div className="p-6 max-w-4xl select-text">
<div className="animate-pulse space-y-4">
<div className="h-8 bg-muted rounded w-1/3" />
<div className="h-4 bg-muted rounded w-1/2" />
</div>
</div>
);
}
+
+ if (error) {
+ console.error("[settings/project] Failed to load project:", error);
+ return (
+ <div className="p-6 max-w-4xl">
+ <div className="mb-8">
+ <h2 className="text-xl font-semibold">Project</h2>
+ <p className="text-sm text-destructive mt-1">
+ Failed to load project settings
+ </p>
+ </div>
+ </div>
+ );
+ }🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/page.tsx
around lines 13 - 22, The component ProjectSettingsPage currently only
destructures data and isLoading from trpc.projects.get.useQuery (and similarly
for trpc.config.getConfigFilePath.useQuery) which swallows query errors and
causes "Project not found" to show on failures; update both hook usages to also
destructure error (e.g., { data: project, isLoading, error }) and handle error
cases explicitly by rendering a clear error UI/message (using error.message) or
returning early, and avoid treating undefined project as "not found" when an
error exists; also optionally log the error for debugging.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
Navigation Updates
/settings/account) rather than modal system✏️ Tip: You can customize this high-level summary in your review settings.