fix(desktop): make it so you can close projects#480
Conversation
WalkthroughThis PR introduces a "close project" feature enabling users to cleanly shut down projects. The backend mutation terminates associated terminal processes, removes related workspaces, hides the project, and handles active workspace updates. A React hook wraps the mutation with automatic cache invalidation, and a UI menu item integrates the feature. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as Context Menu
participant Hook as useCloseProject<br/>(React Query)
participant Server as TRPC Server<br/>(projects.close)
participant TM as Terminal<br/>Manager
participant DB as Database
participant QC as Query<br/>Client
User->>UI: Click "Close Project"
UI->>Hook: mutate({ id })
Hook->>Server: Call projects.close mutation
rect rgb(220, 240, 220)
note right of Server: Close Project Flow
Server->>DB: Get project + workspaces
Server->>TM: killByWorkspaceId (all workspaces)
TM-->>Server: { failures: [...] }
Server->>DB: Delete workspace records
Server->>DB: Null project.tabOrder (hide)
Server->>DB: Update active workspace if needed
Server-->>Hook: { success, terminalWarning }
end
rect rgb(240, 220, 220)
note right of QC: Cache Invalidation
Hook->>QC: Invalidate workspace queries
Hook->>QC: Invalidate project recents queries
end
Hook->>Hook: Call onSuccess callback (if provided)
Hook-->>UI: Mutation complete
UI-->>User: Menu closes, UI updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The backend mutation logic requires careful review for edge cases (terminal cleanup failure aggregation, workspace record deletion integrity, active workspace state updates). The React hook is straightforward but depends on correct TRPC mutation signature and invalidation strategy. UI integration is minimal but requires verification of hook integration pattern. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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 (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx (1)
150-158: Consider adding a confirmation dialog for this destructive action.Closing a project terminates all terminal processes and removes workspace records. Users could accidentally trigger this from the context menu. A confirmation dialog would prevent accidental data loss.
Additionally, the backend returns a
terminalWarningwhen processes fail to terminate cleanly, but this isn't surfaced to the user. Consider showing a toast/notification whenterminalWarningis present.🔎 Suggested approach
const closeProject = useCloseProject({ onSuccess: (data) => { if (data.terminalWarning) { // Show toast notification with terminalWarning } }, }); // In the button onClick, show confirmation first: onClick={() => { if (window.confirm('Close this project? This will terminate all running terminals.')) { closeProject.mutate({ id: projectId }); } }}Or use a proper dialog component from your UI library for a better UX.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/renderer/react-query/projects/index.tsapps/desktop/src/renderer/react-query/projects/useCloseProject.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/renderer/react-query/projects/index.tsapps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/renderer/react-query/projects/useCloseProject.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/renderer/react-query/projects/index.tsapps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/renderer/react-query/projects/useCloseProject.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/react-query/projects/index.tsapps/desktop/src/renderer/react-query/projects/useCloseProject.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/react-query/projects/index.tsapps/desktop/src/renderer/react-query/projects/useCloseProject.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx
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/projects/index.tsapps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/renderer/react-query/projects/useCloseProject.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx
**/{components,features}/**/[!.]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx
**/{components,features}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to **/{components,features}/**/[!.]*.tsx : Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Applied to files:
apps/desktop/src/renderer/react-query/projects/index.ts
🧬 Code graph analysis (3)
apps/desktop/src/lib/trpc/routers/projects/projects.ts (2)
apps/desktop/src/main/lib/db/index.ts (1)
db(69-76)apps/desktop/src/main/lib/analytics/index.ts (1)
track(45-66)
apps/desktop/src/renderer/react-query/projects/useCloseProject.ts (1)
apps/desktop/src/renderer/react-query/projects/index.ts (1)
useCloseProject(1-1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceGroupContextMenu.tsx (1)
apps/desktop/src/renderer/react-query/projects/useCloseProject.ts (1)
useCloseProject(7-24)
⏰ 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 (4)
apps/desktop/src/renderer/react-query/projects/index.ts (1)
1-4: LGTM!The barrel export correctly exposes the new
useCloseProjecthook, consistent with the existing export pattern.apps/desktop/src/renderer/react-query/projects/useCloseProject.ts (1)
7-23: LGTM!Well-structured hook that properly wraps the tRPC mutation with automatic query invalidation. The pattern of spreading options and chaining the user's
onSuccesscallback is correct.apps/desktop/src/lib/trpc/routers/projects/projects.ts (2)
554-613: LGTM!The close mutation implementation is well-structured:
- Properly collects workspaces before modifications
- Terminates terminals gracefully with failure tracking
- Correctly updates
lastActiveWorkspaceIdby sorting remaining workspaces- Hides project (via
tabOrder = null) rather than deleting, allowing future reopening- Analytics tracking appropriately placed after successful operation
569-575: Consider whether terminal cleanup failures should affect the close operation.Currently, failed terminal terminations are aggregated and returned as a warning, but the close operation proceeds regardless. This is likely the correct UX (don't block user from closing), but verify this matches the intended behavior.
If a terminal truly can't be killed, the user might expect the close to fail or at least see the warning prominently.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 Preview resources have been processed for cleanup |
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.