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

test(dashboard): cover the three lane resolvers nobody was testing (#2826)

Browse Source 
## Why this exists

#2821's review found a bug that lived entirely in a lane **builder**
while every test drove the **guard** that consumed it. That is a
structural blind spot, not a one-off: injecting a resolved value into a
synchronous guard makes the guard testable and the resolver invisible.

So I audited every lane helper I added this session. Three had **no
direct coverage at all** — `archivedColumnsForTask`,
`wipColumnsForTask`, `preWipColumnsForTask`. Their callers were tested;
the functions were not.

## They shared the defect that review named

Each read `resolved.length > 0 ? resolved : legacyId`, which conflates
two different boards:

- a **v1 upgrade** — `synthesizeDefaultColumns` emits `traits: []` on
every column, so the legacy id is the only vocabulary that exists, and
falling back is correct;
- a **v2 board that expresses traits** and declares no lane of that role
— where the legacy id names a column the board may still *have* and
deliberately did not give the role. Falling back there widens the guard
onto a role the board explicitly withheld.

`declaresAnyLifecycleTrait` separates them, matching the shape #2821's
review established for `resolveNodeOverrideLanes`.

## The fixture trap, which is the part worth reading

**My first fixture could not see the bug.** It traited the role under
test — and where the role *is* traited, the two shapes agree: both
return the traited lane. Mutating a helper back to the old shape left
all 15 cases green.

The shapes diverge only when the resolved set is **empty while traits
are expressed**. Each helper now has that case explicitly, with a
fixture that traits something *other* than the role under test.

**Mutation-verified per helper:** all three reverted independently now
fail. Before the extra case, none did.

This is the second time this session a fixture built with the production
path normalised away the very thing under test. Worth stating as a rule:
a renamed-lane fixture proves the resolver reads traits; only a
*traits-expressed-but-role-absent* fixture proves what it does when the
answer is legitimately nothing.

## Verification

- `task-lifecycle-lanes.test.ts` → 18 passed (was 15, none covering
these three)
- consumer suites (`github-issue-comment`, `planning-board-tools`,
`register-git-github.review-lanes`) → 64 passed together
- `pnpm test:gate` → 161 + 487 + 13 + 71
- `--strict` → 0; `tsc --noEmit` and `pnpm lint` → 0 errors

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-30 13:18:08 -07:00
committed by GitHub
parent e5c9ea3870
commit d56c635c77
2 changed files with 118 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

98
packages/dashboard/src/__tests__/task-lifecycle-lanes.test.ts
View File

@@ -23,7 +23,7 @@ resolved set is EMPTY even though the columns exist) must BOTH keep the legacy p
*/
import { describe, expect, it, vi } from "vitest";
import "@fusion/core"; // registers the built-in column traits so flags resolve
import { landedColumnsForTask, completeColumnsForTask } from "../task-lifecycle-lanes.js";
import { landedColumnsForTask, completeColumnsForTask, archivedColumnsForTask, wipColumnsForTask, preWipColumnsForTask } from "../task-lifecycle-lanes.js";
function storeWith(ir: unknown, workflowId = "wf") {
const selection = { workflowId, stepIds: [] as string[] };
@@ -113,3 +113,99 @@ describe("completeColumnsForTask is narrower than the landed set", () => {
expect([...(await completeColumnsForTask(store, "FN-1"))]).toEqual(["done"]);
});
});
/*
FNXC:WorkflowResolvedColumns 2026-07-31-00:05 (self-audit after #2821's review):
THE RESOLVERS NOBODY WAS TESTING.
#2821's review found a bug that lived entirely in a lane BUILDER while every test drove the guard
that consumed it — injecting a value makes the guard testable and the resolver invisible. Auditing
the helpers I added this session for the same shape found three with no direct coverage at all:
`archivedColumnsForTask`, `wipColumnsForTask`, `preWipColumnsForTask`. Their callers are tested; the
functions themselves were not.
They also shared the defect that review named. Each read `resolved.length > 0 ? resolved : legacyId`,
which conflates a v1 upgrade (traits synthesised empty, so the legacy id is the only vocabulary) with
a v2 board that expresses traits and declares no lane of that role — where falling back onto the
legacy id widens the guard onto a role the board deliberately did not assign.
Each helper is now pinned on all four outcomes: the renamed lane, an untraited legacy NAME, the v1
fallback, and the unresolvable fallback.
*/
describe("the lane resolvers themselves, not just their callers", () => {
const storeFor = (ir: unknown) => {
const selection = { workflowId: "wf", stepIds: [] as string[] };
return {
getTaskWorkflowSelection: () => selection,
getTaskWorkflowSelectionAsync: async () => selection,
getWorkflowDefinition: async () => (ir === undefined ? undefined : { id: "wf", ir }),
} as never;
};
/* Traits ARE expressed here, and `done`/`archived`/`in-progress`/`todo` appear WITHOUT them. */
const TRAITED_WITH_LEGACY_NAMES = {
version: "v2", id: "wf", name: "wf", nodes: [], edges: [],
columns: [
{ id: "todo", name: "Not intake", traits: [] },
{ id: "in-progress", name: "Not wip", traits: [] },
{ id: "done", name: "Not complete", traits: [] },
{ id: "archived", name: "Not archived", traits: [] },
{ id: "backlog", name: "Backlog", traits: [{ trait: "hold" }] },
{ id: "building", name: "Building", traits: [{ trait: "wip" }] },
{ id: "attic", name: "Attic", traits: [{ trait: "archived" }] },
],
};
const V1 = {
version: "v2", id: "wf", name: "wf", nodes: [], edges: [],
columns: ["todo", "in-progress", "in-review", "done", "archived"].map((id) => ({ id, name: id, traits: [] })),
};
const cases: Array<[string, (s: never, id: string) => Promise<Set<string>>, string, string]> = [
["archivedColumnsForTask", archivedColumnsForTask, "attic", "archived"],
["wipColumnsForTask", wipColumnsForTask, "building", "in-progress"],
["preWipColumnsForTask", preWipColumnsForTask, "backlog", "todo"],
];
for (const [name, resolve, renamed, legacy] of cases) {
it(`${name} returns the traited lane and EXCLUDES the untraited legacy name`, async () => {
const lanes = await resolve(storeFor(TRAITED_WITH_LEGACY_NAMES), "FN-1");
expect(lanes.has(renamed)).toBe(true);
/* The board declares this column and deliberately did not give it the role. */
expect(lanes.has(legacy)).toBe(false);
});
it(`${name} returns EMPTY when traits are expressed but no column carries this role`, async () => {
/*
The case that actually separates the two shapes, and the reason the assertion above could not.
Where the role IS traited, `resolved.length > 0 ? resolved : legacy` and "trust the resolved
set" agree — both return the traited lane. They diverge only when the resolved set is EMPTY
while traits ARE expressed: the old shape falls back onto the legacy NAME, which this board
declares and deliberately did not give the role; the new one takes the board at its word.
Caught by mutation: reverting the helper left every other case in this suite green.
*/
const traitedElsewhere = {
version: "v2", id: "wf", name: "wf", nodes: [], edges: [],
columns: [
{ id: "todo", name: "plain", traits: [] },
{ id: "in-progress", name: "plain", traits: [] },
{ id: "done", name: "plain", traits: [] },
{ id: "archived", name: "plain", traits: [] },
/* One trait, and deliberately never the role under test. */
{ id: "signoff", name: "Signoff", traits: [{ trait: "merge" }] },
],
};
expect([...(await resolve(storeFor(traitedElsewhere), "FN-1"))]).toEqual([]);
});
it(`${name} falls back to the legacy id on a V1-UPGRADED board`, async () => {
expect([...(await resolve(storeFor(V1), "FN-1"))]).toEqual([legacy]);
});
it(`${name} falls back to the legacy id when the workflow cannot be resolved`, async () => {
expect([...(await resolve(storeFor(undefined), "FN-1"))]).toEqual([legacy]);
});
}
});

