fix(dashboard): prefer merge-base over outdated baseCommitSha
When a worktree was rebased onto newer main, baseCommitSha remained an ancestor of HEAD but the range baseCommitSha..HEAD then swept in upstream main commits as if they were task changes (FN-2840 showed 33 files for a 4-file task). The previous display-recovery only fired when baseCommitSha was no longer an ancestor, missing this case. When enableDisplayRecovery is on and baseBranch is missing, also compute merge-base(HEAD, main) and prefer it when it's a descendant of baseCommitSha (tighter fork point). When merge-base is not a descendant (FN-2855: baseCommitSha is on a deleted feature branch), keep baseCommitSha so we don't widen to unrelated upstream files. Also stabilizes the card-vs-Changes-tab divergence: resolution no longer depends on whether a rebase happened between polls. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -359,6 +359,58 @@ describe("resolveDiffBase", () => {
|
|||||||
expect(diffBase).toBe("parent-123");
|
expect(diffBase).toBe("parent-123");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("display recovery: prefers merge-base over outdated-but-ancestor baseCommitSha", async () => {
|
||||||
|
// Regression: FN-2840 showed 33 changed files in review (12 commits)
|
||||||
|
// because the worktree was rebased onto newer main, leaving an old
|
||||||
|
// baseCommitSha as a still-valid ancestor of HEAD. The actual task
|
||||||
|
// touched only 4 files (3 commits). Prefer merge-base(HEAD, main) when
|
||||||
|
// it's a descendant of baseCommitSha — it's a tighter fork point.
|
||||||
|
const runGit = vi.fn(async (args: string[]) => {
|
||||||
|
const cmd = args.join(" ");
|
||||||
|
if (cmd === "merge-base HEAD main") return "rebased-onto-main";
|
||||||
|
if (cmd === "merge-base --is-ancestor old-base HEAD") return "";
|
||||||
|
if (cmd === "merge-base --is-ancestor old-base rebased-onto-main") return "";
|
||||||
|
throw new Error(`Unexpected command: ${cmd}`);
|
||||||
|
});
|
||||||
|
|
||||||
|
const diffBase = await resolveDiffBase(
|
||||||
|
{ baseCommitSha: "old-base" },
|
||||||
|
"/tmp/worktree",
|
||||||
|
"HEAD",
|
||||||
|
runGit,
|
||||||
|
{ enableDisplayRecovery: true },
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(diffBase).toBe("rebased-onto-main");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("display recovery: keeps baseCommitSha when merge-base is not a descendant (FN-2855)", async () => {
|
||||||
|
// Regression: FN-2855 had baseBranch nulled (deleted upstream feature
|
||||||
|
// branch) with baseCommitSha pointing to a commit on that feature
|
||||||
|
// branch. merge-base(HEAD, main) returns an older commit that is NOT
|
||||||
|
// a descendant of baseCommitSha — so widening to it would surface 108
|
||||||
|
// unrelated upstream files. Keep the task-scoped baseCommitSha.
|
||||||
|
const runGit = vi.fn(async (args: string[]) => {
|
||||||
|
const cmd = args.join(" ");
|
||||||
|
if (cmd === "merge-base HEAD main") return "older-main-commit";
|
||||||
|
if (cmd === "merge-base --is-ancestor task-base-789 HEAD") return "";
|
||||||
|
if (cmd === "merge-base --is-ancestor task-base-789 older-main-commit") {
|
||||||
|
throw new Error("not an ancestor — feature branch base");
|
||||||
|
}
|
||||||
|
throw new Error(`Unexpected command: ${cmd}`);
|
||||||
|
});
|
||||||
|
|
||||||
|
const diffBase = await resolveDiffBase(
|
||||||
|
{ baseCommitSha: "task-base-789" },
|
||||||
|
"/tmp/worktree",
|
||||||
|
"HEAD",
|
||||||
|
runGit,
|
||||||
|
{ enableDisplayRecovery: true },
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(diffBase).toBe("task-base-789");
|
||||||
|
});
|
||||||
|
|
||||||
it("display recovery: does NOT trigger when baseBranch is set (merger-parity case)", async () => {
|
it("display recovery: does NOT trigger when baseBranch is set (merger-parity case)", async () => {
|
||||||
// When baseBranch is recorded, the regular merge-base path runs first.
|
// When baseBranch is recorded, the regular merge-base path runs first.
|
||||||
// Recovery is only meant for the post-rebase "no baseBranch + stale
|
// Recovery is only meant for the post-rebase "no baseBranch + stale
|
||||||
|
|||||||
@@ -36,16 +36,23 @@ export interface ResolveDiffBaseTaskInput {
|
|||||||
|
|
||||||
export interface ResolveDiffBaseOptions {
|
export interface ResolveDiffBaseOptions {
|
||||||
/**
|
/**
|
||||||
* Display-only recovery: when the normal resolution would fall through to
|
* Display-only recovery: when `baseBranch` is missing, also try
|
||||||
* `headRef~1` (because `baseBranch` is missing AND `baseCommitSha` is no
|
* `merge-base(headRef, "main")` (then `origin/main`) and prefer it over
|
||||||
* longer an ancestor of HEAD — e.g., the worktree was rebased onto
|
* `baseCommitSha` if it's a descendant of `baseCommitSha` (i.e. tighter).
|
||||||
* `origin/main` after `baseCommitSha` was recorded), attempt one final
|
* Catches two cases:
|
||||||
* `merge-base(headRef, "main")` (then `origin/main`) before giving up to
|
|
||||||
* `headRef~1`.
|
|
||||||
*
|
*
|
||||||
* This is for the dashboard "files changed" UI only. The merger never opts
|
* 1. **Stale baseCommitSha** (FN-2957): worktree rebased onto `origin/main`
|
||||||
* in — its scope checks must stay tied to the recorded task base, not a
|
* after `baseCommitSha` was recorded; baseCommitSha is no longer an
|
||||||
* widened display range.
|
* ancestor of HEAD. Without recovery the code falls to `headRef~1`,
|
||||||
|
* showing only the latest commit's files.
|
||||||
|
* 2. **Outdated baseCommitSha** (FN-2840): worktree rebased onto newer
|
||||||
|
* `main`; baseCommitSha is *still* an ancestor of HEAD but the range
|
||||||
|
* `baseCommitSha..HEAD` now sweeps in upstream main commits as if they
|
||||||
|
* were task changes (33 files instead of the 4 the task actually
|
||||||
|
* touched). The merge-base is a tighter, more accurate fork point.
|
||||||
|
*
|
||||||
|
* Display-only — the merger never opts in. Its scope checks must stay
|
||||||
|
* tied to the recorded task base, not a widened display range.
|
||||||
*
|
*
|
||||||
* Default: false.
|
* Default: false.
|
||||||
*/
|
*/
|
||||||
@@ -114,34 +121,51 @@ export async function resolveDiffBase(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (task.baseCommitSha) {
|
// Display-only recovery: compute merge-base(HEAD, main) when baseBranch is
|
||||||
try {
|
// missing. Used both to recover from a stale baseCommitSha and to tighten
|
||||||
await runGit(["merge-base", "--is-ancestor", task.baseCommitSha, headRef], cwd, 5000);
|
// an outdated-but-still-ancestor baseCommitSha. See ResolveDiffBaseOptions.
|
||||||
return task.baseCommitSha;
|
let recoveredBase: string | undefined;
|
||||||
} catch {
|
|
||||||
// stale or unreachable — fall through
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Display-only recovery before the HEAD~1 fallback. Only kicks in when the
|
|
||||||
// caller explicitly opted in AND the original resolution skipped the
|
|
||||||
// merge-base step (no baseBranch was recorded). This catches the case where
|
|
||||||
// a worktree got rebased onto origin/main after baseCommitSha was
|
|
||||||
// recorded, leaving the SHA as a non-ancestor of HEAD.
|
|
||||||
if (options.enableDisplayRecovery && !task.baseBranch?.trim()) {
|
if (options.enableDisplayRecovery && !task.baseBranch?.trim()) {
|
||||||
try {
|
try {
|
||||||
const out = (await runGit(["merge-base", headRef, "main"], cwd, 5000)).trim();
|
recoveredBase = (await runGit(["merge-base", headRef, "main"], cwd, 5000)).trim() || undefined;
|
||||||
if (out) return out;
|
|
||||||
} catch {
|
} catch {
|
||||||
try {
|
try {
|
||||||
const out = (await runGit(["merge-base", headRef, "origin/main"], cwd, 5000)).trim();
|
recoveredBase = (await runGit(["merge-base", headRef, "origin/main"], cwd, 5000)).trim() || undefined;
|
||||||
if (out) return out;
|
|
||||||
} catch {
|
} catch {
|
||||||
// no recovery possible — fall through to HEAD~1
|
// no recovery available
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (task.baseCommitSha) {
|
||||||
|
let baseShaIsAncestor = false;
|
||||||
|
try {
|
||||||
|
await runGit(["merge-base", "--is-ancestor", task.baseCommitSha, headRef], cwd, 5000);
|
||||||
|
baseShaIsAncestor = true;
|
||||||
|
} catch {
|
||||||
|
// stale or unreachable
|
||||||
|
}
|
||||||
|
|
||||||
|
if (baseShaIsAncestor) {
|
||||||
|
// Prefer recoveredBase only if it's strictly tighter (a descendant of
|
||||||
|
// baseCommitSha). When baseCommitSha is on a deleted feature branch
|
||||||
|
// it won't be an ancestor of merge-base(HEAD, main), so we keep the
|
||||||
|
// task-scoped SHA — preserves FN-2855 behavior.
|
||||||
|
if (recoveredBase && recoveredBase !== task.baseCommitSha) {
|
||||||
|
try {
|
||||||
|
await runGit(["merge-base", "--is-ancestor", task.baseCommitSha, recoveredBase], cwd, 5000);
|
||||||
|
return recoveredBase;
|
||||||
|
} catch {
|
||||||
|
// recoveredBase not a descendant — keep baseCommitSha
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return task.baseCommitSha;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// baseCommitSha unusable (stale or unset) — use recoveredBase if available.
|
||||||
|
if (recoveredBase) return recoveredBase;
|
||||||
|
|
||||||
try {
|
try {
|
||||||
return (await runGit(["rev-parse", `${headRef}~1`], cwd, 5000)).trim() || undefined;
|
return (await runGit(["rev-parse", `${headRef}~1`], cwd, 5000)).trim() || undefined;
|
||||||
} catch {
|
} catch {
|
||||||
|
|||||||
Reference in New Issue
Block a user