fix(merger): stop phantom-merge guard stranding tasks whose commit already landed
Tasks were getting stuck in In Review with "verification fix succeeded but no merge commit could be created" even though the merge commit was already on main. Verification failures on attempt 1 were being swallowed by the smart- conflict-resolution retry path, triggering attempt 2 with a stale baseline, and the in-merge-fix finalizer would then fail its phantom-merge check. - Propagate VerificationError out of executeMergeAttempt so the in-merge fix runs once on attempt 1 with the correct preAttemptHeadSha baseline. - In commitOrAmendMergeWithFixes, recognize "task already on HEAD" via the Fusion-Task-Id trailer (line-anchored match) and treat the no-progress finalize as success instead of tripping the guard. - Add real-git regression test plus update merger.test.ts call counts to reflect the (now correctly absent) attempt-2 AI agent. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
10
.changeset/fix-merger-phantom-merge-false-negative.md
Normal file
10
.changeset/fix-merger-phantom-merge-false-negative.md
Normal file
@@ -0,0 +1,10 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Fix tasks getting stuck in In Review with "verification fix succeeded but no merge commit could be created" even when the merge commit had already landed on main.
|
||||
|
||||
Root cause: when attempt 1 of the merge hit a verification failure (test command failed) under default smart conflict resolution, the catch in `executeMergeAttempt` swallowed the error and returned `false`, triggering a redundant attempt 2. Attempt 2 captured a stale `preAttemptHeadSha` (the AI commit from attempt 1), found the branch already merged, ran the in-merge fix, and the finalizer's phantom-merge guard then saw `!hasStaged && !headMoved` against the wrong baseline — even though the task's content was already on HEAD.
|
||||
|
||||
- `executeMergeAttempt` now propagates `VerificationError` directly so the in-merge fix runs once on attempt 1 with the correct baseline. Auto-conflict-resolution can't fix a verification failure, so retrying with attempt 2 was always wrong for this error.
|
||||
- `commitOrAmendMergeWithFixes` adds a defense-in-depth check: if HEAD already carries the task's `Fusion-Task-Id` trailer, treat the no-progress finalize as success rather than tripping the phantom-merge guard. The trailer match is anchored to line boundaries so unrelated task IDs in the body can't false-positive.
|
||||
@@ -409,6 +409,56 @@ describe("commitOrAmendMergeWithFixes — staging allowlist", () => {
|
||||
const warnMessages = warnSpy.mock.calls.map((c) => String(c[0]));
|
||||
expect(warnMessages.some((m) => m.includes("unrelated.ts") && m.includes("refusing to stage"))).toBe(true);
|
||||
});
|
||||
|
||||
// ── Regression: phantom-merge guard false-negative ────────────────────
|
||||
//
|
||||
// Previously a stale `preAttemptHeadSha` (e.g. captured by a redundant
|
||||
// attempt 2 after attempt 1's AI commit) combined with a fix that touched
|
||||
// no tracked files would trip the phantom-merge guard and strand the task
|
||||
// in In Review even though the merge commit already landed on HEAD. The
|
||||
// guard now defers to the `Fusion-Task-Id` trailer: if HEAD already records
|
||||
// this task, treat the no-op finalize as success.
|
||||
it("returns success when HEAD already carries the Fusion-Task-Id trailer (phantom-merge false-negative defense)", async () => {
|
||||
const taskId = "FN-3727";
|
||||
const git = (cmd: string) => execSync(cmd, { cwd: dir, stdio: "pipe" }).toString();
|
||||
|
||||
// Simulate the state after attempt 1 successfully committed: HEAD carries
|
||||
// the Fusion-Task-Id trailer for this task; the working tree is clean.
|
||||
git("git checkout -b feat/Z");
|
||||
writeFileSync(join(dir, "feature-z.ts"), "export const z = 1;\n");
|
||||
git("git add feature-z.ts");
|
||||
git('git commit -m "feat: add feature-z" -m "Fusion-Task-Id: ' + taskId + '"');
|
||||
git("git checkout main");
|
||||
git("git merge --squash feat/Z");
|
||||
git('git commit -m "feat(' + taskId + '): add feature-z" -m "Fusion-Task-Id: ' + taskId + '"');
|
||||
|
||||
// Now invoke the finalizer with a STALE baseline — preAttemptHeadSha
|
||||
// points at HEAD itself (mimicking attempt 2 capturing HEAD after
|
||||
// attempt 1's commit) and no fix-modified files.
|
||||
const headSha = git("git rev-parse HEAD").trim();
|
||||
const result = await commitOrAmendMergeWithFixes(
|
||||
dir,
|
||||
taskId,
|
||||
"feat/Z",
|
||||
"- feat: add feature-z",
|
||||
false,
|
||||
headSha, // stale baseline — equals current HEAD
|
||||
"",
|
||||
undefined,
|
||||
STUB_SETTINGS,
|
||||
undefined,
|
||||
null,
|
||||
null,
|
||||
new Set<string>(), // fix touched no tracked files
|
||||
);
|
||||
|
||||
// Must NOT trip the phantom-merge guard: the trailer says we're done.
|
||||
expect(result).toBe(true);
|
||||
|
||||
// No new commit should have been fabricated.
|
||||
const newHead = git("git rev-parse HEAD").trim();
|
||||
expect(newHead).toBe(headSha);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -6981,13 +6981,17 @@ describe("aiMergeTask — in-merge verification fix", () => {
|
||||
name: "VerificationError",
|
||||
});
|
||||
|
||||
// Verify that fix agent was spawned (3 calls: summarizer + merger + fix)
|
||||
expect(mockedCreateFnAgent).toHaveBeenCalledTimes(3);
|
||||
// 2 calls: merge AI agent (attempt 1) + verification-fix agent.
|
||||
// VerificationError no longer triggers a redundant attempt 2 — the
|
||||
// in-merge fix runs immediately on attempt 1's catch with the correct
|
||||
// preAttemptHeadSha baseline.
|
||||
expect(mockedCreateFnAgent).toHaveBeenCalledTimes(2);
|
||||
|
||||
// Verify the fix agent was called with correct options
|
||||
const fixAgentCall = mockedCreateFnAgent.mock.calls[1];
|
||||
expect(fixAgentCall[0].tools).toBe("coding");
|
||||
expect(fixAgentCall[0].cwd).toBe("/tmp/root");
|
||||
expect(fixAgentCall[0].systemPrompt).toContain("verification fix agent");
|
||||
});
|
||||
|
||||
it("logs fix-agent startup metadata, streams callbacks, and logs rerun lifecycle", async () => {
|
||||
@@ -7206,8 +7210,9 @@ describe("aiMergeTask — in-merge verification fix", () => {
|
||||
name: "VerificationError",
|
||||
});
|
||||
|
||||
// Verify fix agent was NOT spawned (summarizer + merger only)
|
||||
expect(mockedCreateFnAgent).toHaveBeenCalledTimes(2);
|
||||
// Verify fix agent was NOT spawned — only the merge AI agent (attempt 1).
|
||||
// VerificationError propagates without triggering attempt 2.
|
||||
expect(mockedCreateFnAgent).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Verify no fix attempt was logged
|
||||
const logCalls = (store.logEntry as ReturnType<typeof vi.fn>).mock.calls;
|
||||
@@ -7432,8 +7437,8 @@ describe("aiMergeTask — in-merge verification fix", () => {
|
||||
name: "VerificationError",
|
||||
});
|
||||
|
||||
// Should have 3 fix attempts (capped at 3) + summarizer + merger = 5 calls
|
||||
expect(mockedCreateFnAgent).toHaveBeenCalledTimes(5);
|
||||
// 1 merger AI agent (attempt 1) + 3 fix agent attempts (capped) = 4 calls
|
||||
expect(mockedCreateFnAgent).toHaveBeenCalledTimes(4);
|
||||
});
|
||||
|
||||
it("default verificationFixRetries (omitted) results in 3 fix attempts", async () => {
|
||||
@@ -7483,8 +7488,8 @@ describe("aiMergeTask — in-merge verification fix", () => {
|
||||
name: "VerificationError",
|
||||
});
|
||||
|
||||
// Should have 3 fix attempts (default) + summarizer + merger = 5 calls
|
||||
expect(mockedCreateFnAgent).toHaveBeenCalledTimes(5);
|
||||
// 1 merger AI agent (attempt 1) + 3 fix agent attempts (default) = 4 calls
|
||||
expect(mockedCreateFnAgent).toHaveBeenCalledTimes(4);
|
||||
|
||||
// Verify the log shows 3 fix attempts (2 log entries per attempt: start + failure)
|
||||
const logCalls = (store.logEntry as ReturnType<typeof vi.fn>).mock.calls;
|
||||
|
||||
@@ -2182,6 +2182,17 @@ export async function commitOrAmendMergeWithFixes(
|
||||
const headMoved = currentHead !== preAttemptHeadSha;
|
||||
|
||||
if (!hasStaged && !headMoved) {
|
||||
// Defense-in-depth: if HEAD already carries this task's `Fusion-Task-Id`
|
||||
// trailer, the merge commit landed on a prior code path (e.g. AI commit
|
||||
// in an earlier attempt) and there's simply nothing left for the fix to
|
||||
// fold in. Record success rather than tripping the phantom-merge guard
|
||||
// and stranding the task in In Review when the work is already on main.
|
||||
if (await headCarriesTaskIdTrailer(rootDir, taskId)) {
|
||||
mergerLog.log(
|
||||
`${taskId}: HEAD already carries Fusion-Task-Id trailer — treating in-merge fix finalize as no-op success`,
|
||||
);
|
||||
return true;
|
||||
}
|
||||
// Truly nothing happened — neither a commit nor staged changes. Refuse
|
||||
// to fabricate a successful merge: the caller will report failure.
|
||||
mergerLog.warn(
|
||||
@@ -2709,6 +2720,28 @@ function buildTaskIdTrailerArg(taskId: string): string {
|
||||
return ` -m "${FUSION_TASK_ID_TRAILER_KEY}: ${taskId}"`;
|
||||
}
|
||||
|
||||
/** True iff HEAD's commit message contains the `Fusion-Task-Id: <taskId>`
|
||||
* trailer. Used by the in-merge fix finalizer to recognize that the merge
|
||||
* commit already landed on HEAD (e.g. via the AI commit on a prior attempt)
|
||||
* before tripping the phantom-merge guard. Best-effort: any error returns
|
||||
* false so callers fall back to the conservative "refuse to fabricate" path. */
|
||||
async function headCarriesTaskIdTrailer(rootDir: string, taskId: string): Promise<boolean> {
|
||||
try {
|
||||
const { stdout } = await execAsync("git log -1 --pretty=%B HEAD", {
|
||||
cwd: rootDir,
|
||||
encoding: "utf-8",
|
||||
});
|
||||
// Anchor to line boundaries so e.g. FN-37 doesn't match a body line
|
||||
// mentioning FN-3727. Trailer lines are produced by git itself, so the
|
||||
// exact `Key: Value` form is what we look for.
|
||||
const escapedId = taskId.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
|
||||
const pattern = new RegExp(`(?:^|\\n)${FUSION_TASK_ID_TRAILER_KEY}: ${escapedId}\\s*(?:\\n|$)`);
|
||||
return pattern.test(stdout);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/** Idempotently add the Fusion-Task-Id trailer to HEAD's commit. Used after
|
||||
* the AI agent commits to guarantee the trailer is present even when the
|
||||
* agent didn't include it (especially under includeTaskIdInCommit=false,
|
||||
@@ -5576,13 +5609,25 @@ async function executeMergeAttempt(
|
||||
if (error.message?.includes("Build verification failed")) {
|
||||
throw error; // Fatal - don't retry build failures
|
||||
}
|
||||
|
||||
|
||||
// Check if it's a non-conflict merge failure
|
||||
if (error.message?.includes("Merge failed")) {
|
||||
throw error; // Fatal
|
||||
}
|
||||
|
||||
// For attempt 1, return false to trigger attempt 2
|
||||
// VerificationError must propagate so mergeAttempt's catch can run the
|
||||
// in-merge fix against THIS attempt's preAttemptHeadSha baseline. Falling
|
||||
// through to the attempt-1 retry path here would swallow the error,
|
||||
// trigger attempt 2 with a stale baseline (= AI's commit from attempt 1),
|
||||
// and then the in-merge fix's finalizer would see !hasStaged && !headMoved
|
||||
// and trip the phantom-merge guard even though the task's content is
|
||||
// already on HEAD. Retrying with auto-conflict-resolution can't help a
|
||||
// verification failure anyway — there are no conflicts to resolve.
|
||||
if (error?.name === "VerificationError") {
|
||||
throw error;
|
||||
}
|
||||
|
||||
// For attempt 1, return false to trigger attempt 2 (conflict-only path)
|
||||
if (attemptNum === 1 && smartConflictResolution) {
|
||||
return false;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user