fix(desktop): prevent StartView flash when deleting active workspace#714
fix(desktop): prevent StartView flash when deleting active workspace#714
Conversation
Root cause: When deleting the active workspace, the optimistic update tried to find the next workspace in `previousGrouped` (which only contains visible projects). If the next workspace was in a hidden project or the cache was empty, it fell back to setting `getActive` to null, causing StartView to incorrectly appear. Changes: - Backend: `selectNextActiveWorkspace()` now only selects from visible projects (joins with projects table and filters by tabOrder != null) - Frontend: Iterate through remaining workspaces to find one with full data in cache, instead of only checking the first one - Frontend: Add fallback that sets minimal workspace data instead of null when cache miss occurs, preventing StartView flash 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughModified workspace selection logic to filter visible projects only (non-null tabOrder). Enhanced optimistic updates in close/delete handlers to search for candidates with full grouped data. Improved data integrity and reduced UI flash with minimal safe fallback structure. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 0
🧹 Nitpick comments (2)
apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts (2)
118-128: Verify UI components handleproject: nullgracefully.The fallback prevents the StartView flash, but components consuming
getActivedata may not expectproject: nullduring the brief window before refetch completes. Ensure any UI rendering active workspace metadata uses optional chaining or null checks.Also, the type normalization on line 123 appears redundant if
fallback.typeis already constrained to"branch" | "worktree". Consider simplifying:- type: fallback.type === "branch" ? "branch" : "worktree", + type: fallback.type,
67-132: Consider extracting shared candidate selection logic.The next-active-workspace selection logic (lines 69-131) is nearly identical to
useDeleteWorkspace.ts. This could be extracted to a shared utility function to reduce duplication and ensure consistent behavior.♻️ Example extraction
// In a shared utility file, e.g., workspaceSelectionUtils.ts export function selectNextActiveWorkspaceFromCache({ remainingWorkspaces, previousGrouped, }: { remainingWorkspaces: WorkspaceType[]; previousGrouped: GroupedWorkspaces | undefined; }): ActiveWorkspaceData | null { // ... shared selection logic }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.tsapps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.tsapps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts
🧰 Additional context used
📓 Path-based instructions (5)
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/react-query/workspaces/useCloseWorkspace.tsapps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.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/react-query/workspaces/useCloseWorkspace.tsapps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.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/react-query/workspaces/useCloseWorkspace.tsapps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.tsapps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.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/react-query/workspaces/useCloseWorkspace.tsapps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts
⏰ 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). (2)
- GitHub Check: Cursor Bugbot
- GitHub Check: Build
🔇 Additional comments (5)
apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts (1)
112-130: LGTM! Query correctly filters for visible projects.The
innerJoinwith projects andisNotNull(projects.tabOrder)constraint ensures the selected workspace belongs to a visible project, aligning backend selection with frontend cache expectations. The narrowedselectto onlyidandlastOpenedAtis appropriate since onlyidis used in the return value.apps/desktop/src/renderer/react-query/workspaces/useCloseWorkspace.ts (1)
74-91: Candidate selection logic is sound.The iteration through
remainingWorkspacesto find a workspace with complete grouped data is a good approach. Breaking on the first match ensures deterministic behavior while respecting thelastOpenedAtsort order.apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts (3)
69-86: Candidate selection logic correctly prioritizes data completeness.The loop through
remainingWorkspacessorted bylastOpenedAtensures the most recently opened workspace with full cached data is selected. This aligns backend and frontend selection behavior.
113-123: Same fallback pattern asuseCloseWorkspace- ensure consistency.The fallback logic mirrors
useCloseWorkspace.ts. The same note about verifying UI components handleproject: nullapplies here. Since this is duplicated code, extracting to a shared utility (as noted in the other file) would ensure any future fixes apply to both paths.
88-112: Optimistic update data construction is correct.The worktreeData construction and active workspace update properly combine data from
selectedWorkspace(timestamps, branch) withworkspaceFromGrouped(type, worktreePath) andprojectGroup(project metadata). This ensures the optimistic state is complete and consistent.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| worktreePath: "", | ||
| project: null, | ||
| worktree: null, | ||
| }); |
There was a problem hiding this comment.
Fallback triggers initialization view for worktree workspaces
Medium Severity
The fallback sets worktree: null while also setting type to "worktree" for non-branch workspaces. This combination causes hasIncompleteInit to evaluate to true in WorkspaceView (since worktree?.gitStatus becomes undefined), which displays WorkspaceInitializingView instead of the normal content. The main path provides minimal worktreeData with a gitStatus object for worktree types, but this fallback doesn't, defeating the goal of preventing incorrect views during workspace transitions.
Additional Locations (1)
1 participant
Summary
Changes
Backend (
db-helpers.ts):selectNextActiveWorkspace()now only selects from visible projects by joining with the projects table and filtering bytabOrder != nullFrontend (
useDeleteWorkspace.ts,useCloseWorkspace.ts):previousGrouped, instead of only checking the first onenullwhen cache is missing, preventing StartView flashTest plan
🤖 Generated with Claude Code
Note
Aligns next-workspace selection to visible projects and hardens optimistic switching to avoid StartView flashes when closing/deleting the active workspace.
selectNextActiveWorkspace()now joinsprojectsand filters byprojects.tabOrder != null, selecting most-recent from visible projects onlygetAllGroupedand optimistically set it active; add minimal-data fallback instead ofnullto prevent StartView flash; keep caches in sync and invalidate after ops (plus recents on close)Written by Cursor Bugbot for commit 975f461. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.