Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
120 changes: 60 additions & 60 deletions apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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,
Expand All @@ -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)}`,
};
}
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)}`,
};
}
}),
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) => ({
Expand Down
52 changes: 50 additions & 2 deletions apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -811,14 +815,27 @@ 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;
isDetached: boolean;
isBare: boolean;
}

export async function listExternalWorktrees(
export async function listAllGitWorktrees(
mainRepoPath: string,
): Promise<ExternalWorktree[]> {
try {
Expand Down Expand Up @@ -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<ExternalWorktree[]> {
// 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion bun.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading