FN-5666: exclude base commit from done-task diff ranges
Prevent done-task diff endpoints from counting base-commit-only lineage as task-owned changes. - add base-boundary handling in done-task merge SHA resolution so `baseCommitSha` is excluded by default and only consulted for boundary checks - short-circuit `/diff` and `/file-diffs` responses to empty results when resolved done-task SHA is at or below `baseCommitSha` - export and use an ancestry helper to detect base-boundary cases, while preserving normal and rebase range behavior - extend done-task diff route tests with FN-5666 cases for no-op merge SHA, lineage association at base commit, normal post-base commits, and rebase ranges - add a patch changeset for `@runfusion/fusion` Files changed: .changeset/fn-5666-base-commit-exclusive.md | 5 + .../src/__tests__/routes-diff-done-tasks.test.ts | 217 ++++++++++++++++++++- .../src/routes/register-session-diff-routes.ts | 57 +++++- 3 files changed, 273 insertions(+), 6 deletions(-) Fusion-Task-Id: FN-5666 Fusion-Task-Lineage: 782f9875-856a-4d31-9505-0bcd638c9c29
This commit is contained in:
gsxdsm
5
.changeset/fn-5666-base-commit-exclusive.md
Normal file
5
.changeset/fn-5666-base-commit-exclusive.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Fix per-task diff view incorrectly including a task's base commit when a done task lands as a no-op or its resolved merge SHA equals `baseCommitSha`.
|
||||
@@ -138,6 +138,12 @@ async function getDoneDiff(store: RealGitStore, taskId = "FN-4524") {
|
||||
return get(app, `/api/tasks/${taskId}/diff`);
|
||||
}
|
||||
|
||||
async function getDoneFileDiffs(store: RealGitStore, taskId = "FN-4524") {
|
||||
const app = createServer(store as any);
|
||||
const { get } = await import("../test-request.js");
|
||||
return get(app, `/api/tasks/${taskId}/file-diffs`);
|
||||
}
|
||||
|
||||
describe("FN-4524 done-task diff stats", () => {
|
||||
it("matches shortstat and excludes interleaved foreign commit files", async () => {
|
||||
const rootDir = mkdtempSync(join(tmpdir(), "fn-4524-done-lineage-"));
|
||||
@@ -562,7 +568,8 @@ describe("FN-4524 done-task diff stats", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("matches shortstat when hunks include ++/-- content lines", async () => { const rootDir = mkdtempSync(join(tmpdir(), "fn-4524-done-plusminus-"));
|
||||
it("matches shortstat when hunks include ++/-- content lines", async () => {
|
||||
const rootDir = mkdtempSync(join(tmpdir(), "fn-4524-done-plusminus-"));
|
||||
|
||||
try {
|
||||
git(rootDir, "init", "-b", "main");
|
||||
@@ -576,7 +583,6 @@ describe("FN-4524 done-task diff stats", () => {
|
||||
|
||||
git(rootDir, "checkout", "main");
|
||||
git(rootDir, "merge", "task-branch", "--no-ff", "-m", "merge plusminus");
|
||||
const mergeCommit = git(rootDir, "rev-parse", "HEAD");
|
||||
const expected = shortstatForLineage(rootDir, [patchCommit]);
|
||||
|
||||
const lineageId = "lin-fn-4524-d";
|
||||
@@ -606,4 +612,211 @@ describe("FN-4524 done-task diff stats", () => {
|
||||
rmSync(rootDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
describe("FN-5666 baseCommitSha exclusivity", () => {
|
||||
it("returns empty done-task diff and file-diffs when merge sha equals baseCommitSha", async () => {
|
||||
const rootDir = mkdtempSync(join(tmpdir(), "fn-5666-no-op-merge-sha-"));
|
||||
|
||||
try {
|
||||
git(rootDir, "init", "-b", "main");
|
||||
git(rootDir, "config", "user.email", "fusion@example.com");
|
||||
git(rootDir, "config", "user.name", "Fusion");
|
||||
|
||||
writeFileSync(join(rootDir, "a.ts"), "export const a = 1;\n");
|
||||
writeFileSync(join(rootDir, "b.ts"), "export const b = 1;\n");
|
||||
writeFileSync(join(rootDir, "c.ts"), "export const c = 1;\n");
|
||||
writeFileSync(join(rootDir, "d.ts"), "export const d = 1;\n");
|
||||
writeFileSync(join(rootDir, "e.ts"), "export const e = 1;\n");
|
||||
git(rootDir, "add", "a.ts", "b.ts", "c.ts", "d.ts", "e.ts");
|
||||
git(rootDir, "commit", "-m", "fat base commit");
|
||||
const baseCommit = git(rootDir, "rev-parse", "HEAD");
|
||||
|
||||
const store = new RealGitStore(rootDir);
|
||||
store.addTask({
|
||||
id: "FN-4524",
|
||||
title: "fn-5666 no-op",
|
||||
description: "fn-5666 no-op",
|
||||
column: "done",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-05-29T00:00:00.000Z",
|
||||
updatedAt: "2026-05-29T00:00:00.000Z",
|
||||
columnMovedAt: "2026-05-29T00:00:00.000Z",
|
||||
branch: "main",
|
||||
baseCommitSha: baseCommit,
|
||||
mergeDetails: { commitSha: baseCommit },
|
||||
} as Task);
|
||||
|
||||
const diffResponse = await getDoneDiff(store);
|
||||
expect(diffResponse.status).toBe(200);
|
||||
expect(diffResponse.body.files).toEqual([]);
|
||||
expect(diffResponse.body.stats).toEqual({ filesChanged: 0, additions: 0, deletions: 0 });
|
||||
|
||||
const fileDiffResponse = await getDoneFileDiffs(store);
|
||||
expect(fileDiffResponse.status).toBe(200);
|
||||
expect(fileDiffResponse.body).toEqual([]);
|
||||
} finally {
|
||||
rmSync(rootDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("returns empty done-task diffs when lineage association points at baseCommitSha", async () => {
|
||||
const rootDir = mkdtempSync(join(tmpdir(), "fn-5666-no-op-lineage-"));
|
||||
|
||||
try {
|
||||
git(rootDir, "init", "-b", "main");
|
||||
git(rootDir, "config", "user.email", "fusion@example.com");
|
||||
git(rootDir, "config", "user.name", "Fusion");
|
||||
|
||||
writeFileSync(join(rootDir, "fat-a.ts"), "export const fatA = 1;\n");
|
||||
writeFileSync(join(rootDir, "fat-b.ts"), "export const fatB = 1;\n");
|
||||
writeFileSync(join(rootDir, "fat-c.ts"), "export const fatC = 1;\n");
|
||||
writeFileSync(join(rootDir, "fat-d.ts"), "export const fatD = 1;\n");
|
||||
writeFileSync(join(rootDir, "fat-e.ts"), "export const fatE = 1;\n");
|
||||
git(rootDir, "add", "fat-a.ts", "fat-b.ts", "fat-c.ts", "fat-d.ts", "fat-e.ts");
|
||||
git(rootDir, "commit", "-m", "fat base commit");
|
||||
const baseCommit = git(rootDir, "rev-parse", "HEAD");
|
||||
|
||||
const lineageId = "lin-fn-5666-no-op";
|
||||
const store = new RealGitStore(rootDir);
|
||||
store.addTask({
|
||||
id: "FN-4524",
|
||||
title: "fn-5666 lineage no-op",
|
||||
description: "fn-5666 lineage no-op",
|
||||
column: "done",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-05-29T00:00:00.000Z",
|
||||
updatedAt: "2026-05-29T00:00:00.000Z",
|
||||
columnMovedAt: "2026-05-29T00:00:00.000Z",
|
||||
lineageId,
|
||||
baseCommitSha: baseCommit,
|
||||
branch: "main",
|
||||
mergeDetails: {},
|
||||
} as Task);
|
||||
store.setAssociations(lineageId, [mkAssoc(lineageId, baseCommit, "2026-05-29T00:00:01.000Z")]);
|
||||
|
||||
const diffResponse = await getDoneDiff(store);
|
||||
expect(diffResponse.status).toBe(200);
|
||||
expect(diffResponse.body.files).toEqual([]);
|
||||
expect(diffResponse.body.stats).toEqual({ filesChanged: 0, additions: 0, deletions: 0 });
|
||||
|
||||
const fileDiffResponse = await getDoneFileDiffs(store);
|
||||
expect(fileDiffResponse.status).toBe(200);
|
||||
expect(fileDiffResponse.body).toEqual([]);
|
||||
} finally {
|
||||
rmSync(rootDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("still reports files introduced after baseCommitSha", async () => {
|
||||
const rootDir = mkdtempSync(join(tmpdir(), "fn-5666-normal-range-"));
|
||||
|
||||
try {
|
||||
git(rootDir, "init", "-b", "main");
|
||||
git(rootDir, "config", "user.email", "fusion@example.com");
|
||||
git(rootDir, "config", "user.name", "Fusion");
|
||||
|
||||
const baseCommit = commitFile(rootDir, "base.ts", "export const base = 1;\n", "base");
|
||||
const landedCommit = (() => {
|
||||
writeFileSync(join(rootDir, "one.ts"), "export const one = 1;\n");
|
||||
writeFileSync(join(rootDir, "two.ts"), "export const two = 2;\n");
|
||||
writeFileSync(join(rootDir, "three.ts"), "export const three = 3;\n");
|
||||
git(rootDir, "add", "one.ts", "two.ts", "three.ts");
|
||||
git(rootDir, "commit", "-m", "landed commit");
|
||||
return git(rootDir, "rev-parse", "HEAD");
|
||||
})();
|
||||
|
||||
const expectedPaths = git(rootDir, "diff", "--name-only", `${baseCommit}..${landedCommit}`)
|
||||
.split("\n")
|
||||
.map((line) => line.trim())
|
||||
.filter(Boolean)
|
||||
.sort();
|
||||
|
||||
const store = new RealGitStore(rootDir);
|
||||
store.addTask({
|
||||
id: "FN-4524",
|
||||
title: "fn-5666 normal case",
|
||||
description: "fn-5666 normal case",
|
||||
column: "done",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-05-29T00:00:00.000Z",
|
||||
updatedAt: "2026-05-29T00:00:00.000Z",
|
||||
columnMovedAt: "2026-05-29T00:00:00.000Z",
|
||||
branch: "main",
|
||||
baseCommitSha: baseCommit,
|
||||
mergeDetails: { commitSha: landedCommit },
|
||||
} as Task);
|
||||
|
||||
const diffResponse = await getDoneDiff(store);
|
||||
expect(diffResponse.status).toBe(200);
|
||||
expect(diffResponse.body.stats.filesChanged).toBe(3);
|
||||
expect(diffResponse.body.files.map((f: { path: string }) => f.path).sort()).toEqual(expectedPaths);
|
||||
} finally {
|
||||
rmSync(rootDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps rebaseBaseSha..commitSha done-task ranges intact", async () => {
|
||||
const rootDir = mkdtempSync(join(tmpdir(), "fn-5666-rebase-range-"));
|
||||
|
||||
try {
|
||||
git(rootDir, "init", "-b", "main");
|
||||
git(rootDir, "config", "user.email", "fusion@example.com");
|
||||
git(rootDir, "config", "user.name", "Fusion");
|
||||
|
||||
const baseCommit = commitFile(rootDir, "base.ts", "export const base = 1;\n", "base");
|
||||
const landedCommit = (() => {
|
||||
writeFileSync(join(rootDir, "rebased-one.ts"), "export const rebasedOne = 1;\n");
|
||||
writeFileSync(join(rootDir, "rebased-two.ts"), "export const rebasedTwo = 2;\n");
|
||||
writeFileSync(join(rootDir, "rebased-three.ts"), "export const rebasedThree = 3;\n");
|
||||
git(rootDir, "add", "rebased-one.ts", "rebased-two.ts", "rebased-three.ts");
|
||||
git(rootDir, "commit", "-m", "rebased landed commit");
|
||||
return git(rootDir, "rev-parse", "HEAD");
|
||||
})();
|
||||
|
||||
const expectedPaths = git(rootDir, "diff", "--name-only", `${baseCommit}..${landedCommit}`)
|
||||
.split("\n")
|
||||
.map((line) => line.trim())
|
||||
.filter(Boolean)
|
||||
.sort();
|
||||
|
||||
const store = new RealGitStore(rootDir);
|
||||
store.addTask({
|
||||
id: "FN-4524",
|
||||
title: "fn-5666 rebase case",
|
||||
description: "fn-5666 rebase case",
|
||||
column: "done",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-05-29T00:00:00.000Z",
|
||||
updatedAt: "2026-05-29T00:00:00.000Z",
|
||||
columnMovedAt: "2026-05-29T00:00:00.000Z",
|
||||
branch: "main",
|
||||
baseCommitSha: baseCommit,
|
||||
mergeDetails: { commitSha: landedCommit, rebaseBaseSha: baseCommit },
|
||||
} as Task);
|
||||
|
||||
const diffResponse = await getDoneDiff(store);
|
||||
expect(diffResponse.status).toBe(200);
|
||||
expect(diffResponse.body.stats.filesChanged).toBe(3);
|
||||
expect(diffResponse.body.files.map((f: { path: string }) => f.path).sort()).toEqual(expectedPaths);
|
||||
|
||||
const fileDiffResponse = await getDoneFileDiffs(store);
|
||||
expect(fileDiffResponse.status).toBe(200);
|
||||
expect(fileDiffResponse.body.map((f: { path: string }) => f.path).sort()).toEqual(expectedPaths);
|
||||
} finally {
|
||||
rmSync(rootDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -378,17 +378,43 @@ async function resolveAuditCommitSha(taskId: string, scopedStore: DoneTaskAggreg
|
||||
return undefined;
|
||||
}
|
||||
|
||||
async function resolveDoneTaskMergeSha(task: DoneTaskAggregationTask, scopedStore: DoneTaskAggregationStore): Promise<string | undefined> {
|
||||
export async function isAtOrBelowTaskBase(sha: string, baseCommitSha: string, rootDir: string): Promise<boolean> {
|
||||
if (sha === baseCommitSha) return true;
|
||||
try {
|
||||
await runGitCommand(["merge-base", "--is-ancestor", sha, baseCommitSha], rootDir, 5000);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
async function resolveDoneTaskMergeSha(
|
||||
task: DoneTaskAggregationTask & { baseCommitSha?: string | null },
|
||||
scopedStore: DoneTaskAggregationStore,
|
||||
options?: { includeBaseCommitSha?: boolean },
|
||||
): Promise<string | undefined> {
|
||||
const rootDir = scopedStore.getRootDir();
|
||||
const baseCommitSha = task.baseCommitSha?.trim();
|
||||
const includeBaseCommitSha = options?.includeBaseCommitSha === true;
|
||||
|
||||
const existing = task.mergeDetails?.commitSha?.trim();
|
||||
if (existing) return existing;
|
||||
if (existing) {
|
||||
if (!includeBaseCommitSha && baseCommitSha && existing === baseCommitSha) return undefined;
|
||||
return existing;
|
||||
}
|
||||
|
||||
const auditSha = await resolveAuditCommitSha(task.id, scopedStore);
|
||||
if (auditSha) return auditSha;
|
||||
if (auditSha) {
|
||||
if (!includeBaseCommitSha && baseCommitSha && auditSha === baseCommitSha) return undefined;
|
||||
return auditSha;
|
||||
}
|
||||
|
||||
if (!task.lineageId) return undefined;
|
||||
const associations = await scopedStore.getTaskCommitAssociationsByLineageId(task.lineageId);
|
||||
for (const association of associations) {
|
||||
if (association.commitSha && (await isReachableFromHead(association.commitSha, scopedStore.getRootDir()))) {
|
||||
if (!association.commitSha) continue;
|
||||
if (!includeBaseCommitSha && baseCommitSha && association.commitSha === baseCommitSha) continue;
|
||||
if (await isReachableFromHead(association.commitSha, rootDir)) {
|
||||
return association.commitSha;
|
||||
}
|
||||
}
|
||||
@@ -670,7 +696,19 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
}
|
||||
|
||||
if (task.column === "done") {
|
||||
const mergeShaForBaseBoundary = await resolveDoneTaskMergeSha(task, scopedStore, { includeBaseCommitSha: true });
|
||||
const resolvedMergeSha = await resolveDoneTaskMergeSha(task, scopedStore);
|
||||
if (mergeShaForBaseBoundary && task.baseCommitSha) {
|
||||
// FN-5666: `git diff A..B` is exclusive of A, and baseCommitSha is the
|
||||
// task fork point, so when resolved SHA is at/below base the task has
|
||||
// no owned changes to display.
|
||||
const atOrBelowBase = await isAtOrBelowTaskBase(mergeShaForBaseBoundary, task.baseCommitSha, scopedStore.getRootDir());
|
||||
if (atOrBelowBase) {
|
||||
res.json({ files: [], stats: { filesChanged: 0, additions: 0, deletions: 0 } });
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
const doneTaskForDiff = resolvedMergeSha
|
||||
? {
|
||||
...task,
|
||||
@@ -931,7 +969,18 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
}
|
||||
|
||||
if (task.column === "done") {
|
||||
const mergeShaForBaseBoundary = await resolveDoneTaskMergeSha(task, scopedStore, { includeBaseCommitSha: true });
|
||||
const resolvedMergeSha = await resolveDoneTaskMergeSha(task, scopedStore);
|
||||
if (mergeShaForBaseBoundary && task.baseCommitSha) {
|
||||
// FN-5666: baseCommitSha is the branch fork point and must remain
|
||||
// exclusive in per-task display diffs.
|
||||
const atOrBelowBase = await isAtOrBelowTaskBase(mergeShaForBaseBoundary, task.baseCommitSha, scopedStore.getRootDir());
|
||||
if (atOrBelowBase) {
|
||||
res.json([]);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
const doneTaskForDiff = resolvedMergeSha
|
||||
? {
|
||||
...task,
|
||||
|
||||
Reference in New Issue
Block a user