Skip to content

Commit 8bf7432

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 28f0fb7 commit 8bf7432

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
@@ -478,6 +478,18 @@ export class LocalRuntime implements Runtime {
478478
// Compute workspace path using the canonical method
479479
const deletedPath = this.getWorkspacePath(projectPath, workspaceName);
480480

481+
// Check if directory exists - if not, operation is idempotent
482+
if (!fs.existsSync(deletedPath)) {
483+
// Directory already gone - prune stale git records (best effort)
484+
try {
485+
using pruneProc = execAsync(`git -C "${projectPath}" worktree prune`);
486+
await pruneProc.result;
487+
} catch {
488+
// Ignore prune errors - directory is already deleted, which is the goal
489+
}
490+
return { success: true, deletedPath };
491+
}
492+
481493
try {
482494
// Use git worktree remove to delete the worktree
483495
// This updates git's internal worktree metadata correctly
@@ -492,6 +504,25 @@ export class LocalRuntime implements Runtime {
492504
} catch (error) {
493505
const message = getErrorMessage(error);
494506

507+
// Check if the error is due to missing/stale worktree
508+
const normalizedError = message.toLowerCase();
509+
const looksLikeMissingWorktree =
510+
normalizedError.includes("not a working tree") ||
511+
normalizedError.includes("does not exist") ||
512+
normalizedError.includes("no such file");
513+
514+
if (looksLikeMissingWorktree) {
515+
// Worktree records are stale - prune them
516+
try {
517+
using pruneProc = execAsync(`git -C "${projectPath}" worktree prune`);
518+
await pruneProc.result;
519+
} catch {
520+
// Ignore prune errors
521+
}
522+
// Treat as success - workspace is gone (idempotent)
523+
return { success: true, deletedPath };
524+
}
525+
495526
// If force is enabled and git worktree remove failed, fall back to rm -rf
496527
// This handles edge cases like submodules where git refuses to delete
497528
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";
@@ -604,13 +602,14 @@ export class IpcMain {
604602
}
605603
} catch (copyError) {
606604
// If copy fails, clean up everything we created
607-
// 1. Remove the workspace
608-
await removeWorktree(foundProjectPath, newWorkspacePath);
605+
// 1. Remove the workspace using Runtime abstraction
606+
const cleanupRuntime = createRuntime({ type: "local", srcBaseDir: this.config.srcDir });
607+
await cleanupRuntime.deleteWorkspace(foundProjectPath, newName, true);
609608
// 2. Remove the session directory (may contain partial copies)
610609
try {
611610
await fsPromises.rm(newSessionDir, { recursive: true, force: true });
612611
} catch (cleanupError) {
613-
// Log but don't fail - worktree cleanup is more important
612+
// Log but don't fail - workspace cleanup is more important
614613
log.error(`Failed to clean up session dir ${newSessionDir}:`, cleanupError);
615614
}
616615
const message = copyError instanceof Error ? copyError.message : String(copyError);
@@ -1053,34 +1052,12 @@ export class IpcMain {
10531052
metadata.runtimeConfig ?? { type: "local", srcBaseDir: this.config.srcDir }
10541053
);
10551054

1056-
// Delegate deletion to runtime - it handles all path computation and existence checks
1055+
// Delegate deletion to runtime - it handles all path computation, existence checks, and pruning
10571056
const deleteResult = await runtime.deleteWorkspace(projectPath, metadata.name, options.force);
10581057

10591058
if (!deleteResult.success) {
1060-
const errorMessage = deleteResult.error;
1061-
const normalizedError = errorMessage.toLowerCase();
1062-
const looksLikeMissingWorktree =
1063-
normalizedError.includes("not a working tree") ||
1064-
normalizedError.includes("does not exist") ||
1065-
normalizedError.includes("no such file");
1066-
1067-
if (looksLikeMissingWorktree) {
1068-
// Worktree is already gone or stale - prune git records if this is a local worktree
1069-
if (metadata.runtimeConfig?.type !== "ssh") {
1070-
const pruneResult = await pruneWorktrees(projectPath);
1071-
if (!pruneResult.success) {
1072-
log.info(
1073-
`Failed to prune stale worktrees for ${projectPath} after deleteWorkspace error: ${
1074-
pruneResult.error ?? "unknown error"
1075-
}`
1076-
);
1077-
}
1078-
}
1079-
// Treat missing workspace as success (idempotent operation)
1080-
} else {
1081-
// Real error (e.g., dirty workspace without force) - return it
1082-
return { success: false, error: deleteResult.error };
1083-
}
1059+
// Real error (e.g., dirty workspace without force) - return it
1060+
return { success: false, error: deleteResult.error };
10841061
}
10851062

10861063
// Remove the workspace from AI service

0 commit comments

Comments
 (0)