fix(engine): verify resumed worktrees aren't bootstrap-misbound
The resume path in acquireTaskWorktree returned a reused worktree without checking whether its branch contained foreign commits. If a sibling task's tip had been baked into the branch at creation time, the executor preflight would later fail contamination checks forever (observed in the FN-5475 cascade). The resume path now computes a fresh merge-base and runs classifyBootstrapMisbinding. For the foreign-only / zero-own-commits shape it re-anchors inline and emits a branch:reanchor audit event. Mixed contamination continues to flow through the executor's primary recovery path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
24
.changeset/fix-engine-resume-misbinding-verify.md
Normal file
24
.changeset/fix-engine-resume-misbinding-verify.md
Normal file
@@ -0,0 +1,24 @@
|
|||||||
|
---
|
||||||
|
"@fusion/engine": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
fix(engine): verify resumed worktree branches aren't bootstrap-misbound
|
||||||
|
|
||||||
|
`acquireTaskWorktree` short-circuited the resume path when
|
||||||
|
`task.worktree` existed on disk and classified `ok`, handing the
|
||||||
|
worktree back to the executor without inspecting its branch history.
|
||||||
|
If the branch had been created from a poisoned local-main tip (a
|
||||||
|
sibling task's commit), the executor preflight would later flag every
|
||||||
|
intermediate landing as foreign and the task would loop through
|
||||||
|
contamination recovery until pausing for human adjudication
|
||||||
|
(observed in the FN-5475 cascade).
|
||||||
|
|
||||||
|
The resume path now computes a fresh merge-base against local `main`
|
||||||
|
(falling back to `origin/main`) and runs `classifyBootstrapMisbinding`
|
||||||
|
on the branch. When the range is purely foreign with zero own commits,
|
||||||
|
it re-anchors the branch inline via `reanchorBranchToBase` and emits a
|
||||||
|
`branch:reanchor` audit event with `trigger: "resume-misbinding"`.
|
||||||
|
|
||||||
|
Mixed contamination (own + foreign, or non-attributed commits) is
|
||||||
|
deliberately left to the executor's existing primary path so the
|
||||||
|
richer adjudication flow still applies.
|
||||||
@@ -3,6 +3,7 @@ import { promisify } from "node:util";
|
|||||||
import { acquireTaskWorktree } from "../worktree-acquisition.js";
|
import { acquireTaskWorktree } from "../worktree-acquisition.js";
|
||||||
import { classifyTaskWorktree, PoolDoubleLeaseError } from "../worktree-pool.js";
|
import { classifyTaskWorktree, PoolDoubleLeaseError } from "../worktree-pool.js";
|
||||||
import * as desktopArtifacts from "../worktree-desktop-artifacts.js";
|
import * as desktopArtifacts from "../worktree-desktop-artifacts.js";
|
||||||
|
import * as branchConflicts from "../branch-conflicts.js";
|
||||||
|
|
||||||
vi.mock("../worktree-pool.js", async () => {
|
vi.mock("../worktree-pool.js", async () => {
|
||||||
const actual = await vi.importActual<any>("../worktree-pool.js");
|
const actual = await vi.importActual<any>("../worktree-pool.js");
|
||||||
@@ -13,6 +14,20 @@ vi.mock("../worktree-pool.js", async () => {
|
|||||||
};
|
};
|
||||||
});
|
});
|
||||||
|
|
||||||
|
vi.mock("../branch-conflicts.js", async () => {
|
||||||
|
const actual = await vi.importActual<any>("../branch-conflicts.js");
|
||||||
|
return {
|
||||||
|
...actual,
|
||||||
|
classifyBootstrapMisbinding: vi.fn().mockResolvedValue({
|
||||||
|
isBootstrapMisbinding: false,
|
||||||
|
ownCommitCount: 0,
|
||||||
|
foreignCommitCount: 0,
|
||||||
|
nonAttributedCount: 0,
|
||||||
|
}),
|
||||||
|
reanchorBranchToBase: vi.fn().mockResolvedValue({ previousTipSha: "abc", newTipSha: "def" }),
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
vi.mock("../worktree-db-hydrate.js", () => ({
|
vi.mock("../worktree-db-hydrate.js", () => ({
|
||||||
hydrateWorktreeDb: vi.fn().mockResolvedValue({ degraded: false, tasksCopied: 1, documentsCopied: 1 }),
|
hydrateWorktreeDb: vi.fn().mockResolvedValue({ degraded: false, tasksCopied: 1, documentsCopied: 1 }),
|
||||||
}));
|
}));
|
||||||
@@ -52,6 +67,48 @@ describe("acquireTaskWorktree", () => {
|
|||||||
expect(result.worktreePath).toBe(process.cwd());
|
expect(result.worktreePath).toBe(process.cwd());
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Regression: FN-5475 — when a resumed worktree's branch was created from
|
||||||
|
// a poisoned local-main tip carrying a sibling task's commits and has zero
|
||||||
|
// commits of its own, acquireTaskWorktree must re-anchor inline so the
|
||||||
|
// executor preflight doesn't pause on contamination forever.
|
||||||
|
it("re-anchors a resumed branch when classified as bootstrap-misbinding", async () => {
|
||||||
|
const audit = { git: vi.fn().mockResolvedValue(undefined), filesystem: vi.fn() } as any;
|
||||||
|
vi.mocked(branchConflicts.classifyBootstrapMisbinding).mockResolvedValueOnce({
|
||||||
|
isBootstrapMisbinding: true,
|
||||||
|
ownCommitCount: 0,
|
||||||
|
foreignCommitCount: 2,
|
||||||
|
nonAttributedCount: 0,
|
||||||
|
});
|
||||||
|
|
||||||
|
const result = await acquireTaskWorktree({
|
||||||
|
task: { ...task, worktree: process.cwd(), branch: "fusion/fn-1" },
|
||||||
|
rootDir: process.cwd(),
|
||||||
|
store,
|
||||||
|
settings: {},
|
||||||
|
audit,
|
||||||
|
createWorktree: vi.fn(),
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result.source).toBe("existing");
|
||||||
|
expect(vi.mocked(branchConflicts.reanchorBranchToBase)).toHaveBeenCalledTimes(1);
|
||||||
|
expect(audit.git).toHaveBeenCalledWith(expect.objectContaining({
|
||||||
|
type: "branch:reanchor",
|
||||||
|
metadata: expect.objectContaining({ trigger: "resume-misbinding" }),
|
||||||
|
}));
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not re-anchor a resumed branch when not misbound", async () => {
|
||||||
|
const result = await acquireTaskWorktree({
|
||||||
|
task: { ...task, worktree: process.cwd(), branch: "fusion/fn-1" },
|
||||||
|
rootDir: process.cwd(),
|
||||||
|
store,
|
||||||
|
settings: {},
|
||||||
|
createWorktree: vi.fn(),
|
||||||
|
});
|
||||||
|
expect(result.source).toBe("existing");
|
||||||
|
expect(vi.mocked(branchConflicts.reanchorBranchToBase)).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
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();
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ import { canonicalFusionBranchName, generateWorktreeName, slugify } from "./work
|
|||||||
import { resolveTaskWorktreePathForBackend } from "./worktree-paths.js";
|
import { resolveTaskWorktreePathForBackend } from "./worktree-paths.js";
|
||||||
import { hydrateWorktreeDb } from "./worktree-db-hydrate.js";
|
import { hydrateWorktreeDb } from "./worktree-db-hydrate.js";
|
||||||
import { formatError } from "./logger.js";
|
import { formatError } from "./logger.js";
|
||||||
import { isBranchConflictError } from "./branch-conflicts.js";
|
import { classifyBootstrapMisbinding, isBranchConflictError, reanchorBranchToBase } from "./branch-conflicts.js";
|
||||||
import {
|
import {
|
||||||
type WorktreePool,
|
type WorktreePool,
|
||||||
classifyTaskWorktree,
|
classifyTaskWorktree,
|
||||||
@@ -258,8 +258,19 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro
|
|||||||
await store.logEntry(task.id, `Removed desktop build artifacts from worktree: ${cleanup.removed.join(", ")}`, undefined, runContext);
|
await store.logEntry(task.id, `Removed desktop build artifacts from worktree: ${cleanup.removed.join(", ")}`, undefined, runContext);
|
||||||
}
|
}
|
||||||
const hydrated = await hydrate(worktreePath);
|
const hydrated = await hydrate(worktreePath);
|
||||||
|
const resumedBranch = task.branch ?? branchName;
|
||||||
|
await verifyResumeBranchNotMisbound({
|
||||||
|
worktreePath,
|
||||||
|
branchName: resumedBranch,
|
||||||
|
taskId: task.id,
|
||||||
|
rootDir,
|
||||||
|
store,
|
||||||
|
audit,
|
||||||
|
logger,
|
||||||
|
runContext,
|
||||||
|
});
|
||||||
// FN-4912: resume path reuses the prior on-disk .env (and its fingerprint sidecar). Rewrite is owned by the next fresh acquisition.
|
// FN-4912: resume path reuses the prior on-disk .env (and its fingerprint sidecar). Rewrite is owned by the next fresh acquisition.
|
||||||
return { worktreePath, branch: task.branch ?? branchName, source: "existing", hydrated, isResume: true };
|
return { worktreePath, branch: resumedBranch, source: "existing", hydrated, isResume: true };
|
||||||
}
|
}
|
||||||
|
|
||||||
let acquiredFromPool = false;
|
let acquiredFromPool = false;
|
||||||
@@ -496,3 +507,88 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro
|
|||||||
}
|
}
|
||||||
return { worktreePath, branch, source: acquiredFromPool ? "pool" : "fresh", hydrated, isResume: false };
|
return { worktreePath, branch, source: acquiredFromPool ? "pool" : "fresh", hydrated, isResume: false };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Resume-path safety check: before handing a reused worktree back to the
|
||||||
|
* executor, verify that its branch contains only this task's own commits
|
||||||
|
* since `main`. If the branch was created from a poisoned local-main tip
|
||||||
|
* (a sibling task's commit, observed in the FN-5475 cascade) the only
|
||||||
|
* commits between merge-base and HEAD are foreign-attributed and zero
|
||||||
|
* are this task's — the bootstrap-misbinding shape. Re-anchor inline so
|
||||||
|
* downstream checks see a clean branch.
|
||||||
|
*
|
||||||
|
* Mixed contamination (own + foreign, or non-attributed commits) is
|
||||||
|
* intentionally not handled here — those cases need richer adjudication
|
||||||
|
* and continue to flow through the executor's primary contamination
|
||||||
|
* path at `tryBootstrapMisbindingRecovery` / `classifyForeignCommits`.
|
||||||
|
*/
|
||||||
|
async function verifyResumeBranchNotMisbound(input: {
|
||||||
|
worktreePath: string;
|
||||||
|
branchName: string;
|
||||||
|
taskId: string;
|
||||||
|
rootDir: string;
|
||||||
|
store: TaskStore;
|
||||||
|
audit?: Pick<RunAuditor, "git" | "filesystem">;
|
||||||
|
logger?: { log?: (msg: string) => void; warn?: (msg: string) => void };
|
||||||
|
runContext: RunMutationContext | undefined;
|
||||||
|
}): Promise<void> {
|
||||||
|
const { worktreePath, branchName, taskId, rootDir, store, audit, logger, runContext } = input;
|
||||||
|
|
||||||
|
let baseSha = "";
|
||||||
|
try {
|
||||||
|
const { stdout } = await execAsync(
|
||||||
|
"git merge-base HEAD main 2>/dev/null || git merge-base HEAD origin/main",
|
||||||
|
{ cwd: worktreePath, encoding: "utf-8" },
|
||||||
|
);
|
||||||
|
baseSha = stdout.trim();
|
||||||
|
} catch {
|
||||||
|
// Can't resolve a base — let executor's primary contamination path handle it.
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if (!baseSha) return;
|
||||||
|
|
||||||
|
let classification;
|
||||||
|
try {
|
||||||
|
classification = await classifyBootstrapMisbinding({
|
||||||
|
repoDir: rootDir,
|
||||||
|
branchName,
|
||||||
|
baseSha,
|
||||||
|
taskId,
|
||||||
|
});
|
||||||
|
} catch (err) {
|
||||||
|
logger?.warn?.(`${taskId}: resume misbinding check failed: ${formatError(err)}`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!classification.isBootstrapMisbinding) return;
|
||||||
|
|
||||||
|
await store.logEntry(
|
||||||
|
taskId,
|
||||||
|
`[recovery] resume-path bootstrap misbinding detected on ${branchName}: 0 own commits, ${classification.foreignCommitCount} foreign — re-anchoring to ${baseSha.slice(0, 12)}`,
|
||||||
|
undefined,
|
||||||
|
runContext,
|
||||||
|
);
|
||||||
|
|
||||||
|
try {
|
||||||
|
const reanchor = await reanchorBranchToBase({
|
||||||
|
repoDir: rootDir,
|
||||||
|
worktreePath,
|
||||||
|
branchName,
|
||||||
|
baseSha,
|
||||||
|
taskId,
|
||||||
|
});
|
||||||
|
await audit?.git({
|
||||||
|
type: "branch:reanchor",
|
||||||
|
target: branchName,
|
||||||
|
metadata: {
|
||||||
|
taskId,
|
||||||
|
baseSha,
|
||||||
|
previousTipSha: reanchor.previousTipSha,
|
||||||
|
newTipSha: reanchor.newTipSha,
|
||||||
|
trigger: "resume-misbinding",
|
||||||
|
},
|
||||||
|
});
|
||||||
|
} catch (err) {
|
||||||
|
logger?.warn?.(`${taskId}: resume re-anchor failed (continuing — executor preflight will handle): ${formatError(err)}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user