Skip to content

Commit 298a424

Browse files
committed
fix: detect squash-merged branches for SSH workspace deletion
SSH workspaces required force deletion after squash-merging PRs because the check used 'git log --branches --not --remotes' which always shows commits for squash-merged branches (original commits differ from the squash commit SHA). Added content-based comparison when unpushed commits are detected: - Fetch latest default branch from origin - Get files changed on branch since merge-base - Compare each file's content between HEAD and origin/$DEFAULT - If all files match, treat as merged (squash-merge case) Also prefer origin/main or origin/master over origin/HEAD for default branch detection, since origin/HEAD can point to feature branches in some repo configurations. Includes integration tests for both squash-merge detection and ensuring genuinely unmerged content is still blocked.
1 parent d06a4a2 commit 298a424

File tree

2 files changed

+272
-9
lines changed

2 files changed

+272
-9
lines changed

src/node/runtime/SSHRuntime.ts

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,23 +1087,59 @@ export class SSHRuntime implements Runtime {
10871087
# Get current branch for better error messaging
10881088
BRANCH=$(git rev-parse --abbrev-ref HEAD 2>/dev/null)
10891089
1090-
# Get default branch (try origin/HEAD, fallback to main, then master)
1091-
DEFAULT=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')
1092-
if [ -z "$DEFAULT" ]; then
1093-
if git rev-parse --verify origin/main >/dev/null 2>&1; then
1094-
DEFAULT="main"
1095-
elif git rev-parse --verify origin/master >/dev/null 2>&1; then
1096-
DEFAULT="master"
1090+
# Get default branch (prefer main/master over origin/HEAD since origin/HEAD
1091+
# might point to a feature branch in some setups)
1092+
if git rev-parse --verify origin/main >/dev/null 2>&1; then
1093+
DEFAULT="main"
1094+
elif git rev-parse --verify origin/master >/dev/null 2>&1; then
1095+
DEFAULT="master"
1096+
else
1097+
# Fallback to origin/HEAD if main/master don't exist
1098+
DEFAULT=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')
1099+
fi
1100+
1101+
# Check for squash-merge: if all changed files match origin/$DEFAULT, content is merged
1102+
if [ -n "$DEFAULT" ]; then
1103+
# Fetch latest to ensure we have current remote state
1104+
git fetch origin "$DEFAULT" --quiet 2>/dev/null || true
1105+
1106+
# Get merge-base between current branch and default
1107+
MERGE_BASE=$(git merge-base "origin/$DEFAULT" HEAD 2>/dev/null)
1108+
if [ -n "$MERGE_BASE" ]; then
1109+
# Get files changed on this branch since fork point
1110+
CHANGED_FILES=$(git diff --name-only "$MERGE_BASE" HEAD 2>/dev/null)
1111+
1112+
if [ -n "$CHANGED_FILES" ]; then
1113+
# Check if all changed files match what's in origin/$DEFAULT
1114+
ALL_MERGED=true
1115+
while IFS= read -r f; do
1116+
# Compare file content between HEAD and origin/$DEFAULT
1117+
# If file doesn't exist in one but exists in other, they differ
1118+
if ! git diff --quiet "HEAD:$f" "origin/$DEFAULT:$f" 2>/dev/null; then
1119+
ALL_MERGED=false
1120+
break
1121+
fi
1122+
done <<< "$CHANGED_FILES"
1123+
1124+
if $ALL_MERGED; then
1125+
# All changes are in default branch - safe to delete (squash-merge case)
1126+
exit 0
1127+
fi
1128+
else
1129+
# No changed files means nothing to merge - safe to delete
1130+
exit 0
1131+
fi
10971132
fi
10981133
fi
10991134
1100-
# If we have both branch and default, use show-branch for better output
1135+
# If we get here, there are real unpushed changes
1136+
# Show helpful output for debugging
11011137
if [ -n "$BRANCH" ] && [ -n "$DEFAULT" ] && git show-branch "$BRANCH" "origin/$DEFAULT" >/dev/null 2>&1; then
11021138
echo "Branch status compared to origin/$DEFAULT:" >&2
11031139
echo "" >&2
11041140
git show-branch "$BRANCH" "origin/$DEFAULT" 2>&1 | head -20 >&2
11051141
echo "" >&2
1106-
echo "Note: If your PR was squash-merged, these commits are already in origin/$DEFAULT and safe to delete." >&2
1142+
echo "Note: Branch has changes not yet in origin/$DEFAULT." >&2
11071143
else
11081144
# Fallback to just showing the commit list
11091145
echo "$unpushed" | head -10 >&2

tests/ipcMain/removeWorkspace.test.ts

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -799,5 +799,232 @@ describeIntegration("Workspace deletion integration tests", () => {
799799
},
800800
TEST_TIMEOUT_SSH_MS
801801
);
802+
803+
test.concurrent(
804+
"should allow deletion of squash-merged branches without force flag",
805+
async () => {
806+
const env = await createTestEnvironment();
807+
const tempGitRepo = await createTempGitRepo();
808+
809+
try {
810+
const branchName = generateBranchName("squash-merge-test");
811+
const runtimeConfig = getRuntimeConfig(branchName);
812+
const { workspaceId } = await createWorkspaceWithInit(
813+
env,
814+
tempGitRepo,
815+
branchName,
816+
runtimeConfig,
817+
true, // waitForInit
818+
true // isSSH
819+
);
820+
821+
// Configure git for committing
822+
await executeBash(env, workspaceId, 'git config user.email "test@example.com"');
823+
await executeBash(env, workspaceId, 'git config user.name "Test User"');
824+
825+
// Get the current workspace path (inside SSH container)
826+
const pwdResult = await executeBash(env, workspaceId, "pwd");
827+
const workspacePath = pwdResult.output.trim();
828+
829+
// Create a bare repo inside the SSH container to act as "origin"
830+
// This avoids issues with host paths not being accessible in container
831+
const originPath = `${workspacePath}/../.test-origin-${branchName}`;
832+
await executeBash(env, workspaceId, `git clone --bare . "${originPath}"`);
833+
834+
// Point origin to the bare repo (add if doesn't exist, set-url if it does)
835+
await executeBash(
836+
env,
837+
workspaceId,
838+
`git remote get-url origin >/dev/null 2>&1 && git remote set-url origin "${originPath}" || git remote add origin "${originPath}"`
839+
);
840+
841+
// Create feature commits on the branch
842+
await executeBash(env, workspaceId, 'echo "feature1" > feature.txt');
843+
await executeBash(env, workspaceId, "git add feature.txt");
844+
await executeBash(env, workspaceId, 'git commit -m "Feature commit 1"');
845+
846+
await executeBash(env, workspaceId, 'echo "feature2" >> feature.txt');
847+
await executeBash(env, workspaceId, "git add feature.txt");
848+
await executeBash(env, workspaceId, 'git commit -m "Feature commit 2"');
849+
850+
// Get the feature branch's final file content
851+
const featureContent = await executeBash(env, workspaceId, "cat feature.txt");
852+
853+
// Simulate squash-merge: create a temp worktree, add the squash commit to main, push
854+
// We need to work around bare repo limitations by using a temp checkout
855+
const tempCheckoutPath = `${workspacePath}/../.test-temp-checkout-${branchName}`;
856+
await executeBash(
857+
env,
858+
workspaceId,
859+
`git clone "${originPath}" "${tempCheckoutPath}" && ` +
860+
`cd "${tempCheckoutPath}" && ` +
861+
`git config user.email "test@example.com" && ` +
862+
`git config user.name "Test User" && ` +
863+
// Checkout main (or master, depending on git version)
864+
`(git checkout main 2>/dev/null || git checkout master) && ` +
865+
// Create squash commit with same content (use printf '%s\n' to match echo's newline)
866+
`printf '%s\\n' '${featureContent.output.trim().replace(/'/g, "'\\''")}' > feature.txt && ` +
867+
`git add feature.txt && ` +
868+
`git commit -m "Squash: Feature commits" && ` +
869+
`git push origin HEAD`
870+
);
871+
872+
// Cleanup temp checkout
873+
await executeBash(env, workspaceId, `rm -rf "${tempCheckoutPath}"`);
874+
875+
// Fetch the updated origin in the workspace
876+
await executeBash(env, workspaceId, "git fetch origin");
877+
878+
// Verify we have unpushed commits (branch commits are not ancestors of origin/main)
879+
const logResult = await executeBash(
880+
env,
881+
workspaceId,
882+
"git log --branches --not --remotes --oneline"
883+
);
884+
// Should show commits since our branch commits != squash commit SHA
885+
expect(logResult.output.trim()).not.toBe("");
886+
887+
// Now attempt deletion without force - should succeed because content matches
888+
const deleteResult = await env.mockIpcRenderer.invoke(
889+
IPC_CHANNELS.WORKSPACE_REMOVE,
890+
workspaceId
891+
);
892+
893+
// Should succeed - squash-merge detection should recognize content is in main
894+
if (!deleteResult.success) {
895+
throw new Error(
896+
`Expected deletion to succeed but it failed.\nError: ${deleteResult.error}`
897+
);
898+
}
899+
expect(deleteResult.success).toBe(true);
900+
901+
// Cleanup the bare repo we created
902+
// Note: This runs after workspace is deleted, may fail if path is gone
903+
try {
904+
using cleanupProc = execAsync(`rm -rf "${originPath}"`);
905+
await cleanupProc.result;
906+
} catch {
907+
// Ignore cleanup errors
908+
}
909+
910+
// Verify workspace was removed from config
911+
const config = env.config.loadConfigOrDefault();
912+
const project = config.projects.get(tempGitRepo);
913+
if (project) {
914+
const stillInConfig = project.workspaces.some((w) => w.id === workspaceId);
915+
expect(stillInConfig).toBe(false);
916+
}
917+
} finally {
918+
await cleanupTestEnvironment(env);
919+
await cleanupTempGitRepo(tempGitRepo);
920+
}
921+
},
922+
TEST_TIMEOUT_SSH_MS
923+
);
924+
925+
test.concurrent(
926+
"should block deletion when branch has genuinely unmerged content",
927+
async () => {
928+
const env = await createTestEnvironment();
929+
const tempGitRepo = await createTempGitRepo();
930+
931+
try {
932+
const branchName = generateBranchName("unmerged-content-test");
933+
const runtimeConfig = getRuntimeConfig(branchName);
934+
const { workspaceId } = await createWorkspaceWithInit(
935+
env,
936+
tempGitRepo,
937+
branchName,
938+
runtimeConfig,
939+
true, // waitForInit
940+
true // isSSH
941+
);
942+
943+
// Configure git for committing
944+
await executeBash(env, workspaceId, 'git config user.email "test@example.com"');
945+
await executeBash(env, workspaceId, 'git config user.name "Test User"');
946+
947+
// Get the current workspace path (inside SSH container)
948+
const pwdResult = await executeBash(env, workspaceId, "pwd");
949+
const workspacePath = pwdResult.output.trim();
950+
951+
// Create a bare repo inside the SSH container to act as "origin"
952+
const originPath = `${workspacePath}/../.test-origin-${branchName}`;
953+
await executeBash(env, workspaceId, `git clone --bare . "${originPath}"`);
954+
955+
// Point origin to the bare repo (add if doesn't exist, set-url if it does)
956+
await executeBash(
957+
env,
958+
workspaceId,
959+
`git remote get-url origin >/dev/null 2>&1 && git remote set-url origin "${originPath}" || git remote add origin "${originPath}"`
960+
);
961+
962+
// Create feature commits with unique content (not in origin)
963+
await executeBash(env, workspaceId, 'echo "unique-unmerged-content" > unique.txt');
964+
await executeBash(env, workspaceId, "git add unique.txt");
965+
await executeBash(env, workspaceId, 'git commit -m "Unique commit"');
966+
967+
// Fetch origin (main doesn't have our content - we didn't push)
968+
await executeBash(env, workspaceId, "git fetch origin");
969+
970+
// Debug: verify the setup is correct before attempting delete
971+
const debugBranches = await executeBash(env, workspaceId, "git branch -a");
972+
const debugLog = await executeBash(
973+
env,
974+
workspaceId,
975+
"git log --oneline --all | head -10"
976+
);
977+
const debugRemote = await executeBash(env, workspaceId, "git remote -v");
978+
console.log(
979+
"DEBUG unmerged test - branches:",
980+
debugBranches.output,
981+
"log:",
982+
debugLog.output,
983+
"remote:",
984+
debugRemote.output
985+
);
986+
987+
// Attempt deletion without force - should fail because content differs
988+
const deleteResult = await env.mockIpcRenderer.invoke(
989+
IPC_CHANNELS.WORKSPACE_REMOVE,
990+
workspaceId
991+
);
992+
993+
// Should fail - genuinely unmerged content
994+
if (deleteResult.success) {
995+
// Provide debug info if test would fail
996+
throw new Error(
997+
`Expected deletion to fail but it succeeded.\n` +
998+
`Branches: ${debugBranches.output}\n` +
999+
`Log: ${debugLog.output}\n` +
1000+
`Remote: ${debugRemote.output}\n` +
1001+
`Result: ${JSON.stringify(deleteResult)}`
1002+
);
1003+
}
1004+
expect(deleteResult.error).toMatch(/unpushed|changes/i);
1005+
1006+
// Verify workspace still exists
1007+
const stillExists = await workspaceExists(env, workspaceId);
1008+
expect(stillExists).toBe(true);
1009+
1010+
// Cleanup: force delete
1011+
await env.mockIpcRenderer.invoke(IPC_CHANNELS.WORKSPACE_REMOVE, workspaceId, {
1012+
force: true,
1013+
});
1014+
1015+
// Cleanup the bare repo
1016+
try {
1017+
using cleanupProc = execAsync(`rm -rf "${originPath}"`);
1018+
await cleanupProc.result;
1019+
} catch {
1020+
// Ignore cleanup errors
1021+
}
1022+
} finally {
1023+
await cleanupTestEnvironment(env);
1024+
await cleanupTempGitRepo(tempGitRepo);
1025+
}
1026+
},
1027+
TEST_TIMEOUT_SSH_MS
1028+
);
8021029
});
8031030
});

0 commit comments

Comments
 (0)