fix(merger): require fast-forward ref advances and read integration tip from refs/heads/<branch>
Closes a "non-fast-forward ref overwrite" path where a subsequent merger could orphan a previously-merged squash by advancing the integration branch to a sibling commit. Symptom (observed on fusion/fn-5419): main reflog shows385b6e93-> f6358ce4 (FN-5551 squash) ->63ec7098(FN-5552 squash) with f6358ce4 and63ec7098both parented at385b6e93. The FN-5551 squash was correctly committed to main, then the FN-5552 merger built its own squash off the stale385b6e93base and the CAS update-ref blindly moved main sideways, orphaning f6358ce4 onto whichever feature branch had already branched from it. Two coupled fixes uphold the missing invariant — local <integrationBranch> only advances via fast-forward, and the merger never builds a squash off a stale base sha: 1. advanceIntegrationBranchRef: add a `merge-base --is-ancestor` check before update-ref. Non-FF attempts now return reason: "non-fast-forward-advance" instead of overwriting the ref. The existing concurrent-advance CAS guard is retained. 2. runMerge: resolve the integration-branch tip via `git rev-parse --verify refs/heads/<integrationBranch>` instead of `git rev-parse HEAD` in rootDir. In reuse-task-worktree mode rootDir's HEAD can lag behind the shared ref after a sibling merger advanced it via update-ref without re-checking-out. Adds regression coverage in merger-ref-update-advance.test.ts: a sibling-commit advance with a matching expectedCurrentSha is now refused with the new reason, and multi-commit fast-forwards still succeed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
15
.changeset/merger-ff-only-ref-advance.md
Normal file
15
.changeset/merger-ff-only-ref-advance.md
Normal file
@@ -0,0 +1,15 @@
|
||||
---
|
||||
"@fusion/engine": patch
|
||||
---
|
||||
|
||||
fix(merger): require fast-forward ref advances and read integration tip from refs/heads/<branch>
|
||||
|
||||
Closes a class of "orphaned merge" bug where a subsequent merger could overwrite the integration branch tip with a sibling commit, leaving the previous squash reachable only from a feature branch.
|
||||
|
||||
Two coupled fixes:
|
||||
|
||||
1. `advanceIntegrationBranchRef` now refuses non-fast-forward advances. The CAS check still guards against concurrent ref movement, but the new `merge-base --is-ancestor` check additionally requires the new sha to descend from the expected current sha. Non-FF attempts return `reason: "non-fast-forward-advance"` instead of silently orphaning the prior tip.
|
||||
|
||||
2. `runMerge` resolves the integration-branch tip via `git rev-parse --verify refs/heads/<integrationBranch>` instead of `git rev-parse HEAD` in `rootDir`. In reuse-task-worktree mode, `rootDir`'s HEAD can lag behind the shared ref after a sibling merger advanced it via `update-ref` without re-checking-out — using HEAD there caused the eventual squash commit to parent off an earlier sha and orphan the previously-merged tip.
|
||||
|
||||
Together these uphold the invariant: local `<integrationBranch>` only advances via fast-forward, and the merger never builds a squash off a stale base sha.
|
||||
@@ -174,6 +174,89 @@ describe("advanceIntegrationBranchRef", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("refuses non-fast-forward advance even when expectedCurrentSha matches (sibling-commit orphan guard)", async () => {
|
||||
const dir = setupRepo("main");
|
||||
const events: Array<{ type: string; metadata?: Record<string, unknown> }> = [];
|
||||
try {
|
||||
const baseSha = git(dir, "git rev-parse refs/heads/main");
|
||||
|
||||
// First sibling — legitimate prior merger output that advanced main.
|
||||
git(dir, "git checkout -b sibling-a");
|
||||
writeFileSync(join(dir, "a.txt"), "a\n");
|
||||
git(dir, "git add a.txt");
|
||||
git(dir, "git commit -m a");
|
||||
const siblingASha = git(dir, "git rev-parse HEAD");
|
||||
git(dir, "git checkout main");
|
||||
git(dir, `git update-ref refs/heads/main ${siblingASha} ${baseSha}`);
|
||||
|
||||
// Second sibling — built off the stale base (the bug shape). Both
|
||||
// shas have the same parent, so siblingA is NOT an ancestor of
|
||||
// siblingB. CAS alone would happily move main from siblingA to
|
||||
// siblingB and orphan siblingA.
|
||||
git(dir, `git checkout ${baseSha}`);
|
||||
git(dir, "git checkout -b sibling-b");
|
||||
writeFileSync(join(dir, "b.txt"), "b\n");
|
||||
git(dir, "git add b.txt");
|
||||
git(dir, "git commit -m b");
|
||||
const siblingBSha = git(dir, "git rev-parse HEAD");
|
||||
|
||||
const result = await advanceIntegrationBranchRef({
|
||||
rootDir: dir,
|
||||
projectRootDir: dir,
|
||||
integrationBranch: "main",
|
||||
newSha: siblingBSha,
|
||||
expectedCurrentSha: siblingASha,
|
||||
taskId: "FN-5419",
|
||||
audit: {
|
||||
git: async (event: any) => events.push(event),
|
||||
} as any,
|
||||
});
|
||||
|
||||
expect(result.advanced).toBe(false);
|
||||
if (result.advanced) throw new Error("expected refusal");
|
||||
expect(result.reason).toBe("non-fast-forward-advance");
|
||||
// Ref must NOT have moved — siblingA is still reachable from main.
|
||||
expect(git(dir, "git rev-parse refs/heads/main")).toBe(siblingASha);
|
||||
expect(events[0]?.type).toBe("merge:integration-ref-advance");
|
||||
expect(events[0]?.metadata?.succeeded).toBe(false);
|
||||
expect(String(events[0]?.metadata?.error ?? "")).toContain(
|
||||
"non-fast-forward-advance",
|
||||
);
|
||||
} finally {
|
||||
removeTmpDirSync(dir);
|
||||
}
|
||||
});
|
||||
|
||||
it("allows multi-commit fast-forward advance", async () => {
|
||||
const dir = setupRepo("main");
|
||||
try {
|
||||
const baseSha = git(dir, "git rev-parse refs/heads/main");
|
||||
git(dir, "git checkout -b feat");
|
||||
writeFileSync(join(dir, "f1.txt"), "f1\n");
|
||||
git(dir, "git add f1.txt");
|
||||
git(dir, "git commit -m f1");
|
||||
writeFileSync(join(dir, "f2.txt"), "f2\n");
|
||||
git(dir, "git add f2.txt");
|
||||
git(dir, "git commit -m f2");
|
||||
const newSha = git(dir, "git rev-parse HEAD");
|
||||
|
||||
const result = await advanceIntegrationBranchRef({
|
||||
rootDir: dir,
|
||||
projectRootDir: dir,
|
||||
integrationBranch: "main",
|
||||
newSha,
|
||||
expectedCurrentSha: baseSha,
|
||||
taskId: "FN-5419",
|
||||
audit: { git: async () => undefined } as any,
|
||||
});
|
||||
|
||||
expect(result.advanced).toBe(true);
|
||||
expect(git(dir, "git rev-parse refs/heads/main")).toBe(newSha);
|
||||
} finally {
|
||||
removeTmpDirSync(dir);
|
||||
}
|
||||
});
|
||||
|
||||
it("throws on missing precondition shas", async () => {
|
||||
const dir = setupRepo("main");
|
||||
try {
|
||||
|
||||
@@ -55,7 +55,7 @@ export async function advanceIntegrationBranchRef(args: {
|
||||
| { advanced: true; previousSha: string; newSha: string }
|
||||
| {
|
||||
advanced: false;
|
||||
reason: "concurrent-advance" | "ref-update-refused" | "missing-current-sha";
|
||||
reason: "concurrent-advance" | "ref-update-refused" | "missing-current-sha" | "non-fast-forward-advance";
|
||||
diagnostic: string;
|
||||
observedCurrentSha?: string;
|
||||
}
|
||||
@@ -152,6 +152,34 @@ export async function advanceIntegrationBranchRef(args: {
|
||||
};
|
||||
}
|
||||
|
||||
// Fast-forward-only invariant: the new sha must descend from the current
|
||||
// tip. CAS alone (old-value match) lets a sibling commit overwrite the ref
|
||||
// and orphan the prior tip — the exact shape that left an FN-trailered
|
||||
// squash reachable only from a feature branch when a subsequent merger
|
||||
// built its squash off a stale base. Reject non-FF advances.
|
||||
if (newSha !== expectedCurrentSha) {
|
||||
try {
|
||||
await testHooks.runGit(
|
||||
["merge-base", "--is-ancestor", expectedCurrentSha, newSha],
|
||||
rootDir,
|
||||
);
|
||||
} catch (error: unknown) {
|
||||
const diagnostic = `newSha ${newSha} is not a descendant of ${expectedCurrentSha} on ${ref}`;
|
||||
await emitRefAdvance({
|
||||
succeeded: false,
|
||||
fromSha: expectedCurrentSha,
|
||||
toSha: newSha,
|
||||
error: `non-fast-forward-advance: ${diagnostic}`,
|
||||
});
|
||||
return {
|
||||
advanced: false,
|
||||
reason: "non-fast-forward-advance",
|
||||
diagnostic,
|
||||
observedCurrentSha,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
await testHooks.runGit(["update-ref", ref, newSha, expectedCurrentSha], rootDir);
|
||||
await emitRefAdvance({
|
||||
|
||||
@@ -8000,10 +8000,18 @@ export async function aiMergeTask(
|
||||
if (worktreePath && task.baseCommitSha) {
|
||||
try {
|
||||
throwIfAborted(options.signal, taskId);
|
||||
const { stdout: mainHeadOut } = await execAsync("git rev-parse HEAD", {
|
||||
cwd: rootDir,
|
||||
encoding: "utf-8",
|
||||
});
|
||||
// Read the authoritative integration-branch tip from the shared ref —
|
||||
// NOT rootDir's HEAD. In reuse-task-worktree mode rootDir's HEAD can
|
||||
// lag behind refs/heads/<integrationBranch> when a sibling merger
|
||||
// advanced the ref via update-ref without re-checking-out, and using a
|
||||
// stale base sha here causes the eventual squash commit to parent off
|
||||
// an earlier sha and orphan the previously-merged tip on a subsequent
|
||||
// non-FF ref advance.
|
||||
const refName = `refs/heads/${mergeTarget.branch}`;
|
||||
const { stdout: mainHeadOut } = await execAsync(
|
||||
`git rev-parse --verify ${refName}`,
|
||||
{ cwd: rootDir, encoding: "utf-8" },
|
||||
);
|
||||
const mainHead = mainHeadOut.trim();
|
||||
if (mainHead) {
|
||||
const divergence = await probeDivergence({
|
||||
|
||||
Reference in New Issue
Block a user