diff --git a/packages/engine/src/__tests__/lane-wiring-census-named-types.test.ts b/packages/engine/src/__tests__/lane-wiring-census-named-types.test.ts new file mode 100644 index 0000000000..75c04f2795 --- /dev/null +++ b/packages/engine/src/__tests__/lane-wiring-census-named-types.test.ts @@ -0,0 +1,176 @@ +// @vitest-environment node + +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-23:50: +THE LANE-WIRING CENSUS MUST SEE NAMED CONTEXT TYPES, not only inline type literals. + +`scripts/lib/lane-wiring-census.mjs` recognises a lane-accepting function so its call sites can be +checked for a resolved-lane argument. Its first version matched only `param.type` being a +`TypeLiteralNode`, so a parameter typed by an INTERFACE was invisible — and that is how the real +code is written: + + export function getInReviewStallReason(task: …, context: InReviewStallContext = {}) + +The consequence was not academic. The census shipped unable to detect #2956 — the first case listed +in its own header — and re-introducing that defect left it reporting "none added". Fixing the +detector moved the honest count from 10 unwired call sites across 8 files to 18 across 14. + +The first draft of that fix reported 24, because detecting these functions also exposed a SECOND bug +(the `satisfies` case below) that had been hidden while they were invisible: six of those "new" hits +were correct code. Both fixes ship together, or the baseline records six sites nobody can act on. + +Fixtures rather than the live tree: a test asserting counts over real source would fail every time +someone legitimately wires or adds a call site, which is the churn the baseline exists to absorb. +These pin the DETECTOR's shape instead. The final case is the anti-vacuity check — it asserts the +real corpus still exercises the named-type arm, so the fixtures cannot pass while the tool has +silently stopped applying to this codebase. +*/ + +import { mkdtempSync, writeFileSync, readdirSync, statSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { describe, expect, it } from "vitest"; +import { findLaneAcceptingFunctions, findUnwiredCallSites } from "../../../../scripts/lib/lane-wiring-census.mjs"; + +function fixture(source: string): string[] { + const dir = mkdtempSync(join(tmpdir(), "fusion-lane-census-")); + const file = join(dir, "fixture.ts"); + writeFileSync(file, source); + return [file]; +} + +const REPO_ROOT = resolve(__dirname, "../../../.."); + +describe("the lane-wiring census recognises how lane arguments are actually declared", () => { + it("detects a lane member on a NAMED context interface (the #2956 shape)", () => { + const files = fixture(` + export interface StallContext { now?: number; reviewColumns?: ReadonlySet; } + export function getSignal(task: string, context: StallContext = {}): string { return task; } + `); + const accepting = findLaneAcceptingFunctions(files); + expect(accepting.has("getSignal")).toBe(true); + expect([...accepting.get("getSignal")!.namesByIndex.get(1)!]).toContain("reviewColumns"); + }); + + it("counts a call site that omits the lane argument on that named-type function", () => { + const files = fixture(` + export interface StallContext { reviewColumns?: ReadonlySet; } + export function getSignal(task: string, context: StallContext = {}): string { return task; } + export function wired() { return getSignal("a", { reviewColumns: new Set() }); } + export function unwired() { return getSignal("b", { now: 1 } as never); } + `); + const unwired = findUnwiredCallSites(files, findLaneAcceptingFunctions(files)); + expect(unwired.map((hit) => hit.fn)).toEqual(["getSignal"]); + }); + + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-22:25: + `satisfies` must not hide a wired call. #2956 annotated four wired `reads.ts` call sites with + `satisfies InReviewStalledContext`, and a bare `isObjectLiteralExpression` check reported every one + as unwired — the wrapper node is not the literal. Six false positives, all in correct code. + + This case exists because a census that flags correct code inflates its own baseline with entries + nobody can act on, and the first person to check one stops trusting the number. + */ + it("treats a `satisfies`-annotated options bag as wired", () => { + const files = fixture(` + export interface StallContext { reviewColumns?: ReadonlySet; } + export function getSignal(task: string, context: StallContext = {}): string { return task; } + export function wired() { + return getSignal("a", { reviewColumns: new Set() } satisfies StallContext); + } + `); + expect(findUnwiredCallSites(files, findLaneAcceptingFunctions(files))).toEqual([]); + }); + + it("still detects the inline type-literal and positional spellings", () => { + const files = fixture(` + export function inlineBag(task: string, opts: { terminalColumns?: ReadonlySet }): string { return task; } + export function positional(task: string, activeColumns: ReadonlySet): string { return task; } + `); + const accepting = findLaneAcceptingFunctions(files); + expect([...accepting.get("inlineBag")!.namesByIndex.get(1)!]).toContain("terminalColumns"); + expect([...accepting.get("positional")!.positions]).toEqual([1]); + }); + + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-23:20 (#2974 review — coderabbitai): THE TWO FALSE-GREEN + HOLES, PINNED. Both errors pointed the same way — MORE sites counted as wired — which for a ratchet + means unwired seams vanish and the gate passes with nothing to report. + */ + it("does NOT count a lane key that appears in the wrong argument as wired", () => { + /* + The options bag is parameter 1. Passing `reviewColumns` on the parameter-0 `task` object and an + EMPTY bag means the function receives no lane answer at all. Before the index was retained, the + call-site check scanned every argument and scored this wired. + */ + const files = fixture(` + export function needsLanes(task: { id: string }, opts: { reviewColumns?: ReadonlySet }): string { return ""; } + export function caller(): string { return needsLanes({ id: "x", reviewColumns: new Set() } as never, {}); } + `); + const unwired = findUnwiredCallSites(files, findLaneAcceptingFunctions(files)); + + expect(unwired.map((u) => u.fn)).toContain("needsLanes"); + }); + + it("still counts the SAME key as wired when it is in the declaring argument", () => { + /* Paired positive: the fix must not make every options-bag call look unwired. */ + const files = fixture(` + export function needsLanes(task: { id: string }, opts: { reviewColumns?: ReadonlySet }): string { return ""; } + export function caller(): string { return needsLanes({ id: "x" }, { reviewColumns: new Set() }); } + `); + const unwired = findUnwiredCallSites(files, findLaneAcceptingFunctions(files)); + + expect(unwired.map((u) => u.fn)).not.toContain("needsLanes"); + }); + + it("refuses to merge two same-named lane-carrying types instead of unioning them silently", () => { + /* + Resolution is by NAME with no type checker, so two `Ctx` declarations would union their lane + members and let a call that supplies only the OTHER file's key count as wired. Throwing names both + paths; the previous behaviour returned a quietly wrong answer. + */ + const files = fixture(` + export interface Ctx { reviewColumns?: ReadonlySet; } + `).concat( + fixture(` + export interface Ctx { completeColumns?: ReadonlySet; } + `), + ); + + expect(() => findLaneAcceptingFunctions(files)).toThrow(/two files declare a lane-carrying type named "Ctx"/); + }); + + it("type aliases carry lane members too", () => { + const files = fixture(` + export type MergeContext = { completeColumns?: ReadonlySet }; + export function canMerge(task: string, context: MergeContext): string { return task; } + `); + expect([...findLaneAcceptingFunctions(fixture("")).keys()]).toEqual([]); + expect([...findLaneAcceptingFunctions(files).get("canMerge")!.namesByIndex.get(1)!]).toContain("completeColumns"); + }); + + /* + ANTI-VACUITY against the live tree. The fixtures above would keep passing if the named-type arm + stopped mattering here — if every context interface were inlined, or the census stopped being + pointed at core. This asserts the arm is still load-bearing on real source. + */ + it("resolves a real named-context function in the live tree", () => { + function sources(dir: string, out: string[] = []): string[] { + for (const entry of readdirSync(dir)) { + const path = join(dir, entry); + if (statSync(path).isDirectory()) { + if (entry === "__tests__" || entry === "node_modules" || entry === "dist") continue; + sources(path, out); + } else if (entry.endsWith(".ts") && !entry.endsWith(".d.ts") && !entry.includes(".test.")) { + out.push(path); + } + } + return out; + } + const accepting = findLaneAcceptingFunctions(sources(join(REPO_ROOT, "packages/core/src"))); + expect(accepting.size).toBeGreaterThan(5); + // Declared as `context: InReviewStallContext` — invisible to the census before this arm existed. + expect(accepting.has("getInReviewStallReason")).toBe(true); + }); +}); diff --git a/scripts/lib/lane-wiring-baseline.json b/scripts/lib/lane-wiring-baseline.json index cd9048f3fb..69548ee2ca 100644 --- a/scripts/lib/lane-wiring-baseline.json +++ b/scripts/lib/lane-wiring-baseline.json @@ -1,10 +1,16 @@ { "counts": { "packages/core/src/task-merge.ts": 1, + "packages/core/src/task-store/branch-and-pr-entities.ts": 1, "packages/core/src/task-store/moves.ts": 2, + "packages/core/src/task-store/task-update.ts": 1, "packages/engine/src/auto-merge-finalization.ts": 1, "packages/engine/src/project-engine.ts": 1, "packages/engine/src/runtimes/in-process-runtime.ts": 1, - "packages/engine/src/self-healing.ts": 2 + "packages/engine/src/scheduler.ts": 1, + "packages/engine/src/self-healing.ts": 4, + "packages/dashboard/src/routes/register-task-workflow-routes.ts": 1, + "packages/cli/src/commands/dashboard-tui/bucket-mapping.ts": 1, + "packages/cli/src/extension.ts": 1 } } diff --git a/scripts/lib/lane-wiring-census.mjs b/scripts/lib/lane-wiring-census.mjs index 97d76df0c2..b201e319d3 100644 --- a/scripts/lib/lane-wiring-census.mjs +++ b/scripts/lib/lane-wiring-census.mjs @@ -53,8 +53,79 @@ function parse(file) { * * Exported only: an internal helper's callers are all in one file and visible without a tool. */ +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-23:30: +NAMED context types are resolved, not just inline literals — without this the census could not see +the defect it was built for. + +The first version matched a lane parameter only when `param.type` was a `TypeLiteralNode`. Every +`*Context` interface therefore slipped through, including the motivating case: + + export function getInReviewStallReason(task: …, context: InReviewStallContext = {}) + +`InReviewStallContext` is a TypeReference, so `getInReviewStallReason` never entered `accepting` and +none of its call sites were examined. MEASURED: removing `reviewColumns` from one of them — i.e. +re-introducing #2956, the first case in this file's own header — left the check reporting +"34 known unwired call site(s), none added." + +Five core files declare lane-carrying context interfaces (`in-review-stall`, `in-review-stalled`, +`stale-paused-review`, `stale-paused-todo`, `task-priority`), so the gap was structural rather than +one awkward signature. + +Resolved by NAME across the whole corpus rather than through a type-checker `Program`: these are +plain source scans and a checker would cost a full type-resolution pass for one lookup. + +FNXC:WorkflowLifecycleColumns 2026-07-30-22:55 (#2974 review — coderabbitai, "resolve named type +references by declaration identity"): THE MERGE IS NOT SAFE IN THE DIRECTION THE OLD NOTE CLAIMED. + +That note argued a same-name merge was acceptable because a false member "only widens what counts as +wired". Widening is precisely the unsafe direction HERE: every extra member makes MORE call sites count +as wired, so unwired sites disappear from the census and the ratchet goes green while the seam it +watches is unsupplied. A gate whose errors land on "nothing to report" is the one failure mode a +ratchet must not have — the same reasoning that keeps the sibling SQL gate from pretending it can read +`.sql`. + +A type checker is still the wrong price (a full `Program` over ~1950 files for one lookup, against a +~2s scan). Instead the unsound case is made IMPOSSIBLE TO HIT SILENTLY: two files declaring +lane-carrying types under one name is a hard error naming both paths, not a quiet union. Measured at +the time of writing: 2 lane-carrying types, 0 collisions — so this throws for nobody today and +converts a silent wrong answer into a loud one the moment it would matter. +*/ +function findLaneCarryingTypes(files) { + const byName = new Map(); + for (const file of files) { + const sf = parse(file); + ts.forEachChild(sf, (node) => { + const members = ts.isInterfaceDeclaration(node) + ? node.members + : (ts.isTypeAliasDeclaration(node) && ts.isTypeLiteralNode(node.type) ? node.type.members : null); + if (!members || !node.name) return; + const names = new Set(); + for (const member of members) { + if (member.name && ts.isIdentifier(member.name) && LANE_ARGUMENT_NAMES.has(member.name.text)) { + names.add(member.name.text); + } + } + if (names.size > 0) { + const existing = byName.get(node.name.text); + if (existing && existing.file !== file) { + throw new Error( + `[lane-wiring] two files declare a lane-carrying type named "${node.name.text}": ` + + `${existing.file} and ${file}. Resolving by name would merge their lane members and ` + + `silently mark unwired call sites as wired. Rename one, or resolve by declaration.`, + ); + } + if (existing) for (const n of names) existing.names.add(n); + else byName.set(node.name.text, { file, names }); + } + }); + } + return byName; +} + export function findLaneAcceptingFunctions(files) { const accepting = new Map(); + const laneTypes = findLaneCarryingTypes(files); for (const file of files) { const sf = parse(file); ts.forEachChild(sf, (node) => { @@ -71,24 +142,75 @@ export function findLaneAcceptingFunctions(files) { Both spellings are tracked: option names by name, positional ones by INDEX, so a call is wired if it passes an accepted option key OR supplies an argument in the positional slot. */ - const names = new Set(); + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-23:05 (#2974 review — coderabbitai, "retain the + options-bag parameter index"): A LANE KEY ONLY COUNTS IN THE ARGUMENT THAT DECLARES IT. + + `names` was a flat set with the parameter index thrown away, and the call-site check then + accepted a matching property in ANY argument. So a call could put `reviewColumns` on an earlier + `task` object and pass `{}` as the actual options bag, and the census would score it wired while + the function received nothing — a FALSE GREEN, the same direction as the type-merge above. + + Keyed by index instead: `namesByIndex[i]` are the keys that count when they appear in argument + `i`. Measured before changing it: 0 call sites in the tree match on a mismatched index, so this + is behaviour-preserving today and exists to keep it that way. + */ + const namesByIndex = new Map(); const positions = new Set(); + const addName = (index, name) => { + if (!namesByIndex.has(index)) namesByIndex.set(index, new Set()); + namesByIndex.get(index).add(name); + }; node.parameters.forEach((param, index) => { if (ts.isIdentifier(param.name) && LANE_ARGUMENT_NAMES.has(param.name.text)) positions.add(index); if (param.type && ts.isTypeLiteralNode(param.type)) { for (const member of param.type.members) { if (member.name && ts.isIdentifier(member.name) && LANE_ARGUMENT_NAMES.has(member.name.text)) { - names.add(member.name.text); + addName(index, member.name.text); } } } + /* `context: InReviewStallContext` — see findLaneCarryingTypes for why this arm exists. */ + if (param.type && ts.isTypeReferenceNode(param.type) && ts.isIdentifier(param.type.typeName)) { + for (const member of laneTypes.get(param.type.typeName.text)?.names ?? []) addName(index, member); + } }); - if (names.size > 0 || positions.size > 0) accepting.set(node.name.text, { names, positions }); + if (namesByIndex.size > 0 || positions.size > 0) { + accepting.set(node.name.text, { namesByIndex, positions }); + } }); } return accepting; } +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-22:20: +`{ … } satisfies SomeContext` IS an options bag — a bare `isObjectLiteralExpression` check says no. + +`satisfies` (and `as`, and parentheses) wrap the literal in another node, so the plain check reported +a fully-wired call as unwired. That is not hypothetical: #2956 wired four `getInReviewStalledSignal` +call sites in `reads.ts` and annotated each with `satisfies InReviewStalledContext`, and the census +counted every one of them as missing its lane argument. + +The bug predates the named-type arm but was INVISIBLE behind it: functions typed by an interface were +never detected, so their call sites were never inspected and the false positives never surfaced. +Fixing detection exposed six of them at once, which is how this was found. + +A census that reports correct code as unwired is worse than one that misses cases — it inflates the +baseline with sites nobody can "fix", and the first person to check one learns the number is noise. +*/ +function unwrapObjectLiteral(node) { + let current = node; + while ( + ts.isSatisfiesExpression(current) + || ts.isAsExpression(current) + || ts.isParenthesizedExpression(current) + ) { + current = current.expression; + } + return ts.isObjectLiteralExpression(current) ? current : null; +} + /** Call sites of those functions that pass none of the accepted lane arguments. */ export function findUnwiredCallSites(files, accepting) { const unwired = []; @@ -98,9 +220,13 @@ export function findUnwiredCallSites(files, accepting) { if (ts.isCallExpression(node) && ts.isIdentifier(node.expression)) { const accepted = accepting.get(node.expression.text); if (accepted) { - const passesOption = node.arguments.some((arg) => - ts.isObjectLiteralExpression(arg) - && arg.properties.some((p) => p.name && ts.isIdentifier(p.name) && accepted.names.has(p.name.text))); + const passesOption = node.arguments.some((arg, index) => { + const wanted = accepted.namesByIndex.get(index); + if (wanted === undefined) return false; + const bag = unwrapObjectLiteral(arg); + return bag !== null + && bag.properties.some((p) => p.name && ts.isIdentifier(p.name) && wanted.has(p.name.text)); + }); const passesPositional = [...accepted.positions].some((index) => node.arguments.length > index); const passes = passesOption || passesPositional; if (!passes) {