fix(FN-4417): prefer local main over origin/main in contamination base
Follow-up toa2b494ddc. The previous fix correctly stopped using task.baseCommitSha for the contamination check, but the new resolveContaminationBaseRef tried origin/main first and fell back to local main only if the origin/main lookup failed. On dev machines that have not pushed recently, origin/main is a tracking ref that can lag local main by hundreds of commits. `git merge-base HEAD origin/main` then resolves successfully and returns the last common ancestor between the (force-reset-to-local-main) branch and the stale origin/main ref, which is exactly the same stale SHA the prior bug surfaced. Every commit on local main since that point is then flagged as foreign contamination. Observed: after restarting the engine witha2b494ddc, FN-4315/FN-4403 still failed with `since base e787036b80cc...`. Local main was 166 commits ahead of origin/main;e787036was the last shared commit. Fix: reorder the shell fallback to prefer `main` over `origin/main`. Local main is the canonical integration target for Fusion's merger, and origin/main is only useful as a fallback when local main does not exist (rare in a worktree). Test updated: `resolveContaminationBaseRef returns the current merge-base...` now asserts local `main` appears before `origin/main` in the issued command so the shell `||` resolves to local main first. Fusion-Task-Id: FN-4417
This commit is contained in:
@@ -40,9 +40,19 @@ describe("resolveContaminationBaseRef (FN-4417)", () => {
|
||||
);
|
||||
|
||||
expect(result).toBe("fresh_main_sha");
|
||||
// Must have asked for merge-base against origin/main || main, never
|
||||
// fallen back to HEAD~1 or read task.baseCommitSha.
|
||||
expect(calls.some((c) => c.includes("merge-base HEAD origin/main"))).toBe(true);
|
||||
// Must have asked for merge-base against local main first (preferred
|
||||
// over origin/main, which can lag local main by hundreds of commits on
|
||||
// dev machines and re-introduce the FN-4417 false positive). Must not
|
||||
// fall back to HEAD~1 (which on a force-reset branch is a commit on
|
||||
// main itself).
|
||||
const mergeBaseCall = calls.find((c) => c.includes("merge-base"));
|
||||
expect(mergeBaseCall).toBeDefined();
|
||||
// Local `main` must appear before `origin/main` in the command so the
|
||||
// shell `||` fallback prefers it.
|
||||
const localMainIdx = mergeBaseCall!.indexOf("merge-base HEAD main");
|
||||
const originMainIdx = mergeBaseCall!.indexOf("merge-base HEAD origin/main");
|
||||
expect(localMainIdx).toBeGreaterThanOrEqual(0);
|
||||
expect(localMainIdx).toBeLessThan(originMainIdx === -1 ? Number.MAX_SAFE_INTEGER : originMainIdx);
|
||||
expect(calls.some((c) => c.includes("HEAD~1"))).toBe(false);
|
||||
});
|
||||
|
||||
|
||||
@@ -5768,9 +5768,16 @@ ${failureFeedback}
|
||||
* the caller is expected to treat that as "contamination check skipped".
|
||||
*/
|
||||
private async resolveContaminationBaseRef(worktreePath: string): Promise<string | undefined> {
|
||||
// Prefer LOCAL main over origin/main. origin/main is a tracking ref that
|
||||
// is only as fresh as the last `git fetch` — on dev machines that haven't
|
||||
// pushed in a while it can lag local main by hundreds of commits, which
|
||||
// re-introduces the FN-4417 false positive at a smaller scale (the
|
||||
// merge-base falls back to the last common ancestor between HEAD and the
|
||||
// stale origin/main, and every commit on local main since then looks
|
||||
// "foreign"). Local main is the canonical integration target for Fusion.
|
||||
try {
|
||||
const { stdout } = await execAsync(
|
||||
"git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main",
|
||||
"git merge-base HEAD main 2>/dev/null || git merge-base HEAD origin/main",
|
||||
{ cwd: worktreePath, encoding: "utf-8" },
|
||||
);
|
||||
const ref = stdout.trim();
|
||||
|
||||
Reference in New Issue
Block a user