root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(FN-1193): use sha^ base for done-task diff endpoints

Browse Source 
- Update done-task /diff and /file-diffs routes to always resolve merge base from commitSha^ for squash merges
- Remove baseCommitSha ancestor/merge-base fallback logic that could include unrelated merged task changes
- Expand routes-diff regression tests to assert sha^ behavior and ensure baseCommitSha/baseBranch are ignored for done tasks
- Add done-task coverage in routes-file-diffs tests for sha^ success, missing commitSha, and rev-parse failure paths
This commit is contained in:
gsxdsm
2026-04-08 09:35:24 -07:00
parent cc4c4dc7e1
commit 108c3958ac
3 changed files with 182 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files

80
packages/dashboard/src/__tests__/routes-diff.test.ts
View File

@@ -376,7 +376,7 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
vi.useRealTimers();
});
it("shows only this task's files using merge-base, not unrelated main branch changes", async () => {
it("shows only this task's files using sha^, not unrelated main branch changes", async () => {
const store = new MockStore();
store.addTask(createTask({
column: "done",
@@ -386,7 +386,7 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
mockExecSync.mockImplementation((command) => {
const cmd = String(command);
// No baseCommitSha set, so Priority 2: rev-parse first parent
// Done tasks resolve merge base via sha^
if (cmd === "git rev-parse merge789^") {
return "base456\n" as any;
}
@@ -412,7 +412,7 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
expect(response.body.stats.filesChanged).toBe(1);
});
it("computes correct additions and deletions from merge-base diff", async () => {
it("computes correct additions and deletions from sha^ diff", async () => {
const store = new MockStore();
store.addTask(createTask({
column: "done",
@@ -422,7 +422,7 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
mockExecSync.mockImplementation((command) => {
const cmd = String(command);
// No baseCommitSha, so Priority 2: rev-parse first parent
// Done tasks resolve merge base via sha^
if (cmd === "git rev-parse sha_with_modifications^") {
return "base_xyz\n" as any;
}
@@ -457,7 +457,7 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
});
});
it("falls back to parent commit when merge-base fails", async () => {
it("uses parent commit when rev-parse sha^ succeeds", async () => {
const store = new MockStore();
store.addTask(createTask({
column: "done",
@@ -467,8 +467,7 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
mockExecSync.mockImplementation((command) => {
const cmd = String(command);
// No baseCommitSha — Priority 1 skipped
// Priority 2: rev-parse first parent (used to be merge-base)
// Done tasks always resolve the first parent via sha^
if (cmd === "git rev-parse merge_ff^") {
return "parent_ff\n" as any;
}
@@ -490,7 +489,7 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
expect(response.body.files[0].path).toBe("readme.md");
});
it("returns empty result when both rev-parse and merge-base fail", async () => {
it("returns empty result when rev-parse sha^ fails", async () => {
const store = new MockStore();
store.addTask(createTask({
column: "done",
@@ -524,7 +523,7 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
mockExecSync.mockImplementation((command) => {
const cmd = String(command);
// No baseCommitSha — Priority 2: rev-parse first parent
// Done tasks resolve merge base via sha^
if (cmd === "git rev-parse multi_merge^") {
return "multi_base\n" as any;
}
@@ -553,31 +552,30 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
expect(response.body.files[2]).toMatchObject({ path: "src/removed.ts", status: "deleted" });
});
it("uses task.baseCommitSha as diff base when available and valid ancestor", async () => {
it("ignores baseCommitSha even when valid ancestor to avoid cross-task leaks", async () => {
const store = new MockStore();
store.addTask(createTask({
column: "done",
mergeDetails: { commitSha: "merged_commit" },
baseCommitSha: "original_base",
mergeDetails: { commitSha: "task_a_merge" },
baseCommitSha: "very_old_base",
baseBranch: "main",
}));
mockExecSync.mockImplementation((command) => {
const cmd = String(command);
// Priority 1: validate baseCommitSha is ancestor of merge commit — succeeds
if (cmd === "git merge-base --is-ancestor original_base merged_commit") {
return "" as any; // exit 0 = is ancestor
// Scenario: Task B merged between Task A start and Task A merge.
// Using baseCommitSha would include Task B's file, but sha^ isolates Task A.
if (cmd === "git rev-parse task_a_merge^") {
return "task_a_parent\n" as any;
}
// Should NOT reach Priority 2 (merge-base with branch)
if (cmd.includes("git merge-base merged_commit origin/main") || cmd.includes("git merge-base merged_commit main")) {
throw new Error("Should not reach branch merge-base when baseCommitSha is valid");
if (cmd === "git diff --name-status task_a_parent..task_a_merge") {
return "A\ttask-a.ts\n" as any;
}
// Diff from baseCommitSha to merge commit
if (cmd === "git diff --name-status original_base..merged_commit") {
return "A\tfeature-file.ts\n" as any;
if (cmd === 'git diff task_a_parent..task_a_merge -- "task-a.ts"') {
return "diff --git a/task-a.ts b/task-a.ts\n+task A only\n" as any;
}
if (cmd === 'git diff original_base..merged_commit -- "feature-file.ts"') {
return "diff --git a/feature-file.ts b/feature-file.ts\n+new feature\n" as any;
if (cmd.includes("git merge-base --is-ancestor")) {
throw new Error("Done-task diff must ignore baseCommitSha");
}
throw new Error(`Unexpected command: ${cmd}`);
});
@@ -587,16 +585,19 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
expect(response.status).toBe(200);
expect(response.body.files).toHaveLength(1);
expect(response.body.files[0].path).toBe("feature-file.ts");
expect(response.body.files[0].status).toBe("added");
// Verify baseCommitSha validation was called
expect(response.body.files[0].path).toBe("task-a.ts");
expect(response.body.files.map((f: any) => f.path)).not.toContain("task-b.ts");
expect(mockExecSync).toHaveBeenCalledWith(
expect.stringContaining("git merge-base --is-ancestor original_base merged_commit"),
"git rev-parse task_a_merge^",
expect.any(Object),
);
expect(mockExecSync).not.toHaveBeenCalledWith(
expect.stringContaining("git merge-base --is-ancestor"),
expect.any(Object),
);
});
it("falls through to branch merge-base when baseCommitSha is not an ancestor", async () => {
it("uses sha^ even when baseCommitSha is stale", async () => {
const store = new MockStore();
store.addTask(createTask({
column: "done",
@@ -607,21 +608,18 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
mockExecSync.mockImplementation((command) => {
const cmd = String(command);
// Priority 1: baseCommitSha is NOT a valid ancestor — fails
if (cmd === "git merge-base --is-ancestor stale_base merged_commit") {
throw new Error("not an ancestor");
}
// Priority 2: rev-parse first parent of merge commit
if (cmd === "git rev-parse merged_commit^") {
return "branch_base\n" as any;
}
// Diff from first parent
if (cmd === "git diff --name-status branch_base..merged_commit") {
return "M\tsrc/app.ts\n" as any;
}
if (cmd === 'git diff branch_base..merged_commit -- "src/app.ts"') {
return "diff --git a/src/app.ts b/src/app.ts\n-old\n+new\n" as any;
}
if (cmd.includes("git merge-base --is-ancestor")) {
throw new Error("Done-task diff must not validate baseCommitSha");
}
throw new Error(`Unexpected command: ${cmd}`);
});
@@ -631,30 +629,28 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
expect(response.status).toBe(200);
expect(response.body.files).toHaveLength(1);
expect(response.body.files[0].path).toBe("src/app.ts");
// Verify baseCommitSha was tried first
expect(mockExecSync).toHaveBeenCalledWith(
expect.stringContaining("git merge-base --is-ancestor stale_base merged_commit"),
"git rev-parse merged_commit^",
expect.any(Object),
);
// Verify rev-parse first parent was called as fallback
expect(mockExecSync).toHaveBeenCalledWith(
expect.stringContaining("git rev-parse merged_commit^"),
expect(mockExecSync).not.toHaveBeenCalledWith(
expect.stringContaining("git merge-base --is-ancestor"),
expect.any(Object),
);
});
it("uses custom baseBranch for merge-base computation", async () => {
it("uses sha^ regardless of baseBranch", async () => {
const store = new MockStore();
store.addTask(createTask({
column: "done",
mergeDetails: { commitSha: "custom_merge" },
// No baseCommitSha — should go straight to Priority 2
// baseBranch should be ignored for done-task diffs
baseBranch: "release/v2",
}));
mockExecSync.mockImplementation((command) => {
const cmd = String(command);
// No baseCommitSha — Priority 2: rev-parse first parent
// Done tasks resolve merge base via sha^
if (cmd === "git rev-parse custom_merge^") {
return "release_base\n" as any;
}

121
packages/dashboard/src/__tests__/routes-file-diffs.test.ts
View File

@@ -465,6 +465,127 @@ describe("GET /api/tasks/:id/file-diffs", () => {
expect(diffPaths.sort()).toEqual(sessionFiles.sort());
});
// ── Done task file-diffs: first-parent computation ─────────────────────────────
it("returns done-task file diffs from sha^..sha", async () => {
const store = new MockStore();
store.addTask(createTask({
column: "done",
mergeDetails: { commitSha: "done_merge_sha" },
baseCommitSha: "ignored_base",
worktree: undefined,
}));
mockExecSync.mockImplementation((command) => {
const cmd = String(command);
if (cmd === "git rev-parse done_merge_sha^") {
return "done_parent\n" as any;
}
if (cmd === "git diff --name-status done_parent..done_merge_sha") {
return "A\tsrc/done.ts\n" as any;
}
if (cmd === 'git diff done_parent..done_merge_sha -- "src/done.ts"') {
return "diff --git a/src/done.ts b/src/done.ts\n+done\n" as any;
}
if (cmd.includes("git merge-base --is-ancestor")) {
throw new Error("Done-task file-diffs must ignore baseCommitSha");
}
throw new Error(`Unexpected command: ${cmd}`);
});
const response = await requestFileDiffs(store);
expect(response.status).toBe(200);
expect(response.body).toEqual([
{
path: "src/done.ts",
status: "added",
diff: "diff --git a/src/done.ts b/src/done.ts\n+done\n",
},
]);
expect(mockExecSync).toHaveBeenCalledWith(
"git rev-parse done_merge_sha^",
expect.any(Object),
);
});
it("ignores done-task baseCommitSha even when valid ancestor", async () => {
const store = new MockStore();
store.addTask(createTask({
column: "done",
mergeDetails: { commitSha: "task_a_merge" },
baseCommitSha: "very_old_base",
worktree: undefined,
}));
mockExecSync.mockImplementation((command) => {
const cmd = String(command);
// Task B was merged between Task A start and Task A merge.
// sha^ keeps only Task A file and prevents a cross-task leak.
if (cmd === "git rev-parse task_a_merge^") {
return "task_a_parent\n" as any;
}
if (cmd === "git diff --name-status task_a_parent..task_a_merge") {
return "A\ttask-a.ts\n" as any;
}
if (cmd === 'git diff task_a_parent..task_a_merge -- "task-a.ts"') {
return "diff --git a/task-a.ts b/task-a.ts\n+task A\n" as any;
}
if (cmd.includes("git merge-base --is-ancestor")) {
throw new Error("Done-task file-diffs must not validate baseCommitSha");
}
throw new Error(`Unexpected command: ${cmd}`);
});
const response = await requestFileDiffs(store);
expect(response.status).toBe(200);
expect(response.body).toHaveLength(1);
expect(response.body[0].path).toBe("task-a.ts");
expect(response.body.map((file: any) => file.path)).not.toContain("task-b.ts");
expect(mockExecSync).not.toHaveBeenCalledWith(
expect.stringContaining("git merge-base --is-ancestor"),
expect.any(Object),
);
});
it("returns empty array for done task without commitSha", async () => {
const store = new MockStore();
store.addTask(createTask({
column: "done",
mergeDetails: undefined,
worktree: undefined,
}));
const response = await requestFileDiffs(store);
expect(response.status).toBe(200);
expect(response.body).toEqual([]);
expect(mockExecSync).not.toHaveBeenCalled();
});
it("returns empty array when done-task sha^ cannot be resolved", async () => {
const store = new MockStore();
store.addTask(createTask({
column: "done",
mergeDetails: { commitSha: "broken_merge_sha" },
worktree: undefined,
}));
mockExecSync.mockImplementation((command) => {
const cmd = String(command);
if (cmd === "git rev-parse broken_merge_sha^") {
throw new Error("unknown revision");
}
throw new Error(`Unexpected command: ${cmd}`);
});
const response = await requestFileDiffs(store);
expect(response.status).toBe(200);
expect(response.body).toEqual([]);
});
// ── Regression: shared/recycled worktree produces broader file sets ─────────────────
it("task-scoped baseCommitSha narrows file-diffs to this task's work", async () => {