From b80e01f4d62b5d7b2c7d97f0277904af8f065a03 Mon Sep 17 00:00:00 2001 From: Ammar Date: Tue, 2 Dec 2025 13:40:10 -0600 Subject: [PATCH] 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. --- src/node/runtime/SSHRuntime.ts | 54 +++++-- tests/ipcMain/removeWorkspace.test.ts | 196 ++++++++++++++++++++++++++ 2 files changed, 241 insertions(+), 9 deletions(-) diff --git a/src/node/runtime/SSHRuntime.ts b/src/node/runtime/SSHRuntime.ts index c78e3a7e37..9c624f10df 100644 --- a/src/node/runtime/SSHRuntime.ts +++ b/src/node/runtime/SSHRuntime.ts @@ -1087,23 +1087,59 @@ export class SSHRuntime implements Runtime { # Get current branch for better error messaging BRANCH=$(git rev-parse --abbrev-ref HEAD 2>/dev/null) - # Get default branch (try origin/HEAD, fallback to main, then master) - DEFAULT=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@') - if [ -z "$DEFAULT" ]; then - if git rev-parse --verify origin/main >/dev/null 2>&1; then - DEFAULT="main" - elif git rev-parse --verify origin/master >/dev/null 2>&1; then - DEFAULT="master" + # Get default branch (prefer main/master over origin/HEAD since origin/HEAD + # might point to a feature branch in some setups) + if git rev-parse --verify origin/main >/dev/null 2>&1; then + DEFAULT="main" + elif git rev-parse --verify origin/master >/dev/null 2>&1; then + DEFAULT="master" + else + # Fallback to origin/HEAD if main/master don't exist + DEFAULT=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@') + fi + + # Check for squash-merge: if all changed files match origin/$DEFAULT, content is merged + if [ -n "$DEFAULT" ]; then + # Fetch latest to ensure we have current remote state + git fetch origin "$DEFAULT" --quiet 2>/dev/null || true + + # Get merge-base between current branch and default + MERGE_BASE=$(git merge-base "origin/$DEFAULT" HEAD 2>/dev/null) + if [ -n "$MERGE_BASE" ]; then + # Get files changed on this branch since fork point + CHANGED_FILES=$(git diff --name-only "$MERGE_BASE" HEAD 2>/dev/null) + + if [ -n "$CHANGED_FILES" ]; then + # Check if all changed files match what's in origin/$DEFAULT + ALL_MERGED=true + while IFS= read -r f; do + # Compare file content between HEAD and origin/$DEFAULT + # If file doesn't exist in one but exists in other, they differ + if ! git diff --quiet "HEAD:$f" "origin/$DEFAULT:$f" 2>/dev/null; then + ALL_MERGED=false + break + fi + done <<< "$CHANGED_FILES" + + if $ALL_MERGED; then + # All changes are in default branch - safe to delete (squash-merge case) + exit 0 + fi + else + # No changed files means nothing to merge - safe to delete + exit 0 + fi fi fi - # If we have both branch and default, use show-branch for better output + # If we get here, there are real unpushed changes + # Show helpful output for debugging if [ -n "$BRANCH" ] && [ -n "$DEFAULT" ] && git show-branch "$BRANCH" "origin/$DEFAULT" >/dev/null 2>&1; then echo "Branch status compared to origin/$DEFAULT:" >&2 echo "" >&2 git show-branch "$BRANCH" "origin/$DEFAULT" 2>&1 | head -20 >&2 echo "" >&2 - echo "Note: If your PR was squash-merged, these commits are already in origin/$DEFAULT and safe to delete." >&2 + echo "Note: Branch has changes not yet in origin/$DEFAULT." >&2 else # Fallback to just showing the commit list echo "$unpushed" | head -10 >&2 diff --git a/tests/ipcMain/removeWorkspace.test.ts b/tests/ipcMain/removeWorkspace.test.ts index 89b0e8272b..6f07f794a6 100644 --- a/tests/ipcMain/removeWorkspace.test.ts +++ b/tests/ipcMain/removeWorkspace.test.ts @@ -799,5 +799,201 @@ describeIntegration("Workspace deletion integration tests", () => { }, TEST_TIMEOUT_SSH_MS ); + + test.concurrent( + "should allow deletion of squash-merged branches without force flag", + async () => { + const env = await createTestEnvironment(); + const tempGitRepo = await createTempGitRepo(); + + try { + const branchName = generateBranchName("squash-merge-test"); + const runtimeConfig = getRuntimeConfig(branchName); + const { workspaceId } = await createWorkspaceWithInit( + env, + tempGitRepo, + branchName, + runtimeConfig, + true, // waitForInit + true // isSSH + ); + + // Configure git for committing + await executeBash(env, workspaceId, 'git config user.email "test@example.com"'); + await executeBash(env, workspaceId, 'git config user.name "Test User"'); + + // Get the current workspace path (inside SSH container) + const pwdResult = await executeBash(env, workspaceId, "pwd"); + const workspacePath = pwdResult.output.trim(); + + // Create a bare repo inside the SSH container to act as "origin" + // This avoids issues with host paths not being accessible in container + const originPath = `${workspacePath}/../.test-origin-${branchName}`; + await executeBash(env, workspaceId, `git clone --bare . "${originPath}"`); + + // Point origin to the bare repo (add if doesn't exist, set-url if it does) + await executeBash( + env, + workspaceId, + `git remote get-url origin >/dev/null 2>&1 && git remote set-url origin "${originPath}" || git remote add origin "${originPath}"` + ); + + // Create feature commits on the branch + await executeBash(env, workspaceId, 'echo "feature1" > feature.txt'); + await executeBash(env, workspaceId, "git add feature.txt"); + await executeBash(env, workspaceId, 'git commit -m "Feature commit 1"'); + + await executeBash(env, workspaceId, 'echo "feature2" >> feature.txt'); + await executeBash(env, workspaceId, "git add feature.txt"); + await executeBash(env, workspaceId, 'git commit -m "Feature commit 2"'); + + // Get the feature branch's final file content + const featureContent = await executeBash(env, workspaceId, "cat feature.txt"); + + // Simulate squash-merge: create a temp worktree, add the squash commit to main, push + // We need to work around bare repo limitations by using a temp checkout + const tempCheckoutPath = `${workspacePath}/../.test-temp-checkout-${branchName}`; + await executeBash( + env, + workspaceId, + `git clone "${originPath}" "${tempCheckoutPath}" && ` + + `cd "${tempCheckoutPath}" && ` + + `git config user.email "test@example.com" && ` + + `git config user.name "Test User" && ` + + // Checkout main (or master, depending on git version) + `(git checkout main 2>/dev/null || git checkout master) && ` + + // Create squash commit with same content (use printf '%s\n' to match echo's newline) + `printf '%s\\n' '${featureContent.output.trim().replace(/'/g, "'\\''")}' > feature.txt && ` + + `git add feature.txt && ` + + `git commit -m "Squash: Feature commits" && ` + + `git push origin HEAD` + ); + + // Cleanup temp checkout + await executeBash(env, workspaceId, `rm -rf "${tempCheckoutPath}"`); + + // Fetch the updated origin in the workspace + await executeBash(env, workspaceId, "git fetch origin"); + + // Verify we have unpushed commits (branch commits are not ancestors of origin/main) + const logResult = await executeBash( + env, + workspaceId, + "git log --branches --not --remotes --oneline" + ); + // Should show commits since our branch commits != squash commit SHA + expect(logResult.output.trim()).not.toBe(""); + + // Now attempt deletion without force - should succeed because content matches + const deleteResult = await env.mockIpcRenderer.invoke( + IPC_CHANNELS.WORKSPACE_REMOVE, + workspaceId + ); + + // Should succeed - squash-merge detection should recognize content is in main + expect(deleteResult.success).toBe(true); + + // Cleanup the bare repo we created + // Note: This runs after workspace is deleted, may fail if path is gone + try { + using cleanupProc = execAsync(`rm -rf "${originPath}"`); + await cleanupProc.result; + } catch { + // Ignore cleanup errors + } + + // 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 + ); + + test.concurrent( + "should block deletion when branch has genuinely unmerged content", + async () => { + const env = await createTestEnvironment(); + const tempGitRepo = await createTempGitRepo(); + + try { + const branchName = generateBranchName("unmerged-content-test"); + const runtimeConfig = getRuntimeConfig(branchName); + const { workspaceId } = await createWorkspaceWithInit( + env, + tempGitRepo, + branchName, + runtimeConfig, + true, // waitForInit + true // isSSH + ); + + // Configure git for committing + await executeBash(env, workspaceId, 'git config user.email "test@example.com"'); + await executeBash(env, workspaceId, 'git config user.name "Test User"'); + + // Get the current workspace path (inside SSH container) + const pwdResult = await executeBash(env, workspaceId, "pwd"); + const workspacePath = pwdResult.output.trim(); + + // Create a bare repo inside the SSH container to act as "origin" + const originPath = `${workspacePath}/../.test-origin-${branchName}`; + await executeBash(env, workspaceId, `git clone --bare . "${originPath}"`); + + // Point origin to the bare repo (add if doesn't exist, set-url if it does) + await executeBash( + env, + workspaceId, + `git remote get-url origin >/dev/null 2>&1 && git remote set-url origin "${originPath}" || git remote add origin "${originPath}"` + ); + + // Create feature commits with unique content (not in origin) + await executeBash(env, workspaceId, 'echo "unique-unmerged-content" > unique.txt'); + await executeBash(env, workspaceId, "git add unique.txt"); + await executeBash(env, workspaceId, 'git commit -m "Unique commit"'); + + // Fetch origin (main doesn't have our content - we didn't push) + await executeBash(env, workspaceId, "git fetch origin"); + + // Attempt deletion without force - should fail because content differs + const deleteResult = await env.mockIpcRenderer.invoke( + IPC_CHANNELS.WORKSPACE_REMOVE, + workspaceId + ); + + // Should fail - genuinely unmerged content + expect(deleteResult.success).toBe(false); + expect(deleteResult.error).toMatch(/unpushed|changes/i); + + // Verify workspace still exists + const stillExists = await workspaceExists(env, workspaceId); + expect(stillExists).toBe(true); + + // Cleanup: force delete + await env.mockIpcRenderer.invoke(IPC_CHANNELS.WORKSPACE_REMOVE, workspaceId, { + force: true, + }); + + // Cleanup the bare repo + try { + using cleanupProc = execAsync(`rm -rf "${originPath}"`); + await cleanupProc.result; + } catch { + // Ignore cleanup errors + } + } finally { + await cleanupTestEnvironment(env); + await cleanupTempGitRepo(tempGitRepo); + } + }, + TEST_TIMEOUT_SSH_MS + ); }); });