Skip to content

Commit c202072

Browse files
ammar-agentammar-agent
committed
🤖 Prevent SSH workspace deletion with unpushed refs
- 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`
1 parent ec66ae4 commit c202072

File tree

2 files changed

+186
-24
lines changed

2 files changed

+186
-24
lines changed

‎src/runtime/SSHRuntime.ts‎

Lines changed: 51 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -922,41 +922,68 @@ export class SSHRuntime implements Runtime {
922922
const deletedPath = this.getWorkspacePath(projectPath, workspaceName);
923923

924924
try {
925-
// Check if workspace exists first
926-
const checkExistStream = await this.exec(`test -d ${shescape.quote(deletedPath)}`, {
925+
// Combine all pre-deletion checks into a single bash script to minimize round trips
926+
// Exit codes: 0=ok to delete, 1=uncommitted changes, 2=unpushed commits, 3=doesn't exist
927+
const checkScript = force
928+
? // When force=true, only check existence
929+
`test -d ${shescape.quote(deletedPath)} || exit 3`
930+
: // When force=false, perform all safety checks
931+
`
932+
test -d ${shescape.quote(deletedPath)} || exit 3
933+
cd ${shescape.quote(deletedPath)} || exit 1
934+
git diff --quiet --exit-code && git diff --quiet --cached --exit-code || exit 1
935+
if git remote | grep -q .; then
936+
git log --branches --not --remotes --oneline | head -1 | grep -q . && exit 2
937+
fi
938+
exit 0
939+
`;
940+
941+
const checkStream = await this.exec(checkScript, {
927942
cwd: this.config.srcBaseDir,
928943
timeout: 10,
929944
});
930945

931-
await checkExistStream.stdin.close();
932-
const existsExitCode = await checkExistStream.exitCode;
946+
await checkStream.stdin.close();
947+
const checkExitCode = await checkStream.exitCode;
933948

934-
// If directory doesn't exist, deletion is a no-op (success)
935-
if (existsExitCode !== 0) {
949+
// Handle check results
950+
if (checkExitCode === 3) {
951+
// Directory doesn't exist - deletion is idempotent (success)
936952
return { success: true, deletedPath };
937953
}
938954

939-
// Check if workspace has uncommitted changes (unless force is true)
940-
if (!force) {
941-
// Check for uncommitted changes using git diff
942-
const checkStream = await this.exec(
943-
`cd ${shescape.quote(deletedPath)} && git diff --quiet --exit-code && git diff --quiet --cached --exit-code`,
944-
{
945-
cwd: this.config.srcBaseDir,
946-
timeout: 10,
947-
}
948-
);
955+
if (checkExitCode === 1) {
956+
return {
957+
success: false,
958+
error: `Workspace contains uncommitted changes. Use force flag to delete anyway.`,
959+
};
960+
}
949961

950-
await checkStream.stdin.close();
951-
const checkExitCode = await checkStream.exitCode;
962+
if (checkExitCode === 2) {
963+
return {
964+
success: false,
965+
error: `Workspace contains unpushed commits. Use force flag to delete anyway.`,
966+
};
967+
}
952968

953-
if (checkExitCode !== 0) {
954-
// Workspace has uncommitted changes
955-
return {
956-
success: false,
957-
error: `Workspace contains uncommitted changes. Use force flag to delete anyway.`,
958-
};
969+
if (checkExitCode !== 0) {
970+
// Unexpected error
971+
const stderrReader = checkStream.stderr.getReader();
972+
const decoder = new TextDecoder();
973+
let stderr = "";
974+
try {
975+
while (true) {
976+
const { done, value } = await stderrReader.read();
977+
if (done) break;
978+
stderr += decoder.decode(value, { stream: true });
979+
}
980+
} finally {
981+
stderrReader.releaseLock();
959982
}
983+
return {
984+
success: false,
985+
error: `Failed to check workspace state: ${stderr || `exit code ${checkExitCode}`}`,
986+
};
960987
}
961988

962989
// SSH runtimes use plain directories, not git worktrees

‎tests/ipcMain/removeWorkspace.test.ts‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,141 @@ describeIntegration("Workspace deletion integration tests", () => {
356356
TEST_TIMEOUT
357357
);
358358

359+
test.concurrent(
360+
"should fail to delete SSH workspace with unpushed refs without force flag",
361+
async () => {
362+
// This test only applies to SSH runtime (local worktrees share .git so unpushed refs are safe)
363+
if (type !== "ssh") {
364+
return;
365+
}
366+
367+
const env = await createTestEnvironment();
368+
const tempGitRepo = await createTempGitRepo();
369+
370+
try {
371+
const branchName = generateBranchName("delete-unpushed");
372+
const runtimeConfig = getRuntimeConfig(branchName);
373+
const { workspaceId } = await createWorkspaceWithInit(
374+
env,
375+
tempGitRepo,
376+
branchName,
377+
runtimeConfig,
378+
true, // waitForInit
379+
true // isSSH
380+
);
381+
382+
// Configure git for committing (SSH environment needs this)
383+
await executeBash(env, workspaceId, 'git config user.email "test@example.com"');
384+
await executeBash(env, workspaceId, 'git config user.name "Test User"');
385+
386+
// Add a fake remote (needed for unpushed check to work)
387+
// Without a remote, SSH workspaces have no concept of "unpushed" commits
388+
await executeBash(
389+
env,
390+
workspaceId,
391+
"git remote add origin https://github.com/fake/repo.git"
392+
);
393+
394+
// Create a commit in the workspace (unpushed)
395+
await executeBash(env, workspaceId, 'echo "new content" > newfile.txt');
396+
await executeBash(env, workspaceId, "git add newfile.txt");
397+
await executeBash(env, workspaceId, 'git commit -m "Unpushed commit"');
398+
399+
// Verify commit was created and working tree is clean
400+
const statusResult = await executeBash(env, workspaceId, "git status --porcelain");
401+
expect(statusResult.output.trim()).toBe(""); // Should be clean
402+
403+
// Attempt to delete without force should fail
404+
const deleteResult = await env.mockIpcRenderer.invoke(
405+
IPC_CHANNELS.WORKSPACE_REMOVE,
406+
workspaceId
407+
);
408+
expect(deleteResult.success).toBe(false);
409+
expect(deleteResult.error).toMatch(/unpushed.*commit|unpushed.*ref/i);
410+
411+
// Verify workspace still exists
412+
const stillExists = await workspaceExists(env, workspaceId);
413+
expect(stillExists).toBe(true);
414+
415+
// Cleanup: force delete for cleanup
416+
await env.mockIpcRenderer.invoke(IPC_CHANNELS.WORKSPACE_REMOVE, workspaceId, {
417+
force: true,
418+
});
419+
} finally {
420+
await cleanupTestEnvironment(env);
421+
await cleanupTempGitRepo(tempGitRepo);
422+
}
423+
},
424+
TEST_TIMEOUT
425+
);
426+
427+
test.concurrent(
428+
"should delete SSH workspace with unpushed refs when force flag is set",
429+
async () => {
430+
// This test only applies to SSH runtime
431+
if (type !== "ssh") {
432+
return;
433+
}
434+
435+
const env = await createTestEnvironment();
436+
const tempGitRepo = await createTempGitRepo();
437+
438+
try {
439+
const branchName = generateBranchName("delete-unpushed-force");
440+
const runtimeConfig = getRuntimeConfig(branchName);
441+
const { workspaceId } = await createWorkspaceWithInit(
442+
env,
443+
tempGitRepo,
444+
branchName,
445+
runtimeConfig,
446+
true, // waitForInit
447+
true // isSSH
448+
);
449+
450+
// Configure git for committing (SSH environment needs this)
451+
await executeBash(env, workspaceId, 'git config user.email "test@example.com"');
452+
await executeBash(env, workspaceId, 'git config user.name "Test User"');
453+
454+
// Add a fake remote (needed for unpushed check to work)
455+
// Without a remote, SSH workspaces have no concept of "unpushed" commits
456+
await executeBash(
457+
env,
458+
workspaceId,
459+
"git remote add origin https://github.com/fake/repo.git"
460+
);
461+
462+
// Create a commit in the workspace (unpushed)
463+
await executeBash(env, workspaceId, 'echo "new content" > newfile.txt');
464+
await executeBash(env, workspaceId, "git add newfile.txt");
465+
await executeBash(env, workspaceId, 'git commit -m "Unpushed commit"');
466+
467+
// Verify commit was created and working tree is clean
468+
const statusResult = await executeBash(env, workspaceId, "git status --porcelain");
469+
expect(statusResult.output.trim()).toBe(""); // Should be clean
470+
471+
// Delete with force should succeed
472+
const deleteResult = await env.mockIpcRenderer.invoke(
473+
IPC_CHANNELS.WORKSPACE_REMOVE,
474+
workspaceId,
475+
{ force: true }
476+
);
477+
expect(deleteResult.success).toBe(true);
478+
479+
// Verify workspace was removed from config
480+
const config = env.config.loadConfigOrDefault();
481+
const project = config.projects.get(tempGitRepo);
482+
if (project) {
483+
const stillInConfig = project.workspaces.some((w) => w.id === workspaceId);
484+
expect(stillInConfig).toBe(false);
485+
}
486+
} finally {
487+
await cleanupTestEnvironment(env);
488+
await cleanupTempGitRepo(tempGitRepo);
489+
}
490+
},
491+
TEST_TIMEOUT
492+
);
493+
359494
// Submodule tests only apply to local runtime (SSH doesn't use git worktrees)
360495
if (type === "local") {
361496
test.concurrent(

0 commit comments

Comments
 (0)