Skip to content

Commit 4616813

Browse files
committed
🤖 Remove ipcMain coupling with git worktrees
Problem: ipcMain directly imported and called git worktree functions (removeWorktree, pruneWorktrees), breaking the Runtime abstraction: - Created leaky abstraction with if (type !== 'ssh') checks - Exposed LocalRuntime implementation details (worktrees) to business logic - Inconsistent - some ops went through Runtime, others bypassed it Key insight: Worktrees are an internal concept of LocalRuntime. SSHRuntime uses plain directories. The Runtime interface should never expose worktree-specific operations. Solution: Make LocalRuntime.deleteWorkspace() fully idempotent and self-sufficient: 1. LocalRuntime.deleteWorkspace() now: - Checks if directory exists before attempting deletion - Prunes stale git records when directory is already gone - Handles 'not a working tree' errors gracefully by auto-pruning - Returns success for already-deleted workspaces (idempotent) 2. ipcMain cleanup paths now use runtime.deleteWorkspace(): - Line 608: Fork cleanup now calls runtime.deleteWorkspace(force=true) - Lines 1067-1078: Removed manual pruning logic entirely 3. Removed git imports from ipcMain: - Deleted removeWorktree and pruneWorktrees imports - Only kept branch query operations (getCurrentBranch, etc.) Benefits: ✅ Proper encapsulation - Worktree concerns stay in LocalRuntime ✅ No leaky abstractions - Removed if (type !== 'ssh') check from ipcMain ✅ Consistent - All workspace mutations go through Runtime interface ✅ Idempotent - deleteWorkspace() succeeds on already-deleted workspaces ✅ Zero Runtime interface changes - Solution internal to LocalRuntime Result: Clean architecture, no git coupling in ipcMain, all tests pass. _Generated with `cmux`_
1 parent d4fd1aa commit 4616813

File tree

2 files changed

+38
-30
lines changed

2 files changed

+38
-30
lines changed

src/runtime/LocalRuntime.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,18 @@ export class LocalRuntime implements Runtime {
488488
// Compute workspace path using the canonical method
489489
const deletedPath = this.getWorkspacePath(projectPath, workspaceName);
490490

491+
// Check if directory exists - if not, operation is idempotent
492+
if (!fs.existsSync(deletedPath)) {
493+
// Directory already gone - prune stale git records (best effort)
494+
try {
495+
using pruneProc = execAsync(`git -C "${projectPath}" worktree prune`);
496+
await pruneProc.result;
497+
} catch {
498+
// Ignore prune errors - directory is already deleted, which is the goal
499+
}
500+
return { success: true, deletedPath };
501+
}
502+
491503
try {
492504
// Use git worktree remove to delete the worktree
493505
// This updates git's internal worktree metadata correctly
@@ -502,6 +514,25 @@ export class LocalRuntime implements Runtime {
502514
} catch (error) {
503515
const message = getErrorMessage(error);
504516

517+
// Check if the error is due to missing/stale worktree
518+
const normalizedError = message.toLowerCase();
519+
const looksLikeMissingWorktree =
520+
normalizedError.includes("not a working tree") ||
521+
normalizedError.includes("does not exist") ||
522+
normalizedError.includes("no such file");
523+
524+
if (looksLikeMissingWorktree) {
525+
// Worktree records are stale - prune them
526+
try {
527+
using pruneProc = execAsync(`git -C "${projectPath}" worktree prune`);
528+
await pruneProc.result;
529+
} catch {
530+
// Ignore prune errors
531+
}
532+
// Treat as success - workspace is gone (idempotent)
533+
return { success: true, deletedPath };
534+
}
535+
505536
// If force is enabled and git worktree remove failed, fall back to rm -rf
506537
// This handles edge cases like submodules where git refuses to delete
507538
if (force) {

src/services/ipcMain.ts

Lines changed: 7 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ import {
1010
listLocalBranches,
1111
detectDefaultTrunkBranch,
1212
getCurrentBranch,
13-
removeWorktree,
14-
pruneWorktrees,
1513
} from "@/git";
1614
import { AIService } from "@/services/aiService";
1715
import { HistoryService } from "@/services/historyService";
@@ -628,13 +626,14 @@ export class IpcMain {
628626
}
629627
} catch (copyError) {
630628
// If copy fails, clean up everything we created
631-
// 1. Remove the workspace
632-
await removeWorktree(foundProjectPath, newWorkspacePath);
629+
// 1. Remove the workspace using Runtime abstraction
630+
const cleanupRuntime = createRuntime({ type: "local", srcBaseDir: this.config.srcDir });
631+
await cleanupRuntime.deleteWorkspace(foundProjectPath, newName, true);
633632
// 2. Remove the session directory (may contain partial copies)
634633
try {
635634
await fsPromises.rm(newSessionDir, { recursive: true, force: true });
636635
} catch (cleanupError) {
637-
// Log but don't fail - worktree cleanup is more important
636+
// Log but don't fail - workspace cleanup is more important
638637
log.error(`Failed to clean up session dir ${newSessionDir}:`, cleanupError);
639638
}
640639
const message = copyError instanceof Error ? copyError.message : String(copyError);
@@ -1077,34 +1076,12 @@ export class IpcMain {
10771076
metadata.runtimeConfig ?? { type: "local", srcBaseDir: this.config.srcDir }
10781077
);
10791078

1080-
// Delegate deletion to runtime - it handles all path computation and existence checks
1079+
// Delegate deletion to runtime - it handles all path computation, existence checks, and pruning
10811080
const deleteResult = await runtime.deleteWorkspace(projectPath, metadata.name, options.force);
10821081

10831082
if (!deleteResult.success) {
1084-
const errorMessage = deleteResult.error;
1085-
const normalizedError = errorMessage.toLowerCase();
1086-
const looksLikeMissingWorktree =
1087-
normalizedError.includes("not a working tree") ||
1088-
normalizedError.includes("does not exist") ||
1089-
normalizedError.includes("no such file");
1090-
1091-
if (looksLikeMissingWorktree) {
1092-
// Worktree is already gone or stale - prune git records if this is a local worktree
1093-
if (metadata.runtimeConfig?.type !== "ssh") {
1094-
const pruneResult = await pruneWorktrees(projectPath);
1095-
if (!pruneResult.success) {
1096-
log.info(
1097-
`Failed to prune stale worktrees for ${projectPath} after deleteWorkspace error: ${
1098-
pruneResult.error ?? "unknown error"
1099-
}`
1100-
);
1101-
}
1102-
}
1103-
// Treat missing workspace as success (idempotent operation)
1104-
} else {
1105-
// Real error (e.g., dirty workspace without force) - return it
1106-
return { success: false, error: deleteResult.error };
1107-
}
1083+
// Real error (e.g., dirty workspace without force) - return it
1084+
return { success: false, error: deleteResult.error };
11081085
}
11091086

11101087
// Remove the workspace from AI service

0 commit comments

Comments
 (0)