Skip to content

fix(desktop): fix workspace deletion bug#2889

Draft
AviPeltz wants to merge 2 commits into
mainfrom
fix-worktree-deletion
Draft

fix(desktop): fix workspace deletion bug#2889
AviPeltz wants to merge 2 commits into
mainfrom
fix-worktree-deletion

Conversation

@AviPeltz
Copy link
Copy Markdown
Collaborator

@AviPeltz AviPeltz commented Mar 26, 2026

Summary

  • restore worktree deletion behavior by removing the broken delete-time listExternalWorktrees safety checks and renaming that helper to listGitWorktrees
  • add an in-memory draft provisioning job for workspace creation so Superset does not persist a real workspace/worktree row until git acquisition succeeds
  • surface draft jobs through the existing workspace queries/UI so provisioning progress, retry, and delete still work before persistence
  • document the original deletion regression and the broader creation-flow problem in deletionResearch.md

Why

The old flow persisted a real workspace before git worktree add had succeeded. When provisioning failed early, Superset could leave behind a misleading failed workspace row attached to a path it never actually created. This PR changes that boundary: a workspace only becomes real after the git worktree has been acquired.

Testing

  • bunx tsc -p apps/desktop/tsconfig.json --noEmit --ignoreDeprecations 6.0
  • bun test apps/desktop/src/lib/trpc/routers/workspaces/utils/git.test.ts apps/desktop/src/lib/trpc/routers/workspaces/procedures/external-worktree-import.test.ts
  • bunx biome check apps/desktop/src/main/lib/workspace-init-manager.ts apps/desktop/src/lib/trpc/routers/workspaces/utils/draft-workspace.ts apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts apps/desktop/src/lib/trpc/routers/workspaces/procedures/init.ts apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx

Summary by CodeRabbit

Release Notes

  • New Features

    • Added support for draft workspaces that persist in the workspace list while being provisioned.
    • Enabled deletion of workspaces during the provisioning phase.
    • Added ability to retry workspace initialization on draft workspaces.
  • Improvements

    • Simplified workspace disk deletion logic.
    • Enhanced workspace initialization and recovery workflows.
    • Improved git worktree discovery and enumeration.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 26, 2026

📝 Walkthrough

Walkthrough

This PR introduces a draft workspace provisioning workflow, replacing direct workspace/worktree creation with a DraftWorkspaceProvisioningJob pattern that enables deferred persistence. Additionally, git worktree discovery functions are renamed from listExternalWorktrees to listGitWorktrees throughout the codebase, and draft workspace support is added to delete, query, init, and workspace initialization procedures.

Changes

