From 70985088f6853aa8c7e492aad2454ac7086a5b71 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 10 Apr 2026 20:29:26 +0700 Subject: [PATCH] Make pull abort safely on conflicts --- .../server/src/utils/checkout-git.test.ts | 58 +++++++++++++++++++ packages/server/src/utils/checkout-git.ts | 56 +++++++++++++++++- 2 files changed, 112 insertions(+), 2 deletions(-) diff --git a/packages/server/src/utils/checkout-git.test.ts b/packages/server/src/utils/checkout-git.test.ts index 0d58ddb09..019f8841f 100644 --- a/packages/server/src/utils/checkout-git.test.ts +++ b/packages/server/src/utils/checkout-git.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { execSync } from "child_process"; import { + existsSync, mkdtempSync, rmSync, writeFileSync, @@ -557,6 +558,63 @@ const x = 1; expect(readFileSync(join(repoDir, "pulled.txt"), "utf8")).toBe("remote\n"); }); + it("aborts pull on merge conflicts and leaves no merge in progress", async () => { + const remoteDir = join(tempDir, "remote.git"); + execSync(`git init --bare -b main ${remoteDir}`); + execSync(`git remote add origin ${remoteDir}`, { cwd: repoDir }); + execSync("git push -u origin main", { cwd: repoDir }); + + writeFileSync(join(repoDir, "conflict.txt"), "local\n"); + execSync("git add conflict.txt", { cwd: repoDir }); + execSync("git -c commit.gpgsign=false commit -m 'local conflict commit'", { cwd: repoDir }); + + const otherClone = join(tempDir, "other-clone"); + execSync(`git clone ${remoteDir} ${otherClone}`); + execSync("git config user.email 'test@test.com'", { cwd: otherClone }); + execSync("git config user.name 'Test'", { cwd: otherClone }); + writeFileSync(join(otherClone, "conflict.txt"), "remote\n"); + execSync("git add conflict.txt", { cwd: otherClone }); + execSync("git -c commit.gpgsign=false commit -m 'remote conflict commit'", { cwd: otherClone }); + execSync("git push", { cwd: otherClone }); + + await expect(pullCurrentBranch(repoDir)).rejects.toBeInstanceOf(Error); + + const porcelain = execSync("git status --porcelain", { cwd: repoDir }).toString().trim(); + expect(porcelain).toBe(""); + expect(() => execSync("git rev-parse -q --verify MERGE_HEAD", { cwd: repoDir })).toThrow(); + }); + + it("aborts pull on rebase conflicts and leaves no rebase in progress", async () => { + const remoteDir = join(tempDir, "remote.git"); + execSync(`git init --bare -b main ${remoteDir}`); + execSync(`git remote add origin ${remoteDir}`, { cwd: repoDir }); + execSync("git push -u origin main", { cwd: repoDir }); + execSync("git config pull.rebase true", { cwd: repoDir }); + + writeFileSync(join(repoDir, "conflict.txt"), "local\n"); + execSync("git add conflict.txt", { cwd: repoDir }); + execSync("git -c commit.gpgsign=false commit -m 'local rebase conflict commit'", { cwd: repoDir }); + + const otherClone = join(tempDir, "other-clone"); + execSync(`git clone ${remoteDir} ${otherClone}`); + execSync("git config user.email 'test@test.com'", { cwd: otherClone }); + execSync("git config user.name 'Test'", { cwd: otherClone }); + writeFileSync(join(otherClone, "conflict.txt"), "remote\n"); + execSync("git add conflict.txt", { cwd: otherClone }); + execSync("git -c commit.gpgsign=false commit -m 'remote rebase conflict commit'", { + cwd: otherClone, + }); + execSync("git push", { cwd: otherClone }); + + await expect(pullCurrentBranch(repoDir)).rejects.toBeInstanceOf(Error); + + const gitDir = execSync("git rev-parse --absolute-git-dir", { cwd: repoDir }).toString().trim(); + const porcelain = execSync("git status --porcelain", { cwd: repoDir }).toString().trim(); + expect(porcelain).toBe(""); + expect(existsSync(join(gitDir, "rebase-merge"))).toBe(false); + expect(existsSync(join(gitDir, "rebase-apply"))).toBe(false); + }); + it("pushes the current branch to origin", async () => { const remoteDir = join(tempDir, "remote.git"); execSync(`git init --bare -b main ${remoteDir}`); diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index ef246e32f..db4e3cdfd 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -2,7 +2,7 @@ import { exec, execFile } from "child_process"; import { spawnProcess } from "./spawn.js"; import { promisify } from "util"; import { resolve, dirname, basename } from "path"; -import { realpathSync } from "fs"; +import { existsSync, realpathSync } from "fs"; import { open as openFile, stat as statFile } from "fs/promises"; import { TTLCache } from "@isaacs/ttlcache"; import type { ParsedDiffFile } from "../server/utils/diff-highlighter.js"; @@ -828,6 +828,46 @@ async function hasOriginRemote(cwd: string): Promise { return url !== null; } +async function resolveAbsoluteGitDir(cwd: string): Promise { + try { + const { stdout } = await execAsync("git rev-parse --absolute-git-dir", { + cwd, + env: READ_ONLY_GIT_ENV, + }); + const gitDir = stdout.trim(); + return gitDir.length > 0 ? gitDir : null; + } catch { + return null; + } +} + +async function abortGitPullConflictState(cwd: string): Promise { + const gitDir = await resolveAbsoluteGitDir(cwd); + if (!gitDir) { + return; + } + + const mergeHeadPath = resolve(gitDir, "MERGE_HEAD"); + const rebaseMergePath = resolve(gitDir, "rebase-merge"); + const rebaseApplyPath = resolve(gitDir, "rebase-apply"); + + if (existsSync(mergeHeadPath)) { + try { + await execAsync("git merge --abort", { cwd }); + } catch { + // ignore + } + } + + if (existsSync(rebaseMergePath) || existsSync(rebaseApplyPath)) { + try { + await execAsync("git rebase --abort", { cwd }); + } catch { + // ignore + } + } +} + export async function resolveRepositoryDefaultBranch(repoRoot: string): Promise { try { const { stdout } = await execAsync("git symbolic-ref --quiet refs/remotes/origin/HEAD", { @@ -1463,6 +1503,13 @@ export async function getCheckoutDiff( continue; } + // `git diff -w --name-status` can still report a modified path even when the + // whitespace-filtered patch and numstat are both empty. Skip emitting a + // structured placeholder in that case so whitespace-only edits truly disappear. + if (ignoreWhitespace && !trackedDiffTruncated && stat === null) { + continue; + } + structured.push({ path: change.path, isNew: change.isNew, @@ -1767,7 +1814,12 @@ export async function pullCurrentBranch(cwd: string): Promise { if (!hasRemote) { throw new Error("Remote 'origin' is not configured."); } - await execAsync("git pull", { cwd }); + try { + await execAsync("git pull", { cwd }); + } catch (error) { + await abortGitPullConflictState(cwd); + throw error; + } } export async function pushCurrentBranch(cwd: string): Promise {