Revert PR #2908: workspace deletion fix

[AviPeltz]

Summary

Reverts PR #2908 which introduced changes to workspace deletion behavior.

Reason for Revert

This PR reverts the changes made in #2908 that modified how workspace deletion handles disk cleanup and external worktree detection.

Original PR

• Title: fix(desktop): quickfix restore workspace deletion from disk
• Merged: March 26, 2026
• Commit: a2268c9

Changes Reverted

1. Restoration of workspace file deletion from disk
2. Renaming of listExternalWorktrees → listAllGitWorktrees
3. New listExternalWorktrees(mainRepoPath, projectId) implementation
4. Removal of createdBySuperset flag gating
5. Error handling for stuck 'deleting' status

Files Affected

• 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/utils/git.ts
• apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-creation.ts
• bun.lock

Testing

• Verify workspace deletion behavior returns to pre-fix(desktop): quickfix restore workspace deletion from disk #2908 state
• Confirm external worktree handling works as expected
• Check that no workspaces get stuck in 'deleting' status

---

Summary by cubic

Reverts #2908 to restore the previous workspace deletion behavior and safety checks. Disk deletion now only happens for worktrees created by Superset; external worktrees are never removed from disk.

• Bug Fixes

  • Re-enable createdBySuperset gate for on-disk deletion; external worktrees only remove DB records.
  • Restore path normalization and external worktree safety check in delete flows.
  • Revert listExternalWorktrees to listExternalWorktrees(mainRepoPath) and move DB filtering to callers (updated git-status, creation/import, and tests).

• Dependencies

  • Revert @superset/desktop version in bun.lock to 1.3.2.

^{Written for commit 35e3908. Summary will update on new commits.}

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • External worktree deletion now respects whether worktrees were created by the application or externally—only application-managed worktrees are removed from disk.

• Improvements

  • Enhanced external worktree detection and filtering logic for more accurate identification of managed vs. unmanaged worktrees.
  • Improved safety warnings to better reflect mismatches between tracked and external worktree states.

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request refactors external worktree detection by removing the projectId parameter from listExternalWorktrees, consolidating path normalization logic, and redistributing filtering responsibility to callers. Additional changes modify disk deletion behavior in workspace/worktree cleanup to depend on the createdBySuperset flag and update safety check messaging.

Changes

Cohort / File(s)	Summary
Core Worktree Detection API apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts	Removed projectId parameter from listExternalWorktrees signature; consolidated listAllGitWorktrees into listExternalWorktrees; removed exported normalizePath helper and database filtering logic.
API Call Sites apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts, apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-creation.ts	Updated listExternalWorktrees calls to remove projectId argument passed to the function.
Database-Level Filtering apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts	Added local database query to fetch tracked worktree paths from worktrees table filtered by projectId, then exclude already-tracked worktrees from external worktree results.
Deletion Logic & Path Normalization apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts	Added local normalizePath helper using realpathSync; made disk deletion conditional on worktree.createdBySuperset flag; changed external worktree safety check reason from external_worktree_detected to external_detection_mismatch; simplified error handling by removing outer try/catch in worktree/delete mutation.
Test Updates apps/desktop/src/lib/trpc/routers/workspaces/procedures/external-worktree-import.test.ts	Renamed test to focus on external worktrees; updated dynamic import from listAllGitWorktrees to listExternalWorktrees; adjusted assertions to verify external worktree detection.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• #2908: Directly related—modifies the same listExternalWorktrees signature and updates multiple call sites across the workspace and worktree creation/deletion flows.
• #1503: Touches the same importAllWorktrees function in create.ts and modifies listExternalWorktrees usage by removing the projectId argument.

Poem

🐰 External paths we used to track,
With IDs cluttering every call back.
Now simpler queries, cleaner and bright,
Filtering happens at just the right site.
Worktrees hop freely through our refactored flight! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: reverting PR #2908 related to workspace deletion.
Description check	✅ Passed	The description is comprehensive, covering summary, reason for revert, original PR details, changes reverted, affected files, and testing checklist.

_{✏️ 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 revert-pr-2908

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

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/delete.ts (1)

489-551: ⚠️ Potential issue | 🟡 Minor

Path comparison in worktree/delete is inconsistent with workspace/delete.

The workspace/delete mutation (lines 277-280) uses normalizePath for both the worktree path and external worktree paths when checking isActuallyExternal. However, worktree/delete compares raw paths directly without normalization:

const isActuallyExternal = externalWorktrees.some(
    (wt) => wt.path === worktree.path,  // No normalization
);

Since listExternalWorktrees returns raw, unnormalized paths from git worktree list --porcelain output, this comparison could fail if worktree.path in the database was stored with symlinks resolved. This inconsistency means the same worktree could be handled differently depending on which deletion path is used.

🔧 Suggested fix for consistency

+						const worktreePathNorm = normalizePath(worktree.path);
 						const isActuallyExternal = externalWorktrees.some(
-							(wt) => wt.path === worktree.path,
+							(wt) => normalizePath(wt.path) === worktreePathNorm,
 						);

🤖 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 489 - 551, The path comparison in worktree/delete is brittle—change the
isActuallyExternal check to compare normalized paths like workspace/delete does:
call normalizePath on worktree.path and on each externalWorktrees[].path (the
same function used elsewhere) before comparing; update the logic around
listExternalWorktrees, externalWorktrees, and the isActuallyExternal predicate
so normalization occurs consistently (refer to normalizePath,
listExternalWorktrees, isActuallyExternal, and worktree.path to locate where to
apply the fix).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts`:
- Around line 489-551: The path comparison in worktree/delete is brittle—change
the isActuallyExternal check to compare normalized paths like workspace/delete
does: call normalizePath on worktree.path and on each externalWorktrees[].path
(the same function used elsewhere) before comparing; update the logic around
listExternalWorktrees, externalWorktrees, and the isActuallyExternal predicate
so normalization occurs consistently (refer to normalizePath,
listExternalWorktrees, isActuallyExternal, and worktree.path to locate where to
apply the fix).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 10c9e5ee-2ba2-40d4-b59e-6c1a58da26e1

📥 Commits

Reviewing files that changed from the base of the PR and between b7810d4 and 35e3908.

⛔ Files ignored due to path filters (1)

• bun.lock is excluded by !**/*.lock

📒 Files selected for processing (6)

• 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/utils/git.ts
• apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-creation.ts

💤 Files with no reviewable changes (1)

• apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts

[github-actions]

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

• ✅ Neon database branch
• ✅ Electric Fly.io app

Thank you for your contribution! 🎉