From 7712e0ada2a3e0d062700644d3b5ac4bb74312a5 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 21:50:32 -0700 Subject: [PATCH] engine: merging was broken outright on a board with a renamed review lane (#2963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **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. --- .../merge-blocked-renamed-review-lane.md | 7 ++ .../merge-blocker-renamed-review-lane.test.ts | 69 +++++++++++++++++++ packages/engine/src/merger-ai.ts | 19 ++++- packages/engine/src/merger.ts | 19 ++++- 4 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 .changeset/merge-blocked-renamed-review-lane.md create mode 100644 packages/engine/src/__tests__/merge-blocker-renamed-review-lane.test.ts diff --git a/.changeset/merge-blocked-renamed-review-lane.md b/.changeset/merge-blocked-renamed-review-lane.md new file mode 100644 index 0000000000..9f920c5ebc --- /dev/null +++ b/.changeset/merge-blocked-renamed-review-lane.md @@ -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 '', must be in 'in-review'`. Both now resolve the task's own review lanes. diff --git a/packages/engine/src/__tests__/merge-blocker-renamed-review-lane.test.ts b/packages/engine/src/__tests__/merge-blocker-renamed-review-lane.test.ts new file mode 100644 index 0000000000..9ebadb6fb4 --- /dev/null +++ b/packages/engine/src/__tests__/merge-blocker-renamed-review-lane.test.ts @@ -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'"); + }); +}); diff --git a/packages/engine/src/merger-ai.ts b/packages/engine/src/merger-ai.ts index 0e1196f8c4..7f9a6fdfa0 100644 --- a/packages/engine/src/merger-ai.ts +++ b/packages/engine/src/merger-ai.ts @@ -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(["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(); diff --git a/packages/engine/src/merger.ts b/packages/engine/src/merger.ts index dea6925c35..883e1ade7d 100644 --- a/packages/engine/src/merger.ts +++ b/packages/engine/src/merger.ts @@ -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(["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}`); }