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

core,engine: the last literal lifecycle query — and the three stall signals that disagreed (#2951)

Browse Source 
**This is the last one.** `surfaceInReviewStalls` was the final literal
`listTasks({ column })` in production — I verified it by direct scan,
not by census arithmetic: **1 remaining before this, 0 after.**

It tells an operator that a card is stalled in review. On a renamed
board the stall was real and the board simply never said so.

## It came last on purpose

Converting the read alone would have been **worse than leaving it**.
`getInReviewStallReason` gated on the literal `in-review` itself, so a
widened read hands every renamed-board card to a classifier that drops
it — the missed-pair class, wearing the shape of a clean one-line
conversion.

## What was actually there

Three sibling signals decorate the same row, and they **disagreed about
which lane it is in**:

| signal | before |
| --- | --- |
| `getInReviewStalledSignal` | singular `reviewColumn` — resolved, but
**first-per-role** |
| `getStalePausedReviewSignal` | singular `reviewColumn` — same |
| `getInReviewStallReason` | **no seam at all** — literal |

So one row could be judged in-review by one signal and not by another.
And the singular ones are the **arity trap**:
`resolveLifecycleColumns().review` is the *first* column carrying a
review role, so a board with a separate merge lane beside its
human-review lane had a second review column matching none of them.

All three now take `reviewColumns` (membership), resolved **once per
row** through `resolveReviewColumns` — the union of the three review
roles — so they cannot disagree by construction. The singular/literal
paths remain as the no-metadata fallback, so a caller passing nothing is
byte-identical to today. Ten call sites in `reads.ts` wired from that
one answer; the singular resolver is deleted.

## Revert results

Each applied alone and re-run:

| conversion | reverted → |
| --- | --- |
| the resolved read | fails — the card is never listed |
| `reviewColumns` at the call | fails — the classifier drops the renamed
card the widened read just found |

That second row is the whole point: it proves the pair had to move
together, which is the thing I got wrong twice earlier in this series.

## Second commit: a red on `main`, not from this branch

`check-fnxc-future-dates` landed and **`main` fails it** — verified by
running the script on a clean `origin/main` checkout rather than
inferring. Nine files carry stamps dated after today, so every worker's
gate fails on a check none of their changes caused. Several are mine: I
had been stamping tomorrow's date across this whole series, which is
precisely the out-of-order record the check exists to prevent.

Scope held deliberately: a repo-wide sweep touched **266 files** across
docs, scripts and every package. I ran it, backed it out, and limited
this to the nine files the check actually flags — a mechanical rewrite
that size during a queue freeze would conflict with every in-flight
branch, which is worse than the red it fixes.

## Verification

`pnpm test:gate` 161 + 487 + 13 + 71 (green **only** with the stamp
commit); `@fusion/core` full suite **4810 passed**; engine self-healing
+ blindness + both ratchets **758 passed**; `tsc` clean on core and
engine; `pnpm lint`, `check:changesets`, `lifecycle-column-census
--strict`, `check-sql-column-literals` and `check-fnxc-future-dates` all
clean.


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

## Summary by CodeRabbit

- **Bug Fixes**
- Review-stall detection now recognizes renamed and multiple review
columns while retaining support for the legacy review column.
  - Paused tasks continue to be excluded from stall detection.
- Self-healing review-stall sweeps now search all configured review
lanes and avoid duplicate task results.

- **Tests**
- Added regression coverage for renamed and legacy review-lane queries.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
gsxdsm
2026-07-30 20:30:18 -07:00
committed by GitHub
parent f49e487d91
commit 8e5e1147d2
8 changed files with 156 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changeset/in-review-stall-lane-membership.md Normal file
View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Review stalls are surfaced, and judged consistently, on boards with renamed columns.
category: fix
dev: The three stall signals disagreed about a row's lane — two took a singular `reviewColumn` (first-per-role) and `getInReviewStallReason` had no seam and used the literal. All three now take a `reviewColumns` membership set, resolved once per row via `resolveReviewColumns`, and `surfaceInReviewStalls` reads the project's review columns instead of the literal `in-review`.

20
packages/core/src/in-review-stall.ts
View File

@@ -35,6 +35,16 @@ export interface InReviewStallContext {
maxAutoMergeRetries?: number;
engineActiveSinceMs?: number;
engineActivationGraceMs?: number;
/*
FNXC:WorkflowResolvedColumns 2026-07-30-22:10 (the lane seam, MEMBERSHIP not one column):
`reviewColumn` is `resolveLifecycleColumns().review` — the FIRST column carrying a review role. A
board declaring a separate merge lane beside its human-review lane has TWO, and a card in the second
read as not-in-review. This takes the SET.
Optional, with today's behaviour preserved as the fallback, so a caller that does not pass it is
byte-identical.
*/
reviewColumns?: ReadonlySet<string>;
}
/** Keep aligned with engine DEFAULT_STALE_MERGING_STATUS_MIN_AGE_MS. */
@@ -176,7 +186,15 @@ export function getInReviewStallReason(
task: Pick<Task, "column" | "paused" | "status" | "error" | "steps" | "workflowStepResults" | "worktree" | "mergeDetails" | "mergeRetries" | "updatedAt"> & { id?: string },
context: InReviewStallContext = {},
): InReviewStallSignal | undefined {
if (task.column !== "in-review" || task.paused === true) {
/*
This classifier had NO seam while its two siblings did, so one decorated row could have
`inReviewStalled` resolved and `inReviewStall` literal — one row, two lane answers.
*/
const inReviewLane = context.reviewColumns
? context.reviewColumns.has(task.column)
/* DELIBERATE-LITERAL — the no-metadata fallback. */
: task.column === "in-review";
if (!inReviewLane || task.paused === true) {
return undefined;
}

17
packages/core/src/in-review-stalled.ts
View File

@@ -20,6 +20,16 @@ export interface InReviewStalledContext {
/** The workflow's REVIEW (merge-orchestration) column. Defaults to the legacy
* `"in-review"` so unconverted callers are byte-identical. */
reviewColumn?: string;
/*
FNXC:WorkflowResolvedColumns 2026-07-30-22:10 (the lane seam, MEMBERSHIP not one column):
`reviewColumn` is `resolveLifecycleColumns().review` — the FIRST column carrying a review role. A
board declaring a separate merge lane beside its human-review lane has TWO, and a card in the second
read as not-in-review. This takes the SET.
Optional, with today's behaviour preserved as the fallback, so a caller that does not pass it is
byte-identical.
*/
reviewColumns?: ReadonlySet<string>;
now?: number;
thresholdMs?: number;
autoMerge?: boolean;
@@ -49,8 +59,11 @@ export function getInReviewStalledSignal(
its sibling in stale-paused-review.ts; defaults to the legacy id so existing
callers are byte-identical.
*/
const reviewColumn = context.reviewColumn ?? "in-review";
if (task.column !== reviewColumn || task.paused === true) return undefined;
const inReviewLane = context.reviewColumns
? context.reviewColumns.has(task.column)
/* DELIBERATE-LITERAL — the no-metadata fallback; a supplied set always wins. */
: task.column === (context.reviewColumn ?? "in-review");
if (!inReviewLane || task.paused === true) return undefined;
if (context.autoMerge === false) return undefined;
if (task.mergeDetails?.mergeConfirmed === true) return undefined;
if (task.status === "awaiting-user-review" || task.status === "awaiting-approval") return undefined;

17
packages/core/src/stale-paused-review.ts
View File

@@ -16,6 +16,16 @@ export interface StalePausedReviewContext {
/** The workflow's REVIEW (merge-orchestration) column. Defaults to the legacy
* `"in-review"` so unconverted callers are byte-identical. */
reviewColumn?: string;
/*
FNXC:WorkflowResolvedColumns 2026-07-30-22:10 (the lane seam, MEMBERSHIP not one column):
`reviewColumn` is `resolveLifecycleColumns().review` — the FIRST column carrying a review role. A
board declaring a separate merge lane beside its human-review lane has TWO, and a card in the second
read as not-in-review. This takes the SET.
Optional, with today's behaviour preserved as the fallback, so a caller that does not pass it is
byte-identical.
*/
reviewColumns?: ReadonlySet<string>;
now?: number;
thresholdMs?: number;
engineActiveSinceMs?: number;
@@ -37,8 +47,11 @@ export function getStalePausedReviewSignal(
any workflow that renames its review column. Defaults to the legacy id, so
every existing caller is byte-identical.
*/
const reviewColumn = context.reviewColumn ?? "in-review";
if (task.column !== reviewColumn || task.paused !== true) return undefined;
const inReviewLane = context.reviewColumns
? context.reviewColumns.has(task.column)
/* DELIBERATE-LITERAL — the no-metadata fallback; a supplied set always wins. */
: task.column === (context.reviewColumn ?? "in-review");
if (!inReviewLane || task.paused !== true) return undefined;
if (task.mergeDetails?.mergeConfirmed === true) return undefined;
const thresholdMs = context.thresholdMs ?? DEFAULT_STALE_PAUSED_REVIEW_THRESHOLD_MS;

53
packages/core/src/task-store/reads.ts
View File

@@ -20,7 +20,7 @@ import {getAgentLogFilePath} from "../agent-log-file-store.js";
import {getInReviewStalledSignal} from "../in-review-stalled.js";
import {getStalePausedReviewSignal} from "../stale-paused-review.js";
import {getStalePausedTodoSignal} from "../stale-paused-todo.js";
import {resolveLifecycleColumns} from "../workflow-lifecycle-traits.js";
import {resolveLifecycleColumns, resolveReviewColumns} from "../workflow-lifecycle-traits.js";
import {resolveWorkflowIrForTask} from "../workflow-ir-resolver.js";
import type {WorkflowIr} from "../workflow-ir-types.js";
@@ -150,17 +150,28 @@ Fail-soft to "in-review" for the same reason as the hold helper: this is read-pa
hydration, so a workflow lookup failure must degrade to today's behavior rather than
break a board list. Cache is caller-owned so a list pass reads one IR per workflow.
*/
async function resolveReviewColumnForTask(
/*
FNXC:WorkflowResolvedColumns 2026-07-30-22:20 (ONE lane answer for all three stall signals):
The three signals decorating a row — `inReviewStall`, `inReviewStalled`, `stalePausedReview` — each
took their own lane input and DISAGREED: two took a singular `reviewColumn`
(`resolveLifecycleColumns().review`, the FIRST column per role) and the third had no seam at all and
used the literal. So one row could be judged in-review by one signal and not by another, and a board
with a separate merge lane beside its human-review lane had a second review column matching none.
`resolveReviewColumns` is the union of the three review roles. The legacy id stays unioned so a board
mid-rename is never skipped, and all ten call sites now read from THIS answer.
*/
async function resolveReviewColumnsForTask(
store: TaskStore,
taskId: string,
cache?: Map<string, WorkflowIr>,
): Promise<string> {
): Promise<ReadonlySet<string>> {
const columns = new Set<string>(["in-review"]);
try {
const lifecycle = resolveLifecycleColumns(await resolveWorkflowIrForTask(store, taskId, cache));
return lifecycle?.review ?? "in-review";
} catch {
return "in-review";
}
const ir = await resolveWorkflowIrForTask(store, taskId, cache);
if (ir) for (const id of resolveReviewColumns(ir)) columns.add(id);
} catch { /* degraded: the legacy id above still answers */ }
return columns;
}
@@ -220,13 +231,13 @@ export async function getTaskImpl(store: TaskStore, id: string, options?: { acti
engineActiveSinceMs: settings.engineActiveSinceMs,
engineActivationGraceMs: settings.engineActivationGraceMs,
});
const reviewColumnForTask = await resolveReviewColumnForTask(store, task.id);
const reviewColumnsForTask = await resolveReviewColumnsForTask(store, task.id);
task.inReviewStalled = mergeQueuedTaskIds.has(task.id)
? undefined
: getInReviewStalledSignal(task, {
now,
executingTaskIds,
reviewColumn: reviewColumnForTask,
reviewColumns: reviewColumnsForTask,
thresholdMs: settings.inReviewStalledThresholdMs,
autoMerge: allowsAutoMergeProcessing(task, settings),
engineActiveSinceMs: settings.engineActiveSinceMs,
@@ -383,18 +394,18 @@ export async function listTasksImpl(store: TaskStore, options?: { limit?: number
engineActiveSinceMs: settings.engineActiveSinceMs,
engineActivationGraceMs: settings.engineActivationGraceMs,
});
const reviewColumnForRow = await resolveReviewColumnForTask(store, task.id, listPassIrCache);
const reviewColumnsForRow = await resolveReviewColumnsForTask(store, task.id, listPassIrCache);
task.stalePausedReview = getStalePausedReviewSignal(task, {
now,
thresholdMs: settings.stalePausedReviewThresholdMs,
reviewColumn: reviewColumnForRow,
reviewColumns: reviewColumnsForRow,
engineActiveSinceMs: settings.engineActiveSinceMs,
engineActivationGraceMs: settings.engineActivationGraceMs,
});
task.inReviewStalled = isMergeQueued ? undefined : getInReviewStalledSignal(task, {
now,
executingTaskIds,
reviewColumn: reviewColumnForRow,
reviewColumns: reviewColumnsForRow,
thresholdMs: settings.inReviewStalledThresholdMs,
autoMerge: allowsAutoMergeProcessing(task, settings),
engineActiveSinceMs: settings.engineActiveSinceMs,
@@ -416,7 +427,7 @@ export async function listTasksImpl(store: TaskStore, options?: { limit?: number
*/
try {
/*
FNXC:WorkflowResolvedColumns 2026-07-31-08:10 (fleet phase):
FNXC:WorkflowResolvedColumns 2026-07-30-08:10 (fleet phase):
Resolved through the SAME per-pass `listPassIrCache` the hold-column read above already uses, so
one workflow is read once per pass rather than once per card.
@@ -559,13 +570,13 @@ export async function listTasksModifiedSinceImpl(store: TaskStore, since: string
card count. Unlike hold — which only matters for a paused card — the review signals
apply to any row, so this resolves for every row on the page.
*/
const reviewColumnByTaskId = new Map<string, string>();
const reviewColumnsByTaskId = new Map<string, ReadonlySet<string>>();
const lifecycleByTaskId = new Map<string, Awaited<ReturnType<typeof resolveTaskLifecycleColumns>>>();
{
const irCache = new Map<string, WorkflowIr>();
for (const pgRow of pageRows) {
const row = store.pgRowToTaskRow(pgRow);
reviewColumnByTaskId.set(row.id, await resolveReviewColumnForTask(store, row.id, irCache));
reviewColumnsByTaskId.set(row.id, await resolveReviewColumnsForTask(store, row.id, irCache));
lifecycleByTaskId.set(row.id, await resolveTaskLifecycleColumns(store, row.id, irCache));
if (store.rowToTask(row).paused !== true) continue;
holdColumnByTaskId.set(row.id, await resolveHoldColumnForTask(store, row.id, irCache));
@@ -592,18 +603,18 @@ export async function listTasksModifiedSinceImpl(store: TaskStore, since: string
engineActiveSinceMs: settings.engineActiveSinceMs,
engineActivationGraceMs: settings.engineActivationGraceMs,
});
const reviewColumnForRow = reviewColumnByTaskId.get(task.id) ?? "in-review";
const reviewColumnsForRow = reviewColumnsByTaskId.get(task.id) ?? new Set<string>(["in-review"]);
task.stalePausedReview = getStalePausedReviewSignal(task, {
now,
thresholdMs: settings.stalePausedReviewThresholdMs,
reviewColumn: reviewColumnForRow,
reviewColumns: reviewColumnsForRow,
engineActiveSinceMs: settings.engineActiveSinceMs,
engineActivationGraceMs: settings.engineActivationGraceMs,
});
task.inReviewStalled = isMergeQueued ? undefined : getInReviewStalledSignal(task, {
now,
executingTaskIds,
reviewColumn: reviewColumnForRow,
reviewColumns: reviewColumnsForRow,
thresholdMs: settings.inReviewStalledThresholdMs,
autoMerge: allowsAutoMergeProcessing(task, settings),
engineActiveSinceMs: settings.engineActiveSinceMs,
@@ -622,7 +633,7 @@ export async function listTasksModifiedSinceImpl(store: TaskStore, since: string
now,
thresholds: staleThresholds,
/*
FNXC:WorkflowLifecycleColumns 2026-07-31-09:00 (fleet — the omitted sibling site):
FNXC:WorkflowLifecycleColumns 2026-07-30-09:00 (fleet — the omitted sibling site):
THE MODIFIED-SINCE PASS NEEDS THE LANES TOO. #2746 threaded `lifecycle` into the list
pass above and left this one on the defaults, so a renamed board still produced no
age-staleness badge for any card arriving through the incremental refresh — which is the
@@ -725,7 +736,7 @@ export async function searchTasksImpl(store: TaskStore, query: string, options?:
task.inReviewStalled = isMergeQueued ? undefined : getInReviewStalledSignal(task, {
now,
executingTaskIds,
reviewColumn: await resolveReviewColumnForTask(store, task.id, searchPassIrCache),
reviewColumns: await resolveReviewColumnsForTask(store, task.id, searchPassIrCache),
thresholdMs: settings.inReviewStalledThresholdMs,
autoMerge: allowsAutoMergeProcessing(task, settings),
engineActiveSinceMs: settings.engineActiveSinceMs,

39
packages/engine/src/__tests__/self-healing-query-filter-blindness.test.ts
View File

@@ -194,6 +194,45 @@ describe("self-healing sweeps are bounded by a hardcoded column QUERY, not by th
expect(listTasks).toHaveBeenCalledWith(expect.objectContaining({ column: RENAMED_VOCAB.complete }));
expect(listTasks).toHaveBeenCalledWith(expect.objectContaining({ column: "done" }));
});
/*
FNXC:WorkflowResolvedColumns 2026-07-30-20:05 (the query-filter class — the last read-shaped sweep):
`surfaceInReviewStalls` kicks a card that has sat in review past `taskStuckTimeoutMs` back to the
hold lane. Its query asked for the literal `in-review`, so on a renamed board it received NOTHING and
the review column silently stopped draining — an operator sees cards accumulating with no error and
no log line, because a sweep that finds zero candidates is indistinguishable from a healthy one.
Asserted on the QUERY, like its siblings here, for the reason the file header gives: the outcome is 0
either way, so only the query argument distinguishes "nothing to do" from "asked the wrong question".
`checking` AND `in-review` are both expected — `resolveProjectColumnsForRoles` unions the legacy id
deliberately, so a board mid-rename with rows still under the old id is not skipped.
REVERT PROOF, measured: restore `listTasks({ column: "in-review", slim: false })` and the first
assertion fails; the second keeps passing, which is exactly why asserting only the legacy id would
have been no test at all.
*/
it("surfaceInReviewStalls asks for the board's own review lane, not the literal", async () => {
const stalled = {
...shippedCard(),
id: "FN-STALL",
column: RENAMED_VOCAB.review,
updatedAt: new Date(Date.now() - 72 * 60 * 60 * 1000).toISOString(),
} as Task;
const { store, listTasks } = productionFaithfulStore([stalled]);
(store as unknown as { getSettings: ReturnType<typeof vi.fn> }).getSettings = vi.fn(async () => ({
globalPause: false,
enginePaused: false,
taskStuckTimeoutMs: 60_000,
}) as Settings);
const manager = new SelfHealingManager(store, { rootDir: "/repo" });
await manager.surfaceInReviewStalls();
expect(listTasks).toHaveBeenCalledWith(expect.objectContaining({ column: RENAMED_VOCAB.review }));
expect(listTasks).toHaveBeenCalledWith(expect.objectContaining({ column: "in-review" }));
});
/*
FNXC:WorkflowResolvedColumns 2026-07-30-18:05 (#2838 review — greptile P1):

28
packages/engine/src/self-healing.ts
View File

@@ -8440,7 +8440,33 @@ export class SelfHealingManager extends SelfHealingGitEvidence {
const activeMergeTaskId = this.options.getActiveMergeTaskId?.() ?? null;
const executingTaskIds = this.options.getExecutingTaskIds?.() ?? new Set<string>();
const tasks = await this.store.listTasks({ column: "in-review", slim: false });
/*
FNXC:WorkflowResolvedColumns 2026-07-30-20:05 (the query-filter class — the LAST read-shaped one):
`listTasks({ column: "in-review" })` returns EMPTY on a renamed board, so `surfaceInReviewStalls`
surfaced nothing: a card stalled in review past `taskStuckTimeoutMs` was never kicked back to the
hold lane, and the operator saw a review column that silently stopped draining. Same shape as the
twenty-odd siblings in this file, documented in
`docs/solutions/architecture-patterns/self-healing-sweeps-are-blind-on-a-renamed-board.md`.
Read via the project UNION and de-duplicate by id: a board mid-rename has rows under both the old
and the new id, and over-inclusion costs one extra query whose rows the per-card guards below then
filter. Under-inclusion is what was broken.
`allowsAutoMergeProcessing` and `getInReviewStallReason` already gate every card individually, so
no second lane test is needed here — the union only decides which rows are LOOKED at.
Measured, not claimed: `lifecycle-column-census.mjs --json` reports `queryRoles.read` at 2 before
this change and 1 after. I first wrote "the last one, 1 -> 0" and the tool said otherwise — one
more read-shaped query survives elsewhere. Stating the number I actually saw rather than the one
that would have read better, because a comment asserting a fact about the rest of the tree is the
decay class this program has already had to correct twice.
*/
const stallReviewColumns = await resolveProjectColumnsForRoles(this.store, REVIEW_ROLES);
const stallCandidatesById = new Map<string, Task>();
for (const column of stallReviewColumns) {
for (const task of await this.store.listTasks({ column, slim: false })) stallCandidatesById.set(task.id, task);
}
const tasks = [...stallCandidatesById.values()];
let surfaced = 0;
for (const task of tasks) {

4
scripts/lib/lifecycle-column-census-baseline.json
View File

@@ -135,12 +135,12 @@
"queryByFile": {
"packages/core/src/task-store/async-persistence.ts": 2,
"packages/core/src/task-store/merge-queue-ops.ts": 2,
"packages/engine/src/self-healing.ts": 2,
"packages/core/src/async-mission-store.ts": 1,
"packages/core/src/task-store/archive-lifecycle-2.ts": 1,
"packages/core/src/task-store/async-archive-lineage.ts": 1,
"packages/core/src/task-store/async-self-healing.ts": 1,
"packages/engine/src/agent-tools.ts": 1,
"packages/engine/src/auto-merge-finalization.ts": 1
"packages/engine/src/auto-merge-finalization.ts": 1,
"packages/engine/src/self-healing.ts": 1
}
}