Cohort / File(s) Summary
Git Worktree API Rename
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts, apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-creation.ts, apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts, apps/desktop/src/lib/trpc/routers/workspaces/procedures/external-worktree-import.test.ts
Renamed listExternalWorktrees to listGitWorktrees and ExternalWorktree interface to GitWorktreeEntry, updating all callers and test assertions accordingly.
Draft Workspace Creation & Provisioning
apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts, apps/desktop/src/lib/trpc/routers/workspaces/utils/draft-workspace.ts, apps/desktop/src/main/lib/workspace-init-manager.ts, apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts
Introduced DraftWorkspaceProvisioningJob type and builder functions (buildDraftWorkspaceRow, buildDraftWorktreeRow); modified workspace creation to use draft workflow with deferred persistence; added draft job management methods to WorkspaceInitManager; extended initializeWorkspaceWorktree to conditionally persist draft workspaces during initialization.
Draft Workspace Deletion
apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
Added draft workspace handling to canDelete and delete mutation; removed path canonicalization and external-worktree safety checks; simplified disk deletion to be unconditional when worktree exists; added draft job cancellation and tracking support.
Draft Workspace Querying
apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts
Integrated draft workspace/job support into listing and lookup flows via workspaceInitManager; updated visual ordering to include draft workspaces; extended getAllGrouped to include isDraft boolean and append draft workspaces to results.
Retry Initialization for Draft Workspaces
apps/desktop/src/lib/trpc/routers/workspaces/procedures/init.ts
Expanded retry-initialization lookup to branch between persisted and draft targets; updated resolveRetryTarget and retryInit to handle draft jobs with in-place updates via workspaceInitManager.updateDraftJob.
UI Updates for Draft Workspaces
apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx, apps/desktop/src/renderer/screens/main/components/WorkspaceView/.../Terminal.tsx
Gated file event bridge and reconciliation based on draft status; simplified terminal restart command fallback logic.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant WorkspaceInitMgr as WorkspaceInitManager
    participant Procedures
    participant Database
    participant FileSystem as File System

    rect rgba(100, 200, 100, 0.5)
    Note over Client,FileSystem: Draft Workspace Creation Flow (New)
    Client->>Procedures: create(projectId, branch)
    Procedures->>WorkspaceInitMgr: startJob(workspaceId, projectId, draftJob)
    WorkspaceInitMgr->>WorkspaceInitMgr: Store draft in InitJob
    Procedures->>Procedures: buildDraftWorkspaceRow(draftJob)
    Procedures->>Procedures: initializeWorkspaceWorktree(params with draftJob)
    Procedures->>FileSystem: Create worktree on disk
    Procedures->>Procedures: persistDraftWorkspaceIfNeeded()
    Procedures->>Database: Insert workspace row (on persistence)
    Procedures->>Database: Insert worktree row (on persistence)
    Procedures-->>Client: Return created workspace
    end

    rect rgba(100, 100, 200, 0.5)
    Note over Client,FileSystem: Draft Workspace Deletion Flow (New)
    Client->>Procedures: delete(workspaceId)
    Procedures->>WorkspaceInitMgr: getDraftJob(workspaceId)
    alt Draft job exists
        Procedures->>WorkspaceInitMgr: Cancel job (up to 30s)
        Procedures->>FileSystem: Remove worktree disk path
        Procedures->>WorkspaceInitMgr: Clear draft job
    else Persisted workspace exists
        Procedures->>Database: Delete workspace record
        Procedures->>FileSystem: Remove worktree disk path
    end
    Procedures-->>Client: Return success
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

Possibly related PRs

  • feat(desktop): add project run commands #2511: Both PRs modify apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts to enhance workspace querying, with this PR adding draft-workspace support and the related PR adding resolved run commands integration.

Poem

🐰 A draft takes shape, not yet declared,
Until the user's ready care,
No rows committed, just prepared—
A workspace waiting in the air!
Then click persist and it's all there!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ⚠️ Warning The PR title 'fix(desktop): fix workspace deletion bug' describes one component (deletion safety checks) but misses the primary objective: introducing draft provisioning jobs to defer persistence until after successful git worktree acquisition. Revise the title to reflect the main change: 'feat(desktop): defer workspace persistence until successful git worktree acquisition' or similar, which better captures the core refactoring across create, delete, init, and query procedures.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The PR description covers all required template sections: Summary explains the changes and naming updates, Related Issues context is clear from objectives, Type of Change is implied as refactor/feature, and Testing section lists comprehensive verification steps.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-worktree-deletion

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

🚀 Preview Deployment

🔗 Preview Links

Service Status Link
Neon Database (Neon) View Branch
Fly.io Electric (Fly.io) View App
Vercel API (Vercel) Open Preview
Vercel Web (Vercel) Open Preview
Vercel Marketing (Vercel) Open Preview
Vercel Admin (Vercel) Open Preview
Vercel Docs (Vercel) Open Preview

Preview updates automatically with new commits

@AviPeltz AviPeltz marked this pull request as ready for review March 26, 2026 00:49
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/lib/trpc/routers/workspaces/procedures/external-worktree-import.test.ts (1)

154-161: ⚠️ Potential issue | 🟡 Minor

Duplicate assertion detected.

Lines 155-157 and 158-160 both assert the same condition (important-data.txt existence). This appears to be a copy-paste artifact.

🔧 Suggested fix
 		// Verify data exists before
 		expect(existsSync(join(externalWorktreePath, "important-data.txt"))).toBe(
 			true,
 		);
