#2966 shipped a gate that **cannot detect the defect named first in its own header.** `findLaneAcceptingFunctions` matched a lane parameter only when `param.type` was a `TypeLiteralNode` — an inline `{ reviewColumns?: … }`. But the real code declares these as interfaces: ```ts export function getInReviewStallReason( task: Pick<Task, …>, context: InReviewStallContext = {}, // TypeReference — invisible ): InReviewStallSignal | undefined ``` so the function never entered `accepting` and none of its call sites were examined. ### Measured, both directions | | before | after | |---|---|---| | lane-accepting functions detected | 20 | **30** | | `getInReviewStallReason` detected | no | **yes** | | re-introduce #2956 (drop `reviewColumns` from one call site) | `none added` — **passes** | **fails**: `reads.ts: 7 unwired now, baseline allows 6` | The gate now catches the thing it was built for. ### The baseline moves 10 → 24, and that number needs context `10 unwired call site(s) across 8 files` → `24 across 15`. **No entry was removed** — every previously-recorded file kept its count and 14 sites became visible for the first time: ``` core/task-store/reads.ts 0 -> 6 engine/self-healing.ts 2 -> 4 core/task-store/branch-and-pr-entities.ts 0 -> 1 core/task-store/task-update.ts 0 -> 1 engine/scheduler.ts 0 -> 1 dashboard/routes/register-task-workflow-routes 0 -> 1 cli/commands/dashboard-tui/bucket-mapping.ts 0 -> 1 cli/extension.ts 0 -> 1 ``` **These are newly VISIBLE, not newly broken** — they have been unwired all along. I have **not** audited them, and recording them in the baseline is not a claim that they are fine; it is the ratchet doing what its header describes, since the census's own note says roughly half of the original hits were legitimately unwired (identity proven by a stronger means, sentinel columns, dead exports). Someone should walk the 14. Two stand out as worth a look first: **`reads.ts` at 6** is the file #2956 was about, and **`scheduler.ts`** is a dispatch path. Flagging rather than fixing, because wiring a call site that should not be wired is its own defect and each needs the judgement call the census header describes. ### Regression test `packages/engine/src/__tests__/lane-wiring-census-named-types.test.ts` pins the detector's shape — named interface, type alias, inline literal, positional — against fixtures rather than live counts, so it does not churn when someone legitimately wires a call site. Plus one anti-vacuity case asserting the named-type arm is still load-bearing on real source (`getInReviewStallReason` resolves in the live tree), so the fixtures cannot pass while the tool has quietly stopped applying here. **Mutation:** removing the `TypeReference` arm fails **4 of 5**. ### Also worth knowing `findLaneAcceptingFunctions` still only visits `ts.isFunctionDeclaration` at top level, so `export const fn = (ctx) => …` remains invisible. I checked — no exported arrow function currently takes a lane argument, so nothing is missed today, and I left it rather than widen the surface in the same change. Resolved by **name across the corpus** instead of a type-checker `Program`: these are plain source scans and a checker would cost a full type-resolution pass for one lookup. Two same-named types merge, which only ever widens what counts as wired — safe for a ratchet. **Verified:** 5/5 new tests, `check-lane-wiring` clean at the new baseline, lint clean, FNXC gate exit 0. Core suite on main is green (4923 passed / 0 failed) — unrelated, but I had it running. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved lane-wiring analysis to recognize named interfaces and type aliases. * Added support for wrapped configuration expressions and positional parameters when detecting lane information. * **Tests** * Added comprehensive coverage for lane-wiring detection, including named contexts and live-tree validation. * **Chores** * Updated baseline counts to reflect newly recognized application areas and improved self-healing detection. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user