core: every review card reported a false stall on a renamed board (#2970)
**The failure mode worth distinguishing: the rest of this family went
quiet on a renamed board. This one shouted.**
`getInReviewStallReason` satisfied its **own** lane check from
`context.reviewColumns` — then called `getTaskMergeBlocker` **without**
them. That helper re-ran its column-identity check against the literal
`in-review` and returned, for a perfectly healthy card:
```
task is in 'signoff', must be in 'in-review'
```
…which was surfaced as `{ code: "merge-blocker" }`. **Every in-review
card on a renamed board was flagged as stalled**, each citing a lane the
board does not have. That is how a signal stops being read at all.
## A second symptom, found by the revert rather than by reading
On a **genuinely failed** card, the identity message wins over the real
one. The operator saw the bogus column complaint instead of `task is
marked 'failed': merge verification failed`.
So it did not only invent stalls — it **masked the true reason for real
ones**. I would not have noticed that from the diff; it showed up
because the revert run asserted on the reason text.
## Same shape, last one in the family
The outer question was resolved and the inner one was not — the
half-conversion the helper's own comment records for `moves.ts`, and
#2963/#2964 fixed for the merge entry points. This is the last site the
audit turned up where the lane answer was already in scope and simply
not forwarded.
## Revert results
| | reverted → |
| --- | --- |
| the unforwarded call (what ships today) | **2 of 3 fail** — healthy
card reports a merge-blocker stall; failed card reports the wrong reason
|
**Fixture note worth keeping:** `paused` is deliberately *not* the
genuine-stall case. An earlier guard returns `undefined` for a paused
card before the merge blocker is ever consulted, so that case would pass
whether or not the lanes are forwarded — the vacuous shape this series
has produced eight times.
## Verification
`pnpm test:gate` 161 + 487 + 13 + 71; `@fusion/core` full suite **4878
passed** (457 files); `tsc` core clean; lint, lifecycle census
`--strict`, FNXC gate, changesets all clean.
This commit is contained in:
7
.changeset/stall-signal-forwards-lanes.md
Normal file
7
.changeset/stall-signal-forwards-lanes.md
Normal file
@@ -0,0 +1,7 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
summary: Review cards no longer all report a false stall on boards with renamed columns.
|
||||||
|
category: fix
|
||||||
|
dev: `getInReviewStallReason` satisfied its own lane check from `context.reviewColumns` but called `getTaskMergeBlocker` without them, so the helper re-checked against the literal `in-review` and returned an identity message for every healthy card — surfaced as a merge-blocker stall, and masking the real reason on genuinely failed ones.
|
||||||
@@ -0,0 +1,80 @@
|
|||||||
|
/*
|
||||||
|
FNXC:WorkflowResolvedColumns 2026-07-30-21:10 (a false-alarm generator, not a silent miss):
|
||||||
|
|
||||||
|
`getInReviewStallReason` satisfied its OWN lane check from `context.reviewColumns`, then called
|
||||||
|
`getTaskMergeBlocker` WITHOUT them. That helper re-ran its column-identity check against the literal
|
||||||
|
`in-review`, so on a renamed board a perfectly healthy review card came back with
|
||||||
|
|
||||||
|
task is in 'signoff', must be in 'in-review'
|
||||||
|
|
||||||
|
which was surfaced as `{ code: "merge-blocker" }` — a stall reason naming a column the board does not
|
||||||
|
have, for EVERY card in review.
|
||||||
|
|
||||||
|
This is the failure mode worth distinguishing from the rest of the family: the others went quiet on a
|
||||||
|
renamed board, this one shouted. An operator would see every in-review card flagged as stalled, each
|
||||||
|
citing a lane that does not exist, which is how a signal stops being read at all.
|
||||||
|
|
||||||
|
The outer question was resolved and the inner one was not — the same half-conversion the helper's own
|
||||||
|
comment records for `moves.ts`, and #2963/#2964 fixed for the merge paths.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import { getInReviewStallReason } from "../in-review-stall.js";
|
||||||
|
import { getTaskMergeBlocker } from "../task-merge.js";
|
||||||
|
import type { Task } from "../types.js";
|
||||||
|
|
||||||
|
/** A healthy card sitting in a renamed review lane: nothing wrong with it. */
|
||||||
|
function healthyReviewCard(column: string): Task {
|
||||||
|
return {
|
||||||
|
id: "FN-HEALTHY",
|
||||||
|
column,
|
||||||
|
paused: false,
|
||||||
|
status: null,
|
||||||
|
error: null,
|
||||||
|
steps: [{ id: "s1", status: "done" }],
|
||||||
|
workflowStepResults: [],
|
||||||
|
worktree: "/tmp/wt",
|
||||||
|
mergeDetails: {},
|
||||||
|
mergeRetries: 0,
|
||||||
|
updatedAt: new Date().toISOString(),
|
||||||
|
} as unknown as Task;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("the stall signal forwards its resolved lanes to the merge blocker", () => {
|
||||||
|
it("reports NO stall for a healthy card on a RENAMED review lane", () => {
|
||||||
|
const signal = getInReviewStallReason(healthyReviewCard("signoff"), {
|
||||||
|
reviewColumns: new Set(["signoff"]),
|
||||||
|
now: Date.now(),
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(signal).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("reproduces the shipped false alarm when the lanes are not forwarded", () => {
|
||||||
|
/*
|
||||||
|
Drives the helper the way the unfixed code did — lanes known to the caller, not passed on. The
|
||||||
|
reason string is the operator-visible text, so a regression reports what they would actually see.
|
||||||
|
*/
|
||||||
|
const blocker = getTaskMergeBlocker(healthyReviewCard("signoff"));
|
||||||
|
|
||||||
|
expect(blocker).toBe("task is in 'signoff', must be in 'in-review'");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("still reports a genuine stall on a renamed lane", () => {
|
||||||
|
/*
|
||||||
|
Non-vacuous companion: forwarding lanes must not silence REAL blockers, or the fix would trade a
|
||||||
|
wall of false alarms for silence. A failed card in the board's own review lane is genuinely stalled
|
||||||
|
and must still be reported.
|
||||||
|
|
||||||
|
Note `paused` is deliberately not the case used here — an earlier guard returns undefined for a
|
||||||
|
paused card before the merge blocker is ever consulted, so it would pass whether or not the lanes
|
||||||
|
are forwarded. That is the vacuous shape this file exists to avoid.
|
||||||
|
*/
|
||||||
|
const signal = getInReviewStallReason(
|
||||||
|
{ ...healthyReviewCard("signoff"), status: "failed", error: "merge verification failed" } as Task,
|
||||||
|
{ reviewColumns: new Set(["signoff"]), now: Date.now() },
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(signal?.code).toBe("merge-blocker");
|
||||||
|
expect(signal?.reason).toContain("failed");
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -252,7 +252,21 @@ export function getInReviewStallReason(
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
const mergeBlocker = getTaskMergeBlocker(task);
|
/*
|
||||||
|
FNXC:WorkflowResolvedColumns 2026-07-30-21:05 (this reported EVERY healthy review card as stalled):
|
||||||
|
Forward the resolved lanes. This function had already satisfied its OWN lane check above using
|
||||||
|
`context.reviewColumns`, then called getTaskMergeBlocker without them — so the helper re-ran its
|
||||||
|
identity check against the literal `in-review` and, on a renamed board, returned
|
||||||
|
`task is in 'signoff', must be in 'in-review'` for a perfectly healthy card.
|
||||||
|
|
||||||
|
That string was then surfaced as `{ code: "merge-blocker" }`, i.e. a stall reason naming a column
|
||||||
|
the board does not have — for every card in review. Worse than silence: the operator gets a wall of
|
||||||
|
false stalls, each citing a lane that does not exist.
|
||||||
|
|
||||||
|
The outer question was resolved and the inner one was not — the same half-conversion recorded at the
|
||||||
|
helper itself for moves.ts, and fixed in #2963/#2964 for the merge paths.
|
||||||
|
*/
|
||||||
|
const mergeBlocker = getTaskMergeBlocker(task, { reviewColumns: context.reviewColumns });
|
||||||
if (mergeBlocker) {
|
if (mergeBlocker) {
|
||||||
if (mergeBlocker.startsWith(FAILED_TASK_MERGE_BLOCKER_PREFIX)) {
|
if (mergeBlocker.startsWith(FAILED_TASK_MERGE_BLOCKER_PREFIX)) {
|
||||||
const error = mergeBlocker.slice(FAILED_TASK_MERGE_BLOCKER_PREFIX.length).trim();
|
const error = mergeBlocker.slice(FAILED_TASK_MERGE_BLOCKER_PREFIX.length).trim();
|
||||||
|
|||||||
@@ -1,6 +1,5 @@
|
|||||||
{
|
{
|
||||||
"counts": {
|
"counts": {
|
||||||
"packages/core/src/in-review-stall.ts": 1,
|
|
||||||
"packages/core/src/task-merge.ts": 1,
|
"packages/core/src/task-merge.ts": 1,
|
||||||
"packages/core/src/task-store/moves.ts": 2,
|
"packages/core/src/task-store/moves.ts": 2,
|
||||||
"packages/engine/src/auto-merge-finalization.ts": 1,
|
"packages/engine/src/auto-merge-finalization.ts": 1,
|
||||||
|
|||||||
Reference in New Issue
Block a user