-		expect(existsSync(join(externalWorktreePath, "important-data.txt"))).toBe(
-			true,
-		);
 		expect(existsSync(join(externalWorktreePath, "test.txt"))).toBe(true);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/lib/trpc/routers/workspaces/procedures/external-worktree-import.test.ts`
around lines 154 - 161, The test contains a duplicated assertion checking
existsSync(join(externalWorktreePath, "important-data.txt")) — remove the
redundant expect so the setup only asserts that "important-data.txt" exists once
(leave the distinct expect for "test.txt" intact); locate the duplicate expect
calls that reference externalWorktreePath and delete one to avoid the copy‑paste
artifact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts`:
- Around line 200-213: The branch clears the draft job and returns success even
if workspaceInitManager.waitForInit(input.id, 30000) timed out and the
initializer is still running, which allows persistDraftWorkspaceIfNeeded() to
recreate the draft; change the logic in the block that calls
workspaceInitManager.cancel(input.id) and await
workspaceInitManager.waitForInit(input.id, 30000) so you check the result (or
re-check workspaceInitManager.isInitializing(input.id)) and only call
workspaceInitManager.clearJob(input.id),
hideProjectIfNoWorkspaces(draftJob.projectId), and track("workspace_deleted",
...) and return { success: true } when the initializer has actually stopped; if
the wait timed out keep the job intact (or surface an error) so the background
initializer cannot later recreate the draft workspace.
- Around line 309-316: The delete flow currently always calls
removeWorktreeFromDisk for every workspace/worktree; instead, check the
worktree's origin flag (createdBySuperset) before performing destructive disk
removal: in the branch around the removeWorktreeFromDisk call (and the other
branch that mirrors this logic) use worktree.createdBySuperset (or the
equivalent flag set by createWorkspaceFromExternalWorktree) and only call
removeWorktreeFromDisk when createdBySuperset === true; if false, skip disk
deletion and proceed to remove only the Superset record and
clearWorkspaceDeletingStatus(input.id), returning a successful result for the
DB-only delete path. Ensure both affected code paths that call
removeWorktreeFromDisk are updated to respect createdBySuperset.

In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/init.ts`:
- Around line 196-250: The code currently calls
workspaceInitManager.clearJob/startJob but never cancels a running
initializeWorkspaceWorktree, allowing concurrent inits for the same workspace;
update the flow to explicitly cancel or await the previous initializer before
starting a new one by using the workspaceInitManager cancellation API (e.g., a
cancelJob/abortJob or returned AbortController) and/or making
initializeWorkspaceWorktree accept an AbortSignal and check it; specifically,
before calling workspaceInitManager.clearJob/startJob and before calling
initializeWorkspaceWorktree (both the regular and draft paths where
persistDraftWorkspaceIfNeeded races), invoke
workspaceInitManager.cancelJob(input.workspaceId) or await the previous job’s
completion, pass the cancellation signal into initializeWorkspaceWorktree, and
ensure any in-flight persistDraftWorkspaceIfNeeded is guarded by that signal to
prevent duplicate inserts or resurrected state.

In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts`:
- Around line 128-188: The DB insert sequence in persistDraftWorkspaceIfNeeded
must be made atomic and the draft should be marked persisted immediately after
the DB writes succeed: wrap the localDb.insert(...) calls that create worktrees
and workspaces (the insert into worktrees with draftJob.worktreeId and the
insert into workspaces with workspaceId and tabOrder) in a single transaction,
commit the transaction, then set workspacePersisted = true right after the
transaction succeeds (before calling setBranchBaseConfig, activateProject,
setLastActiveWorkspace, or track). This ensures the persisted boundary is the
successful DB commit and prevents later exceptions (in setBranchBaseConfig,
activateProject, track, etc.) from leaving rows in SQLite while
workspacePersisted remains false.

---

Outside diff comments:
In
`@apps/desktop/src/lib/trpc/routers/workspaces/procedures/external-worktree-import.test.ts`:
- Around line 154-161: The test contains a duplicated assertion checking
existsSync(join(externalWorktreePath, "important-data.txt")) — remove the
redundant expect so the setup only asserts that "important-data.txt" exists once
(leave the distinct expect for "test.txt" intact); locate the duplicate expect
calls that reference externalWorktreePath and delete one to avoid the copy‑paste
artifact.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 18a70215-bc92-4b73-9701-e5c00edc2902

