From 551de5cb41e0b61c1c49a795901476b857fe1753 Mon Sep 17 00:00:00 2001 From: Ammar Date: Tue, 28 Oct 2025 15:39:45 +0000 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=A4=96=20Prevent=20SSH=20workspace=20?= =?UTF-8?q?deletion=20with=20unpushed=20refs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add unpushed refs check to SSHRuntime.deleteWorkspace() - Only checks when git remotes are configured (no remote = no concept of unpushed) - Rejects deletion when unpushed commits exist unless force=true - Optimize: combine all pre-deletion checks into single bash script - Reduces SSH round trips from 5 to 2 (60% reduction) - Add tests for unpushed refs protection (force=false and force=true cases) Generated with `cmux` --- tests/ipcMain/removeWorkspace.test.ts | 135 ++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/tests/ipcMain/removeWorkspace.test.ts b/tests/ipcMain/removeWorkspace.test.ts index 38af81f93d..5c480295a0 100644 --- a/tests/ipcMain/removeWorkspace.test.ts +++ b/tests/ipcMain/removeWorkspace.test.ts @@ -356,6 +356,141 @@ describeIntegration("Workspace deletion integration tests", () => { TEST_TIMEOUT ); + test.concurrent( + "should fail to delete SSH workspace with unpushed refs without force flag", + async () => { + // This test only applies to SSH runtime (local worktrees share .git so unpushed refs are safe) + if (type !== "ssh") { + return; + } + + 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 + ); + + test.concurrent( + "should delete SSH workspace with unpushed refs when force flag is set", + async () => { + // This test only applies to SSH runtime + if (type !== "ssh") { + return; + } + + 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 + ); + // Submodule tests only apply to local runtime (SSH doesn't use git worktrees) if (type === "local") { test.concurrent( From 93a575f4948107d058f77905a147bfb8a3911af8 Mon Sep 17 00:00:00 2001 From: Ammar Date: Tue, 28 Oct 2025 16:41:58 +0000 Subject: [PATCH 2/3] Refactor: move SSH-only tests outside matrix - Extract SSH-specific unpushed refs tests into separate describe block - Removes conditional type checks inside tests (cleaner test structure) - Test output now clearly separates SSH-only tests from matrix tests - Follows same pattern as local-only submodule tests --- tests/ipcMain/removeWorkspace.test.ts | 135 -------------------------- 1 file changed, 135 deletions(-) diff --git a/tests/ipcMain/removeWorkspace.test.ts b/tests/ipcMain/removeWorkspace.test.ts index 5c480295a0..38af81f93d 100644 --- a/tests/ipcMain/removeWorkspace.test.ts +++ b/tests/ipcMain/removeWorkspace.test.ts @@ -356,141 +356,6 @@ describeIntegration("Workspace deletion integration tests", () => { TEST_TIMEOUT ); - test.concurrent( - "should fail to delete SSH workspace with unpushed refs without force flag", - async () => { - // This test only applies to SSH runtime (local worktrees share .git so unpushed refs are safe) - if (type !== "ssh") { - return; - } - - 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 - ); - - test.concurrent( - "should delete SSH workspace with unpushed refs when force flag is set", - async () => { - // This test only applies to SSH runtime - if (type !== "ssh") { - return; - } - - 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 - ); - // Submodule tests only apply to local runtime (SSH doesn't use git worktrees) if (type === "local") { test.concurrent( From 78b6664b8309638d0d6acf3a6608f3e6a960f15a Mon Sep 17 00:00:00 2001 From: Ammar Date: Wed, 29 Oct 2025 02:04:04 +0000 Subject: [PATCH 3/3] Show unpushed commit details in SSH deletion error When SSH workspace deletion is blocked due to unpushed commits, now shows: - List of unpushed commits (up to 10) in git log --oneline format - Commit hashes and messages in the error modal - More specific warning text when unpushed commits are detected Implementation: - Capture git log output to stderr in deletion check script - Read stderr and include in error message (no parsing needed) - Update ForceDeleteModal to detect and show better message - Add test verifying commit details appear in error Users can now see exactly what they're about to lose before force deleting. Generated with `cmux` --- src/components/ForceDeleteModal.tsx | 7 ++- src/runtime/SSHRuntime.ts | 27 +++++++++++- tests/ipcMain/removeWorkspace.test.ts | 62 +++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 4 deletions(-) diff --git a/src/components/ForceDeleteModal.tsx b/src/components/ForceDeleteModal.tsx index b183dace6b..8b87b0e34a 100644 --- a/src/components/ForceDeleteModal.tsx +++ b/src/components/ForceDeleteModal.tsx @@ -60,8 +60,11 @@ export const ForceDeleteModal: React.FC = ({ This action cannot be undone - Force deleting will permanently remove the workspace and may discard uncommitted work or - lose data. + Force deleting will permanently remove the workspace and{" "} + {error.includes("unpushed commits:") + ? "discard the unpushed commits shown above" + : "may discard uncommitted work or lose data"} + . This action cannot be undone. diff --git a/src/runtime/SSHRuntime.ts b/src/runtime/SSHRuntime.ts index 1403a40e55..6ee0ae9321 100644 --- a/src/runtime/SSHRuntime.ts +++ b/src/runtime/SSHRuntime.ts @@ -984,7 +984,11 @@ export class SSHRuntime implements Runtime { 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 + unpushed=$(git log --branches --not --remotes --oneline) + if [ -n "$unpushed" ]; then + echo "$unpushed" | head -10 >&2 + exit 2 + fi fi exit 0 `; @@ -1012,9 +1016,28 @@ export class SSHRuntime implements Runtime { } if (checkExitCode === 2) { + // Read stderr which contains the unpushed commits output + 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(); + } + + const commitList = stderr.trim(); + const errorMsg = commitList + ? `Workspace contains unpushed commits:\n\n${commitList}` + : `Workspace contains unpushed commits. Use force flag to delete anyway.`; + return { success: false, - error: `Workspace contains unpushed commits. Use force flag to delete anyway.`, + error: errorMsg, }; } diff --git a/tests/ipcMain/removeWorkspace.test.ts b/tests/ipcMain/removeWorkspace.test.ts index 38af81f93d..aa395b1079 100644 --- a/tests/ipcMain/removeWorkspace.test.ts +++ b/tests/ipcMain/removeWorkspace.test.ts @@ -614,5 +614,67 @@ describeIntegration("Workspace deletion integration tests", () => { }, TEST_TIMEOUT_SSH_MS ); + + test.concurrent( + "should include commit list in error for unpushed refs", + async () => { + const env = await createTestEnvironment(); + const tempGitRepo = await createTempGitRepo(); + + try { + const branchName = generateBranchName("delete-unpushed-details"); + 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) + await executeBash( + env, + workspaceId, + "git remote add origin https://github.com/fake/repo.git" + ); + + // Create multiple commits with descriptive messages + await executeBash(env, workspaceId, 'echo "1" > file1.txt'); + await executeBash(env, workspaceId, "git add file1.txt"); + await executeBash(env, workspaceId, 'git commit -m "First commit"'); + + await executeBash(env, workspaceId, 'echo "2" > file2.txt'); + await executeBash(env, workspaceId, "git add file2.txt"); + await executeBash(env, workspaceId, 'git commit -m "Second commit"'); + + // Attempt to delete + const deleteResult = await env.mockIpcRenderer.invoke( + IPC_CHANNELS.WORKSPACE_REMOVE, + workspaceId + ); + + // Should fail with error containing commit details + expect(deleteResult.success).toBe(false); + expect(deleteResult.error).toContain("unpushed commits:"); + expect(deleteResult.error).toContain("First commit"); + expect(deleteResult.error).toContain("Second commit"); + + // 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 + ); }); });