diff --git a/src/runtime/SSHRuntime.ts b/src/runtime/SSHRuntime.ts index ba10128f5..39dbe7f60 100644 --- a/src/runtime/SSHRuntime.ts +++ b/src/runtime/SSHRuntime.ts @@ -922,41 +922,68 @@ export class SSHRuntime implements Runtime { const deletedPath = this.getWorkspacePath(projectPath, workspaceName); try { - // Check if workspace exists first - const checkExistStream = await this.exec(`test -d ${shescape.quote(deletedPath)}`, { + // Combine all pre-deletion checks into a single bash script to minimize round trips + // Exit codes: 0=ok to delete, 1=uncommitted changes, 2=unpushed commits, 3=doesn't exist + const checkScript = force + ? // When force=true, only check existence + `test -d ${shescape.quote(deletedPath)} || exit 3` + : // When force=false, perform all safety checks + ` + test -d ${shescape.quote(deletedPath)} || exit 3 + cd ${shescape.quote(deletedPath)} || exit 1 + git diff --quiet --exit-code && git diff --quiet --cached --exit-code || exit 1 + if git remote | grep -q .; then + git log --branches --not --remotes --oneline | head -1 | grep -q . && exit 2 + fi + exit 0 + `; + + const checkStream = await this.exec(checkScript, { cwd: this.config.srcBaseDir, timeout: 10, }); - await checkExistStream.stdin.close(); - const existsExitCode = await checkExistStream.exitCode; + await checkStream.stdin.close(); + const checkExitCode = await checkStream.exitCode; - // If directory doesn't exist, deletion is a no-op (success) - if (existsExitCode !== 0) { + // Handle check results + if (checkExitCode === 3) { + // Directory doesn't exist - deletion is idempotent (success) return { success: true, deletedPath }; } - // Check if workspace has uncommitted changes (unless force is true) - if (!force) { - // Check for uncommitted changes using git diff - const checkStream = await this.exec( - `cd ${shescape.quote(deletedPath)} && git diff --quiet --exit-code && git diff --quiet --cached --exit-code`, - { - cwd: this.config.srcBaseDir, - timeout: 10, - } - ); + if (checkExitCode === 1) { + return { + success: false, + error: `Workspace contains uncommitted changes. Use force flag to delete anyway.`, + }; + } - await checkStream.stdin.close(); - const checkExitCode = await checkStream.exitCode; + if (checkExitCode === 2) { + return { + success: false, + error: `Workspace contains unpushed commits. Use force flag to delete anyway.`, + }; + } - if (checkExitCode !== 0) { - // Workspace has uncommitted changes - return { - success: false, - error: `Workspace contains uncommitted changes. Use force flag to delete anyway.`, - }; + if (checkExitCode !== 0) { + // Unexpected error + const stderrReader = checkStream.stderr.getReader(); + const decoder = new TextDecoder(); + let stderr = ""; + try { + while (true) { + const { done, value } = await stderrReader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + } + } finally { + stderrReader.releaseLock(); } + return { + success: false, + error: `Failed to check workspace state: ${stderr || `exit code ${checkExitCode}`}`, + }; } // SSH runtimes use plain directories, not git worktrees diff --git a/tests/ipcMain/removeWorkspace.test.ts b/tests/ipcMain/removeWorkspace.test.ts index c5a448de6..38af81f93 100644 --- a/tests/ipcMain/removeWorkspace.test.ts +++ b/tests/ipcMain/removeWorkspace.test.ts @@ -474,4 +474,145 @@ describeIntegration("Workspace deletion integration tests", () => { } } ); + + // SSH-specific tests (unpushed refs only matter for SSH, not local worktrees which share .git) + describe("SSH-only tests", () => { + const getRuntimeConfig = (branchName: string): RuntimeConfig | undefined => { + if (!sshConfig) { + throw new Error("SSH config not initialized"); + } + return { + type: "ssh", + host: `testuser@localhost`, + srcBaseDir: sshConfig.workdir, + identityFile: sshConfig.privateKeyPath, + port: sshConfig.port, + }; + }; + + test.concurrent( + "should fail to delete SSH workspace with unpushed refs without force flag", + async () => { + const env = await createTestEnvironment(); + const tempGitRepo = await createTempGitRepo(); + + try { + const branchName = generateBranchName("delete-unpushed"); + const runtimeConfig = getRuntimeConfig(branchName); + const { workspaceId } = await createWorkspaceWithInit( + env, + tempGitRepo, + branchName, + runtimeConfig, + true, // waitForInit + true // isSSH + ); + + // Configure git for committing (SSH environment needs this) + await executeBash(env, workspaceId, 'git config user.email "test@example.com"'); + await executeBash(env, workspaceId, 'git config user.name "Test User"'); + + // Add a fake remote (needed for unpushed check to work) + // Without a remote, SSH workspaces have no concept of "unpushed" commits + await executeBash( + env, + workspaceId, + "git remote add origin https://github.com/fake/repo.git" + ); + + // Create a commit in the workspace (unpushed) + await executeBash(env, workspaceId, 'echo "new content" > newfile.txt'); + await executeBash(env, workspaceId, "git add newfile.txt"); + await executeBash(env, workspaceId, 'git commit -m "Unpushed commit"'); + + // Verify commit was created and working tree is clean + const statusResult = await executeBash(env, workspaceId, "git status --porcelain"); + expect(statusResult.output.trim()).toBe(""); // Should be clean + + // Attempt to delete without force should fail + const deleteResult = await env.mockIpcRenderer.invoke( + IPC_CHANNELS.WORKSPACE_REMOVE, + workspaceId + ); + expect(deleteResult.success).toBe(false); + expect(deleteResult.error).toMatch(/unpushed.*commit|unpushed.*ref/i); + + // Verify workspace still exists + const stillExists = await workspaceExists(env, workspaceId); + expect(stillExists).toBe(true); + + // Cleanup: force delete for cleanup + await env.mockIpcRenderer.invoke(IPC_CHANNELS.WORKSPACE_REMOVE, workspaceId, { + force: true, + }); + } finally { + await cleanupTestEnvironment(env); + await cleanupTempGitRepo(tempGitRepo); + } + }, + TEST_TIMEOUT_SSH_MS + ); + + test.concurrent( + "should delete SSH workspace with unpushed refs when force flag is set", + async () => { + const env = await createTestEnvironment(); + const tempGitRepo = await createTempGitRepo(); + + try { + const branchName = generateBranchName("delete-unpushed-force"); + const runtimeConfig = getRuntimeConfig(branchName); + const { workspaceId } = await createWorkspaceWithInit( + env, + tempGitRepo, + branchName, + runtimeConfig, + true, // waitForInit + true // isSSH + ); + + // Configure git for committing (SSH environment needs this) + await executeBash(env, workspaceId, 'git config user.email "test@example.com"'); + await executeBash(env, workspaceId, 'git config user.name "Test User"'); + + // Add a fake remote (needed for unpushed check to work) + // Without a remote, SSH workspaces have no concept of "unpushed" commits + await executeBash( + env, + workspaceId, + "git remote add origin https://github.com/fake/repo.git" + ); + + // Create a commit in the workspace (unpushed) + await executeBash(env, workspaceId, 'echo "new content" > newfile.txt'); + await executeBash(env, workspaceId, "git add newfile.txt"); + await executeBash(env, workspaceId, 'git commit -m "Unpushed commit"'); + + // Verify commit was created and working tree is clean + const statusResult = await executeBash(env, workspaceId, "git status --porcelain"); + expect(statusResult.output.trim()).toBe(""); // Should be clean + + // Delete with force should succeed + const deleteResult = await env.mockIpcRenderer.invoke( + IPC_CHANNELS.WORKSPACE_REMOVE, + workspaceId, + { force: true } + ); + expect(deleteResult.success).toBe(true); + + // Verify workspace was removed from config + const config = env.config.loadConfigOrDefault(); + const project = config.projects.get(tempGitRepo); + if (project) { + const stillInConfig = project.workspaces.some((w) => w.id === workspaceId); + expect(stillInConfig).toBe(false); + } + } finally { + await cleanupTestEnvironment(env); + await cleanupTempGitRepo(tempGitRepo); + } + }, + TEST_TIMEOUT_SSH_MS + ); + }); });