diff --git a/.changeset/fix-base-commit-sha-local-main.md b/.changeset/fix-base-commit-sha-local-main.md new file mode 100644 index 0000000000..adc5e98796 --- /dev/null +++ b/.changeset/fix-base-commit-sha-local-main.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": patch +--- + +Fix in-review tasks showing other tasks' files in the "files changed" list. `baseCommitSha` was captured as `merge-base(HEAD, origin/main)` at task start, but task branches fork from local main — when local main was ahead by merged-but-unpushed task commits, the recorded base rewound past them, and after the post-merge rebase-and-push rewrote their SHAs the diff range permanently swept the predecessors' files into the new task's diff. The capture now measures against local main first (origin/main as fallback), matching the contamination-base sites. diff --git a/packages/engine/src/__tests__/base-commit-capture.real-git.test.ts b/packages/engine/src/__tests__/base-commit-capture.real-git.test.ts new file mode 100644 index 0000000000..45ca1641f6 --- /dev/null +++ b/packages/engine/src/__tests__/base-commit-capture.real-git.test.ts @@ -0,0 +1,106 @@ +import { execSync, spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { resolveCapturedBaseCommitSha } from "../base-commit-capture.js"; + +const hasGit = spawnSync("git", ["--version"], { stdio: "pipe" }).status === 0; +const describeIfGit = hasGit ? describe : describe.skip; + +function git(repo: string, command: string): string { + return execSync(command, { cwd: repo, encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"] }).trim(); +} + +describeIfGit("resolveCapturedBaseCommitSha real-git scenarios", { timeout: 30_000 }, () => { + const dirs: string[] = []; + afterEach(() => { + for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); + }); + + function tmp(prefix: string): string { + const dir = mkdtempSync(join(tmpdir(), prefix)); + dirs.push(dir); + return dir; + } + + function originFixture(): string { + const origin = tmp("fusion-base-capture-origin-"); + git(origin, "git init -b main"); + git(origin, 'git config user.email "test@example.com"'); + git(origin, 'git config user.name "Test User"'); + writeFileSync(join(origin, "README.md"), "init\n"); + git(origin, "git add README.md && git commit -m 'init'"); + return origin; + } + + function cloneFixture(origin: string): string { + const clone = tmp("fusion-base-capture-clone-"); + git(clone, `git clone ${JSON.stringify(origin)} .`); + git(clone, 'git config user.email "test@example.com"'); + git(clone, 'git config user.name "Test User"'); + return clone; + } + + it("captures the local-main fork point when local main is ahead of origin/main (unpushed merges)", async () => { + // Models the FN-5937 regression: the merger lands other tasks' commits on + // LOCAL main first; new task branches fork from that tip before the + // rebase-and-push rewrites those SHAs. Capturing merge-base against + // origin/main rewinds past the unpushed merges, so the dashboard diff + // (baseCommitSha..HEAD) later surfaces the predecessors' files as this + // task's "files changed". + const origin = originFixture(); + const clone = cloneFixture(origin); + + // Local main advances by a merged-but-unpushed predecessor task commit. + writeFileSync(join(clone, "predecessor.txt"), "FN-5936 work\n"); + git(clone, "git add predecessor.txt && git commit -m 'FN-5936: predecessor task'"); + const localMainTip = git(clone, "git rev-parse HEAD"); + + // New task branch forks from local main (prepareForTask behavior). + git(clone, "git checkout -B fusion/fn-5937-test main"); + + const captured = await resolveCapturedBaseCommitSha(clone); + expect(captured).toBe(localMainTip); + }); + + it("captures the merge-base with main for a branch with its own commits", async () => { + const origin = originFixture(); + const clone = cloneFixture(origin); + const forkPoint = git(clone, "git rev-parse HEAD"); + + git(clone, "git checkout -B fusion/fn-100-test main"); + writeFileSync(join(clone, "feature.txt"), "feature\n"); + git(clone, "git add feature.txt && git commit -m 'FN-100: feature'"); + + const captured = await resolveCapturedBaseCommitSha(clone); + expect(captured).toBe(forkPoint); + }); + + it("falls back to origin/main when no local main branch exists", async () => { + const origin = originFixture(); + const clone = cloneFixture(origin); + const originMainSha = git(clone, "git rev-parse origin/main"); + + // Detach and delete local main so only origin/main can resolve. + git(clone, "git checkout --detach origin/main"); + git(clone, "git branch -D main"); + git(clone, "git checkout -B fusion/fn-200-test"); + + const captured = await resolveCapturedBaseCommitSha(clone); + expect(captured).toBe(originMainSha); + }); + + it("falls back to HEAD when neither main nor origin/main resolves", async () => { + const repo = tmp("fusion-base-capture-nomain-"); + git(repo, "git init -b trunk"); + git(repo, 'git config user.email "test@example.com"'); + git(repo, 'git config user.name "Test User"'); + writeFileSync(join(repo, "README.md"), "init\n"); + git(repo, "git add README.md && git commit -m 'init'"); + const head = git(repo, "git rev-parse HEAD"); + + const captured = await resolveCapturedBaseCommitSha(repo); + expect(captured).toBe(head); + }); +}); diff --git a/packages/engine/src/base-commit-capture.ts b/packages/engine/src/base-commit-capture.ts new file mode 100644 index 0000000000..c449a97558 --- /dev/null +++ b/packages/engine/src/base-commit-capture.ts @@ -0,0 +1,55 @@ +import { exec } from "node:child_process"; +import { promisify } from "node:util"; + +const execAsync = promisify(exec); + +/** + * Resolve the fork-point base SHA for a freshly acquired task worktree. + * + * Called immediately after worktree acquisition, when the task branch was + * just created/force-reset from the local integration branch + * (`prepareForTask` forks from local `main` via `resolveIntegrationBranch`). + * + * The merge-base MUST be measured against LOCAL main first (origin/main only + * as a fallback), matching the contamination-base sites in + * `worktree-acquisition.ts` and `auto-recovery-handlers/branch-worktree.ts`. + * The merger lands tasks on local main before pushing, so at fork time local + * main can be ahead of origin/main by merged-but-unpushed commits. Measuring + * against origin/main rewinds the base past those commits; once the + * post-merge rebase-and-push rewrites their SHAs, `baseCommitSha..HEAD` + * permanently sweeps the predecessors' files into this task's diff (FN-5937: + * in-review tasks showing 31 "files changed" instead of 12). + * + * Returns `undefined` only when every git invocation fails (caller treats a + * missing base as non-fatal). + */ +export async function resolveCapturedBaseCommitSha( + worktreePath: string, + logger?: { warn: (msg: string) => void }, +): Promise { + let baseCommitSha: string | undefined; + try { + const { stdout } = await execAsync( + "git merge-base HEAD main 2>/dev/null || git merge-base HEAD origin/main", + { cwd: worktreePath, encoding: "utf-8" }, + ); + baseCommitSha = stdout.trim() || undefined; + } catch (err: unknown) { + const errorMessage = err instanceof Error ? err.message : String(err); + logger?.warn(`merge-base failed, falling back to HEAD: ${errorMessage}`); + } + + if (!baseCommitSha) { + try { + const { stdout } = await execAsync("git rev-parse HEAD", { + cwd: worktreePath, + encoding: "utf-8", + }); + baseCommitSha = stdout.trim() || undefined; + } catch { + return undefined; + } + } + + return baseCommitSha; +} diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index a46088fe13..e5d4ab4579 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -93,6 +93,7 @@ import type { PluginRunner } from "./plugin-runner.js"; import { isContextLimitError } from "./context-limit-detector.js"; import { StepSessionExecutor } from "./step-session-executor.js"; import { acquireTaskWorktree } from "./worktree-acquisition.js"; +import { resolveCapturedBaseCommitSha } from "./base-commit-capture.js"; import { installTaskWorktreeIdentityGuard } from "./worktree-hooks.js"; import { resolveAgentInstructions, @@ -7559,24 +7560,11 @@ ${failureFeedback} } } - let baseCommitSha: string | undefined; - try { - const { stdout } = await execAsync( - "git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main", - { cwd: worktreePath, encoding: "utf-8" }, - ); - baseCommitSha = stdout.trim() || undefined; - } catch (err: unknown) { - const errorMessage = err instanceof Error ? err.message : String(err); - executorLog.warn(`${task.id}: merge-base failed, falling back to HEAD: ${errorMessage}`); - } - + const baseCommitSha = await resolveCapturedBaseCommitSha(worktreePath, { + warn: (msg) => executorLog.warn(`${task.id}: ${msg}`), + }); if (!baseCommitSha) { - const { stdout } = await execAsync("git rev-parse HEAD", { - cwd: worktreePath, - encoding: "utf-8", - }); - baseCommitSha = stdout.trim(); + throw new Error("could not resolve base commit SHA"); } await this.store.updateTask(task.id, { baseCommitSha });