From 9e242ea29485cd0df3d15937cd366a9ca67d48d1 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 03:51:43 -0700 Subject: [PATCH] fix(engine): backlog pressure called every dependency unfinished on a renamed board (#3081) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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) --- .../backlog-pressure-reporter.test.ts | 81 ++++++++++++++++++- .../engine/src/backlog-pressure-reporter.ts | 32 +++++++- .../lib/lifecycle-column-census-baseline.json | 1 - 3 files changed, 106 insertions(+), 8 deletions(-) diff --git a/packages/engine/src/__tests__/backlog-pressure-reporter.test.ts b/packages/engine/src/__tests__/backlog-pressure-reporter.test.ts index 79c6e59cce..efe0a5d732 100644 --- a/packages/engine/src/__tests__/backlog-pressure-reporter.test.ts +++ b/packages/engine/src/__tests__/backlog-pressure-reporter.test.ts @@ -30,6 +30,24 @@ function createTask(overrides: Partial = {}): 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; todoSlim?: Task[]; @@ -38,11 +56,17 @@ function createStore(params: { allTasks?: Task[]; insightStore?: { upsertInsight: ReturnType; listInsights: ReturnType }; 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"); + }); }); diff --git a/packages/engine/src/backlog-pressure-reporter.ts b/packages/engine/src/backlog-pressure-reporter.ts index 78a7fcff64..c9978abdd8 100644 --- a/packages/engine/src/backlog-pressure-reporter.ts +++ b/packages/engine/src/backlog-pressure-reporter.ts @@ -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, slim: boolean): Promise => { const byId = new Map(); @@ -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): 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, finishedColumns: ReadonlySet): 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; } } diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index bc4571ba48..e49c15d314 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -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": {