fix(desktop): prevent workspace from reappearing during deletion#702
fix(desktop): prevent workspace from reappearing during deletion#702
Conversation
📝 WalkthroughWalkthroughThis PR introduces a soft-delete mechanism for workspaces using a Changes
Sequence DiagramsequenceDiagram
participant UI as User/UI
participant Client as React Query Hook
participant Server as Delete Procedure
participant DB as Database
UI->>Client: Request workspace delete
Client->>Client: Cancel related caches (onMutate)
Client->>Client: Remove workspace from local cache
Client->>Server: Delete RPC call
activate Server
Server->>DB: Validate workspace exists
DB-->>Server: Workspace found
Server->>DB: Mark workspace as deleting (set deletingAt)
DB-->>Server: Updated
Note over Server: Long-running operations<br/>(teardown, worktree cleanup)
alt Teardown Success
Server->>DB: Remove workspace/worktree records
DB-->>Server: Deleted
Server-->>Client: Success
Client->>Client: onSettled: invalidate caches
else Teardown Fails
Server->>DB: Clear deleting status (reset deletingAt)
DB-->>Server: Updated
Server-->>Client: Error
Client->>Client: onError: restore previous cache state
Client->>UI: Display error, workspace reappears
end
deactivate Server
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 |
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts (1)
145-256: Prevent “stuck hidden” workspaces: cleardeletingAton any deletion failure, not just worktree removal.
Right nowmarkWorkspaceAsDeleting()(Line 155) happens early, butclearWorkspaceDeletingStatus()is only called in theremoveWorktreecatch (Lines 219-220). Any other exception/throw after Line 155 can leave the row present withdeletingAt != null, and your updated queries will permanently filter it out.Proposed fix (wrap mutation in try/catch and clear deleting status on failure)
delete: publicProcedure .input(z.object({ id: z.string() })) .mutation(async ({ input }) => { const workspace = getWorkspace(input.id); if (!workspace) { return { success: false, error: "Workspace not found" }; } // Mark workspace as deleting IMMEDIATELY so it's hidden from UI. // This prevents the workspace from reappearing if a refetch occurs // during the slow git operations below. markWorkspaceAsDeleting(input.id); + + try { // Cancel any ongoing initialization and wait for it to complete // This ensures we don't race with init's git operations if (workspaceInitManager.isInitializing(input.id)) { console.log( `[workspace/delete] Cancelling init for ${input.id}, waiting for completion...`, ); workspaceInitManager.cancel(input.id); // Wait for init to finish (up to 30s) - it will see cancellation and exit await workspaceInitManager.waitForInit(input.id, 30000); } // Kill all terminal processes in this workspace first const terminalResult = await terminalManager.killByWorkspaceId( input.id, ); const project = getProject(workspace.projectId); let worktree: SelectWorktree | undefined; // Branch workspaces don't have worktrees - skip worktree operations if (workspace.type === "worktree" && workspace.worktreeId) { worktree = getWorktree(workspace.worktreeId); if (worktree && project) { // Acquire project lock before any git operations // This prevents racing with any concurrent init operations await workspaceInitManager.acquireProjectLock(project.id); try { // Run teardown scripts before removing worktree const exists = await worktreeExists( project.mainRepoPath, worktree.path, ); if (exists) { const teardownResult = await runTeardown( project.mainRepoPath, worktree.path, workspace.name, ); if (!teardownResult.success) { console.error( `Teardown failed for workspace ${workspace.name}:`, teardownResult.error, ); } } try { if (exists) { await removeWorktree(project.mainRepoPath, worktree.path); } else { console.warn( `Worktree ${worktree.path} not found in git, skipping removal`, ); } } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); console.error("Failed to remove worktree:", errorMessage); // Clear deleting status so workspace reappears in UI clearWorkspaceDeletingStatus(input.id); return { success: false, error: `Failed to remove worktree: ${errorMessage}`, }; } } finally { workspaceInitManager.releaseProjectLock(project.id); } } } // Proceed with DB cleanup deleteWorkspace(input.id); if (worktree) { deleteWorktreeRecord(worktree.id); } if (project) { hideProjectIfNoWorkspaces(workspace.projectId); } updateActiveWorkspaceIfRemoved(input.id); const terminalWarning = terminalResult.failed > 0 ? `${terminalResult.failed} terminal process(es) may still be running` : undefined; track("workspace_deleted", { workspace_id: input.id }); // Clear init job state only after all cleanup is complete // This ensures cancellation signals remain visible during cleanup workspaceInitManager.clearJob(input.id); return { success: true, terminalWarning }; + } catch (error) { + // Best effort: ensure workspace becomes visible again if anything fails + clearWorkspaceDeletingStatus(input.id); + throw error; + } }),apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts (1)
15-43: Don’t dropoptions.onMutate(and its returned context).
Because you spread...optionsand then overrideonMutate, any consumer-suppliedonMutatewon’t run anymore, and its context won’t flow intoonError/onSettled.Proposed fix (call + return user onMutate context)
onMutate: async ({ id }) => { // Cancel outgoing refetches to avoid overwriting optimistic update await Promise.all([ utils.workspaces.getAll.cancel(), utils.workspaces.getAllGrouped.cancel(), utils.workspaces.getActive.cancel(), ]); + // Preserve user onMutate behavior/context + const userContext = await options?.onMutate?.({ id } as never); + // Optimistically remove workspace from caches for instant feedback utils.workspaces.getAll.setData(undefined, (old) => old?.filter((w) => w.id !== id), ); utils.workspaces.getAllGrouped.setData(undefined, (old) => old ?.map((group) => ({ ...group, workspaces: group.workspaces.filter((w) => w.id !== id), })) .filter((group) => group.workspaces.length > 0), ); // Clear active if it's the one being deleted - server will select next utils.workspaces.getActive.setData(undefined, (old) => old?.id === id ? null : old, ); + + return userContext; },
🧹 Nitpick comments (4)
apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts (3)
28-43: Consider selecting only needed columns (avoids full-row scans).
getMaxWorkspaceTabOrder()only needstabOrder, but selects entire workspace rows. Same logic, smaller payload.
90-105: ConsiderLIMIT 1existence checks for “has any workspaces”.
hideProjectIfNoWorkspaces()only needs to know whether at least one non-deleting workspace exists.
214-237: New helpers are fine; consider returning affected-row count for observability.
Right now these are fire-and-forgetvoid. Returning.run()result (or checking changes) can help detect “id not found” / unexpected states when debugging deletion issues.apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts (1)
44-54: Invalidate-on-settled is a solid simplification; consider narrowing invalidation scope (optional).
utils.workspaces.invalidate()may refetch more than needed vs invalidatinggetAll/getAllGrouped/getActive.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.tsapps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.tspackages/local-db/drizzle/0010_add_workspace_deleting_at.sqlpackages/local-db/drizzle/meta/0010_snapshot.jsonpackages/local-db/drizzle/meta/_journal.jsonpackages/local-db/src/schema/schema.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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:
packages/local-db/src/schema/schema.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.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:
packages/local-db/src/schema/schema.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.tsapps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts
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/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.tsapps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.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/useDeleteWorkspace.ts
🧠 Learnings (3)
📚 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 packages/db/src/**/*.ts : Schema definitions must be in packages/db/src/ using Drizzle ORM
Applied to files:
packages/local-db/src/schema/schema.tsapps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.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/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/lib/trpc/routers/workspaces/procedures/query.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/**/*.{ts,tsx} : Use Drizzle ORM for all database operations - never use raw SQL
Applied to files:
apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts
🧬 Code graph analysis (3)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts (1)
packages/local-db/src/schema/schema.ts (1)
workspaces(80-121)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts (2)
markWorkspaceAsDeleting(219-225)clearWorkspaceDeletingStatus(231-237)
apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts (2)
apps/desktop/src/main/lib/local-db/index.ts (1)
localDb(82-82)packages/local-db/src/schema/schema.ts (1)
workspaces(80-121)
⏰ 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 Marketing
- GitHub Check: Deploy API
- GitHub Check: Deploy Web
- GitHub Check: Build
🔇 Additional comments (12)
packages/local-db/drizzle/0010_add_workspace_deleting_at.sql (1)
1-1: Migration looks correct for nullable “deletion in progress” sentinel.deleting_atbeing nullable aligns with theIS NULLfilters. Consider (optional) indexing only if the table is expected to grow large.packages/local-db/drizzle/meta/_journal.json (1)
74-81: Drizzle journal entry looks consistent (idx/tag progression).packages/local-db/src/schema/schema.ts (1)
105-109: Schema addition is aligned (nullabledeletingAttimestamp). This should work cleanly withisNull(workspaces.deletingAt)filtering and rollback-on-failure by clearing the column.apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts (3)
1-3: Import change is fine; avoid unused operators drift.and/isNullare now used; just ensureisNotNullis still needed elsewhere in this module after the change.
24-31: Filtering out deleting workspaces should fix the “reappears during deletion” refetch. Looks correct usingisNull(workspaces.deletingAt)in both list endpoints.Also applies to: 89-95
121-143: No action required—null handling is already well-implemented across consumers and the get/getActive asymmetry is intentional.Consumers of
getActivehandlenullsafely via optional chaining throughout the codebase (e.g.,activeWorkspace?.id,activeWorkspace?.projectId). TheuseCloseWorkspacemutation explicitly expects and setsgetActivetonullas a valid state. No regressions detected.The filtering difference between
getActive(excludes deleting workspaces) andget/getWorkspace(includes deleting) is correct by design:getActiveis UI-facing and must never return a mid-deletion workspace, whilegetis backend-facing and correctly allows the delete procedure to query the workspace state during the deletion flow (aftermarkWorkspaceAsDeletingis called).packages/local-db/drizzle/meta/0010_snapshot.json (1)
749-836: Snapshot correctly reflects nullableworkspaces.deleting_at. As long as this file is generated by your migration tooling, this looks consistent with the schema + migration.apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts (1)
7-17: Imports look consistent with the new deletingAt flow.apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts (3)
10-10: Good:isNull(workspaces.deletingAt)filtering is now available where needed.
107-120: Next-active selection excluding deleting workspaces matches the new semantics.
253-272: Branch workspace query correctly excludes deleting rows.apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts (1)
3-9: Doc comment matches the new server-drivendeletingAtapproach.
Kitenite
force-pushed
the
deleting-with-open-threads
branch
from
87e2f33 to
4398f7b
Compare
Kitenite
force-pushed
the
deleting-with-open-threads
branch
from
f4665ac to
ca47593
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts (2)
14-37: Consider returning previous state from onMutate as a React Query best practice.The optimistic updates correctly remove the workspace from all three caches. However, React Query convention suggests capturing and returning the previous state from
onMutateas context, even if the current rollback strategy relies on invalidation. This improves debuggability and provides flexibility for future enhancements.♻️ Suggested enhancement
onMutate: async ({ id }) => { await Promise.all([ utils.workspaces.getAll.cancel(), utils.workspaces.getAllGrouped.cancel(), utils.workspaces.getActive.cancel(), ]); + const previousAll = utils.workspaces.getAll.getData(); + const previousGrouped = utils.workspaces.getAllGrouped.getData(); + const previousActive = utils.workspaces.getActive.getData(); + utils.workspaces.getAll.setData(undefined, (old) => old?.filter((w) => w.id !== id), ); utils.workspaces.getAllGrouped.setData(undefined, (old) => old ?.map((group) => ({ ...group, workspaces: group.workspaces.filter((w) => w.id !== id), })) .filter((group) => group.workspaces.length > 0), ); utils.workspaces.getActive.setData(undefined, (old) => old?.id === id ? null : old, ); + + return { previousAll, previousGrouped, previousActive }; },
45-47: Consider adding error logging for debugging failed deletions.The error handler correctly delegates to the user's callback, and rollback occurs via invalidation in
onSettled. For improved observability when troubleshooting deletion failures, consider adding a prefixed console log before delegating.♻️ Suggested enhancement
onError: async (...args) => { + const [error, variables] = args; + console.error('[workspaces/delete] Failed to delete workspace:', { + id: variables.id, + error, + }); await options?.onError?.(...args); },apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts (3)
28-43: deletingAt filter is correct; consider selecting only tabOrder to avoid pulling full rows.
On Line 33-39, you can limit the payload to{ tabOrder: workspaces.tabOrder }(and/or a DB-side max) since only tabOrder is used.
107-120: Filter is correct; optional: select onlyidand add a stable tiebreaker.
Line 113-119 can be reduced to selectingworkspaces.idonly; optionally add a second orderBy (e.g.,desc(workspaces.updatedAt)ordesc(workspaces.id)) for deterministic results on timestamp ties.
214-230: Good helpers; consider makingmarkWorkspaceAsDeletingidempotent (don’t overwrite existing deletingAt).
If you wantdeletingAtto mean “when deletion started”, gate the update withisNull(workspaces.deletingAt)on Line 219.Proposed diff (idempotent mark)
export function markWorkspaceAsDeleting(workspaceId: string): void { localDb .update(workspaces) .set({ deletingAt: Date.now() }) - .where(eq(workspaces.id, workspaceId)) + .where(and(eq(workspaces.id, workspaceId), isNull(workspaces.deletingAt))) .run(); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.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/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/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/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/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/useDeleteWorkspace.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts
🧠 Learnings (2)
📚 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/**/*.{ts,tsx} : Use Drizzle ORM for all database operations - never use raw SQL
Applied to files:
apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.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 packages/db/src/**/*.ts : Schema definitions must be in packages/db/src/ using Drizzle ORM
Applied to files:
apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts
🧬 Code graph analysis (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts (2)
apps/desktop/src/main/lib/local-db/index.ts (1)
localDb(82-82)packages/local-db/src/schema/schema.ts (1)
workspaces(80-119)
⏰ 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 API
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Web
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Marketing
- GitHub Check: Build
🔇 Additional comments (4)
apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts (1)
4-5: Simplified rollback approach works, but AI summary is inconsistent.The comment correctly describes the server-side
deletingAtmechanism. However, the AI summary claims this file "uses stored rollback context to restore prior cache states" in onError, but the actual implementation relies on query invalidation in onSettled for rollback—no explicit context restoration occurs. The simplified approach (relying onutils.workspaces.invalidate()to trigger refetches after the server clearsdeletingAton error) is valid and reduces complexity as intended.apps/desktop/src/lib/trpc/routers/workspaces/utils/db-helpers.ts (3)
10-10: Import update looks correct (isNull used for deletingAt filters).
This stays within Drizzle (no raw SQL), consistent with repo guidelines/learnings.
246-265: Branch workspace query now correctly excludes deleting rows.
This matches the “hide during deletion” model and should prevent branch workspace flicker during slow teardown.
90-105: This review comment is based on code that has been reverted and is no longer in the codebase.The code snippet shown in the review includes
isNull(workspaces.deletingAt)filter, but the current implementation ofhideProjectIfNoWorkspaces(lines 92-101) contains no such filter. Additionally, theworkspacestable schema does not have adeletingAtcolumn.Git history shows commit 55579de added
deletingAtlogic and deletion rollback handling, but this was reverted by commit f4665ac. The current codebase has neither thedeletingAtcolumn nor the filtering logic, so the concern about projects staying hidden after a failed deletion rollback is not applicable.Likely an incorrect or invalid review comment.
1 participant
Summary
deletingAttimestamp field to immediately hide workspaces when deletion startsProblem
When deleting a workspace:
Solution
Added a
deletingAttimestamp field that is set immediately when deletion starts, before any slow operations. All workspace queries now filter out entries withdeletingAtset, so even if refetches occur mid-deletion, the workspace stays hidden.Changes
packages/local-db/src/schema/schema.tsdeletingAtfieldpackages/local-db/drizzle/0010_*.sqlprocedures/delete.tsprocedures/query.tsutils/db-helpers.tsuseDeleteWorkspace.tsTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.