diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts index 1f28701f2bd..5f48e68ac51 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts @@ -866,6 +866,7 @@ export const createCreateProcedures = () => { // 2. Import external worktrees (on disk, not tracked in DB) const allExternalWorktrees = await listExternalWorktrees( project.mainRepoPath, + project.id, ); const trackedPaths = new Set(projectWorktrees.map((wt) => wt.path)); 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..2f3740247c2 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts @@ -1,5 +1,4 @@ -import { existsSync, realpathSync } from "node:fs"; -import { resolve } from "node:path"; +import { existsSync } from "node:fs"; import type { SelectWorktree } from "@superset/local-db"; import { track } from "main/lib/analytics"; import { workspaceInitManager } from "main/lib/workspace-init-manager"; @@ -22,23 +21,11 @@ import { hasUncommittedChanges, hasUnpushedCommits, listExternalWorktrees, + normalizePath, worktreeExists, } from "../utils/git"; import { removeWorktreeFromDisk, runTeardown } from "../utils/teardown"; -/** - * Normalize a filesystem path for comparison. - * Uses realpathSync to resolve symlinks and get canonical path. - * Falls back to resolve if realpathSync fails (e.g., path doesn't exist). - */ -const normalizePath = (p: string): string => { - try { - return realpathSync(p); - } catch { - return resolve(p); - } -}; - export const createDeleteProcedures = () => { return router({ canDelete: publicProcedure @@ -267,12 +254,12 @@ 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) { + try { // Safety: Double-check it's not actually external (catches race conditions) + // Only delete from disk if the worktree is tracked in our database const externalWorktrees = await listExternalWorktrees( project.mainRepoPath, + project.id, ); const worktreePathNorm = normalizePath(worktree.path); const isActuallyExternal = externalWorktrees.some( @@ -281,16 +268,16 @@ export const createDeleteProcedures = () => { if (isActuallyExternal) { console.warn( - `[workspace/delete] Worktree at ${worktree.path} marked as created by Superset but found in external list - preserving as safety measure`, + `[workspace/delete] Worktree at ${worktree.path} 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", + reason: "external_worktree_detected", }); } else { - // Confirmed safe to delete + // Confirmed safe to delete - worktree is tracked in our DB const removeResult = await removeWorktreeFromDisk({ mainRepoPath: project.mainRepoPath, worktreePath: worktree.path, @@ -300,27 +287,33 @@ export const createDeleteProcedures = () => { return removeResult; } } - } else { - console.log( - `[workspace/delete] Skipping disk deletion for external worktree at ${worktree.path}`, - ); + } finally { + workspaceInitManager.releaseProjectLock(project.id); } - } finally { - workspaceInitManager.releaseProjectLock(project.id); - } - if (input.deleteLocalBranch && workspace.branch) { - try { - await deleteLocalBranch({ - mainRepoPath: project.mainRepoPath, - branch: workspace.branch, - }); - } catch (error) { - console.error( - `[workspace/delete] Branch cleanup failed (non-blocking):`, - error instanceof Error ? error.message : String(error), - ); + if (input.deleteLocalBranch && workspace.branch) { + try { + await deleteLocalBranch({ + mainRepoPath: project.mainRepoPath, + branch: workspace.branch, + }); + } catch (error) { + console.error( + `[workspace/delete] Branch cleanup failed (non-blocking):`, + error instanceof Error ? error.message : String(error), + ); + } } + } catch (error) { + console.error( + `[workspace/delete] Worktree deletion failed:`, + error instanceof Error ? error.message : String(error), + ); + clearWorkspaceDeletingStatus(input.id); + return { + success: false, + error: `Failed to delete worktree from disk: ${error instanceof Error ? error.message : String(error)}`, + }; } } @@ -481,32 +474,34 @@ export const createDeleteProcedures = () => { await workspaceInitManager.acquireProjectLock(project.id); try { - const exists = await worktreeExists( - project.mainRepoPath, - worktree.path, - ); + try { + const exists = await worktreeExists( + project.mainRepoPath, + 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) + // Only delete from disk if the worktree is tracked in our database const externalWorktrees = await listExternalWorktrees( project.mainRepoPath, + project.id, ); + const worktreePathNorm = normalizePath(worktree.path); const isActuallyExternal = externalWorktrees.some( - (wt) => wt.path === worktree.path, + (wt) => normalizePath(wt.path) === 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`, + `[worktree/delete] Worktree at ${worktree.path} 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", + reason: "external_worktree_detected", }); } else { - // Confirmed safe to delete + // Confirmed safe to delete - worktree is tracked in our DB if (exists) { const teardownResult = await runTeardown({ mainRepoPath: project.mainRepoPath, @@ -544,21 +539,26 @@ export const createDeleteProcedures = () => { ); } } - } else { - console.log( - `[worktree/delete] Skipping disk deletion for external worktree at ${worktree.path}`, - ); + } finally { + workspaceInitManager.releaseProjectLock(project.id); } - } finally { - workspaceInitManager.releaseProjectLock(project.id); - } - deleteWorktreeRecord(input.worktreeId); - hideProjectIfNoWorkspaces(worktree.projectId); + deleteWorktreeRecord(input.worktreeId); + hideProjectIfNoWorkspaces(worktree.projectId); - track("worktree_deleted", { worktree_id: input.worktreeId }); + track("worktree_deleted", { worktree_id: input.worktreeId }); - return { success: true }; + return { success: true }; + } catch (error) { + console.error( + `[worktree/delete] Worktree deletion failed:`, + error instanceof Error ? error.message : String(error), + ); + return { + success: false, + error: `Failed to delete worktree: ${error instanceof Error ? error.message : String(error)}`, + }; + } }), }); }; diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/external-worktree-import.test.ts b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/external-worktree-import.test.ts index 8ddf2edae7d..0904cbb822e 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/external-worktree-import.test.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/external-worktree-import.test.ts @@ -112,18 +112,18 @@ describe("External worktree detection and import", () => { expect(worktreeList).toContain("feature-external"); }); - test("listExternalWorktrees detects external worktree", async () => { + test("listAllGitWorktrees detects all git worktrees", async () => { // Create external worktree createExternalWorktree(mainRepoPath, "feature-test", externalWorktreePath); - // Import the listExternalWorktrees function - const { listExternalWorktrees } = await import("../utils/git"); + // Import the listAllGitWorktrees function + const { listAllGitWorktrees } = await import("../utils/git"); - // List external worktrees - const externalWorktrees = await listExternalWorktrees(mainRepoPath); + // List all git worktrees + const allWorktrees = await listAllGitWorktrees(mainRepoPath); // Find our external worktree - const found = externalWorktrees.find((wt) => wt.branch === "feature-test"); + const found = allWorktrees.find((wt) => wt.branch === "feature-test"); expect(found).toBeDefined(); expect(found?.path).toBe(externalWorktreePath); diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts index 07843a68cf6..8b7b6d3706f 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts @@ -296,22 +296,17 @@ export const createGitStatusProcedures = () => { return []; } - const allWorktrees = await listExternalWorktrees(project.mainRepoPath); - - const trackedWorktrees = localDb - .select({ path: worktrees.path }) - .from(worktrees) - .where(eq(worktrees.projectId, input.projectId)) - .all(); - const trackedPaths = new Set(trackedWorktrees.map((wt) => wt.path)); + const externalWorktrees = await listExternalWorktrees( + project.mainRepoPath, + input.projectId, + ); - return allWorktrees + return externalWorktrees .filter((wt) => { if (wt.path === project.mainRepoPath) return false; if (wt.isBare) return false; if (wt.isDetached) return false; if (!wt.branch) return false; - if (trackedPaths.has(wt.path)) return false; return true; }) .map((wt) => ({ diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts index 84033314620..517b583175d 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts @@ -1,10 +1,14 @@ import { execFile, spawn } from "node:child_process"; import { randomUUID } from "node:crypto"; +import { realpathSync } from "node:fs"; import { mkdir, rename } from "node:fs/promises"; import { dirname, join, resolve } from "node:path"; import { promisify } from "node:util"; import type { BranchPrefixMode } from "@superset/local-db"; +import { worktrees } from "@superset/local-db"; +import { eq } from "drizzle-orm"; import friendlyWords from "friendly-words"; +import { localDb } from "main/lib/local-db"; import { sanitizeAuthorPrefix, sanitizeBranchName, @@ -811,6 +815,19 @@ export async function worktreeExists( } } +/** + * Normalize a filesystem path for comparison. + * Uses realpathSync to resolve symlinks and get canonical path. + * Falls back to resolve if realpathSync fails (e.g., path doesn't exist). + */ +export const normalizePath = (p: string): string => { + try { + return realpathSync(p); + } catch { + return resolve(p); + } +}; + export interface ExternalWorktree { path: string; branch: string | null; @@ -818,7 +835,7 @@ export interface ExternalWorktree { isBare: boolean; } -export async function listExternalWorktrees( +export async function listAllGitWorktrees( mainRepoPath: string, ): Promise { try { @@ -859,11 +876,42 @@ export async function listExternalWorktrees( return result; } catch (error) { - console.error(`Failed to list external worktrees: ${error}`); + console.error(`Failed to list all git worktrees: ${error}`); throw error; } } +/** + * Lists worktrees that exist on disk but are NOT tracked in the database. + * This is the proper "external worktrees" - ones created outside of Superset. + * @param mainRepoPath - Path to the main repository + * @param projectId - Project ID to check against database records + * @returns Array of external worktrees (not tracked in DB) + */ +export async function listExternalWorktrees( + mainRepoPath: string, + projectId: string, +): Promise { + // Get all git worktrees + const allGitWorktrees = await listAllGitWorktrees(mainRepoPath); + + // Get all tracked worktree paths from database + const trackedWorktrees = localDb + .select({ path: worktrees.path }) + .from(worktrees) + .where(eq(worktrees.projectId, projectId)) + .all(); + + const trackedPaths = new Set( + trackedWorktrees.map((wt: { path: string }) => normalizePath(wt.path)), + ); + + // Filter to only worktrees NOT in database + return allGitWorktrees.filter( + (wt) => !trackedPaths.has(normalizePath(wt.path)), + ); +} + /** * Checks if a branch is already checked out in a worktree. * @param mainRepoPath - Path to the main repository diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-creation.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-creation.ts index 52d3f6b87c0..1106e5ecc29 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-creation.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-creation.ts @@ -110,7 +110,10 @@ export async function createWorkspaceFromExternalWorktree({ } // Check for external worktree (exists on disk but not tracked in DB) - const externalWorktrees = await listExternalWorktrees(project.mainRepoPath); + const externalWorktrees = await listExternalWorktrees( + project.mainRepoPath, + projectId, + ); // Filter candidates: exclude main repo, bare, and detached const candidates = externalWorktrees.filter( diff --git a/bun.lock b/bun.lock index 2b9a3d4371b..7a22a694213 100644 --- a/bun.lock +++ b/bun.lock @@ -110,7 +110,7 @@ }, "apps/desktop": { "name": "@superset/desktop", - "version": "1.3.2", + "version": "1.4.1", "dependencies": { "@ai-sdk/anthropic": "^3.0.43", "@ai-sdk/openai": "3.0.36",