Skip to content

Commit ca16308

Browse files
ammar-agentammario
andauthored
🤖 Show unpushed commit details in SSH workspace deletion errors (#479)
## Problem When SSH workspace deletion is blocked due to unpushed commits, users only see: ``` Workspace contains unpushed commits. Use force flag to delete anyway. ``` This lacks critical context about what commits would be lost, making it hard to decide whether to force delete. ## Solution Enhanced the unpushed commits error to include the actual commit list (up to 10 commits) in standard git log format. **Implementation approach:** - Capture `git log --branches --not --remotes --oneline` output to stderr in the deletion check - Include stderr directly in error message (no parsing - simpler and more robust) - Update ForceDeleteModal warning text to be more specific when unpushed commits detected - Add test verifying commit details appear in error ## UX Impact **Before:** ``` Git Error Workspace contains unpushed commits. Use force flag to delete anyway. ``` **After:** ``` Git Error Workspace contains unpushed commits: a1b2c3d Fix critical bug in validation d4e5f6g Add user authentication h7i8j9k Update documentation ⚠️ This action cannot be undone Force deleting will permanently remove the workspace and discard the unpushed commits shown above. This action cannot be undone. ``` Users can now see exactly which commits they're about to lose, making informed decisions about force deletion. ## Testing - ✅ New test verifies commit list appears in error message - ✅ All 15 deletion tests pass (7 local + 5 SSH + 3 SSH-only) - ✅ Typecheck passes - ✅ Formatting correct _Generated with `cmux`_ --------- Co-authored-by: Ammar Bandukwala <ammar@ammar.io>
1 parent 0d5f997 commit ca16308

File tree

3 files changed

+92
-4
lines changed

3 files changed

+92
-4
lines changed

src/components/ForceDeleteModal.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,11 @@ export const ForceDeleteModal: React.FC<ForceDeleteModalProps> = ({
6060
<WarningBox>
6161
<WarningTitle>This action cannot be undone</WarningTitle>
6262
<WarningText>
63-
Force deleting will permanently remove the workspace and may discard uncommitted work or
64-
lose data.
63+
Force deleting will permanently remove the workspace and{" "}
64+
{error.includes("unpushed commits:")
65+
? "discard the unpushed commits shown above"
66+
: "may discard uncommitted work or lose data"}
67+
. This action cannot be undone.
6568
</WarningText>
6669
</WarningBox>
6770

src/runtime/SSHRuntime.ts

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -984,7 +984,11 @@ export class SSHRuntime implements Runtime {
984984
cd ${shescape.quote(deletedPath)} || exit 1
985985
git diff --quiet --exit-code && git diff --quiet --cached --exit-code || exit 1
986986
if git remote | grep -q .; then
987-
git log --branches --not --remotes --oneline | head -1 | grep -q . && exit 2
987+
unpushed=$(git log --branches --not --remotes --oneline)
988+
if [ -n "$unpushed" ]; then
989+
echo "$unpushed" | head -10 >&2
990+
exit 2
991+
fi
988992
fi
989993
exit 0
990994
`;
@@ -1012,9 +1016,28 @@ export class SSHRuntime implements Runtime {
10121016
}
10131017

10141018
if (checkExitCode === 2) {
1019+
// Read stderr which contains the unpushed commits output
1020+
const stderrReader = checkStream.stderr.getReader();
1021+
const decoder = new TextDecoder();
1022+
let stderr = "";
1023+
try {
1024+
while (true) {
1025+
const { done, value } = await stderrReader.read();
1026+
if (done) break;
1027+
stderr += decoder.decode(value, { stream: true });
1028+
}
1029+
} finally {
1030+
stderrReader.releaseLock();
1031+
}
1032+
1033+
const commitList = stderr.trim();
1034+
const errorMsg = commitList
1035+
? `Workspace contains unpushed commits:\n\n${commitList}`
1036+
: `Workspace contains unpushed commits. Use force flag to delete anyway.`;
1037+
10151038
return {
10161039
success: false,
1017-
error: `Workspace contains unpushed commits. Use force flag to delete anyway.`,
1040+
error: errorMsg,
10181041
};
10191042
}
10201043

tests/ipcMain/removeWorkspace.test.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,5 +614,67 @@ describeIntegration("Workspace deletion integration tests", () => {
614614
},
615615
TEST_TIMEOUT_SSH_MS
616616
);
617+
618+
test.concurrent(
619+
"should include commit list in error for unpushed refs",
620+
async () => {
621+
const env = await createTestEnvironment();
622+
const tempGitRepo = await createTempGitRepo();
623+
624+
try {
625+
const branchName = generateBranchName("delete-unpushed-details");
626+
const runtimeConfig = getRuntimeConfig(branchName);
627+
const { workspaceId } = await createWorkspaceWithInit(
628+
env,
629+
tempGitRepo,
630+
branchName,
631+
runtimeConfig,
632+
true, // waitForInit
633+
true // isSSH
634+
);
635+
636+
// Configure git for committing (SSH environment needs this)
637+
await executeBash(env, workspaceId, 'git config user.email "test@example.com"');
638+
await executeBash(env, workspaceId, 'git config user.name "Test User"');
639+
640+
// Add a fake remote (needed for unpushed check to work)
641+
await executeBash(
642+
env,
643+
workspaceId,
644+
"git remote add origin https://github.com/fake/repo.git"
645+
);
646+
647+
// Create multiple commits with descriptive messages
648+
await executeBash(env, workspaceId, 'echo "1" > file1.txt');
649+
await executeBash(env, workspaceId, "git add file1.txt");
650+
await executeBash(env, workspaceId, 'git commit -m "First commit"');
651+
652+
await executeBash(env, workspaceId, 'echo "2" > file2.txt');
653+
await executeBash(env, workspaceId, "git add file2.txt");
654+
await executeBash(env, workspaceId, 'git commit -m "Second commit"');
655+
656+
// Attempt to delete
657+
const deleteResult = await env.mockIpcRenderer.invoke(
658+
IPC_CHANNELS.WORKSPACE_REMOVE,
659+
workspaceId
660+
);
661+
662+
// Should fail with error containing commit details
663+
expect(deleteResult.success).toBe(false);
664+
expect(deleteResult.error).toContain("unpushed commits:");
665+
expect(deleteResult.error).toContain("First commit");
666+
expect(deleteResult.error).toContain("Second commit");
667+
668+
// Cleanup: force delete for cleanup
669+
await env.mockIpcRenderer.invoke(IPC_CHANNELS.WORKSPACE_REMOVE, workspaceId, {
670+
force: true,
671+
});
672+
} finally {
673+
await cleanupTestEnvironment(env);
674+
await cleanupTempGitRepo(tempGitRepo);
675+
}
676+
},
677+
TEST_TIMEOUT_SSH_MS
678+
);
617679
});
618680
});

0 commit comments

Comments
 (0)