28
packages/dashboard/src/task-lifecycle-lanes.ts
View File

@@ -1,4 +1,4 @@
import { columnsWithFlag, resolveWorkflowIrForTask, type WorkflowIr } from "@fusion/core";
import { columnsWithFlag, declaresAnyLifecycleTrait, resolveWorkflowIrForTask, type WorkflowIr } from "@fusion/core";
/*
FNXC:WorkflowResolvedColumns 2026-07-30-08:45 (#2783 review — coderabbit):
@@ -95,6 +95,20 @@ which a complete-but-not-archived card is not.
The two roles resolve independently and have failed independently before, so they get independent
helpers rather than one flag argument — a caller that wants both asks `landedColumnsForTask`.
*/
/*
FNXC:WorkflowResolvedColumns 2026-07-30-23:55 (batch-core, following #2821's review):
THE EMPTY SET AND THE UNEXPRESSED ONE ARE DIFFERENT, and these three conflated them.
Each read `resolved.length > 0 ? resolved : legacyId`, which is right for a v1 upgrade — every column
emitted with `traits: []`, so the legacy id is the only vocabulary that exists — and wrong for a v2
board that expresses traits and simply declares no lane of that role. There the legacy id is a column
the board may still HAVE without meaning it: a `done` or `archived` column left untraited on purpose.
Falling back onto it widens the guard onto a role the board explicitly did not assign.
`declaresAnyLifecycleTrait` separates the two, matching the shape #2821's review established for
`resolveNodeOverrideLanes`. A board that traits nothing keeps the legacy id; a board that traits
something is taken at its word, including when the answer is "no such lane".
*/
export async function archivedColumnsForTask(
store: LaneResolverStore,
taskId: string,
@@ -102,8 +116,8 @@ export async function archivedColumnsForTask(
): Promise<Set<string>> {
try {
const ir = await resolveWorkflowIrForTask(store, taskId, irCache);
const archived = columnsWithFlag(ir, "archived");
return new Set(archived.length > 0 ? archived : ["archived"]);
if (!declaresAnyLifecycleTrait(ir)) return new Set(["archived"]);
return new Set(columnsWithFlag(ir, "archived"));
} catch {
return new Set(["archived"]);
}
@@ -122,8 +136,8 @@ export async function wipColumnsForTask(
): Promise<Set<string>> {
try {
const ir = await resolveWorkflowIrForTask(store, taskId, irCache);
const wip = columnsWithFlag(ir, "countsTowardWip");
return new Set(wip.length > 0 ? wip : ["in-progress"]);
if (!declaresAnyLifecycleTrait(ir)) return new Set(["in-progress"]);
return new Set(columnsWithFlag(ir, "countsTowardWip"));
} catch {
return new Set(["in-progress"]);
}
@@ -142,8 +156,8 @@ export async function preWipColumnsForTask(
): Promise<Set<string>> {
try {
const ir = await resolveWorkflowIrForTask(store, taskId, irCache);
const preWip = [...columnsWithFlag(ir, "intake"), ...columnsWithFlag(ir, "hold")];
return new Set(preWip.length > 0 ? preWip : ["todo"]);
if (!declaresAnyLifecycleTrait(ir)) return new Set(["todo"]);
return new Set([...columnsWithFlag(ir, "intake"), ...columnsWithFlag(ir, "hold")]);
} catch {
return new Set(["todo"]);
}