📥 Commits

Reviewing files that changed from the base of the PR and between f062fdc and 77ff7fb.

📒 Files selected for processing (13)
  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts
  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/external-worktree-import.test.ts
  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts
  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/init.ts
  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts
  • apps/desktop/src/lib/trpc/routers/workspaces/utils/draft-workspace.ts
  • apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts
  • apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-creation.ts
  • apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts
  • apps/desktop/src/main/lib/workspace-init-manager.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx

Comment on lines +200 to +213
if (!workspace && draftJob) {
if (workspaceInitManager.isInitializing(input.id)) {
console.log(
`[workspace/delete] Cancelling draft init for ${input.id}, waiting for completion...`,
);
workspaceInitManager.cancel(input.id);
await workspaceInitManager.waitForInit(input.id, 30000);
}

workspaceInitManager.clearJob(input.id);
hideProjectIfNoWorkspaces(draftJob.projectId);
track("workspace_deleted", { workspace_id: input.id, draft: true });

return { success: true };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Don't clear the draft job unless the initializer actually stopped.

waitForInit(input.id, 30000) resolves on timeout, so this branch can fall through to clearJob() and return { success: true } while the original init is still running. If that task reaches persistDraftWorkspaceIfNeeded() later, the draft workspace/worktree rows get recreated after the user already deleted them.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts` around
lines 200 - 213, The branch clears the draft job and returns success even if
workspaceInitManager.waitForInit(input.id, 30000) timed out and the initializer
is still running, which allows persistDraftWorkspaceIfNeeded() to recreate the
draft; change the logic in the block that calls
workspaceInitManager.cancel(input.id) and await
workspaceInitManager.waitForInit(input.id, 30000) so you check the result (or
re-check workspaceInitManager.isInitializing(input.id)) and only call
workspaceInitManager.clearJob(input.id),
hideProjectIfNoWorkspaces(draftJob.projectId), and track("workspace_deleted",
...) and return { success: true } when the initializer has actually stopped; if
the wait timed out keep the job intact (or surface an error) so the background
initializer cannot later recreate the draft workspace.

Comment thread apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
Comment on lines +196 to +250
workspaceInitManager.clearJob(input.workspaceId);
workspaceInitManager.startJob(input.workspaceId, workspace.projectId);

initializeWorkspaceWorktree({
workspaceId: input.workspaceId,
projectId: workspace.projectId,
worktreeId: worktree.id,
worktreePath,
branch,
mainRepoPath: project.mainRepoPath,
});
} else {
const { draftJob, project } = target;
const { branch, worktreePath } = await resolveRetryTarget({
currentBranch: draftJob.branch,
currentPath: draftJob.worktreePath,
project,
deduplicateBranchName: input.deduplicateBranchName,
applyUpdate: (next) => {
workspaceInitManager.updateDraftJob(input.workspaceId, {
branch: next.branch,
worktreePath: next.worktreePath,
workspaceName: draftJob.isUnnamed
? next.branch
: draftJob.workspaceName,
});
},
});
const nextDraftJob = {
...(workspaceInitManager.getDraftJob(input.workspaceId) ??
draftJob),
branch,
worktreePath,
workspaceName: draftJob.isUnnamed ? branch : draftJob.workspaceName,
};

workspaceInitManager.clearJob(input.workspaceId);
workspaceInitManager.startJob(
input.workspaceId,
nextDraftJob.projectId,
nextDraftJob,
);

initializeWorkspaceWorktree({
workspaceId: input.workspaceId,
projectId: nextDraftJob.projectId,
worktreeId: nextDraftJob.worktreeId,
worktreePath,
branch,
mainRepoPath: project.mainRepoPath,
startPointBranch: nextDraftJob.startPointBranch,
namingPrompt: nextDraftJob.namingPrompt,
useExistingBranch: nextDraftJob.useExistingBranch,
draftJob: nextDraftJob,
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Cancel the old initializer before reusing this workspace ID.

clearJob()/startJob() only swap the in-memory record; they do not stop an already-running initializeWorkspaceWorktree() call. A quick retry can therefore run a second init against the same workspace while the first attempt is still unwinding. In the draft path, that races persistDraftWorkspaceIfNeeded() and can produce duplicate inserts or resurrect stale state.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/init.ts` around lines
196 - 250, The code currently calls workspaceInitManager.clearJob/startJob but
never cancels a running initializeWorkspaceWorktree, allowing concurrent inits
for the same workspace; update the flow to explicitly cancel or await the
previous initializer before starting a new one by using the workspaceInitManager
cancellation API (e.g., a cancelJob/abortJob or returned AbortController) and/or
making initializeWorkspaceWorktree accept an AbortSignal and check it;
specifically, before calling workspaceInitManager.clearJob/startJob and before
calling initializeWorkspaceWorktree (both the regular and draft paths where
persistDraftWorkspaceIfNeeded races), invoke
workspaceInitManager.cancelJob(input.workspaceId) or await the previous job’s
completion, pass the cancellation signal into initializeWorkspaceWorktree, and
ensure any in-flight persistDraftWorkspaceIfNeeded is guarded by that signal to
prevent duplicate inserts or resurrected state.

Comment on lines +128 to +188
const persistDraftWorkspaceIfNeeded = async (): Promise<void> => {
if (!draftJob) return;

const existingWorkspace = localDb
.select()
.from(workspaces)
.where(eq(workspaces.id, workspaceId))
.get();

if (!existingWorkspace) {
localDb
.insert(worktrees)
.values({
id: draftJob.worktreeId,
projectId,
path: worktreePath,
branch,
baseBranch: effectiveCompareBaseBranch,
gitStatus: null,
createdBySuperset: true,
})
.run();

const maxTabOrder = getMaxProjectChildTabOrder(projectId);
localDb
.insert(workspaces)
.values({
id: workspaceId,
projectId,
worktreeId: draftJob.worktreeId,
type: "worktree",
branch,
name: draftJob.workspaceName,
isUnnamed: draftJob.isUnnamed,
tabOrder: maxTabOrder + 1,
})
.run();

setLastActiveWorkspace(workspaceId);
if (project) {
activateProject(project);
}

track("workspace_created", {
workspace_id: workspaceId,
project_id: projectId,
branch,
base_branch: effectiveCompareBaseBranch,
use_existing_branch: useExistingBranch ?? false,
});
}

await setBranchBaseConfig({
repoPath: mainRepoPath,
branch,
compareBaseBranch: effectiveCompareBaseBranch,
isExplicit: draftJob.compareBaseBranchIsExplicit,
});

workspacePersisted = true;
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Treat the draft as persisted as soon as the DB writes succeed.

Any exception after the inserts run—setBranchBaseConfig(), activateProject(), analytics, etc.—still leaves the new workspaces/worktrees rows in SQLite while workspacePersisted stays false. The outer catch then removes the worktree from disk, recreating the orphaned-state this PR is meant to avoid. Please move the persisted boundary up to the insert step and make that insert sequence atomic.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts` around
lines 128 - 188, The DB insert sequence in persistDraftWorkspaceIfNeeded must be
made atomic and the draft should be marked persisted immediately after the DB
writes succeed: wrap the localDb.insert(...) calls that create worktrees and
workspaces (the insert into worktrees with draftJob.worktreeId and the insert
into workspaces with workspaceId and tabOrder) in a single transaction, commit
the transaction, then set workspacePersisted = true right after the transaction
succeeds (before calling setBranchBaseConfig, activateProject,
setLastActiveWorkspace, or track). This ensures the persisted boundary is the
successful DB commit and prevents later exceptions (in setBranchBaseConfig,
activateProject, track, etc.) from leaving rows in SQLite while
workspacePersisted remains false.

@AviPeltz AviPeltz changed the title feat(desktop): stage workspace provisioning before persistence fix(desktop): fix workspace deletion bug Mar 26, 2026
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 13 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts">

<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts:206">
P1: After cancelling a draft initialization, verify the initializer has actually stopped before clearing the job and returning success; otherwise a still-running init can re-persist the workspace after deletion.</violation>

<violation number="2" location="apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts:309">
P0: Deletion no longer checks `createdBySuperset`, so imported external worktrees can now be removed from disk instead of only being unlinked from the DB.</violation>
</file>

<file name="apps/desktop/src/lib/trpc/routers/workspaces/procedures/init.ts">

<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/procedures/init.ts:233">
P1: Cancel (or await completion of) any in-flight initializer before starting a retry for the same workspace ID; clearing in-memory job state alone can allow two init flows to run concurrently.</violation>
</file>

<file name="apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts">

<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts:180">
P1: Set the persisted boundary immediately after successful workspace/worktree inserts (or make the insert sequence atomic) before later side effects. Otherwise a later exception can trigger cleanup that removes the worktree while the new DB rows remain.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

console.log(
`[workspace/delete] Skipping disk deletion for external worktree at ${worktree.path}`,
);
const removeResult = await removeWorktreeFromDisk({
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0: Deletion no longer checks createdBySuperset, so imported external worktrees can now be removed from disk instead of only being unlinked from the DB.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts, line 309:

<comment>Deletion no longer checks `createdBySuperset`, so imported external worktrees can now be removed from disk instead of only being unlinked from the DB.</comment>

<file context>
@@ -267,43 +306,13 @@ export const createDeleteProcedures = () => {
-							console.log(
-								`[workspace/delete] Skipping disk deletion for external worktree at ${worktree.path}`,
-							);
+						const removeResult = await removeWorktreeFromDisk({
+							mainRepoPath: project.mainRepoPath,
+							worktreePath: worktree.path,
</file context>
Fix with Cubic

`[workspace/delete] Cancelling draft init for ${input.id}, waiting for completion...`,
);
workspaceInitManager.cancel(input.id);
await workspaceInitManager.waitForInit(input.id, 30000);
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: After cancelling a draft initialization, verify the initializer has actually stopped before clearing the job and returning success; otherwise a still-running init can re-persist the workspace after deletion.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts, line 206:

<comment>After cancelling a draft initialization, verify the initializer has actually stopped before clearing the job and returning success; otherwise a still-running init can re-persist the workspace after deletion.</comment>

<file context>
@@ -173,6 +193,25 @@ export const createDeleteProcedures = () => {
+							`[workspace/delete] Cancelling draft init for ${input.id}, waiting for completion...`,
+						);
+						workspaceInitManager.cancel(input.id);
+						await workspaceInitManager.waitForInit(input.id, 30000);
+					}
+
</file context>
Fix with Cubic

};

workspaceInitManager.clearJob(input.workspaceId);
workspaceInitManager.startJob(
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Cancel (or await completion of) any in-flight initializer before starting a retry for the same workspace ID; clearing in-memory job state alone can allow two init flows to run concurrently.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/workspaces/procedures/init.ts, line 233:

<comment>Cancel (or await completion of) any in-flight initializer before starting a retry for the same workspace ID; clearing in-memory job state alone can allow two init flows to run concurrently.</comment>

<file context>
@@ -150,27 +174,81 @@ export const createInitProcedures = () => {
+					};
+
+					workspaceInitManager.clearJob(input.workspaceId);
+					workspaceInitManager.startJob(
+						input.workspaceId,
+						nextDraftJob.projectId,
</file context>
Fix with Cubic

});
}

await setBranchBaseConfig({
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Set the persisted boundary immediately after successful workspace/worktree inserts (or make the insert sequence atomic) before later side effects. Otherwise a later exception can trigger cleanup that removes the worktree while the new DB rows remain.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts, line 180:

<comment>Set the persisted boundary immediately after successful workspace/worktree inserts (or make the insert sequence atomic) before later side effects. Otherwise a later exception can trigger cleanup that removes the worktree while the new DB rows remain.</comment>

<file context>
@@ -101,13 +113,79 @@ export async function initializeWorkspaceWorktree({
+				});
+			}
+
+			await setBranchBaseConfig({
+				repoPath: mainRepoPath,
+				branch,
</file context>
Fix with Cubic

@AviPeltz AviPeltz marked this pull request as draft March 26, 2026 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant