From d2327408fb834f9a6d8f9766d5ae99483ae87325 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Wed, 5 Aug 2026 14:39:02 +0700 Subject: [PATCH] fix(lanes): mergeSync commits a fully rerere-auto-resolved merge instead of rethrowing (E2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: the catch block only fell through to the auto-commit path when `unmergedFiles().length && MERGE_HEAD exists` — but a merge rerere resolved completely has ZERO unmerged files (git already staged the resolution), so that guard was always false and the raw git error was rethrown instead. Found via an audit against the Shipyard source this was ported from. Fixed by checking MERGE_HEAD first (unconditionally — its absence means the merge never started, a real failure), then branching on whether any files are still unmerged. Added a real rerere fixture test (teach a resolution, recreate the identical conflict, confirm mergeSync auto-commits) — the existing test suite had no coverage for this path. --- server/__tests__/lane-sync.test.js | 77 ++++++++++++++++++++++++++++++ server/lib/lane-sync.js | 14 ++---- 2 files changed, 82 insertions(+), 9 deletions(-) diff --git a/server/__tests__/lane-sync.test.js b/server/__tests__/lane-sync.test.js index 7dc3429..66d37ed 100644 --- a/server/__tests__/lane-sync.test.js +++ b/server/__tests__/lane-sync.test.js @@ -246,6 +246,83 @@ describe("lane-sync merge: conflict is left in place, --continue finishes it", ( }); }); +describe("lane-sync merge: rerere auto-resolves a previously-seen conflict", () => { + // Bug this guards: mergeSync's catch block used to check + // `conflicted.length && MERGE_HEAD exists` before falling through to the + // rerere-commit path — but a fully rerere-auto-resolved merge has ZERO + // unmerged files (git already staged the resolution), so that condition + // was always false and the code rethrew the raw git error instead of + // committing. Fixed by checking MERGE_HEAD first, unconditionally. + const RR_ROOT = path.join(ROOT, "rerere-fixture"); + const RR_ORIGIN = path.join(RR_ROOT, "origin.git"); + const RR_LANE = path.join(RR_ROOT, "lane"); + const RR_PUSHER = path.join(RR_ROOT, "pusher"); + + before(() => { + fs.mkdirSync(RR_ROOT, { recursive: true }); + g(RR_ROOT, "init", "-q", "--bare", RR_ORIGIN); + + const seed = path.join(RR_ROOT, "seed"); + g(RR_ROOT, "init", "-q", "-b", "development", seed); + fs.writeFileSync(path.join(seed, "config.txt"), "base\n"); + gc(seed, "add", "-A"); + gc(seed, "commit", "-qm", "init"); + gc(seed, "remote", "add", "origin", RR_ORIGIN); + gc(seed, "push", "-q", "origin", "development"); + g(RR_ORIGIN, "symbolic-ref", "HEAD", "refs/heads/development"); + + g(RR_ROOT, "clone", "-q", RR_ORIGIN, RR_LANE); + g(RR_ROOT, "clone", "-q", RR_ORIGIN, RR_PUSHER); + gc(RR_LANE, "config", "rerere.enabled", "true"); + // autoUpdate is what stages a rerere-recognized resolution automatically — + // without it, git restores the resolved CONTENT but still leaves the file + // as "unmerged" (ls-files -u non-empty), so MERGE_HEAD + zero unmerged + // files (the exact condition the fixed code branches on) never occurs. + gc(RR_LANE, "config", "rerere.autoupdate", "true"); + + const baseSha = g(RR_LANE, "rev-parse", "development").trim(); + + // Upstream's side of the conflict — pushed once, applies to both rounds. + fs.writeFileSync(path.join(RR_PUSHER, "config.txt"), "dev version\n"); + gc(RR_PUSHER, "add", "-A"); + gc(RR_PUSHER, "commit", "-qm", "dev edits config"); + gc(RR_PUSHER, "push", "-q", "origin", "development"); + gc(RR_LANE, "fetch", "-q", "origin"); + + // Round 1 — teach rerere the resolution. + gc(RR_LANE, "checkout", "-qb", "feat/rerere-teach", baseSha); + fs.writeFileSync(path.join(RR_LANE, "config.txt"), "lane version\n"); + gc(RR_LANE, "add", "-A"); + gc(RR_LANE, "commit", "-qm", "lane edits config"); + let conflicted = false; + try { + gc(RR_LANE, "merge", "--no-edit", "origin/development"); + } catch { + conflicted = true; + } + if (!conflicted) throw new Error("fixture bug: expected the teach-round merge to conflict"); + fs.writeFileSync(path.join(RR_LANE, "config.txt"), "resolved version\n"); + gc(RR_LANE, "add", "config.txt"); + gc(RR_LANE, "commit", "-q", "--no-edit"); + + // Round 2 — the actual test branch: an IDENTICAL edit to config.txt from + // the same base, so the conflict signature matches what rerere just + // learned and git auto-applies the recorded resolution during the merge + // this test's assertion drives. + gc(RR_LANE, "checkout", "-qb", "feat/rerere-actual", baseSha); + fs.writeFileSync(path.join(RR_LANE, "config.txt"), "lane version\n"); + gc(RR_LANE, "add", "-A"); + gc(RR_LANE, "commit", "-qm", "lane edits config (again)"); + }); + + it("commits automatically instead of reporting a conflict", async () => { + const result = await laneSync.mergeSync({ cwd: RR_LANE }, profile(), "feat/rerere-actual"); + assert.equal(result.code, 0); + assert.equal(fs.readFileSync(path.join(RR_LANE, "config.txt"), "utf8"), "resolved version\n"); + assert.ok(!fs.existsSync(path.join(RR_LANE, ".git", "MERGE_HEAD"))); + }); +}); + describe("lane-sync against a real git-worktree lane", () => { it("resolves MERGE_HEAD and info/attributes correctly under git worktree add", async () => { const wt = require("../lib/worktree"); diff --git a/server/lib/lane-sync.js b/server/lib/lane-sync.js index 1869f94..f5ac59f 100644 --- a/server/lib/lane-sync.js +++ b/server/lib/lane-sync.js @@ -259,18 +259,14 @@ async function mergeSync(lane, profile, branchArg) { try { await git(lane.cwd, ["merge", "--no-edit", `origin/${INTEGRATION_BRANCH}`]); } catch (err) { - const conflicted = await unmergedFiles(lane.cwd); const mergeHeadPath = path.join(await gitDir(lane.cwd), "MERGE_HEAD"); - if (conflicted.length && fs.existsSync(mergeHeadPath)) { + if (!fs.existsSync(mergeHeadPath)) throw err; // merge never started — a real git failure + + const conflicted = await unmergedFiles(lane.cwd); + if (conflicted.length) { return { code: 4, conflictedFiles: conflicted }; } - throw err; - } - - // rerere may have auto-resolved every conflict but left the merge - // uncommitted — finish it. - const mergeHeadPath = path.join(await gitDir(lane.cwd), "MERGE_HEAD"); - if (fs.existsSync(mergeHeadPath) && !(await unmergedFiles(lane.cwd)).length) { + // rerere auto-resolved every conflict but left the merge uncommitted — finish it. await git(lane.cwd, ["commit", "--no-edit"]); }