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

fix(FN-951): use baseCommitSha for done task diff base calculation

Browse Source 
- Fix diff base resolution for done/merged tasks to use task.baseCommitSha as priority
- Add 3-tier fallback: baseCommitSha → merge-base with base branch → first parent
- Validate baseCommitSha is a valid ancestor before using it
- Use task.baseBranch (defaulting to main) for merge-base computation
- Update and expand test cases to cover new priority resolution logic
This commit is contained in:
gsxdsm
2026-04-04 23:21:39 -07:00
parent 818f5786bd
commit 2b857cd245
2 changed files with 215 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -381,16 +381,13 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
store.addTask(createTask({
column: "done",
mergeDetails: { commitSha: "merge789" },
baseBranch: "main",
}));
mockExecSync.mockImplementation((command) => {
const cmd = String(command);
// git rev-parse merge789^ → first parent (main branch tip at merge time)
if (cmd === "git rev-parse merge789^") {
return "parent123\n" as any;
}
// git merge-base merge789 parent123 → true divergence point
if (cmd === "git merge-base merge789 parent123") {
// No baseCommitSha set, so Priority 2: merge-base with base branch
if (cmd.includes("git merge-base merge789 origin/main") || cmd.includes("git merge-base merge789 main")) {
return "base456\n" as any;
}
// git diff --name-status base456..merge789 → only this task's files
@@ -420,14 +417,13 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
store.addTask(createTask({
column: "done",
mergeDetails: { commitSha: "sha_with_modifications" },
baseBranch: "main",
}));
mockExecSync.mockImplementation((command) => {
const cmd = String(command);
if (cmd === "git rev-parse sha_with_modifications^") {
return "parent_abc\n" as any;
}
if (cmd === "git merge-base sha_with_modifications parent_abc") {
// No baseCommitSha, so Priority 2: merge-base with base branch
if (cmd.includes("git merge-base sha_with_modifications origin/main") || cmd.includes("git merge-base sha_with_modifications main")) {
return "base_xyz\n" as any;
}
if (cmd === "git diff --name-status base_xyz..sha_with_modifications") {
@@ -466,19 +462,20 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
store.addTask(createTask({
column: "done",
mergeDetails: { commitSha: "merge_ff" },
baseBranch: "main",
}));
mockExecSync.mockImplementation((command) => {
const cmd = String(command);
// git rev-parse merge_ff^ succeeds (fast-forward: single parent)
// No baseCommitSha — Priority 1 skipped
// Priority 2: merge-base with base branch fails
if (cmd.includes("git merge-base merge_ff origin/main") || cmd.includes("git merge-base merge_ff main")) {
throw new Error("fatal: not a git repository");
}
// Priority 3: fall back to first parent
if (cmd === "git rev-parse merge_ff^") {
return "parent_ff\n" as any;
}
// git merge-base fails (e.g., shallow clone)
if (cmd === "git merge-base merge_ff parent_ff") {
throw new Error("fatal: not a git repository");
}
// Fallback: git rev-parse merge_ff^ → parent_ff (called again in catch block)
// Name-status from parent to merge commit
if (cmd === "git diff --name-status parent_ff..merge_ff") {
return "M\treadme.md\n" as any;
@@ -526,14 +523,13 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
store.addTask(createTask({
column: "done",
mergeDetails: { commitSha: "multi_merge" },
baseBranch: "main",
}));
mockExecSync.mockImplementation((command) => {
const cmd = String(command);
if (cmd === "git rev-parse multi_merge^") {
return "multi_parent\n" as any;
}
if (cmd === "git merge-base multi_merge multi_parent") {
// No baseCommitSha — Priority 2: merge-base with base branch
if (cmd.includes("git merge-base multi_merge origin/main") || cmd.includes("git merge-base multi_merge main")) {
return "multi_base\n" as any;
}
if (cmd === "git diff --name-status multi_base..multi_merge") {
@@ -560,4 +556,131 @@ describe("GET /api/tasks/:id/diff — done tasks", () => {
expect(response.body.files[1]).toMatchObject({ path: "src/changed.ts", status: "modified" });
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 () => {
const store = new MockStore();
store.addTask(createTask({
column: "done",
mergeDetails: { commitSha: "merged_commit" },
baseCommitSha: "original_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
}
// 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");
}
// 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 original_base..merged_commit -- "feature-file.ts"') {
return "diff --git a/feature-file.ts b/feature-file.ts\n+new feature\n" as any;
}
throw new Error(`Unexpected command: ${cmd}`);
});
const app = createServer(store as any);
const response = await requestDiff(app);
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(mockExecSync).toHaveBeenCalledWith(
expect.stringContaining("git merge-base --is-ancestor original_base merged_commit"),
expect.any(Object),
);
});
it("falls through to branch merge-base when baseCommitSha is not an ancestor", async () => {
const store = new MockStore();
store.addTask(createTask({
column: "done",
mergeDetails: { commitSha: "merged_commit" },
baseCommitSha: "stale_base",
baseBranch: "develop",
}));
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: merge-base with base branch succeeds
if (cmd.includes("git merge-base merged_commit origin/develop") || cmd.includes("git merge-base merged_commit develop")) {
return "branch_base\n" as any;
}
// Diff from branch merge-base
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;
}
throw new Error(`Unexpected command: ${cmd}`);
});
const app = createServer(store as any);
const response = await requestDiff(app);
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"),
expect.any(Object),
);
// Verify branch merge-base was called as fallback
expect(mockExecSync).toHaveBeenCalledWith(
expect.stringContaining("git merge-base merged_commit"),
expect.any(Object),
);
});
it("uses custom baseBranch for merge-base computation", async () => {
const store = new MockStore();
store.addTask(createTask({
column: "done",
mergeDetails: { commitSha: "custom_merge" },
// No baseCommitSha — should go straight to Priority 2
baseBranch: "release/v2",
}));
mockExecSync.mockImplementation((command) => {
const cmd = String(command);
// Priority 2: merge-base with custom base branch
if (cmd.includes("git merge-base custom_merge origin/release/v2") || cmd.includes("git merge-base custom_merge release/v2")) {
return "release_base\n" as any;
}
if (cmd === "git diff --name-status release_base..custom_merge") {
return "A\trelease-file.ts\n" as any;
}
if (cmd === 'git diff release_base..custom_merge -- "release-file.ts"') {
return "diff --git a/release-file.ts b/release-file.ts\n+release stuff\n" as any;
}
throw new Error(`Unexpected command: ${cmd}`);
});
const app = createServer(store as any);
const response = await requestDiff(app);
expect(response.status).toBe(200);
expect(response.body.files).toHaveLength(1);
expect(response.body.files[0].path).toBe("release-file.ts");
// Verify the custom base branch was used
expect(mockExecSync).toHaveBeenCalledWith(
expect.stringContaining("release/v2"),
expect.any(Object),
);
});
});

102
packages/dashboard/src/routes.ts
View File

@@ -7504,26 +7504,46 @@ Output ONLY the prompt text (no markdown, no explanations).`;
// Done tasks: compute diff from merge base to isolate only this task's changes.
// Using `git show SHA` on merge commits shows ALL files changed in the merge,
// including files from unrelated commits on main. Instead, find the merge base
// (where the feature branch diverged) and diff from that point.
// including files from unrelated commits on main. Instead, find the true
// divergence point (where the feature branch started) and diff from that point.
if (task.column === "done" && task.mergeDetails?.commitSha) {
const rootDir = scopedStore.getRootDir();
const sha = task.mergeDetails.commitSha;
// Resolve the merge base: the common ancestor between this commit and its first parent.
// For a merge commit, SHA^1 is the first parent (main branch tip at merge time).
let mergeBase = sha;
try {
const parentSha = nodeChildProcess.execSync(
`git rev-parse ${sha}^`,
{ cwd: rootDir, encoding: "utf-8", timeout: 5000 },
).trim();
mergeBase = nodeChildProcess.execSync(
`git merge-base ${sha} ${parentSha}`,
{ cwd: rootDir, encoding: "utf-8", timeout: 5000 },
).trim();
} catch {
// Fallback: use the parent commit as base if merge-base fails
// Resolve the diff base using the same priority as resolveDiffBase():
// 1. task.baseCommitSha (exact starting commit of the worktree)
// 2. merge-base between the merge commit and the base branch
// 3. First parent of the merge commit as fallback
let mergeBase: string | undefined;
const baseBranch = task.baseBranch ?? "main";
// Priority 1: Use task.baseCommitSha if it's a valid ancestor of the merge commit
if (task.baseCommitSha) {
try {
nodeChildProcess.execSync(
`git merge-base --is-ancestor ${task.baseCommitSha} ${sha}`,
{ cwd: rootDir, encoding: "utf-8", timeout: 5000, stdio: "pipe" },
);
mergeBase = task.baseCommitSha;
} catch {
// baseCommitSha is stale or not an ancestor — fall through
}
}
// Priority 2: Compute merge-base between merge commit and base branch
if (!mergeBase) {
try {
mergeBase = nodeChildProcess.execSync(
`git merge-base ${sha} origin/${baseBranch} 2>/dev/null || git merge-base ${sha} ${baseBranch}`,
{ cwd: rootDir, encoding: "utf-8", timeout: 5000 },
).trim();
} catch {
// merge-base with branch failed — fall through
}
}
// Priority 3: Fall back to first parent of the merge commit
if (!mergeBase) {
try {
mergeBase = nodeChildProcess.execSync(
`git rev-parse ${sha}^`,
@@ -7681,24 +7701,46 @@ Output ONLY the prompt text (no markdown, no explanations).`;
// Done tasks: compute diff from merge base to isolate only this task's changes.
// Using `git show SHA` on merge commits shows ALL files changed in the merge,
// including files from unrelated commits on main. Instead, find the merge base
// (where the feature branch diverged) and diff from that point.
// including files from unrelated commits on main. Instead, find the true
// divergence point (where the feature branch started) and diff from that point.
if (task.column === "done" && task.mergeDetails?.commitSha) {
const rootDir = scopedStore.getRootDir();
const sha = task.mergeDetails.commitSha;
// Resolve the merge base: the common ancestor between this commit and its first parent.
let mergeBase = sha;
try {
const parentSha = nodeChildProcess.execSync(
`git rev-parse ${sha}^`,
{ cwd: rootDir, encoding: "utf-8", timeout: 5000 },
).trim();
mergeBase = nodeChildProcess.execSync(
`git merge-base ${sha} ${parentSha}`,
{ cwd: rootDir, encoding: "utf-8", timeout: 5000 },
).trim();
} catch {
// Resolve the diff base using the same priority as resolveDiffBase():
// 1. task.baseCommitSha (exact starting commit of the worktree)
// 2. merge-base between the merge commit and the base branch
// 3. First parent of the merge commit as fallback
let mergeBase: string | undefined;
const baseBranch = task.baseBranch ?? "main";
// Priority 1: Use task.baseCommitSha if it's a valid ancestor of the merge commit
if (task.baseCommitSha) {
try {
nodeChildProcess.execSync(
`git merge-base --is-ancestor ${task.baseCommitSha} ${sha}`,
{ cwd: rootDir, encoding: "utf-8", timeout: 5000, stdio: "pipe" },
);
mergeBase = task.baseCommitSha;
} catch {
// baseCommitSha is stale or not an ancestor — fall through
}
}
// Priority 2: Compute merge-base between merge commit and base branch
if (!mergeBase) {
try {
mergeBase = nodeChildProcess.execSync(
`git merge-base ${sha} origin/${baseBranch} 2>/dev/null || git merge-base ${sha} ${baseBranch}`,
{ cwd: rootDir, encoding: "utf-8", timeout: 5000 },
).trim();
} catch {
// merge-base with branch failed — fall through
}
}
// Priority 3: Fall back to first parent of the merge commit
if (!mergeBase) {
try {
mergeBase = nodeChildProcess.execSync(
`git rev-parse ${sha}^`,