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
This commit is contained in:
7
.changeset/fix-worktree-contamination.md
Normal file
7
.changeset/fix-worktree-contamination.md
Normal file
@@ -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.
|
||||||
@@ -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<typeof vi.fn>;
|
||||||
|
updateTask: ReturnType<typeof vi.fn>;
|
||||||
|
moveTask: ReturnType<typeof vi.fn>;
|
||||||
|
recordRunAuditEvent: ReturnType<typeof vi.fn>;
|
||||||
|
};
|
||||||
|
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 () => {
|
it("blocks loose merged results that lack durable merge confirmation", async () => {
|
||||||
const strandedTask = {
|
const strandedTask = {
|
||||||
id: "FN-MERGED-PROOF",
|
id: "FN-MERGED-PROOF",
|
||||||
|
|||||||
@@ -168,8 +168,8 @@ describe("acquireTaskWorktree", () => {
|
|||||||
expect(first.branch).toBe("fusion/fn-100");
|
expect(first.branch).toBe("fusion/fn-100");
|
||||||
expect(second.branch).toBe("fusion/fn-101");
|
expect(second.branch).toBe("fusion/fn-101");
|
||||||
expect(first.branch).not.toBe(second.branch);
|
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-100", expect.any(String), "FN-100", "main", false);
|
||||||
expect(createWorktree).toHaveBeenCalledWith("fusion/fn-101", expect.any(String), "FN-101", undefined, 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 () => {
|
it("keeps per-task-derived and ungrouped branch derivation unchanged", async () => {
|
||||||
@@ -195,6 +195,33 @@ describe("acquireTaskWorktree", () => {
|
|||||||
expect(ungrouped.branch).toBe("fusion/fn-103");
|
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 () => {
|
it("acquires from pool when enabled", async () => {
|
||||||
const prepareForTask = vi.fn().mockResolvedValue({ branch: "fusion/fn-1", worktreePath: "/tmp/pooled", reclaimed: false });
|
const prepareForTask = vi.fn().mockResolvedValue({ branch: "fusion/fn-1", worktreePath: "/tmp/pooled", reclaimed: false });
|
||||||
const release = vi.fn();
|
const release = vi.fn();
|
||||||
@@ -215,7 +242,7 @@ describe("acquireTaskWorktree", () => {
|
|||||||
expect(prepareForTask).toHaveBeenCalledWith(
|
expect(prepareForTask).toHaveBeenCalledWith(
|
||||||
"/tmp/pooled",
|
"/tmp/pooled",
|
||||||
"fusion/fn-1",
|
"fusion/fn-1",
|
||||||
undefined,
|
"main",
|
||||||
expect.objectContaining({ requestingTaskId: "FN-1" }),
|
expect.objectContaining({ requestingTaskId: "FN-1" }),
|
||||||
);
|
);
|
||||||
expect(store.updateTask).toHaveBeenCalledWith("FN-1", { worktree: "/tmp/pooled", branch: "fusion/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(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({
|
expect(auditGit).toHaveBeenCalledWith(expect.objectContaining({
|
||||||
type: "worktree:incomplete-detected",
|
type: "worktree:incomplete-detected",
|
||||||
target: rootDir,
|
target: rootDir,
|
||||||
|
|||||||
@@ -137,11 +137,16 @@ function branchDiffFilesMissingFromMergeProof(task: Task, branchFiles: string[],
|
|||||||
async function readBranchDiffFiles(rootDir: string, task: Task): Promise<string[] | null> {
|
async function readBranchDiffFiles(rootDir: string, task: Task): Promise<string[] | null> {
|
||||||
const branch = task.branch;
|
const branch = task.branch;
|
||||||
if (!branch) return null;
|
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 {
|
try {
|
||||||
await execAsync(`git rev-parse --verify ${shellQuote(`refs/heads/${branch}`)}`, { cwd: rootDir, maxBuffer: 1024 * 1024 });
|
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 });
|
await execAsync(`git rev-parse --verify ${shellQuote(diffBase)}`, { cwd: rootDir, maxBuffer: 1024 * 1024 });
|
||||||
const { stdout } = await execAsync(`git diff --name-only ${shellQuote(`${baseBranch}...${branch}`)}`, {
|
const range = task.baseCommitSha ? `${diffBase}..${branch}` : `${diffBase}...${branch}`;
|
||||||
|
const { stdout } = await execAsync(`git diff --name-only ${shellQuote(range)}`, {
|
||||||
cwd: rootDir,
|
cwd: rootDir,
|
||||||
maxBuffer: 1024 * 1024,
|
maxBuffer: 1024 * 1024,
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -219,6 +219,11 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro
|
|||||||
const naming = settings.worktreeNaming || "random";
|
const naming = settings.worktreeNaming || "random";
|
||||||
const allowSiblingBranchRename = settings.executorAllowSiblingBranchRename === true;
|
const allowSiblingBranchRename = settings.executorAllowSiblingBranchRename === true;
|
||||||
const baseBranch = task.executionStartBranch || null;
|
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 ... <startPoint>` 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;
|
let worktreePath = task.worktree;
|
||||||
if (!worktreePath) {
|
if (!worktreePath) {
|
||||||
@@ -371,8 +376,8 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro
|
|||||||
if (created.branch !== branchName) {
|
if (created.branch !== branchName) {
|
||||||
logger?.log(`Branch conflict resolved: using ${created.branch} instead of ${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);
|
await store.logEntry(task.id, `Worktree created at ${worktreePath} (branch conflict: using ${created.branch})`, undefined, runContext);
|
||||||
} else if (baseBranch) {
|
} else if (freshStartPoint) {
|
||||||
await store.logEntry(task.id, `Worktree created at ${worktreePath} (based on ${baseBranch})`, undefined, runContext);
|
await store.logEntry(task.id, `Worktree created at ${worktreePath} (based on ${freshStartPoint})`, undefined, runContext);
|
||||||
} else {
|
} else {
|
||||||
await store.logEntry(task.id, `Worktree created at ${worktreePath}`, undefined, runContext);
|
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 });
|
await store.updateTask(task.id, { worktree: null, branch: null, sessionFile: null });
|
||||||
const fallbackName = generateWorktreeName(rootDir, settings);
|
const fallbackName = generateWorktreeName(rootDir, settings);
|
||||||
const fallbackPath = await resolveTaskWorktreePathForBackend(rootDir, fallbackName, settings, backend, branchName);
|
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");
|
return finalizeCreatedWorktree(created, "fresh", "return-guard");
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -485,7 +490,7 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro
|
|||||||
}
|
}
|
||||||
if (pooled) {
|
if (pooled) {
|
||||||
try {
|
try {
|
||||||
const preparedRaw = await pool.prepareForTask(pooled, branchName, baseBranch ?? undefined, {
|
const preparedRaw = await pool.prepareForTask(pooled, branchName, freshStartPoint, {
|
||||||
allowSiblingBranchRename,
|
allowSiblingBranchRename,
|
||||||
repoDir: rootDir,
|
repoDir: rootDir,
|
||||||
requestingTaskId: task.id,
|
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
|
// Worktree removal in merger.ts, worktree-pool.ts, and self-healing.ts is now
|
||||||
// backend-mediated via WorktreeBackend.remove(). executor.ts and
|
// backend-mediated via WorktreeBackend.remove(). executor.ts and
|
||||||
// step-session-executor.ts remain native-only paths (tracked separately).
|
// 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");
|
return finalizeCreatedWorktree(created, acquiredFromPool ? "pool" : "fresh", "normal");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user