root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(core): thread review lanes through merge readiness (#3514)

Browse Source 
## Summary
- thread resolved review lanes through `isTaskReadyForMerge`
- preserve required pre-merge step filtering
- add coverage for a renamed review lane

## Test plan
- `pnpm --filter @fusion/core exec vitest run --silent=passed-only
--reporter=dot src/__tests__/task-merge.test.ts`
- `pnpm --filter @fusion/core typecheck`
- `pnpm check:lane-wiring`
- `pnpm check:changesets`
- `pnpm exec eslint packages/core/src/merge/task-merge.ts
packages/core/src/__tests__/task-merge.test.ts`

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Custom review lanes are now honored during merge-readiness checks and
auto-merge processing.
  * Renamed workflow lanes correctly determine whether tasks can merge.
* Tasks resumed from a paused state are routed and evaluated using the
appropriate review lane.
* The default `in-review` lane remains supported when no custom review
lanes are configured.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: gsxdsm <gsxdsm@users.noreply.github.com>
This commit is contained in:
Phil Larson
2026-08-23 15:31:17 -07:00
committed by GitHub
parent febe375b14
commit bc82d8e0e1
5 changed files with 198 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changeset/task-ready-lane-wiring.md Normal file
View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Honor custom review lanes across live merge-readiness checks.
category: fix
dev: Threads resolved review lanes through ProjectEngine admission and preserves the empty-set legacy fallback.

21
packages/core/src/__tests__/task-merge.test.ts
View File

@@ -862,6 +862,27 @@ describe("isTaskReadyForMerge", () => {
expect(isTaskReadyForMerge(baseTask)).toBe(true);
});
it("uses the caller's resolved review lanes", () => {
expect(isTaskReadyForMerge(
{ ...baseTask, column: "signoff" },
{ reviewColumns: new Set(["signoff"]) },
)).toBe(true);
});
/*
FNXC:MergeReadiness 2026-08-23-18:49:
A resolved workflow can return an empty review-lane set when it has no usable trait answer. Empty is
therefore "unresolved", not an authoritative board with no review lane; keep the legacy `in-review`
identity fallback until the caller can supply at least one resolved lane.
*/
it("preserves the legacy review lane fallback for an empty resolved set", () => {
expect(isTaskReadyForMerge(baseTask, { reviewColumns: new Set() })).toBe(true);
expect(isTaskReadyForMerge(
{ ...baseTask, column: "signoff" },
{ reviewColumns: new Set() },
)).toBe(false);
});
it("returns false when pre-merge step failed", () => {
expect(isTaskReadyForMerge({
...baseTask,

22
packages/core/src/merge/task-merge.ts
View File

@@ -430,12 +430,19 @@ export function getTaskMergeBlocker(
the resolved lanes when they are known, so it can never point at a column the board does not have.
*/
if (!options.skipColumnIdentityCheck) {
const inReviewLane = options.reviewColumns
? options.reviewColumns.has(task.column)
/*
FNXC:MergeReadiness 2026-08-23-18:49:
An empty resolved lane set means the workflow supplied no usable trait answer. Preserve the
documented legacy identity fallback until at least one resolved review lane is available; treating
an empty Set as authoritative would make every column fail while the error still names `in-review`.
*/
const reviewColumns = options.reviewColumns?.size ? options.reviewColumns : undefined;
const inReviewLane = reviewColumns
? reviewColumns.has(task.column)
: task.column === "in-review";
if (!inReviewLane) {
const expected = options.reviewColumns && options.reviewColumns.size > 0
? [...options.reviewColumns].map((c) => `'${c}'`).join(" or ")
const expected = reviewColumns
? [...reviewColumns].map((c) => `'${c}'`).join(" or ")
: "'in-review'";
return `task is in '${task.column}', must be in ${expected}`;
}
@@ -665,9 +672,12 @@ export function getTaskDoneBypassBlocker(
export function isTaskReadyForMerge(
task: Pick<Task, "column" | "paused" | "status" | "error" | "steps" | "workflowStepResults">,
options: { requiredPreMergeStepIds?: ReadonlySet<string> } = {},
options: { reviewColumns?: ReadonlySet<string>; requiredPreMergeStepIds?: ReadonlySet<string> } = {},
): boolean {
return getTaskMergeBlocker(task, options) === undefined;
return getTaskMergeBlocker(task, {
reviewColumns: options.reviewColumns,
requiredPreMergeStepIds: options.requiredPreMergeStepIds,
}) === undefined;
}
export interface TaskCompletionBlockerOptions {

129
packages/engine/src/__tests__/project-engine-merge-lane-resolved.test.ts
View File

@@ -26,7 +26,7 @@ REVERT PROOF, measured: restore the literal and the renamed-board case returns `
routing to `onMerge`.
*/
import { describe, expect, it, vi } from "vitest";
import type { Settings, Task, TaskStore, WorkflowIr } from "@fusion/core";
import { getTaskMergeBlocker, type Settings, type Task, type TaskStore, type WorkflowIr } from "@fusion/core";
import { ProjectEngine } from "../project-engine.js";
@@ -191,4 +191,131 @@ describe("the other merge-lane surfaces on a renamed board", () => {
expect([...self.pausedReviewTaskIds]).toEqual([]);
});
/*
FNXC:MergeReadiness 2026-08-23-18:49:
ProjectEngine's live admission callbacks must receive the same resolved merge lane used by their
surrounding column guard. Passing only the task makes the injected core blocker silently restore the
`in-review` literal and prevents a renamed-lane card from re-entering the production merge queue.
*/
it("re-enqueues an unpaused renamed-lane card through the live merge blocker", async () => {
const { store, task } = storeFor("signoff", RENAMED_IR);
const self = {
options: { getTaskMergeBlocker },
pausedReviewTaskIds: new Set<string>([task.id]),
mergeQueue: [] as string[],
mergeActive: new Set<string>(),
activeMergeTaskId: null as string | null,
taskUpdatedHandler: undefined as unknown,
abortActiveMerge: vi.fn(),
allowInReviewMergeProcessing: vi.fn(async () => true),
classifyMergeSweepCandidate: vi.fn(async () => ({ admit: true })),
internalEnqueueMerge: vi.fn(),
};
(ProjectEngine.prototype as unknown as {
wireTaskPauseMergeInterruption: (this: unknown, s: TaskStore) => void;
}).wireTaskPauseMergeInterruption.call(self, store);
await (self.taskUpdatedHandler as (t: Task) => Promise<void>)({ ...task, paused: false } as Task);
expect(self.internalEnqueueMerge).toHaveBeenCalledWith(task.id);
expect(self.options.getTaskMergeBlocker(task, { reviewColumns: new Set(["signoff"]) })).toBeUndefined();
});
/*
FNXC:MergeReadiness 2026-08-23-20:25:
Lane forwarding is required at every live ProjectEngine merge-blocker door, not only unpause. The
periodic sweep, final dequeue, and both sides of the handoff grace window must give the blocker the
resolved `signoff` lane; otherwise any one of those callers can silently reject a renamed-lane card.
*/
it("passes the resolved lane from the periodic sweep into the merge blocker", async () => {
const { store, task } = storeFor("signoff", RENAMED_IR);
const blocker = vi.fn(() => "blocked for assertion");
const self = {
options: { getTaskMergeBlocker: blocker, getMergeStrategy: vi.fn(() => "direct") },
runtime: { getTaskStore: () => store },
canMergeTask: (ProjectEngine.prototype as unknown as { canMergeTask: (...args: unknown[]) => boolean }).canMergeTask,
allowInReviewMergeProcessing: vi.fn(async () => true),
loadMergeSweepBatch: vi.fn(async () => ({})),
classifyMergeSweepCandidate: vi.fn(async () => ({ admit: true })),
mergeSweepHoldReasons: new Map<string, string>(),
internalEnqueueMerge: vi.fn(),
};
const admitted = await (ProjectEngine.prototype as unknown as {
enqueueEligibleInReviewTasks: (this: unknown, tasks: readonly Task[], settings: Pick<Settings, "autoMerge" | "maxAutoMergeRetries">) => Promise<number>;
}).enqueueEligibleInReviewTasks.call(self, [task], { autoMerge: true, maxAutoMergeRetries: 3 });
expect(admitted).toBe(0);
expect(blocker).toHaveBeenCalledWith(task, { reviewColumns: new Set(["signoff"]) });
});
it("passes the resolved lane from the final dequeue into the merge blocker", async () => {
const { store, task } = storeFor("signoff", RENAMED_IR);
const blocker = vi.fn(() => "blocked for assertion");
const mergeQueue = [task.id];
const self = {
options: { getTaskMergeBlocker: blocker, getMergeStrategy: vi.fn(() => "direct") },
runtime: { getTaskStore: () => store },
config: { workingDirectory: "/unused" },
mergeQueue,
coordinatorAdmittedMergeTaskIds: new Set<string>(),
mergeRunning: false,
mergeRunningSince: 0,
mergeAbortController: null,
shuttingDown: false,
reconcileStaleMergeActive: vi.fn(),
getShadowMergeRequestCandidateId: vi.fn(async () => null),
pickNextMergeTaskId: vi.fn(async () => mergeQueue.shift()),
hasMergeResolvers: vi.fn(() => false),
allowInReviewMergeProcessing: vi.fn(async () => true),
canMergeTask: (ProjectEngine.prototype as unknown as { canMergeTask: (...args: unknown[]) => boolean }).canMergeTask,
schedulePrMergeRetry: vi.fn(),
clearActiveMergeClaim: vi.fn(),
clearMergeActive: vi.fn(),
};
await (ProjectEngine.prototype as unknown as {
drainMergeQueue: (this: unknown) => Promise<void>;
}).drainMergeQueue.call(self);
expect(blocker).toHaveBeenCalledWith(task, { reviewColumns: new Set(["signoff"]) });
});
it("passes the resolved lane to both immediate and post-grace handoff blockers", async () => {
vi.useFakeTimers();
try {
const { store, task } = storeFor("signoff", RENAMED_IR);
const blocker = vi.fn(() => undefined);
const self = {
options: { getTaskMergeBlocker: blocker },
taskMovedHandler: undefined as unknown,
mergeActive: new Set<string>(),
mergeQueue: [] as string[],
activeMergeTaskId: null as string | null,
clearMergeActive: vi.fn(),
allowInReviewMergeProcessing: vi.fn(async () => true),
classifyMergeSweepCandidate: vi.fn(async () => ({ admit: true })),
internalEnqueueMerge: vi.fn(),
};
(ProjectEngine.prototype as unknown as {
wireAutoMerge: (this: unknown, s: TaskStore, cwd: string) => void;
}).wireAutoMerge.call(self, store, "/unused");
await (self.taskMovedHandler as (event: { task: Task; to: string }) => Promise<void>)({ task, to: "signoff" });
expect(blocker).toHaveBeenCalledTimes(1);
expect(blocker).toHaveBeenLastCalledWith(task, { reviewColumns: new Set(["signoff"]) });
await vi.runOnlyPendingTimersAsync();
expect(blocker).toHaveBeenCalledTimes(2);
expect(blocker).toHaveBeenLastCalledWith(task, { reviewColumns: new Set(["signoff"]) });
expect(self.internalEnqueueMerge).toHaveBeenCalledWith(task.id);
} finally {
vi.useRealTimers();
}
});
});

36
packages/engine/src/project-engine.ts
View File

@@ -447,7 +447,10 @@ export interface ProjectEngineOptions {
* Returns the merge blocker reason for a task, or null/undefined if
* the task is eligible for merge. Imported from @fusion/core.
*/
getTaskMergeBlocker?: (task: Task) => string | null | undefined;
getTaskMergeBlocker?: (
task: Task,
options?: { reviewColumns?: ReadonlySet<string> },
) => string | null | undefined;
/**
* Callback for insight extraction run processing.
* Invoked after CronRunner completes a memory insight extraction schedule.
@@ -3160,14 +3163,20 @@ export class ProjectEngine {
log?: Array<{ action?: string }>;
updatedAt?: string | null;
mergeDetails?: { mergeConfirmed?: boolean } | null;
}, maxAutoMergeRetries: number, isReviewColumn?: boolean, enforcePrRetryBackoff = false): boolean {
}, maxAutoMergeRetries: number, reviewColumns?: ReadonlySet<string>, enforcePrRetryBackoff = false): boolean {
// Merge-confirmed tasks use the fast-path finalizer, which applies blocker
// checks after clearing transient status/error state. Once that path parks
// a blocked task as failed, skip future auto-merge retries.
if (task.mergeDetails?.mergeConfirmed) {
return true;
}
if (this.options.getTaskMergeBlocker?.(task as Task)) return false;
/*
FNXC:MergeReadiness 2026-08-23-18:49:
This shared admission predicate serves both the periodic sweep and the final queue dispatch. Forward
their resolved lane set into the injected core blocker so neither production path reverts to the
legacy `in-review` literal after its surrounding column check accepted a renamed merge lane.
*/
if (this.options.getTaskMergeBlocker?.(task as Task, { reviewColumns })) return false;
// Terminal failure: don't let the cooldown sweep re-attempt a merge that
// already gave up (verification cap, conflict-bounce cap, or non-conflict
// error). The task is parked for human/follow-up intervention.
@@ -3185,7 +3194,11 @@ export class ProjectEngine {
}
return (
(task.mergeRetries ?? 0) < maxAutoMergeRetries ||
this.hasAutoHealableVerificationBufferFailure(task, maxAutoMergeRetries, isReviewColumn) ||
this.hasAutoHealableVerificationBufferFailure(
task,
maxAutoMergeRetries,
reviewColumns === undefined ? undefined : reviewColumns.has(task.column),
) ||
this.isRetryCooldownElapsed(task)
);
}
@@ -3459,7 +3472,7 @@ export class ProjectEngine {
return this.canMergeTask(
t as any,
maxAutoMergeRetries,
reviewLane === undefined ? undefined : t.column === reviewLane,
reviewLane === undefined ? undefined : new Set([reviewLane]),
enforcePrRetryBackoff,
);
}) as Task[];
@@ -4143,7 +4156,7 @@ export class ProjectEngine {
if (!this.canMergeTask(
task as any,
maxAutoMergeRetries,
mergeLoopReviewLane === undefined ? undefined : task.column === mergeLoopReviewLane,
mergeLoopReviewLane === undefined ? undefined : new Set([mergeLoopReviewLane]),
pullRequestMerge,
)) {
// A queued retry can be rejected after an engine restart or a racing
@@ -6106,7 +6119,7 @@ export class ProjectEngine {
const handoffReviewColumn = (await resolveTaskLifecycleColumns(store, task.id))?.review ?? "in-review";
if (to !== handoffReviewColumn) return;
if (task.paused) return;
if (this.options.getTaskMergeBlocker?.(task)) return;
if (this.options.getTaskMergeBlocker?.(task, { reviewColumns: new Set([handoffReviewColumn]) })) return;
// Grace period before handing off to the merger. The executor's finally
// block (session disposal, child-agent termination, in-flight reviewer
@@ -6133,7 +6146,9 @@ export class ProjectEngine {
runtimeLog.log(`Auto-merge handoff (${task.id}) skipped: task paused`);
return;
}
const blockerReason = this.options.getTaskMergeBlocker?.(latestTask);
const blockerReason = this.options.getTaskMergeBlocker?.(latestTask, {
reviewColumns: new Set([handoffReviewColumn]),
});
if (blockerReason) {
runtimeLog.log(`Auto-merge handoff (${task.id}) skipped: ${blockerReason}`);
return;
@@ -6252,7 +6267,8 @@ export class ProjectEngine {
this.taskUpdatedHandler = async (task: Task) => {
/* FNXC:WorkflowLifecycleColumns 2026-08-01-19:25 (fleet): on a renamed board this dropped EVERY card
from the paused-review set on its next update, so a merge paused mid-flight was never interrupted. */
if (task.column !== ((await resolveTaskLifecycleColumns(store, task.id))?.review ?? "in-review")) {
const taskReviewColumn = (await resolveTaskLifecycleColumns(store, task.id))?.review ?? "in-review";
if (task.column !== taskReviewColumn) {
this.pausedReviewTaskIds.delete(task.id);
return;
}
@@ -6291,7 +6307,7 @@ export class ProjectEngine {
if (settings.globalPause || settings.enginePaused || !(await this.allowInReviewMergeProcessing(task, settings, store))) {
return;
}
if (this.options.getTaskMergeBlocker?.(task)) {
if (this.options.getTaskMergeBlocker?.(task, { reviewColumns: new Set([taskReviewColumn]) })) {
return;
}