engine: merging was broken outright on a board with a renamed review lane (#2963)
**Not a degraded message — no task on such a board could be merged at all.** `getTaskMergeBlocker`'s column-identity check *returns a blocker* when the task's column is not a review lane. Both merge entry points called it without `reviewColumns`, so the check ran against the literal `in-review`: ``` Cannot merge FN-1: task is in 'signoff', must be in 'in-review' ``` `aiMergeTask` (`merger.ts`) and `runAiMerge` (`merger-ai.ts`) turn that into a thrown error. Every merge on a renamed board fails, with a message naming a column the board does not have. ## This exact defect was already found once The helper's own FNXC comment records it, in `moves.ts`: > *"so on a renamed board that move threw `Cannot move FN-1 to done: task is in 'signoff', must be in 'in-review'` even though the transition had just been validated as legal. A half-conversion, where the outer question is resolved and the inner one is not."* That fix added the `reviewColumns` option and wired `moves.ts`. **These two callers were missed** — same shape, one layer out. A fix that adds an optional parameter is only as good as the call-site sweep that follows it. ## How it was found By enumerating the call sites of every lane-taking helper, rather than trusting the `unwired-lane-parameter` guard. That guard is deliberately conservative — a mention of the parameter *anywhere* satisfies it — so **partial** wiring is invisible to it, and `reviewColumns` is mentioned plentifully elsewhere. This is the method #2956 used on a sibling defect, applied to every seam I have touched. ## Two sites deliberately unchanged - **`moves.ts`** passes `skipColumnIdentityCheck: true`. It has already proven lane identity from resolved IR traits, so supplying lanes *as well* would be contradictory rather than additive — the helper's comment is explicit that the two options answer different questions. - **`isTaskReadyForMerge`** has **zero** production callers. Adding a parameter there is precisely the unwired-parameter anti-pattern this program keeps removing. ## Revert result | | reverted → | | --- | --- | | `reviewColumns` at either call | reproduces the shipped string exactly | The middle test pins that string deliberately: it is the operator-visible failure, so if the wiring regresses the test says what the operator would have seen. A third case checks that supplying lanes does **not** switch the identity check off — a card in the wip lane is still blocked, and the message names the resolved lanes rather than a column the board lacks. The cases drive `getTaskMergeBlocker` directly: reaching it through the merge entry points needs a real repo, worktree and merge run, while the defect is entirely in *which columns the blocker is asked about*. The wiring itself is covered by tsc and the guard. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71; `merger` + `merger-ai` + `self-healing` suites 461; `tsc` engine clean; lint, census `--strict`, FNXC gate, changesets all clean.
This commit is contained in:
7
.changeset/merge-blocked-renamed-review-lane.md
Normal file
7
.changeset/merge-blocked-renamed-review-lane.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Tasks can be merged again on boards whose review column is renamed.
|
||||
category: fix
|
||||
dev: Both merge entry points called `getTaskMergeBlocker` without `reviewColumns`, so its column-identity check used the literal `in-review` and returned a blocker for any renamed review lane — `aiMergeTask` and `runAiMerge` then threw `Cannot merge FN-x: task is in '<lane>', must be in 'in-review'`. Both now resolve the task's own review lanes.
|
||||
@@ -0,0 +1,69 @@
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-17:20 (merging was broken outright on a renamed board):
|
||||
|
||||
`getTaskMergeBlocker`'s column-identity check RETURNS A BLOCKER when the task's column is not a review
|
||||
lane. Both merge entry points called it WITHOUT `reviewColumns`, so on a board whose review lane is
|
||||
renamed the literal check failed and the merge threw:
|
||||
|
||||
Cannot merge FN-1: task is in 'signoff', must be in 'in-review'
|
||||
|
||||
That is not a degraded message — no task on such a board could be merged at all. The helper's own
|
||||
comment records this exact defect being found and fixed in `moves.ts`; these two callers were missed,
|
||||
which is the same half-conversion shape (outer question resolved, inner one still literal).
|
||||
|
||||
WHY THE SEAM, NOT THE MERGE. These cases drive `getTaskMergeBlocker` directly. Reaching it through
|
||||
`aiMergeTask`/`runAiMerge` needs a real git repo, a worktree and a merge run; the defect is entirely in
|
||||
which columns the blocker is asked about, so the seam is where it is decidable. The wiring at the two
|
||||
call sites is covered by the unwired-lane-parameter guard plus tsc.
|
||||
|
||||
REVERT CHECK, measured: drop `reviewColumns` from either call and the renamed case reports
|
||||
`task is in 'signoff', must be in 'in-review'` instead of undefined.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { getTaskMergeBlocker } from "@fusion/core";
|
||||
import type { Task } from "@fusion/core";
|
||||
|
||||
function reviewReadyCard(column: string): Task {
|
||||
return {
|
||||
id: "FN-MERGE",
|
||||
column,
|
||||
paused: false,
|
||||
status: null,
|
||||
error: null,
|
||||
steps: [{ id: "s1", status: "done" }],
|
||||
workflowStepResults: [],
|
||||
} as unknown as Task;
|
||||
}
|
||||
|
||||
describe("the merge blocker judges the board's OWN review lanes", () => {
|
||||
it("does not block a merge-ready card sitting in a RENAMED review lane", () => {
|
||||
const blocker = getTaskMergeBlocker(reviewReadyCard("signoff"), {
|
||||
reviewColumns: new Set(["signoff", "in-review"]),
|
||||
});
|
||||
|
||||
expect(blocker).toBeUndefined();
|
||||
});
|
||||
|
||||
it("reproduces the shipped failure when the lanes are not supplied", () => {
|
||||
/*
|
||||
This is exactly what both merge entry points did before the fix, and it is the operator-visible
|
||||
string: the merge threw with this as its reason.
|
||||
*/
|
||||
const blocker = getTaskMergeBlocker(reviewReadyCard("signoff"));
|
||||
|
||||
expect(blocker).toBe("task is in 'signoff', must be in 'in-review'");
|
||||
});
|
||||
|
||||
it("still blocks a card that is genuinely outside the review lanes", () => {
|
||||
/*
|
||||
Non-vacuous companion: supplying lanes must not turn the identity check off — a card in the wip
|
||||
lane is not merge-ready on any board, and the message names the resolved lanes rather than a
|
||||
column the board does not have.
|
||||
*/
|
||||
const blocker = getTaskMergeBlocker(reviewReadyCard("building"), {
|
||||
reviewColumns: new Set(["signoff"]),
|
||||
});
|
||||
|
||||
expect(blocker).toBe("task is in 'building', must be in 'signoff'");
|
||||
});
|
||||
});
|
||||
@@ -60,7 +60,7 @@ import {
|
||||
type MergeTargetResolution,
|
||||
type Settings,
|
||||
type Task,
|
||||
type TaskStore,
|
||||
type TaskStore, resolveReviewColumns
|
||||
} from "@fusion/core";
|
||||
import { selectUserCommentsForAgentContext } from "./agent-user-comments.js";
|
||||
import { resolveTaskWorkingBranch } from "./worktree-names.js";
|
||||
@@ -1190,7 +1190,22 @@ export async function runAiMerge(
|
||||
if (await isAlreadyFinalizedColumn(store, task)) {
|
||||
return noOpResult(task, branch, "already-finalized");
|
||||
}
|
||||
const blocker = getTaskMergeBlocker(task, { manual: options.manual === true });
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-17:10 (MERGING WAS BROKEN ON A RENAMED BOARD):
|
||||
`getTaskMergeBlocker`'s identity check RETURNS A BLOCKER when the column is not a review lane, so
|
||||
calling it without `reviewColumns` on a board whose review lane is renamed produced
|
||||
`Cannot merge FN-x: task is in 'signoff', must be in 'in-review'` — and the merge threw. Not a
|
||||
degraded message: no task could be merged at all.
|
||||
|
||||
The helper's own comment records this exact defect being fixed in `moves.ts`; these two merge
|
||||
entry points were missed. Resolve the task's own review lanes and pass them.
|
||||
*/
|
||||
const aiReviewColumns = new Set<string>(["in-review"]);
|
||||
try {
|
||||
const aiIr = await resolveWorkflowIrForTask(store, taskId);
|
||||
if (aiIr) for (const id of resolveReviewColumns(aiIr)) aiReviewColumns.add(id);
|
||||
} catch { /* degraded: the legacy id above still answers */ }
|
||||
const blocker = getTaskMergeBlocker(task, { manual: options.manual === true, reviewColumns: aiReviewColumns });
|
||||
if (blocker) throw new Error(`Cannot merge ${taskId}: ${blocker}`);
|
||||
|
||||
const settings = await store.getSettings();
|
||||
|
||||
@@ -110,7 +110,7 @@ import {
|
||||
resolveCompleteColumn,
|
||||
resolveMergeOrchestrationColumn,
|
||||
resolveTaskLifecycleColumns,
|
||||
type WorkflowIr,
|
||||
type WorkflowIr, resolveReviewColumns
|
||||
} from "@fusion/core";
|
||||
import { evaluateAutoMergeFactProviders } from "./auto-merge-fact-providers.js";
|
||||
import { resolveMergePolicy } from "./merge-trait.js";
|
||||
@@ -6642,7 +6642,22 @@ export async function aiMergeTask(
|
||||
branchDeleted: false,
|
||||
};
|
||||
}
|
||||
const mergeBlocker = getTaskMergeBlocker(task, { manual: options.manual === true });
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-17:10 (MERGING WAS BROKEN ON A RENAMED BOARD):
|
||||
`getTaskMergeBlocker`'s identity check RETURNS A BLOCKER when the column is not a review lane, so
|
||||
calling it without `reviewColumns` on a board whose review lane is renamed produced
|
||||
`Cannot merge FN-x: task is in 'signoff', must be in 'in-review'` — and the merge threw. Not a
|
||||
degraded message: no task could be merged at all.
|
||||
|
||||
The helper's own comment records this exact defect being fixed in `moves.ts`; these two merge
|
||||
entry points were missed. Resolve the task's own review lanes and pass them.
|
||||
*/
|
||||
const mergeReviewColumns = new Set<string>(["in-review"]);
|
||||
try {
|
||||
const mergeIr = await resolveWorkflowIrForTask(store, taskId);
|
||||
if (mergeIr) for (const id of resolveReviewColumns(mergeIr)) mergeReviewColumns.add(id);
|
||||
} catch { /* degraded: the legacy id above still answers */ }
|
||||
const mergeBlocker = getTaskMergeBlocker(task, { manual: options.manual === true, reviewColumns: mergeReviewColumns });
|
||||
if (mergeBlocker) {
|
||||
throw new Error(`Cannot merge ${taskId}: ${mergeBlocker}`);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user