🤖 Remove ipcMain coupling with git worktrees #457
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ammario
approved these changes
Oct 27, 2025
The tests were failing because a shared global workspace directory caused workspace collisions and leftover state from previous test runs. Root cause: - mockConfig used a fixed srcDir path shared across all tests - Multiple tests created workspaces with the same names - Failed runs left directories behind causing 'Workspace already exists' errors Solution: - Created createMockConfig(tempDir) helper to generate unique configs per test - Each test now gets its own isolated workspace directory - Removed debug logging All 807 tests now pass.
removeWorktreeSafe(), isWorktreeClean(), and hasSubmodules() were orphaned during the Runtime architecture refactoring (commit 95c0a14, Oct 25). Production now uses Runtime.deleteWorkspace() for all workspace deletion. These functions existed only in tests and provided no value. Changes: - Removed 153 lines of dead code from gitService.ts (189 → 36 lines) - Removed 265 lines of dead tests (275 → 10 lines) - Total: 418 lines removed - Tests: 807 → 796 (removed 11 tests for dead functions) The active functions (removeWorktree, pruneWorktrees) are preserved as they're used by ipcMain for cleanup operations. _Generated with `cmux`_
Eliminated duplicate WorktreeResult interface and consolidated all git worktree operations into a single file for better organization. Changes: - Moved removeWorktree() and pruneWorktrees() from gitService.ts to git.ts - Removed duplicate WorktreeResult interface (was in both files) - Deleted src/services/gitService.ts (36 lines) - Deleted src/services/gitService.test.ts (10 lines) - Updated ipcMain.ts to import from @/git instead of gitService git.ts now exports all git operations: - listLocalBranches() - getCurrentBranch() - detectDefaultTrunkBranch() - createWorktree() - getMainWorktreeFromWorktree() - removeWorktree() [moved from gitService] - pruneWorktrees() [moved from gitService] All 796 tests pass, type checking passes. _Generated with `cmux`_
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`_
Complete removal of git coupling from ipcMain by adding fork as a
first-class Runtime operation.
Changes:
1. Runtime.ts:
- Added WorkspaceForkParams and WorkspaceForkResult types
- Added forkWorkspace() method to Runtime interface
- Fork creates new workspace branching from source workspace's branch
2. LocalRuntime.forkWorkspace():
- Detects source workspace's current branch via git
- Delegates to createWorkspace() with source branch as trunk
- Returns workspace path and source branch
3. SSHRuntime.forkWorkspace():
- Returns error: "Forking SSH workspaces is not yet implemented"
- Explains complexity: users expect remote filesystem state match,
not local project (which may differ)
- Suggests creating new workspace from desired branch instead
4. ipcMain WORKSPACE_FORK handler:
- Now calls runtime.forkWorkspace() instead of direct git operations
- Removed getCurrentBranch() and createWorktree() calls
- Uses same runtime config as source workspace
- Cleanup on failure uses runtime.deleteWorkspace()
- Session file copying remains direct fs ops (local-only resources)
5. Removed imports from ipcMain:
- getCurrentBranch - no longer called
- createWorktree - no longer called
- Only kept listLocalBranches and detectDefaultTrunkBranch
(project-level queries for UI, not workspace operations)
Benefits:
✅ Zero git coupling in ipcMain
✅ Fork is a proper Runtime abstraction
✅ Consistent with other workspace operations
✅ Clear error for SSH fork limitation
✅ All workspace operations go through Runtime
Result: Clean architecture, proper abstraction, all 796 tests passing.
_Generated with `cmux`_
- Replace fs.existsSync() with async fsPromises.access() - Remove async keyword from SSHRuntime.forkWorkspace() (returns Promise directly) - Fix prettier formatting
Both local and SSH deleteWorkspace are now idempotent - they return success for non-existent workspaces instead of error.
ammar-agent
force-pushed
the
git-test-fail
branch
from
6a3a621 to
19f1947
Compare
ammario
approved these changes
Oct 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
ipcMain.tsdirectly imported and called git worktree functions (removeWorktree,pruneWorktrees), breaking the Runtime abstraction:if (type !== 'ssh')checks in business logicKey Insight
Worktrees are an internal concept of LocalRuntime. SSHRuntime uses plain directories. The Runtime interface should never expose worktree-specific operations.
Solution
Made
LocalRuntime.deleteWorkspace()fully idempotent and self-sufficient, eliminating the need for manual worktree management in ipcMain.1. LocalRuntime.deleteWorkspace() - Now Idempotent (+31 lines)
2. ipcMain.ts - Clean Abstraction (-30 lines)
runtime.deleteWorkspace(force=true)pruneWorktrees()logic entirelyif (metadata.runtimeConfig?.type !== 'ssh')checkremoveWorktree,pruneWorktreesBenefits
✅ Proper encapsulation - Worktree concerns stay in LocalRuntime
✅ No leaky abstractions - Zero runtime-specific checks in ipcMain
✅ Consistent - All workspace mutations go through Runtime interface
✅ Idempotent - deleteWorkspace() succeeds on already-deleted workspaces
✅ Zero interface changes - No new Runtime methods needed
Testing
Context
This PR is part of a series cleaning up git utility organization:
Generated with
cmux