Skip to content

Commit 7b5291b

Browse files
committed
fix: allow deleting worktrees that are even with main
- and add testing
1 parent 27df17f commit 7b5291b

File tree

3 files changed

+203
-36
lines changed

3 files changed

+203
-36
lines changed

src/git.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,15 @@ export async function createWorktree(
6060
}
6161

6262
export async function removeWorktree(
63+
projectPath: string,
6364
workspacePath: string,
6465
options: { force: boolean } = { force: false }
6566
): Promise<WorktreeResult> {
6667
try {
67-
// Remove the worktree
68-
await execAsync(`git worktree remove "${workspacePath}" ${options.force ? "--force" : ""}`);
68+
// Remove the worktree (from the main repository context)
69+
await execAsync(
70+
`git -C "${projectPath}" worktree remove "${workspacePath}" ${options.force ? "--force" : ""}`
71+
);
6972
return { success: true };
7073
} catch (error) {
7174
const message = error instanceof Error ? error.message : String(error);
@@ -120,7 +123,7 @@ export async function listWorktrees(projectPath: string): Promise<string[]> {
120123

121124
for (const line of lines) {
122125
if (line.startsWith("worktree ")) {
123-
const path = line.substring(9);
126+
const path = line.slice("worktree ".length);
124127
if (path !== projectPath) {
125128
// Exclude main worktree
126129
worktrees.push(path);
@@ -143,3 +146,27 @@ export async function isGitRepository(projectPath: string): Promise<boolean> {
143146
return false;
144147
}
145148
}
149+
150+
/**
151+
* Get the main repository path from a worktree path
152+
* @param worktreePath Path to a git worktree
153+
* @returns Path to the main repository, or null if not found
154+
*/
155+
export async function getMainWorktreeFromWorktree(worktreePath: string): Promise<string | null> {
156+
try {
157+
// Get the worktree list from the worktree itself
158+
const { stdout } = await execAsync(`git -C "${worktreePath}" worktree list --porcelain`);
159+
const lines = stdout.split("\n");
160+
161+
// The first worktree in the list is always the main worktree
162+
for (const line of lines) {
163+
if (line.startsWith("worktree ")) {
164+
return line.slice("worktree ".length);
165+
}
166+
}
167+
168+
return null;
169+
} catch {
170+
return null;
171+
}
172+
}

src/services/ipcMain.ts

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@ import { spawn } from "child_process";
33
import * as path from "path";
44
import * as fsPromises from "fs/promises";
55
import type { Config, ProjectConfig } from "@/config";
6-
import { createWorktree, removeWorktree, moveWorktree, pruneWorktrees } from "@/git";
6+
import {
7+
createWorktree,
8+
removeWorktree,
9+
moveWorktree,
10+
pruneWorktrees,
11+
getMainWorktreeFromWorktree,
12+
} from "@/git";
713
import { AIService } from "@/services/aiService";
814
import { HistoryService } from "@/services/historyService";
915
import { PartialService } from "@/services/partialService";
@@ -170,34 +176,30 @@ export class IpcMain {
170176

171177
ipcMain.handle(IPC_CHANNELS.WORKSPACE_REMOVE, async (_event, workspaceId: string) => {
172178
try {
173-
// Load current config
174-
const projectsConfig = this.config.loadConfigOrDefault();
175-
176-
// Find workspace path from config by generating IDs
177-
let workspacePath: string | null = null;
178-
let foundProjectPath: string | null = null;
179-
180-
for (const [projectPath, projectConfig] of projectsConfig.projects.entries()) {
181-
for (const workspace of projectConfig.workspaces) {
182-
const generatedId = this.config.generateWorkspaceId(projectPath, workspace.path);
183-
if (generatedId === workspaceId) {
184-
workspacePath = workspace.path;
185-
foundProjectPath = projectPath;
186-
break;
187-
}
188-
}
189-
if (workspacePath) break;
179+
// Get workspace path from metadata
180+
const metadataResult = await this.aiService.getWorkspaceMetadata(workspaceId);
181+
if (!metadataResult.success) {
182+
// If metadata doesn't exist, workspace is already gone - consider it success
183+
log.info(`Workspace ${workspaceId} metadata not found, considering removal successful`);
184+
return { success: true };
190185
}
191186

192-
// Remove git worktree if we found the path
193-
if (workspacePath) {
187+
const workspacePath = metadataResult.data.workspacePath;
188+
189+
// Get project path from the worktree itself
190+
const foundProjectPath = await getMainWorktreeFromWorktree(workspacePath);
191+
192+
// Remove git worktree if we found the project path
193+
if (foundProjectPath) {
194194
const worktreeExists = await fsPromises
195195
.access(workspacePath)
196196
.then(() => true)
197197
.catch(() => false);
198198

199199
if (worktreeExists) {
200-
const gitResult = await removeWorktree(workspacePath, { force: false });
200+
const gitResult = await removeWorktree(foundProjectPath, workspacePath, {
201+
force: false,
202+
});
201203
if (!gitResult.success) {
202204
const errorMessage = gitResult.error ?? "Unknown error";
203205
const normalizedError = errorMessage.toLowerCase();
@@ -207,21 +209,19 @@ export class IpcMain {
207209
normalizedError.includes("no such file");
208210

209211
if (looksLikeMissingWorktree) {
210-
if (foundProjectPath) {
211-
const pruneResult = await pruneWorktrees(foundProjectPath);
212-
if (!pruneResult.success) {
213-
log.info(
214-
`Failed to prune stale worktrees for ${foundProjectPath} after removeWorktree error: ${
215-
pruneResult.error ?? "unknown error"
216-
}`
217-
);
218-
}
212+
const pruneResult = await pruneWorktrees(foundProjectPath);
213+
if (!pruneResult.success) {
214+
log.info(
215+
`Failed to prune stale worktrees for ${foundProjectPath} after removeWorktree error: ${
216+
pruneResult.error ?? "unknown error"
217+
}`
218+
);
219219
}
220220
} else {
221221
return gitResult;
222222
}
223223
}
224-
} else if (foundProjectPath) {
224+
} else {
225225
const pruneResult = await pruneWorktrees(foundProjectPath);
226226
if (!pruneResult.success) {
227227
log.info(
@@ -239,8 +239,9 @@ export class IpcMain {
239239
return { success: false, error: aiResult.error };
240240
}
241241

242-
// Update config to remove the workspace
243-
if (foundProjectPath && workspacePath) {
242+
// Update config to remove the workspace (if it's in config)
243+
if (foundProjectPath) {
244+
const projectsConfig = this.config.loadConfigOrDefault();
244245
const projectConfig = projectsConfig.projects.get(foundProjectPath);
245246
if (projectConfig) {
246247
projectConfig.workspaces = projectConfig.workspaces.filter(
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
import { shouldRunIntegrationTests, createTestEnvironment, cleanupTestEnvironment } from "./setup";
2+
import { IPC_CHANNELS } from "../../src/constants/ipc-constants";
3+
import {
4+
createTempGitRepo,
5+
cleanupTempGitRepo,
6+
createWorkspace,
7+
generateBranchName,
8+
waitForFileNotExists,
9+
} from "./helpers";
10+
import * as fs from "fs/promises";
11+
12+
// Skip all tests if TEST_INTEGRATION is not set
13+
const describeIntegration = shouldRunIntegrationTests() ? describe : describe.skip;
14+
15+
describeIntegration("IpcMain remove workspace integration tests", () => {
16+
test.concurrent(
17+
"should successfully remove workspace and git worktree",
18+
async () => {
19+
const env = await createTestEnvironment();
20+
const tempGitRepo = await createTempGitRepo();
21+
22+
try {
23+
const branchName = generateBranchName("remove-test");
24+
25+
// Create a workspace
26+
const createResult = await createWorkspace(env.mockIpcRenderer, tempGitRepo, branchName);
27+
expect(createResult.success).toBe(true);
28+
if (!createResult.success) {
29+
throw new Error("Failed to create workspace");
30+
}
31+
32+
const { metadata } = createResult;
33+
const workspacePath = metadata.workspacePath;
34+
35+
// Verify the worktree exists
36+
const worktreeExistsBefore = await fs
37+
.access(workspacePath)
38+
.then(() => true)
39+
.catch(() => false);
40+
expect(worktreeExistsBefore).toBe(true);
41+
42+
// Remove the workspace
43+
const removeResult = await env.mockIpcRenderer.invoke(
44+
IPC_CHANNELS.WORKSPACE_REMOVE,
45+
metadata.id
46+
);
47+
expect(removeResult.success).toBe(true);
48+
49+
// Verify the worktree no longer exists
50+
const worktreeRemoved = await waitForFileNotExists(workspacePath, 5000);
51+
expect(worktreeRemoved).toBe(true);
52+
53+
// Verify workspace is no longer in config
54+
const config = env.config.loadConfigOrDefault();
55+
const project = config.projects.get(tempGitRepo);
56+
if (project) {
57+
const workspaceStillInConfig = project.workspaces.some((w) => w.path === workspacePath);
58+
expect(workspaceStillInConfig).toBe(false);
59+
}
60+
} finally {
61+
await cleanupTestEnvironment(env);
62+
await cleanupTempGitRepo(tempGitRepo);
63+
}
64+
},
65+
15000
66+
);
67+
68+
test.concurrent(
69+
"should handle removal of non-existent workspace gracefully",
70+
async () => {
71+
const env = await createTestEnvironment();
72+
73+
try {
74+
// Try to remove a workspace that doesn't exist
75+
const removeResult = await env.mockIpcRenderer.invoke(
76+
IPC_CHANNELS.WORKSPACE_REMOVE,
77+
"non-existent-workspace-id"
78+
);
79+
80+
// Should succeed (idempotent operation)
81+
expect(removeResult.success).toBe(true);
82+
} finally {
83+
await cleanupTestEnvironment(env);
84+
}
85+
},
86+
15000
87+
);
88+
89+
test.concurrent(
90+
"should handle removal when worktree directory is already deleted",
91+
async () => {
92+
const env = await createTestEnvironment();
93+
const tempGitRepo = await createTempGitRepo();
94+
95+
try {
96+
const branchName = generateBranchName("remove-deleted");
97+
98+
// Create a workspace
99+
const createResult = await createWorkspace(env.mockIpcRenderer, tempGitRepo, branchName);
100+
expect(createResult.success).toBe(true);
101+
if (!createResult.success) {
102+
throw new Error("Failed to create workspace");
103+
}
104+
105+
const { metadata } = createResult;
106+
const workspacePath = metadata.workspacePath;
107+
108+
// Manually delete the worktree directory (simulating external deletion)
109+
await fs.rm(workspacePath, { recursive: true, force: true });
110+
111+
// Verify it's gone
112+
const worktreeExists = await fs
113+
.access(workspacePath)
114+
.then(() => true)
115+
.catch(() => false);
116+
expect(worktreeExists).toBe(false);
117+
118+
// Remove the workspace via IPC - should succeed and prune stale worktree
119+
const removeResult = await env.mockIpcRenderer.invoke(
120+
IPC_CHANNELS.WORKSPACE_REMOVE,
121+
metadata.id
122+
);
123+
expect(removeResult.success).toBe(true);
124+
125+
// Verify workspace is no longer in config
126+
const config = env.config.loadConfigOrDefault();
127+
const project = config.projects.get(tempGitRepo);
128+
if (project) {
129+
const workspaceStillInConfig = project.workspaces.some((w) => w.path === workspacePath);
130+
expect(workspaceStillInConfig).toBe(false);
131+
}
132+
} finally {
133+
await cleanupTestEnvironment(env);
134+
await cleanupTempGitRepo(tempGitRepo);
135+
}
136+
},
137+
15000
138+
);
139+
});

0 commit comments

Comments
 (0)