libgit2's merge implementation creates MERGE_HEAD even for squash merges, unlike CLI git. When the merge fails with conflicts, the error paths in mergeMilestoneToMain cleaned SQUASH_MSG and MERGE_MSG but left MERGE_HEAD on disk. This blocked all subsequent merge attempts and caused doctor to report corrupt merge state. Add MERGE_HEAD cleanup (via nativeMergeAbort + explicit unlink) to: - The code-conflict error path (before MergeConflictError throw) - The dirty-working-tree error path (defensive) - The success path (alongside existing SQUASH_MSG cleanup) Closes #2912 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
9dc6a6a97d
commit
571b382075
2 changed files with 133 additions and 5 deletions
|
|
@ -63,6 +63,7 @@ import {
|
|||
nativeDiffNumstat,
|
||||
nativeUpdateRef,
|
||||
nativeIsAncestor,
|
||||
nativeMergeAbort,
|
||||
} from "./native-git-bridge.js";
|
||||
|
||||
const gsdHome = process.env.GSD_HOME || join(homedir(), ".gsd");
|
||||
|
|
@ -1573,6 +1574,16 @@ export function mergeMilestoneToMain(
|
|||
// untracked .gsd/ files left by syncStateToProjectRoot). Preserve the
|
||||
// milestone branch so commits are not lost.
|
||||
if (mergeResult.conflicts.includes("__dirty_working_tree__")) {
|
||||
// Defensively clean merge state — the native path may leave MERGE_HEAD
|
||||
// even when the merge is rejected (#2912).
|
||||
try {
|
||||
const gitDir_ = resolveGitDir(originalBasePath_);
|
||||
for (const f of ["SQUASH_MSG", "MERGE_MSG", "MERGE_HEAD"]) {
|
||||
const p = join(gitDir_, f);
|
||||
if (existsSync(p)) unlinkSync(p);
|
||||
}
|
||||
} catch { /* best-effort */ }
|
||||
|
||||
// Pop stash before throwing so local work is not lost.
|
||||
if (stashed) {
|
||||
try {
|
||||
|
|
@ -1630,6 +1641,18 @@ export function mergeMilestoneToMain(
|
|||
|
||||
// If there are still real code conflicts, escalate
|
||||
if (codeConflicts.length > 0) {
|
||||
// Abort merge state so MERGE_HEAD is not left on disk (#2912).
|
||||
// libgit2's merge creates MERGE_HEAD even for squash merges; if left
|
||||
// dangling, subsequent merges fail and doctor reports corrupt state.
|
||||
try { nativeMergeAbort(originalBasePath_); } catch { /* best-effort */ }
|
||||
try {
|
||||
const gitDir_ = resolveGitDir(originalBasePath_);
|
||||
for (const f of ["SQUASH_MSG", "MERGE_MSG", "MERGE_HEAD"]) {
|
||||
const p = join(gitDir_, f);
|
||||
if (existsSync(p)) unlinkSync(p);
|
||||
}
|
||||
} catch { /* best-effort */ }
|
||||
|
||||
// Pop stash before throwing so local work is not lost (#2151).
|
||||
if (stashed) {
|
||||
try {
|
||||
|
|
@ -1656,14 +1679,18 @@ export function mergeMilestoneToMain(
|
|||
const commitResult = nativeCommit(originalBasePath_, commitMessage);
|
||||
const nothingToCommit = commitResult === null;
|
||||
|
||||
// 9a. Clean up SQUASH_MSG left by git merge --squash (#1853).
|
||||
// 9a. Clean up merge state files left by git merge --squash (#1853, #2912).
|
||||
// git only removes SQUASH_MSG when the commit reads it directly (plain
|
||||
// `git commit`). nativeCommit uses `-F -` (stdin) or libgit2, neither
|
||||
// of which trigger git's SQUASH_MSG cleanup. If left on disk, doctor
|
||||
// reports `corrupt_merge_state` on every subsequent run.
|
||||
// of which trigger git's SQUASH_MSG cleanup. MERGE_HEAD is created by
|
||||
// libgit2's merge even in squash mode and is not removed by nativeCommit.
|
||||
// If left on disk, doctor reports `corrupt_merge_state` on every subsequent run.
|
||||
try {
|
||||
const squashMsgPath = join(resolveGitDir(originalBasePath_), "SQUASH_MSG");
|
||||
if (existsSync(squashMsgPath)) unlinkSync(squashMsgPath);
|
||||
const gitDir_ = resolveGitDir(originalBasePath_);
|
||||
for (const f of ["SQUASH_MSG", "MERGE_MSG", "MERGE_HEAD"]) {
|
||||
const p = join(gitDir_, f);
|
||||
if (existsSync(p)) unlinkSync(p);
|
||||
}
|
||||
} catch { /* best-effort */ }
|
||||
|
||||
// 9a-ii. Restore stashed files now that the merge+commit is complete (#2151).
|
||||
|
|
|
|||
|
|
@ -638,6 +638,107 @@ describe("auto-worktree-milestone-merge", { timeout: 300_000 }, () => {
|
|||
"#1906: codeFilesChanged must be false when only .gsd/ files were merged");
|
||||
});
|
||||
|
||||
test("#2912: MERGE_HEAD cleaned up after squash-merge conflict", () => {
|
||||
const repo = freshRepo();
|
||||
const wtPath = createAutoWorktree(repo, "M291");
|
||||
|
||||
// Create a file on main that will conflict with the milestone branch
|
||||
run("git checkout main", repo);
|
||||
writeFileSync(join(repo, "conflict.ts"), "// main version\nexport const x = 1;\n");
|
||||
run("git add .", repo);
|
||||
run("git commit -m 'add conflict.ts on main'", repo);
|
||||
|
||||
// Switch back to milestone branch and create conflicting content
|
||||
run("git checkout milestone/M291", wtPath);
|
||||
writeFileSync(join(wtPath, "conflict.ts"), "// milestone version\nexport const x = 2;\n");
|
||||
run("git add .", wtPath);
|
||||
run("git commit -m 'add conflict.ts on milestone'", wtPath);
|
||||
|
||||
const roadmap = makeRoadmap("M291", "Conflict milestone", [
|
||||
{ id: "S01", title: "Conflict test" },
|
||||
]);
|
||||
|
||||
// The merge should throw MergeConflictError due to conflict.ts
|
||||
let threw = false;
|
||||
try {
|
||||
mergeMilestoneToMain(repo, "M291", roadmap);
|
||||
} catch (err: unknown) {
|
||||
threw = true;
|
||||
// Verify it's a merge conflict error
|
||||
assert.ok(
|
||||
err instanceof Error && err.message.includes("conflict"),
|
||||
"should throw a conflict-related error",
|
||||
);
|
||||
}
|
||||
assert.ok(threw, "mergeMilestoneToMain must throw on code conflict");
|
||||
|
||||
// BUG #2912: MERGE_HEAD must NOT be left on disk after the error
|
||||
const mergeHeadPath = join(repo, ".git", "MERGE_HEAD");
|
||||
assert.ok(
|
||||
!existsSync(mergeHeadPath),
|
||||
"#2912: MERGE_HEAD must be cleaned up after merge conflict error",
|
||||
);
|
||||
});
|
||||
|
||||
test("#2912: stale MERGE_HEAD from native merge is cleaned after successful commit", () => {
|
||||
const repo = freshRepo();
|
||||
const wtPath = createAutoWorktree(repo, "M292");
|
||||
|
||||
addSliceToMilestone(repo, wtPath, "M292", "S01", "Feature A", [
|
||||
{ file: "feature-a.ts", content: "export const a = true;\n", message: "add feature a" },
|
||||
]);
|
||||
|
||||
const roadmap = makeRoadmap("M292", "Clean merge", [
|
||||
{ id: "S01", title: "Feature A" },
|
||||
]);
|
||||
|
||||
// Simulate what libgit2's merge implementation does: it creates MERGE_HEAD
|
||||
// even for squash merges (unlike CLI git). We plant MERGE_HEAD before calling
|
||||
// mergeMilestoneToMain to verify the success path cleans it up.
|
||||
// We cannot plant it before the call because the function manages checkout
|
||||
// internally, so instead we verify after the call.
|
||||
mergeMilestoneToMain(repo, "M292", roadmap);
|
||||
|
||||
// After successful merge+commit, MERGE_HEAD must not linger
|
||||
const mergeHeadPath = join(repo, ".git", "MERGE_HEAD");
|
||||
assert.ok(
|
||||
!existsSync(mergeHeadPath),
|
||||
"#2912: MERGE_HEAD must be cleaned up after successful merge",
|
||||
);
|
||||
});
|
||||
|
||||
test("#2912: planted MERGE_HEAD is cleaned up in success path", () => {
|
||||
// This test directly verifies the cleanup code handles a MERGE_HEAD file
|
||||
// left by the native (libgit2) merge path. We hook into the merge by
|
||||
// planting MERGE_HEAD right after nativeMergeSquash would create it.
|
||||
const repo = freshRepo();
|
||||
const wtPath = createAutoWorktree(repo, "M293");
|
||||
|
||||
addSliceToMilestone(repo, wtPath, "M293", "S01", "Feature B", [
|
||||
{ file: "feature-b.ts", content: "export const b = true;\n", message: "add feature b" },
|
||||
]);
|
||||
|
||||
const roadmap = makeRoadmap("M293", "Planted MERGE_HEAD", [
|
||||
{ id: "S01", title: "Feature B" },
|
||||
]);
|
||||
|
||||
// Plant a fake MERGE_HEAD in the git dir to simulate libgit2 behavior.
|
||||
// We need to do this after the function checks out main but before it
|
||||
// commits. Since we can't intercept mid-function, we plant it before
|
||||
// the call. If the function cleans it up, the test passes.
|
||||
const gitDir = join(repo, ".git");
|
||||
const fakeHead = run("git rev-parse HEAD", repo);
|
||||
writeFileSync(join(gitDir, "MERGE_HEAD"), fakeHead + "\n");
|
||||
|
||||
mergeMilestoneToMain(repo, "M293", roadmap);
|
||||
|
||||
// The planted MERGE_HEAD must be cleaned up
|
||||
assert.ok(
|
||||
!existsSync(join(gitDir, "MERGE_HEAD")),
|
||||
"#2912: planted MERGE_HEAD must be removed by success-path cleanup",
|
||||
);
|
||||
});
|
||||
|
||||
test("#1906: codeFilesChanged=true when real code is merged", () => {
|
||||
const repo = freshRepo();
|
||||
const wtPath = createAutoWorktree(repo, "M190");
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue