fix(engine): backlog pressure called every dependency unfinished on a renamed board (#3081)
## The third lane question This reporter had **three** lane questions. Two were resolved when the file's query-blindness was fixed — hold and wip, both through `resolveProjectColumnsForRoles`. The third sat one method down and was never touched: ```ts if (dependency.column !== "done") return false; ``` One board, two lane answers. ## What it cost On a renamed board every dependency reads unfinished, so `isRunnableCandidate` rejects every card that has one. The backlog-pressure alert then names **only dependency-free cards** as the runnable ones. The failure mode is the quiet kind: the report still renders, the counts are right, and the candidate list looks plausible. The operator is told the queue is blocked on nothing in particular. No default-board test can see it — which is exactly why the earlier conversion of this same file, which fixed its reads, left this behind. ## Fix `finishedColumns` (complete ∪ archived) resolved once by the async caller alongside hold and wip, then passed into the sync predicate. - **Required parameter, not optional-with-a-literal-default.** An optional parameter leaves `done` in the file as a silent fallback and the next caller gets pre-conversion behaviour by writing nothing. - **Archived is included** because a dependency that has been archived is finished too — and this reporter already reads with `includeArchived: true` precisely so archived blockers resolve. - **Async resolution.** `resolveProjectColumnsForRoles`' only store read is `listWorkflowDefinitions()`, a project-wide async read that works under PostgreSQL. That is the line between a real conversion and the inert sync-IR kind (#3058), and the new test supplies its board through that same reader so it exercises the production path. ## Census | | before | after | |---|---|---| | `backlog-pressure-reporter.ts` | 1 | **0** | ## Measured - One new case; file **11/11 pass**. - **MUTATION**: restoring `dependency.column !== "done"` fails it. - The case asserts **both directions in one test** — a dependency resting in the board's own complete lane makes its card runnable, *and* a dependency still in the hold lane still blocks it. Asserting only the first would pass against a predicate that had simply stopped checking dependencies. - The file already had a `RENAMED_IR` scoped to its second describe; mine is a distinct `RENAMED_DEPENDENCY_IR` with different lane names. I hit the shadowing first and the test failed as `under-threshold` — worth noting because a same-named fixture that silently resolves to the *other* board is precisely how a renamed-lane test goes vacuous. - `tsc --noEmit -p packages/engine` clean; census `--strict`, `check-lane-wiring`, `check-inert-sync-lane-conversions`, `check-fnxc-future-dates` clean. ## Flagged, not guessed Adjacent census entries I looked at and deliberately left: - **`executor.ts` (4)** — all inside a sync `task:moved` listener. Converting via `resolveTaskWorkflowIrSync` would be inert for #3058's reason, and making the listener async reorders it against every other subscriber. Correctly out of scope, as #3048 judged. - **`triage.ts:724`** — half-converted in the same shape: `disposeLanes.hold`/`.intake` come from a sync resolver, so the resolved arms are themselves inert and "finishing" the guard would add a third inert comparison. - **`auto-merge-finalization.ts` (2)** — one is the resolver's documented degraded fallback (the live arm calls `columnHasFlag`), the other is already recorded as a deferred signature-widening whose cost exceeds the error string it sharpens. - **`in-review-stall.ts:196`** — an explicitly marked DELIBERATE-LITERAL no-metadata fallback. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -30,6 +30,24 @@ function createTask(overrides: Partial<Task> = {}): Task {
|
||||
} as Task;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-19:20:
|
||||
A board whose hold lane is `drafting`, wip is `building` and complete is `shipped`. Supplied through
|
||||
`listWorkflowDefinitions`, which is the ONLY store read `resolveProjectColumnsForRoles` makes — a
|
||||
project-wide async read that works under PostgreSQL, unlike the sync workflow-SELECTION reader.
|
||||
*/
|
||||
const RENAMED_DEPENDENCY_IR = {
|
||||
version: "v2",
|
||||
id: "custom:deps",
|
||||
nodes: [],
|
||||
edges: [],
|
||||
columns: [
|
||||
{ id: "drafting", name: "drafting", traits: [{ trait: "hold", config: { release: "capacity" } }] },
|
||||
{ id: "brewing", name: "brewing", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
|
||||
{ id: "shipped", name: "shipped", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
};
|
||||
|
||||
function createStore(params: {
|
||||
settings?: Record<string, unknown>;
|
||||
todoSlim?: Task[];
|
||||
@@ -38,11 +56,17 @@ function createStore(params: {
|
||||
allTasks?: Task[];
|
||||
insightStore?: { upsertInsight: ReturnType<typeof vi.fn>; listInsights: ReturnType<typeof vi.fn> };
|
||||
throwInsightStore?: boolean;
|
||||
/** Omitted → the helper keeps the legacy ids, which is every pre-existing case in this file. */
|
||||
workflowIr?: unknown;
|
||||
holdColumn?: string;
|
||||
wipColumn?: string;
|
||||
}): TaskStore {
|
||||
const holdColumn = params.holdColumn ?? "todo";
|
||||
const wipColumn = params.wipColumn ?? "in-progress";
|
||||
const listTasks = vi.fn().mockImplementation(async (options?: { column?: string; slim?: boolean }) => {
|
||||
if (options?.column === "todo" && options?.slim) return params.todoSlim ?? [];
|
||||
if (options?.column === "in-progress" && options?.slim) return params.inProgressSlim ?? [];
|
||||
if (options?.column === "todo" && !options?.slim) return params.todoFull ?? [];
|
||||
if (options?.column === holdColumn && options?.slim) return params.todoSlim ?? [];
|
||||
if (options?.column === wipColumn && options?.slim) return params.inProgressSlim ?? [];
|
||||
if (options?.column === holdColumn && !options?.slim) return params.todoFull ?? [];
|
||||
if (!options?.column && options?.slim) return params.allTasks ?? [];
|
||||
return [];
|
||||
});
|
||||
@@ -50,6 +74,7 @@ function createStore(params: {
|
||||
return {
|
||||
getSettings: vi.fn().mockResolvedValue(params.settings ?? {}),
|
||||
listTasks,
|
||||
...(params.workflowIr ? { listWorkflowDefinitions: vi.fn(async () => [{ ir: params.workflowIr }]) } : {}),
|
||||
getInsightStore: vi.fn().mockImplementation(() => {
|
||||
if (params.throwInsightStore) throw new Error("missing insight store");
|
||||
return params.insightStore;
|
||||
@@ -284,4 +309,54 @@ describe("backlog pressure resolves the board's own lanes", () => {
|
||||
|
||||
expect((await reporter.report()).alerted).toBe(false);
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-19:20:
|
||||
THE THIRD LANE QUESTION. Hold and wip were already resolved in this reporter; "is this card's
|
||||
dependency finished?" was still `dependency.column !== "done"`.
|
||||
|
||||
On a renamed board that makes EVERY dependency look unfinished, so `isRunnableCandidate` rejects
|
||||
every card that has one and the alert names only dependency-free cards as runnable. The report
|
||||
still renders, still looks plausible, and tells the operator the queue is blocked on nothing in
|
||||
particular — which is why no default-board test could see it.
|
||||
|
||||
Both directions are asserted in one case: a card whose dependency rests in the board's own complete
|
||||
lane must be RUNNABLE, and a card whose dependency is still in the hold lane must not be. Asserting
|
||||
only the first would pass against a predicate that had simply stopped checking dependencies.
|
||||
*/
|
||||
it("treats a dependency resting in the board's RENAMED complete lane as satisfied", async () => {
|
||||
const todoSlim = Array.from({ length: 44 }, (_, i) => createTask({ id: `FN-T${i}`, column: "drafting" }));
|
||||
const inProgressSlim = [1, 2, 3].map((n) => createTask({ id: `FN-P${n}`, column: "brewing" }));
|
||||
const todoFull = [
|
||||
createTask({ id: "FN-SATISFIED", column: "drafting", priority: "urgent", dependencies: ["FN-DEP-SHIPPED"] }),
|
||||
createTask({ id: "FN-BLOCKED", column: "drafting", priority: "urgent", dependencies: ["FN-DEP-DRAFTING"] }),
|
||||
createTask({ id: "FN-FREE-1", column: "drafting", priority: "high" }),
|
||||
createTask({ id: "FN-FREE-2", column: "drafting", priority: "high" }),
|
||||
createTask({ id: "FN-FREE-3", column: "drafting", priority: "normal" }),
|
||||
];
|
||||
const allTasks = [
|
||||
...todoFull,
|
||||
createTask({ id: "FN-DEP-SHIPPED", column: "shipped" }),
|
||||
createTask({ id: "FN-DEP-DRAFTING", column: "drafting" }),
|
||||
];
|
||||
const insightStore = { upsertInsight: vi.fn(), listInsights: vi.fn().mockResolvedValue([]) };
|
||||
const reporter = new BacklogPressureReporter({
|
||||
store: createStore({
|
||||
todoSlim, inProgressSlim, todoFull, allTasks, insightStore,
|
||||
workflowIr: RENAMED_DEPENDENCY_IR, holdColumn: "drafting", wipColumn: "brewing",
|
||||
}),
|
||||
projectId: "/tmp/project",
|
||||
logger: { warn: vi.fn(), error: vi.fn() },
|
||||
now: () => Date.parse("2026-05-18T12:00:00.000Z"),
|
||||
});
|
||||
|
||||
const result = await reporter.report();
|
||||
expect(result.alerted).toBe(true);
|
||||
const content = JSON.parse(insightStore.upsertInsight.mock.calls[0][1].content);
|
||||
const ids = content.candidates.map((candidate: { id: string }) => candidate.id);
|
||||
|
||||
expect(ids).toContain("FN-SATISFIED");
|
||||
/* The paired negative: a dependency still waiting must still block. */
|
||||
expect(ids).not.toContain("FN-BLOCKED");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -69,9 +69,24 @@ export class BacklogPressureReporter {
|
||||
read needs (there is no task in hand yet to resolve from) and always unions the legacy id, so a
|
||||
board mid-rename still counts rows stored under the old one.
|
||||
*/
|
||||
const [holdColumns, wipColumns] = await Promise.all([
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-19:10:
|
||||
THE THIRD LANE QUESTION IN THIS REPORTER WAS STILL LITERAL. Hold and wip were resolved here;
|
||||
"is this card's dependency finished?" was asked one method down with `dependency.column !== "done"`.
|
||||
Two lane answers about the same board, one resolved and one not.
|
||||
|
||||
Consequence is a report that reads plausible and is wrong: on a renamed board EVERY dependency
|
||||
looks unfinished, so `isRunnableCandidate` rejects every card that has one and the backlog-pressure
|
||||
alert names only dependency-free cards as the runnable ones. The operator is told the queue is
|
||||
blocked on nothing in particular.
|
||||
|
||||
MEMBERSHIP over complete ∪ archived, because a dependency that has been archived is finished too —
|
||||
this reporter reads with `includeArchived: true` precisely so archived blockers resolve.
|
||||
*/
|
||||
const [holdColumns, wipColumns, dependencyFinishedColumns] = await Promise.all([
|
||||
resolveProjectColumnsForRoles(this.store, ["hold"]),
|
||||
resolveProjectColumnsForRoles(this.store, ["countsTowardWip"]),
|
||||
resolveProjectColumnsForRoles(this.store, ["complete", "archived"]),
|
||||
]);
|
||||
const listByColumns = async (columns: ReadonlySet<string>, slim: boolean): Promise<Task[]> => {
|
||||
const byId = new Map<string, Task>();
|
||||
@@ -98,7 +113,7 @@ export class BacklogPressureReporter {
|
||||
]);
|
||||
const byId = new Map(allTasks.map((task) => [task.id, task]));
|
||||
const candidates = todoFull
|
||||
.filter((task) => this.isRunnableCandidate(task, byId))
|
||||
.filter((task) => this.isRunnableCandidate(task, byId, dependencyFinishedColumns))
|
||||
.sort((a, b) => {
|
||||
const pa = PRIORITY_WEIGHT[a.priority ?? "normal"];
|
||||
const pb = PRIORITY_WEIGHT[b.priority ?? "normal"];
|
||||
@@ -185,7 +200,16 @@ export class BacklogPressureReporter {
|
||||
}
|
||||
}
|
||||
|
||||
private isRunnableCandidate(task: Task, byId: Map<string, Task>): boolean {
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-19:10:
|
||||
`finishedColumns` is REQUIRED and resolved by the caller — this predicate is sync (it runs inside
|
||||
`Array.filter`, where an `await` would make every element pass) and its caller is async.
|
||||
|
||||
Required rather than optional-with-a-literal-default: an optional parameter leaves `done` in the
|
||||
file as a silent fallback, and the next caller gets the pre-conversion answer by writing nothing.
|
||||
`resolveProjectColumnsForRoles` seeds the legacy ids itself, so an unconverted board is unchanged.
|
||||
*/
|
||||
private isRunnableCandidate(task: Task, byId: Map<string, Task>, finishedColumns: ReadonlySet<string>): boolean {
|
||||
if (task.paused) return false;
|
||||
if ((task.blockedBy ?? "").trim().length > 0) return false;
|
||||
if ((task.overlapBlockedBy ?? "").trim().length > 0) return false;
|
||||
@@ -194,7 +218,7 @@ export class BacklogPressureReporter {
|
||||
for (const depId of task.dependencies ?? []) {
|
||||
const dependency = byId.get(depId);
|
||||
if (!dependency) continue;
|
||||
if (dependency.column !== "done") {
|
||||
if (!finishedColumns.has(dependency.column)) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,7 +19,6 @@
|
||||
"packages/core/src/task-store/task-id-integrity.ts": 1,
|
||||
"packages/dashboard/app/components/ResearchTaskActionModal.tsx": 1,
|
||||
"packages/dashboard/app/components/TaskCard.tsx": 1,
|
||||
"packages/engine/src/backlog-pressure-reporter.ts": 1,
|
||||
"packages/engine/src/triage.ts": 1
|
||||
},
|
||||
"deliberateByFile": {
|
||||
|
||||
Reference in New Issue
Block a user