From 88bcbce0fe601d1c77736cfd1363beea65b15026 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 1 Jul 2026 08:39:58 -0700 Subject: [PATCH] fix(FN-7360): prevent contaminated task branch bases Pin fresh task worktree creation to the resolved integration branch when no explicit executionStartBranch is present, so ambient root checkout state cannot leak sibling task commits into new branches. Use task baseCommitSha for merge-finalization branch proof when available, allowing already-landed mergeConfirmed tasks to finalize even if historical branch ancestry contains foreign commits. Fusion-Task-Id: FN-7360 --- .changeset/fix-worktree-contamination.md | 7 ++ .../__tests__/merger-merge-lifecycle.test.ts | 67 +++++++++++++++++++ .../__tests__/worktree-acquisition.test.ts | 35 ++++++++-- .../engine/src/auto-merge-finalization.ts | 11 ++- packages/engine/src/worktree-acquisition.ts | 15 +++-- 5 files changed, 123 insertions(+), 12 deletions(-) create mode 100644 .changeset/fix-worktree-contamination.md diff --git a/.changeset/fix-worktree-contamination.md b/.changeset/fix-worktree-contamination.md new file mode 100644 index 0000000000..50648d25df --- /dev/null +++ b/.changeset/fix-worktree-contamination.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Prevent task branches from inheriting unrelated checked-out task commits. +category: fix +dev: Fresh worktree acquisition now pins the integration branch as the default start point, and merge finalization validates task-owned branch diffs from baseCommitSha when available. diff --git a/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts b/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts index 7b7a9da463..678ad38aa8 100644 --- a/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts +++ b/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts @@ -315,6 +315,73 @@ describe("auto-merge proven finalization helper", () => { ); }); + it("uses the task base commit for branch proof so inherited foreign commits do not block finalization", async () => { + const strandedTask = { + id: "FN-7360", + title: "Contaminated branch but landed task files", + description: "Test", + column: "in-review", + status: "landing", + error: null, + blockedBy: null, + overlapBlockedBy: null, + dependencies: [], + steps: [{ status: "done" }], + currentStep: 0, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + branch: "fusion/fn-7360", + baseCommitSha: "foreign-base", + mergeDetails: { + mergeConfirmed: true, + commitSha: "landed", + mergedAt: "2026-07-01T15:06:13.748Z", + landedFiles: ["packages/desktop/scripts/build.ts"], + }, + } as Task; + const doneTask = { ...strandedTask, column: "done", status: null } as Task; + const store = createMockStore(strandedTask) as unknown as TaskStore & { + getTask: ReturnType; + updateTask: ReturnType; + moveTask: ReturnType; + recordRunAuditEvent: ReturnType; + }; + store.getTask.mockResolvedValue(strandedTask); + store.moveTask.mockResolvedValue(doneTask); + mockedExecSync.mockImplementation((cmd: any) => { + const command = String(cmd); + if (command.includes("rev-parse --verify")) return "ok\n" as any; + if (command.includes("git diff --name-only 'foreign-base..fusion/fn-7360'")) { + return "packages/desktop/scripts/build.ts\n" as any; + } + if (command.includes("git diff --name-only 'main...fusion/fn-7360'")) { + throw new Error("should not read ambient main branch diff for task proof"); + } + return "" as any; + }); + + const result = await finalizeProvenAutoMergeTask({ + store, + taskId: "FN-7360", + result: { task: strandedTask, ok: true, merged: true, commitSha: "landed", mergeConfirmed: true } as MergeResult, + rootDir: "/tmp/repo", + source: "workflow-graph-merge-finalize", + auditAgentId: "executor", + auditPhase: "workflow-graph-merge-finalize", + }); + + expect(result.outcome).toBe("done"); + expect(store.moveTask).toHaveBeenCalledWith( + "FN-7360", + "done", + expect.objectContaining({ moveSource: "engine", preserveProgress: true }), + ); + expect(store.recordRunAuditEvent).not.toHaveBeenCalledWith(expect.objectContaining({ + mutationType: "task:auto-merge-finalize-column-mismatch-no-action", + })); + }); + it("blocks loose merged results that lack durable merge confirmation", async () => { const strandedTask = { id: "FN-MERGED-PROOF", diff --git a/packages/engine/src/__tests__/worktree-acquisition.test.ts b/packages/engine/src/__tests__/worktree-acquisition.test.ts index e56f4f0dca..afa65a9dc5 100644 --- a/packages/engine/src/__tests__/worktree-acquisition.test.ts +++ b/packages/engine/src/__tests__/worktree-acquisition.test.ts @@ -168,8 +168,8 @@ describe("acquireTaskWorktree", () => { expect(first.branch).toBe("fusion/fn-100"); expect(second.branch).toBe("fusion/fn-101"); expect(first.branch).not.toBe(second.branch); - expect(createWorktree).toHaveBeenCalledWith("fusion/fn-100", expect.any(String), "FN-100", undefined, false); - expect(createWorktree).toHaveBeenCalledWith("fusion/fn-101", expect.any(String), "FN-101", undefined, false); + expect(createWorktree).toHaveBeenCalledWith("fusion/fn-100", expect.any(String), "FN-100", "main", false); + expect(createWorktree).toHaveBeenCalledWith("fusion/fn-101", expect.any(String), "FN-101", "main", false); }); it("keeps per-task-derived and ungrouped branch derivation unchanged", async () => { @@ -195,6 +195,33 @@ describe("acquireTaskWorktree", () => { expect(ungrouped.branch).toBe("fusion/fn-103"); }); + it("creates fresh worktrees from the integration branch instead of ambient root HEAD", async () => { + const rootDir = makeRepo(); + writeFileSync(join(rootDir, "foreign.txt"), "foreign\n", "utf-8"); + git(rootDir, "git checkout -b fusion/fn-foreign"); + git(rootDir, "git add foreign.txt"); + git(rootDir, 'git commit -m "FN-9999: foreign work"'); + const foreignHead = git(rootDir, "git rev-parse HEAD"); + const mainHead = git(rootDir, "git rev-parse main"); + expect(foreignHead).not.toBe(mainHead); + + const createWorktree = vi.fn(async (branchName: string, worktreePath: string, _taskId: string, startPoint?: string) => { + git(rootDir, `git worktree add -b ${branchName} ${JSON.stringify(worktreePath)} ${startPoint ?? ""}`); + return { path: worktreePath, branch: branchName }; + }); + + const result = await acquireTaskWorktree({ + task: { ...task, id: "FN-200", worktree: null, branch: null }, + rootDir, + store, + settings: {}, + createWorktree, + }); + + expect(createWorktree).toHaveBeenCalledWith("fusion/fn-200", expect.any(String), "FN-200", "main", false); + expect(git(result.worktreePath, "git rev-parse HEAD")).toBe(mainHead); + }); + it("acquires from pool when enabled", async () => { const prepareForTask = vi.fn().mockResolvedValue({ branch: "fusion/fn-1", worktreePath: "/tmp/pooled", reclaimed: false }); const release = vi.fn(); @@ -215,7 +242,7 @@ describe("acquireTaskWorktree", () => { expect(prepareForTask).toHaveBeenCalledWith( "/tmp/pooled", "fusion/fn-1", - undefined, + "main", expect.objectContaining({ requestingTaskId: "FN-1" }), ); expect(store.updateTask).toHaveBeenCalledWith("FN-1", { worktree: "/tmp/pooled", branch: "fusion/fn-1" }); @@ -370,7 +397,7 @@ describe("acquireTaskWorktree", () => { }); expect(result).toMatchObject({ worktreePath: freshPath, source: "fresh", isResume: false }); - expect(createWorktree).toHaveBeenCalledWith("fusion/fn-1", expect.stringContaining(`${join(rootDir, ".worktrees")}/`), "FN-1", undefined, false); + expect(createWorktree).toHaveBeenCalledWith("fusion/fn-1", expect.stringContaining(`${join(rootDir, ".worktrees")}/`), "FN-1", "main", false); expect(auditGit).toHaveBeenCalledWith(expect.objectContaining({ type: "worktree:incomplete-detected", target: rootDir, diff --git a/packages/engine/src/auto-merge-finalization.ts b/packages/engine/src/auto-merge-finalization.ts index df33f432db..d9975bc89f 100644 --- a/packages/engine/src/auto-merge-finalization.ts +++ b/packages/engine/src/auto-merge-finalization.ts @@ -137,11 +137,16 @@ function branchDiffFilesMissingFromMergeProof(task: Task, branchFiles: string[], async function readBranchDiffFiles(rootDir: string, task: Task): Promise { const branch = task.branch; if (!branch) return null; - const baseBranch = task.mergeDetails?.mergeTargetBranch ?? task.baseBranch ?? "main"; + /* + * FNXC:AutoMergeFinalization 2026-07-01-08:35: + * Branch-proof validation must measure the task's own diff, not every commit reachable from the task branch but absent from current main. Fresh-worktree bugs and historical recovery paths can leave a branch with foreign ancestor commits; when the merger has already landed the recorded task files, `baseCommitSha..branch` is the authoritative task-owned range and prevents unrelated ancestor files from stranding a mergeConfirmed task in `landing`. + */ + const diffBase = task.baseCommitSha ?? task.mergeDetails?.mergeTargetBranch ?? task.baseBranch ?? "main"; try { await execAsync(`git rev-parse --verify ${shellQuote(`refs/heads/${branch}`)}`, { cwd: rootDir, maxBuffer: 1024 * 1024 }); - await execAsync(`git rev-parse --verify ${shellQuote(baseBranch)}`, { cwd: rootDir, maxBuffer: 1024 * 1024 }); - const { stdout } = await execAsync(`git diff --name-only ${shellQuote(`${baseBranch}...${branch}`)}`, { + await execAsync(`git rev-parse --verify ${shellQuote(diffBase)}`, { cwd: rootDir, maxBuffer: 1024 * 1024 }); + const range = task.baseCommitSha ? `${diffBase}..${branch}` : `${diffBase}...${branch}`; + const { stdout } = await execAsync(`git diff --name-only ${shellQuote(range)}`, { cwd: rootDir, maxBuffer: 1024 * 1024, }); diff --git a/packages/engine/src/worktree-acquisition.ts b/packages/engine/src/worktree-acquisition.ts index daeb860cec..e0bda12776 100644 --- a/packages/engine/src/worktree-acquisition.ts +++ b/packages/engine/src/worktree-acquisition.ts @@ -219,6 +219,11 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro const naming = settings.worktreeNaming || "random"; const allowSiblingBranchRename = settings.executorAllowSiblingBranchRename === true; const baseBranch = task.executionStartBranch || null; + /* + * FNXC:WorktreeIsolation 2026-07-01-08:35: + * Fresh task worktrees must never inherit the project root checkout's ambient HEAD. The root checkout can temporarily point at a sibling task branch/commit during merge or recovery work, so an omitted `git worktree add -b ... ` contaminates new task branches with unrelated task commits. Use the task's explicit executionStartBranch when present; otherwise pin creation to the resolved integration branch. + */ + const freshStartPoint = baseBranch ?? await resolveIntegrationBranch(rootDir, settings, { logger: logger ?? console }); let worktreePath = task.worktree; if (!worktreePath) { @@ -371,8 +376,8 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro if (created.branch !== branchName) { logger?.log(`Branch conflict resolved: using ${created.branch} instead of ${branchName}`); await store.logEntry(task.id, `Worktree created at ${worktreePath} (branch conflict: using ${created.branch})`, undefined, runContext); - } else if (baseBranch) { - await store.logEntry(task.id, `Worktree created at ${worktreePath} (based on ${baseBranch})`, undefined, runContext); + } else if (freshStartPoint) { + await store.logEntry(task.id, `Worktree created at ${worktreePath} (based on ${freshStartPoint})`, undefined, runContext); } else { await store.logEntry(task.id, `Worktree created at ${worktreePath}`, undefined, runContext); } @@ -439,7 +444,7 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro await store.updateTask(task.id, { worktree: null, branch: null, sessionFile: null }); const fallbackName = generateWorktreeName(rootDir, settings); const fallbackPath = await resolveTaskWorktreePathForBackend(rootDir, fallbackName, settings, backend, branchName); - const created = await createWorktreeImpl(branchName, fallbackPath, task.id, baseBranch ?? undefined, allowSiblingBranchRename); + const created = await createWorktreeImpl(branchName, fallbackPath, task.id, freshStartPoint, allowSiblingBranchRename); return finalizeCreatedWorktree(created, "fresh", "return-guard"); }; @@ -485,7 +490,7 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro } if (pooled) { try { - const preparedRaw = await pool.prepareForTask(pooled, branchName, baseBranch ?? undefined, { + const preparedRaw = await pool.prepareForTask(pooled, branchName, freshStartPoint, { allowSiblingBranchRename, repoDir: rootDir, requestingTaskId: task.id, @@ -601,7 +606,7 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro // Worktree removal in merger.ts, worktree-pool.ts, and self-healing.ts is now // backend-mediated via WorktreeBackend.remove(). executor.ts and // step-session-executor.ts remain native-only paths (tracked separately). - const created = await createWorktreeImpl(branchName, worktreePath, task.id, baseBranch ?? undefined, allowSiblingBranchRename); + const created = await createWorktreeImpl(branchName, worktreePath, task.id, freshStartPoint, allowSiblingBranchRename); return finalizeCreatedWorktree(created, acquiredFromPool ? "pool" : "fresh", "normal"); }