feat(FN-5353): fix merge-reuse handoff race by gating reacquire and strict
Fixes merge handoff stalls by enforcing strict queue targeting and self-enqueue in the merger (FN-5353 Step 4), forcing reacquire before reuse handoff gates (Step 5), and aligning integration-root contract tests (Steps 1 & 7). Consolidates two prior changesets into `fn-5353-merge-reuse-stall-fix.md` Fusion-Task-Id: FN-5353
This commit is contained in:
committed by
gsxdsm
parent
f21464070a
commit
8df21a6b68
@@ -63,7 +63,7 @@ describe("resolveMergeIntegrationRoot", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("uses the project root when the task worktree is missing", () => {
|
||||
it("returns empty sentinel rootDir when the task worktree is missing", () => {
|
||||
expect(
|
||||
resolveMergeIntegrationRoot({
|
||||
task: { id: "FN-5279", worktree: undefined } as any,
|
||||
@@ -72,7 +72,7 @@ describe("resolveMergeIntegrationRoot", () => {
|
||||
}),
|
||||
).toEqual({
|
||||
mode: "reuse-task-worktree",
|
||||
rootDir: "/tmp/project-root",
|
||||
rootDir: "",
|
||||
branchName: "fusion/fn-5279",
|
||||
});
|
||||
});
|
||||
@@ -550,11 +550,11 @@ describe("acquireReuseHandoff", () => {
|
||||
worktreePath: "/tmp/task-worktree",
|
||||
}),
|
||||
"lease-handoff-failed",
|
||||
"no-lease",
|
||||
"target-not-queued",
|
||||
);
|
||||
expect(refusal.payload).toMatchObject({
|
||||
queueHeadTaskId: "FN-5329",
|
||||
queueHeadLeasedBy: "merger-reuse-handoff",
|
||||
taskId: "FN-5279",
|
||||
worktreePath: "/tmp/task-worktree",
|
||||
});
|
||||
});
|
||||
|
||||
@@ -625,6 +625,7 @@ describe("aiMergeTask integration-root behavior", () => {
|
||||
});
|
||||
store.acquireMergeQueueLease = vi.fn().mockReturnValue({ taskId: "FN-5279" });
|
||||
store.releaseMergeQueueLease = vi.fn();
|
||||
store.enqueueMergeQueue = vi.fn();
|
||||
store.listTasks.mockResolvedValue([{ id: "FN-5279", column: "in-review", worktree: "/tmp/task-worktree" }]);
|
||||
|
||||
const baseImpl = mockedExecSync.getMockImplementation();
|
||||
|
||||
@@ -223,7 +223,7 @@ describe("FN-5279 reliability interactions: merge reuse task worktree", () => {
|
||||
}
|
||||
}, 30_000);
|
||||
|
||||
it.skipIf(!hasGit)("missing merge queue lease refuses handoff with no-lease diagnostics", async () => {
|
||||
it.skipIf(!hasGit)("missing merge queue lease refuses handoff with target-not-queued diagnostics", async () => {
|
||||
const fixture = await makeReliabilityFixture({
|
||||
taskId: "FN-5279-RI-NO-LEASE",
|
||||
settings: {
|
||||
@@ -264,18 +264,182 @@ describe("FN-5279 reliability interactions: merge reuse task worktree", () => {
|
||||
await expect(aiMergeTask(store, rootDir, task.id)).rejects.toMatchObject({
|
||||
name: "MergeHandoffRefusedError",
|
||||
gate: "lease-handoff-failed",
|
||||
reason: "no-lease",
|
||||
reason: "target-not-queued",
|
||||
});
|
||||
const refused = store.getRunAuditEvents({ taskId: task.id }).find((event) => event.mutationType === "merge:reuse-handoff-refused");
|
||||
expect(refused?.metadata).toMatchObject({
|
||||
gate: "lease-handoff-failed",
|
||||
reason: "no-lease",
|
||||
reason: "target-not-queued",
|
||||
});
|
||||
} finally {
|
||||
await fixture.cleanup();
|
||||
}
|
||||
}, 30_000);
|
||||
|
||||
it.skipIf(!hasGit)("FN-5353: aiMergeTask succeeds without pre-enqueue by self-enqueueing before handoff", async () => {
|
||||
const fixture = await makeReliabilityFixture({
|
||||
taskId: "FN-5353-RI-SELF-ENQUEUE",
|
||||
settings: {
|
||||
baseBranch: "master",
|
||||
mergeIntegrationWorktree: "reuse-task-worktree",
|
||||
} as any,
|
||||
});
|
||||
|
||||
try {
|
||||
const { rootDir, store, task } = fixture;
|
||||
const actualTask = await store.getTask(task.id);
|
||||
const branch = `fusion/${actualTask!.id.toLowerCase()}`;
|
||||
const worktreeRoot = `${rootDir}-worktrees`;
|
||||
const worktreePath = join(worktreeRoot, actualTask!.id.toLowerCase());
|
||||
|
||||
git(rootDir, "git branch -m main master");
|
||||
const completedSteps = (actualTask?.steps ?? []).map((step) => ({ ...step, status: "done" as const }));
|
||||
await store.updateTask(task.id, { baseBranch: "master", branch, steps: completedSteps, currentStep: completedSteps.length } as any);
|
||||
await fixture.createBranch(branch);
|
||||
await fixture.writeAndCommit("packages/engine/src/fn-5353-ri-self-enqueue.ts", "export const selfEnqueue = true;\n", "feat: add self enqueue merge content");
|
||||
await fixture.checkout("master");
|
||||
await mkdir(worktreeRoot, { recursive: true });
|
||||
git(rootDir, `git worktree add ${JSON.stringify(worktreePath)} ${JSON.stringify(branch)}`);
|
||||
await store.updateTask(task.id, { worktree: worktreePath, branch } as any);
|
||||
store.getDatabase().prepare("DELETE FROM mergeQueue WHERE taskId = ?").run(task.id);
|
||||
|
||||
const result = await aiMergeTask(store, rootDir, task.id);
|
||||
expect(result.merged).toBe(true);
|
||||
expect((await store.getTask(task.id))?.column).toBe("done");
|
||||
const auditTypes = store.getRunAuditEvents({ taskId: task.id }).map((event) => event.mutationType);
|
||||
expect(auditTypes).toContain("merge:reuse-handoff-acquired");
|
||||
} finally {
|
||||
await fixture.cleanup();
|
||||
}
|
||||
}, 30_000);
|
||||
|
||||
it.skipIf(!hasGit)("FN-5353: cross-task queue entries remain untouched when aiMergeTask self-enqueues target", async () => {
|
||||
const fixtureA = await makeReliabilityFixture({
|
||||
taskId: "FN-5353-RI-TARGET-A",
|
||||
settings: { baseBranch: "master", mergeIntegrationWorktree: "reuse-task-worktree" } as any,
|
||||
});
|
||||
|
||||
try {
|
||||
const { rootDir, store, task } = fixtureA;
|
||||
const actualTask = await store.getTask(task.id);
|
||||
const branch = `fusion/${actualTask!.id.toLowerCase()}`;
|
||||
const worktreeRoot = `${rootDir}-worktrees`;
|
||||
const worktreePath = join(worktreeRoot, actualTask!.id.toLowerCase());
|
||||
|
||||
const other = await store.createTask({ description: "queue head other", priority: "normal" });
|
||||
await store.moveTask(other.id, "todo");
|
||||
await store.moveTask(other.id, "in-progress");
|
||||
await store.handoffToReview(other.id, {
|
||||
ownerAgentId: "agent-1",
|
||||
evidence: { reason: "fn_task_done", runId: "run-1", agentId: "agent-1" },
|
||||
});
|
||||
store.enqueueMergeQueue(other.id, { now: "2026-05-19T00:00:00.000Z" });
|
||||
|
||||
git(rootDir, "git branch -m main master");
|
||||
const completedSteps = (actualTask?.steps ?? []).map((step) => ({ ...step, status: "done" as const }));
|
||||
await store.updateTask(task.id, { baseBranch: "master", branch, steps: completedSteps, currentStep: completedSteps.length } as any);
|
||||
await fixtureA.createBranch(branch);
|
||||
await fixtureA.writeAndCommit("packages/engine/src/fn-5353-ri-target-not-queued.ts", "export const targetNotQueued = true;\n", "feat: add target not queued reproduction");
|
||||
await fixtureA.checkout("master");
|
||||
await mkdir(worktreeRoot, { recursive: true });
|
||||
git(rootDir, `git worktree add ${JSON.stringify(worktreePath)} ${JSON.stringify(branch)}`);
|
||||
await store.updateTask(task.id, { worktree: worktreePath, branch } as any);
|
||||
store.getDatabase().prepare("DELETE FROM mergeQueue WHERE taskId = ?").run(task.id);
|
||||
|
||||
const result = await aiMergeTask(store, rootDir, task.id);
|
||||
expect(result.merged).toBe(true);
|
||||
expect((await store.getTask(task.id))?.column).toBe("done");
|
||||
|
||||
const otherRow = store.getDatabase().prepare("SELECT taskId, leasedBy FROM mergeQueue WHERE taskId = ?").get(other.id) as {
|
||||
taskId: string;
|
||||
leasedBy: string | null;
|
||||
};
|
||||
expect(otherRow.taskId).toBe(other.id);
|
||||
expect(otherRow.leasedBy).toBeNull();
|
||||
} finally {
|
||||
await fixtureA.cleanup();
|
||||
}
|
||||
}, 30_000);
|
||||
|
||||
it.skipIf(!hasGit)("FN-5353: reuse handoff rejects project-root worktree misconfiguration", async () => {
|
||||
const fixture = await makeReliabilityFixture({
|
||||
taskId: "FN-5353-RI-PROJECT-ROOT-WORKTREE",
|
||||
settings: {
|
||||
baseBranch: "master",
|
||||
mergeIntegrationWorktree: "reuse-task-worktree",
|
||||
} as any,
|
||||
});
|
||||
|
||||
try {
|
||||
const { rootDir, store, task } = fixture;
|
||||
const actualTask = await store.getTask(task.id);
|
||||
const branch = `fusion/${actualTask!.id.toLowerCase()}`;
|
||||
git(rootDir, "git branch -m main master");
|
||||
const completedSteps = (actualTask?.steps ?? []).map((step) => ({ ...step, status: "done" as const }));
|
||||
await store.updateTask(task.id, {
|
||||
baseBranch: "master",
|
||||
branch,
|
||||
worktree: rootDir,
|
||||
steps: completedSteps,
|
||||
currentStep: completedSteps.length,
|
||||
} as any);
|
||||
await fixture.createBranch(branch);
|
||||
await fixture.writeAndCommit("packages/engine/src/fn-5353-ri-project-root.ts", "export const projectRootReuse = true;\n", "feat: add project root misconfiguration content");
|
||||
await fixture.checkout("master");
|
||||
store.enqueueMergeQueue(task.id);
|
||||
|
||||
await expect(aiMergeTask(store, rootDir, task.id)).rejects.toMatchObject({
|
||||
name: "MergeHandoffRefusedError",
|
||||
gate: "reuse-misconfigured",
|
||||
reason: "worktree-equals-project-root",
|
||||
});
|
||||
} finally {
|
||||
await fixture.cleanup();
|
||||
}
|
||||
}, 30_000);
|
||||
|
||||
it.skipIf(!hasGit)("FN-5353: missing task.worktree reacquires a reusable worktree before handoff gates", async () => {
|
||||
const fixture = await makeReliabilityFixture({
|
||||
taskId: "FN-5353-RI-MISSING-WORKTREE-HANDOFF",
|
||||
settings: {
|
||||
baseBranch: "master",
|
||||
mergeIntegrationWorktree: "reuse-task-worktree",
|
||||
} as any,
|
||||
});
|
||||
|
||||
try {
|
||||
const { rootDir, store, task } = fixture;
|
||||
const actualTask = await store.getTask(task.id);
|
||||
const branch = `fusion/${actualTask!.id.toLowerCase()}`;
|
||||
git(rootDir, "git branch -m main master");
|
||||
|
||||
const completedSteps = (actualTask?.steps ?? []).map((step) => ({ ...step, status: "done" as const }));
|
||||
await store.updateTask(task.id, {
|
||||
baseBranch: "master",
|
||||
branch,
|
||||
worktree: null,
|
||||
steps: completedSteps,
|
||||
currentStep: completedSteps.length,
|
||||
} as any);
|
||||
await fixture.createBranch(branch);
|
||||
await fixture.writeAndCommit("packages/engine/src/fn-5353-ri-missing-worktree-handoff.ts", "export const missingHandoff = true;\n", "feat: add missing worktree handoff content");
|
||||
await fixture.checkout("master");
|
||||
store.enqueueMergeQueue(task.id);
|
||||
|
||||
const result = await aiMergeTask(store, rootDir, task.id);
|
||||
expect(result.merged).toBe(true);
|
||||
expect((await store.getTask(task.id))?.column).toBe("done");
|
||||
const audits = store.getRunAuditEvents({ taskId: task.id });
|
||||
const auditTypes = audits.map((event) => event.mutationType);
|
||||
expect(auditTypes).toContain("merge:reuse-fallback-new-worktree");
|
||||
expect(auditTypes).not.toContain("merge:reuse-handoff-refused");
|
||||
const refused = audits.find((event) => event.mutationType === "merge:reuse-handoff-refused");
|
||||
expect((refused?.metadata as { reason?: string } | undefined)?.reason).not.toBe("worktree-equals-project-root");
|
||||
} finally {
|
||||
await fixture.cleanup();
|
||||
}
|
||||
}, 30_000);
|
||||
|
||||
it.skipIf(!hasGit)("FN-5363: queue-head pollution by non-in-review tasks does not block target reuse handoff", async () => {
|
||||
const fixture = await makeReliabilityFixture({
|
||||
taskId: "FN-5363-RI-POLLUTED",
|
||||
@@ -329,7 +493,7 @@ describe("FN-5279 reliability interactions: merge reuse task worktree", () => {
|
||||
}
|
||||
}, 30_000);
|
||||
|
||||
it.skipIf(!hasGit)("FN-5363: target row leased by another worker refuses with no-lease and queue-head diagnostics", async () => {
|
||||
it.skipIf(!hasGit)("FN-5363: target row leased by another worker refuses with target-not-queued diagnostics", async () => {
|
||||
const fixture = await makeReliabilityFixture({
|
||||
taskId: "FN-5363-RI-NO-LEASE-TARGET",
|
||||
settings: {
|
||||
@@ -374,10 +538,10 @@ describe("FN-5279 reliability interactions: merge reuse task worktree", () => {
|
||||
await expect(aiMergeTask(store, rootDir, task.id)).rejects.toMatchObject({
|
||||
name: "MergeHandoffRefusedError",
|
||||
gate: "lease-handoff-failed",
|
||||
reason: "no-lease",
|
||||
reason: "target-not-queued",
|
||||
});
|
||||
const refused = store.getRunAuditEvents({ taskId: task.id }).find((event) => event.mutationType === "merge:reuse-handoff-refused");
|
||||
expect(refused?.metadata).toMatchObject({ reason: "no-lease" });
|
||||
expect(refused?.metadata).toMatchObject({ reason: "target-not-queued" });
|
||||
} finally {
|
||||
await fixture.cleanup();
|
||||
}
|
||||
|
||||
@@ -32,6 +32,8 @@ const MERGE_HANDOFF_WORKER_ID = "merger-reuse-handoff";
|
||||
|
||||
export interface MergeIntegrationRootResolution {
|
||||
mode: MergeIntegrationWorktreeMode;
|
||||
// Sentinel: empty string means reuse mode is requested but no reusable
|
||||
// task.worktree is currently recorded; caller must reacquire before use.
|
||||
rootDir: string;
|
||||
branchName: string;
|
||||
}
|
||||
@@ -51,10 +53,11 @@ export function resolveMergeIntegrationRoot(
|
||||
input.settings.mergeIntegrationWorktree,
|
||||
);
|
||||
|
||||
const reusablePath = input.task.worktree?.trim() || "";
|
||||
return {
|
||||
mode,
|
||||
rootDir: mode === "reuse-task-worktree"
|
||||
? input.task.worktree?.trim() || input.projectRoot
|
||||
? reusablePath
|
||||
: input.projectRoot,
|
||||
branchName,
|
||||
};
|
||||
@@ -295,6 +298,13 @@ function asCentralClaimAccessor(store: TaskStore): {
|
||||
export async function acquireReuseHandoff(input: ReuseHandoffInput): Promise<HandoffResult> {
|
||||
const expectedBranch = canonicalFusionBranchName(input.task.id);
|
||||
const worktreePath = input.worktreePath;
|
||||
if (canonicalizePath(worktreePath) === canonicalizePath(input.projectRoot)) {
|
||||
throw new MergeHandoffRefusedError("reuse-misconfigured", "worktree-equals-project-root", {
|
||||
taskId: input.task.id,
|
||||
projectRoot: input.projectRoot,
|
||||
worktreePath,
|
||||
});
|
||||
}
|
||||
const dirtyPaths = Array.from(await snapshotDirtyFilesLocal(worktreePath)).sort();
|
||||
const dirtyFingerprint = await gitDirtyFingerprintLocal(worktreePath);
|
||||
if (dirtyPaths.length > 0 || dirtyFingerprint) {
|
||||
@@ -481,14 +491,21 @@ export async function acquireReuseHandoff(input: ReuseHandoffInput): Promise<Han
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
if (!lease || !("taskId" in lease) || lease.taskId !== input.task.id) {
|
||||
if (!lease) {
|
||||
throw new MergeHandoffRefusedError("lease-handoff-failed", "target-not-queued", {
|
||||
taskId: input.task.id,
|
||||
worktreePath,
|
||||
});
|
||||
}
|
||||
|
||||
if (!("taskId" in lease) || lease.taskId !== input.task.id) {
|
||||
const queueHead = (input.store as TaskStore & {
|
||||
peekMergeQueueHead?: () => { taskId: string; leasedBy: string | null; column: string | null } | null;
|
||||
}).peekMergeQueueHead?.();
|
||||
throw new MergeHandoffRefusedError("lease-handoff-failed", "no-lease", {
|
||||
taskId: input.task.id,
|
||||
worktreePath,
|
||||
acquiredTaskId: lease && "taskId" in lease ? lease.taskId : null,
|
||||
acquiredTaskId: "taskId" in lease ? lease.taskId : null,
|
||||
queueHeadTaskId: queueHead?.taskId ?? null,
|
||||
queueHeadLeasedBy: queueHead?.leasedBy ?? null,
|
||||
});
|
||||
|
||||
@@ -6760,12 +6760,7 @@ export async function aiMergeTask(
|
||||
projectRoot: projectRootDir,
|
||||
});
|
||||
let reuseTaskWorktreeMerge = integrationRoot.mode === "reuse-task-worktree";
|
||||
rootDir = integrationRoot.rootDir;
|
||||
let integrationRemote = await resolveIntegrationRemote({
|
||||
settings,
|
||||
rootDir: rootDir,
|
||||
integrationBranch: mergeTarget.branch,
|
||||
});
|
||||
let integrationRemote: string | undefined;
|
||||
const reacquireReuseIntegrationWorktree = async (
|
||||
reason: string,
|
||||
diagnostics: Record<string, unknown>,
|
||||
@@ -7006,6 +7001,14 @@ export async function aiMergeTask(
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
rootDir = integrationRoot.rootDir;
|
||||
integrationRemote = await resolveIntegrationRemote({
|
||||
settings,
|
||||
rootDir,
|
||||
integrationBranch: mergeTarget.branch,
|
||||
});
|
||||
|
||||
try {
|
||||
const integrationWorktreeState = await probeIntegrationWorktreeState({
|
||||
rootDir: integrationRoot.rootDir,
|
||||
@@ -7032,6 +7035,9 @@ export async function aiMergeTask(
|
||||
}
|
||||
|
||||
if (integrationRoot.mode === "reuse-task-worktree") {
|
||||
// FN-5353: ensure the target task is in mergeQueue before attempting strict
|
||||
// targetTaskId lease acquisition for reuse handoff.
|
||||
store.enqueueMergeQueue(task.id, { priority: task.priority });
|
||||
try {
|
||||
reuseHandoff = await acquireReuseHandoff({
|
||||
task,
|
||||
|
||||
@@ -5532,6 +5532,8 @@ export class SelfHealingManager {
|
||||
await this.store.logEntry(task.id, "Auto-recovered (FN-4999): task in 'in-review' past handoff grace with no merge fan-out — re-emitting auto-merge handoff");
|
||||
if (this.options.requeueForAutoMerge) {
|
||||
try {
|
||||
// FN-5353: strict targetTaskId leasing in reuse handoff requires an
|
||||
// explicit queue row before re-emitting auto-merge.
|
||||
await this.store.enqueueMergeQueue(task.id);
|
||||
} catch (err) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
|
||||
Reference in New Issue
Block a user