From 0085c88a543c62f128b7f8b573557b0098233012 Mon Sep 17 00:00:00 2001 From: AviPeltz Date: Thu, 26 Mar 2026 17:38:58 -0700 Subject: [PATCH 1/2] fix(desktop): improve external worktree detection in deletion safety check Enhance the safety mechanism that prevents deletion of external worktrees by cross-referencing git worktrees with database-tracked worktrees. Previously, the check relied solely on listExternalWorktrees(), which could miss cases where a worktree was incorrectly marked as createdBySuperset. Now the logic: - Gets all git worktrees from the repository - Queries all tracked worktrees from the database for the project - Normalizes paths for accurate comparison (handles symlinks) - Determines if a worktree is external by checking if it exists in git but is NOT tracked in the database This prevents accidental deletion of external worktrees that may have been incorrectly flagged, providing an additional layer of safety. Applied to both workspace.delete and workspace.deleteWorktree procedures. --- .../routers/workspaces/procedures/delete.ts | 44 ++++++++++++++++--- bun.lock | 2 +- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts index 100c7028f48..889cfb30ae7 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts @@ -1,7 +1,10 @@ import { existsSync, realpathSync } from "node:fs"; import { resolve } from "node:path"; import type { SelectWorktree } from "@superset/local-db"; +import { worktrees } from "@superset/local-db"; +import { eq } from "drizzle-orm"; import { track } from "main/lib/analytics"; +import { localDb } from "main/lib/local-db"; import { workspaceInitManager } from "main/lib/workspace-init-manager"; import { getWorkspaceRuntimeRegistry } from "main/lib/workspace-runtime"; import { z } from "zod"; @@ -271,13 +274,28 @@ export const createDeleteProcedures = () => { // External worktrees should only have their DB records removed if (worktree.createdBySuperset) { // Safety: Double-check it's not actually external (catches race conditions) - const externalWorktrees = await listExternalWorktrees( + // Get all git worktrees + const allGitWorktrees = await listExternalWorktrees( project.mainRepoPath, ); + + // Get all tracked worktree paths from database for this project + const trackedWorktrees = localDb + .select({ path: worktrees.path }) + .from(worktrees) + .where(eq(worktrees.projectId, project.id)) + .all(); + const trackedPaths = new Set( + trackedWorktrees.map((wt) => normalizePath(wt.path)), + ); + + // Check if this worktree exists in git but is NOT tracked in our DB const worktreePathNorm = normalizePath(worktree.path); - const isActuallyExternal = externalWorktrees.some( + const existsInGit = allGitWorktrees.some( (wt) => normalizePath(wt.path) === worktreePathNorm, ); + const isActuallyExternal = + existsInGit && !trackedPaths.has(worktreePathNorm); if (isActuallyExternal) { console.warn( @@ -489,12 +507,28 @@ export const createDeleteProcedures = () => { // Only delete from disk if this worktree was created by Superset if (worktree.createdBySuperset) { // Safety: Double-check it's not actually external (catches race conditions) - const externalWorktrees = await listExternalWorktrees( + // Get all git worktrees + const allGitWorktrees = await listExternalWorktrees( project.mainRepoPath, ); - const isActuallyExternal = externalWorktrees.some( - (wt) => wt.path === worktree.path, + + // Get all tracked worktree paths from database for this project + const trackedWorktrees = localDb + .select({ path: worktrees.path }) + .from(worktrees) + .where(eq(worktrees.projectId, project.id)) + .all(); + const trackedPaths = new Set( + trackedWorktrees.map((wt) => normalizePath(wt.path)), + ); + + // Check if this worktree exists in git but is NOT tracked in our DB + const worktreePathNorm = normalizePath(worktree.path); + const existsInGit = allGitWorktrees.some( + (wt) => normalizePath(wt.path) === worktreePathNorm, ); + const isActuallyExternal = + existsInGit && !trackedPaths.has(worktreePathNorm); if (isActuallyExternal) { console.warn( diff --git a/bun.lock b/bun.lock index 2b9a3d4371b..fdf3278907e 100644 --- a/bun.lock +++ b/bun.lock @@ -110,7 +110,7 @@ }, "apps/desktop": { "name": "@superset/desktop", - "version": "1.3.2", + "version": "1.4.2", "dependencies": { "@ai-sdk/anthropic": "^3.0.43", "@ai-sdk/openai": "3.0.36", From 8625167a88c1d3eac552bf0c36eb6f5d7dc8031d Mon Sep 17 00:00:00 2001 From: AviPeltz Date: Thu, 26 Mar 2026 18:53:26 -0700 Subject: [PATCH 2/2] fix(desktop): delete external worktrees from disk when removed from Superset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update deletion logic to remove all worktrees from disk when deleted through Superset, regardless of whether they were created by Superset or imported as external worktrees. Previously: - Superset-created worktrees (createdBySuperset=true) → deleted from disk - External worktrees (createdBySuperset=false) → only removed from database Now: - All worktrees tracked in Superset → deleted from disk when removed - Safety check still prevents deletion of untracked worktrees This provides consistent UX: once a worktree is managed in Superset (even if originally created externally), deleting it removes it completely. The safety mechanism still protects against edge cases where a worktree exists in git but is not properly tracked in the database. Updated both workspace.delete and workspace.deleteWorktree procedures. --- .../routers/workspaces/procedures/delete.ts | 221 +++++++++--------- 1 file changed, 104 insertions(+), 117 deletions(-) diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts index 889cfb30ae7..83bfab7102a 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts @@ -270,58 +270,51 @@ export const createDeleteProcedures = () => { await workspaceInitManager.acquireProjectLock(project.id); try { - // Only delete from disk if this worktree was created by Superset - // External worktrees should only have their DB records removed - if (worktree.createdBySuperset) { - // Safety: Double-check it's not actually external (catches race conditions) - // Get all git worktrees - const allGitWorktrees = await listExternalWorktrees( - project.mainRepoPath, - ); + // Safety: Check if this worktree is tracked in our database + // Prevents deletion of worktrees that were never properly imported + // Get all git worktrees + const allGitWorktrees = await listExternalWorktrees( + project.mainRepoPath, + ); - // Get all tracked worktree paths from database for this project - const trackedWorktrees = localDb - .select({ path: worktrees.path }) - .from(worktrees) - .where(eq(worktrees.projectId, project.id)) - .all(); - const trackedPaths = new Set( - trackedWorktrees.map((wt) => normalizePath(wt.path)), - ); + // Get all tracked worktree paths from database for this project + const trackedWorktrees = localDb + .select({ path: worktrees.path }) + .from(worktrees) + .where(eq(worktrees.projectId, project.id)) + .all(); + const trackedPaths = new Set( + trackedWorktrees.map((wt) => normalizePath(wt.path)), + ); - // Check if this worktree exists in git but is NOT tracked in our DB - const worktreePathNorm = normalizePath(worktree.path); - const existsInGit = allGitWorktrees.some( - (wt) => normalizePath(wt.path) === worktreePathNorm, + // Check if this worktree exists in git but is NOT tracked in our DB + const worktreePathNorm = normalizePath(worktree.path); + const existsInGit = allGitWorktrees.some( + (wt) => normalizePath(wt.path) === worktreePathNorm, + ); + const isActuallyExternal = + existsInGit && !trackedPaths.has(worktreePathNorm); + + if (isActuallyExternal) { + console.warn( + `[workspace/delete] Worktree at ${worktree.path} exists in git but not tracked in database - preserving as safety measure`, ); - const isActuallyExternal = - existsInGit && !trackedPaths.has(worktreePathNorm); - - if (isActuallyExternal) { - console.warn( - `[workspace/delete] Worktree at ${worktree.path} marked as created by Superset but found in external list - preserving as safety measure`, - ); - track("worktree_delete_safety_trigger", { - workspace_id: input.id, - worktree_id: worktree.id, - worktree_path: worktree.path, - reason: "external_detection_mismatch", - }); - } else { - // Confirmed safe to delete - const removeResult = await removeWorktreeFromDisk({ - mainRepoPath: project.mainRepoPath, - worktreePath: worktree.path, - }); - if (!removeResult.success) { - clearWorkspaceDeletingStatus(input.id); - return removeResult; - } - } + track("worktree_delete_safety_trigger", { + workspace_id: input.id, + worktree_id: worktree.id, + worktree_path: worktree.path, + reason: "untracked_worktree_detected", + }); } else { - console.log( - `[workspace/delete] Skipping disk deletion for external worktree at ${worktree.path}`, - ); + // Safe to delete - worktree is tracked in our database + const removeResult = await removeWorktreeFromDisk({ + mainRepoPath: project.mainRepoPath, + worktreePath: worktree.path, + }); + if (!removeResult.success) { + clearWorkspaceDeletingStatus(input.id); + return removeResult; + } } } finally { workspaceInitManager.releaseProjectLock(project.id); @@ -504,84 +497,78 @@ export const createDeleteProcedures = () => { worktree.path, ); - // Only delete from disk if this worktree was created by Superset - if (worktree.createdBySuperset) { - // Safety: Double-check it's not actually external (catches race conditions) - // Get all git worktrees - const allGitWorktrees = await listExternalWorktrees( - project.mainRepoPath, - ); + // Safety: Check if this worktree is tracked in our database + // Prevents deletion of worktrees that were never properly imported + // Get all git worktrees + const allGitWorktrees = await listExternalWorktrees( + project.mainRepoPath, + ); - // Get all tracked worktree paths from database for this project - const trackedWorktrees = localDb - .select({ path: worktrees.path }) - .from(worktrees) - .where(eq(worktrees.projectId, project.id)) - .all(); - const trackedPaths = new Set( - trackedWorktrees.map((wt) => normalizePath(wt.path)), - ); + // Get all tracked worktree paths from database for this project + const trackedWorktrees = localDb + .select({ path: worktrees.path }) + .from(worktrees) + .where(eq(worktrees.projectId, project.id)) + .all(); + const trackedPaths = new Set( + trackedWorktrees.map((wt) => normalizePath(wt.path)), + ); - // Check if this worktree exists in git but is NOT tracked in our DB - const worktreePathNorm = normalizePath(worktree.path); - const existsInGit = allGitWorktrees.some( - (wt) => normalizePath(wt.path) === worktreePathNorm, - ); - const isActuallyExternal = - existsInGit && !trackedPaths.has(worktreePathNorm); + // Check if this worktree exists in git but is NOT tracked in our DB + const worktreePathNorm = normalizePath(worktree.path); + const existsInGit = allGitWorktrees.some( + (wt) => normalizePath(wt.path) === worktreePathNorm, + ); + const isActuallyExternal = + existsInGit && !trackedPaths.has(worktreePathNorm); - if (isActuallyExternal) { - console.warn( - `[worktree/delete] Worktree at ${worktree.path} marked as created by Superset but found in external list - preserving as safety measure`, - ); - track("worktree_delete_safety_trigger", { - worktree_id: input.worktreeId, - worktree_path: worktree.path, - reason: "external_detection_mismatch", + if (isActuallyExternal) { + console.warn( + `[worktree/delete] Worktree at ${worktree.path} exists in git but not tracked in database - preserving as safety measure`, + ); + track("worktree_delete_safety_trigger", { + worktree_id: input.worktreeId, + worktree_path: worktree.path, + reason: "untracked_worktree_detected", + }); + } else { + // Safe to delete - worktree is tracked in our database + if (exists) { + const teardownResult = await runTeardown({ + mainRepoPath: project.mainRepoPath, + worktreePath: worktree.path, + workspaceName: worktree.branch, + projectId: project.id, }); - } else { - // Confirmed safe to delete - if (exists) { - const teardownResult = await runTeardown({ - mainRepoPath: project.mainRepoPath, - worktreePath: worktree.path, - workspaceName: worktree.branch, - projectId: project.id, - }); - if (!teardownResult.success) { - if (input.force) { - console.warn( - `[worktree/delete] Teardown failed but force=true, continuing deletion:`, - teardownResult.error, - ); - } else { - return { - success: false, - error: `Teardown failed: ${teardownResult.error}`, - output: teardownResult.output, - }; - } + if (!teardownResult.success) { + if (input.force) { + console.warn( + `[worktree/delete] Teardown failed but force=true, continuing deletion:`, + teardownResult.error, + ); + } else { + return { + success: false, + error: `Teardown failed: ${teardownResult.error}`, + output: teardownResult.output, + }; } } + } - if (exists) { - const removeResult = await removeWorktreeFromDisk({ - mainRepoPath: project.mainRepoPath, - worktreePath: worktree.path, - }); - if (!removeResult.success) { - return removeResult; - } - } else { - console.warn( - `Worktree ${worktree.path} not found in git, skipping removal`, - ); + if (exists) { + const removeResult = await removeWorktreeFromDisk({ + mainRepoPath: project.mainRepoPath, + worktreePath: worktree.path, + }); + if (!removeResult.success) { + return removeResult; } + } else { + console.warn( + `Worktree ${worktree.path} not found in git, skipping removal`, + ); } - } else { - console.log( - `[worktree/delete] Skipping disk deletion for external worktree at ${worktree.path}`, - ); } } finally { workspaceInitManager.releaseProjectLock(project.id);