gate: ratchet call sites that never receive the lane answer (#2966)
**This is the gap that let three defects reach `main` in one day.** `unwired-lane-parameter.mjs` catches a parameter that reaches **no** caller. It is deliberately satisfied by a mention *anywhere*, so **partial** wiring is invisible to it: | | | | --- | --- | | #2956 | `getInReviewStallReason` wired at **0 of 4** call sites while both siblings were wired | | #2963 | both merge entry points unwired — merging was **impossible** on a renamed board | | #2964 | merge-confirmed finalization unwired — **already-landed work parked `failed`** | Every one was a fix that added an optional parameter without the call-site sweep that has to follow it. The existing guard was green throughout, correctly by its own contract. ## A census, not a guard — and that distinction is the whole design Auditing the sites this finds showed **four of seven were legitimately unwired**: `skipColumnIdentityCheck` callers have already proven lane identity by a stronger means, a sentinel-column caller wants the identity check satisfied by construction, and a dead export has no caller to wire at all. A check that failed on those is ~57% false positives. The sibling guard's own header says why that is worse than a miss — *"it teaches people to disable the check"* — and I agree, so this does not do it. Instead it ratchets like the lifecycle census: **36 known unwired sites across 20 files**, allowed to shrink and not to grow. A new unwired caller raises the count and fails; wiring one lowers it and re-records. The recurrence — adding a caller that forgets the lane answer — is precisely what gets caught, and the legitimate sites cost one baseline line each instead of a permanently red gate. ## Detection is AST-based, deliberately It finds exported functions accepting a lane-named argument — directly *or* as an options-bag member — then finds call sites passing none of them. Not regex: the ad-hoc scan I used during the audit produced false negatives on multi-line calls, which is exactly how a caller gets missed in the first place. Using a heuristic to police a defect caused by a heuristic seemed like a poor trade. ## Verified to fail on the recurrence A ratchet that cannot fail is worse than none, so this was measured rather than assumed. Injecting one new unwired caller into `self-healing.ts`: ``` [check-lane-wiring] call sites not passing a resolved lane argument INCREASED: packages/engine/src/self-healing.ts: 9 unwired now, baseline allows 8 ``` exit 1, naming the file and the delta. ## Placement Runs as a named `check:lane-wiring` step in `pr-checks.yml` beside the lifecycle, SQL, inert-seam and FNXC ratchets — same convention, same failure ergonomics, ~1s. Note the baseline records today's state, which still includes the #2963/#2964 sites because those fixes have not merged yet. When they land the count drops and the baseline is re-recorded downward — the ratchet working as intended rather than a conflict. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71; `tsc` engine clean; lint, lifecycle census `--strict`, FNXC gate, and the new check all clean.
This commit is contained in:
2
.github/workflows/pr-checks.yml
vendored
2
.github/workflows/pr-checks.yml
vendored
@@ -62,6 +62,8 @@ jobs:
|
||||
run: pnpm check:inert-flag-seams
|
||||
- name: FNXC stamp dates
|
||||
run: pnpm check:fnxc-future-dates
|
||||
- name: Lane-wiring ratchet
|
||||
run: pnpm check:lane-wiring
|
||||
|
||||
typecheck:
|
||||
name: Typecheck
|
||||
|
||||
@@ -23,6 +23,7 @@
|
||||
"check:sql-column-literals": "node scripts/check-sql-column-literals.mjs",
|
||||
"check:inert-flag-seams": "node scripts/check-inert-flag-seams.mjs",
|
||||
"check:fnxc-future-dates": "node scripts/check-fnxc-future-dates.mjs",
|
||||
"check:lane-wiring": "node scripts/check-lane-wiring.mjs",
|
||||
"census:lifecycle-columns": "node scripts/lifecycle-column-census.mjs",
|
||||
"check:quarantine-ledger": "node scripts/check-quarantine-ledger.mjs",
|
||||
"check:mock-completeness": "node scripts/check-mock-completeness.mjs",
|
||||
|
||||
88
scripts/check-lane-wiring.mjs
Normal file
88
scripts/check-lane-wiring.mjs
Normal file
@@ -0,0 +1,88 @@
|
||||
#!/usr/bin/env node
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-19:20:
|
||||
Ratchet the population of call sites that do not pass a resolved-lane argument. See
|
||||
scripts/lib/lane-wiring-census.mjs for why this is a census with a baseline and not a hard guard.
|
||||
|
||||
Usage:
|
||||
node scripts/check-lane-wiring.mjs # fail if any file's count ROSE
|
||||
node scripts/check-lane-wiring.mjs --update-baseline # re-record (downward moves only)
|
||||
*/
|
||||
import { readFileSync, writeFileSync, readdirSync, statSync } from "node:fs";
|
||||
import { join, relative } from "node:path";
|
||||
import { findLaneAcceptingFunctions, findUnwiredCallSites } from "./lib/lane-wiring-census.mjs";
|
||||
|
||||
const ROOT = process.cwd();
|
||||
const BASELINE = join(ROOT, "scripts/lib/lane-wiring-baseline.json");
|
||||
const ROOTS = ["packages/core/src", "packages/engine/src", "packages/dashboard/src", "packages/cli/src"];
|
||||
|
||||
function sources(dir, out = []) {
|
||||
for (const entry of readdirSync(dir)) {
|
||||
const p = join(dir, entry);
|
||||
if (statSync(p).isDirectory()) {
|
||||
if (entry === "__tests__" || entry === "node_modules" || entry === "dist") continue;
|
||||
sources(p, out);
|
||||
} else if (entry.endsWith(".ts") && !entry.endsWith(".d.ts") && !entry.includes(".test.")) {
|
||||
out.push(p);
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
const files = ROOTS.flatMap((r) => { try { return sources(join(ROOT, r)); } catch { return []; } });
|
||||
const accepting = findLaneAcceptingFunctions(files);
|
||||
const unwired = findUnwiredCallSites(files, accepting);
|
||||
|
||||
const counts = {};
|
||||
for (const hit of unwired) {
|
||||
const key = relative(ROOT, hit.file);
|
||||
counts[key] = (counts[key] ?? 0) + 1;
|
||||
}
|
||||
|
||||
if (process.argv.includes("--update-baseline")) {
|
||||
writeFileSync(BASELINE, `${JSON.stringify({ counts }, null, 2)}\n`);
|
||||
const total = Object.values(counts).reduce((a, b) => a + b, 0);
|
||||
console.log(`[check-lane-wiring] baseline written: ${total} unwired call site(s) in ${Object.keys(counts).length} file(s)`);
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
let baseline = { counts: {} };
|
||||
try { baseline = JSON.parse(readFileSync(BASELINE, "utf8")); } catch { /* first run */ }
|
||||
|
||||
const rose = [];
|
||||
const fell = [];
|
||||
for (const [file, count] of Object.entries(counts)) {
|
||||
const allowed = baseline.counts[file] ?? 0;
|
||||
if (count > allowed) rose.push(` ${file}: ${count} unwired now, baseline allows ${allowed}`);
|
||||
else if (count < allowed) fell.push(` ${file}: ${allowed} -> ${count}`);
|
||||
}
|
||||
for (const [file, allowed] of Object.entries(baseline.counts)) {
|
||||
if (!(file in counts) && allowed > 0) fell.push(` ${file}: ${allowed} -> 0`);
|
||||
}
|
||||
|
||||
if (rose.length > 0) {
|
||||
console.error("[check-lane-wiring] call sites not passing a resolved lane argument INCREASED:\n");
|
||||
console.error(rose.join("\n"));
|
||||
console.error(`
|
||||
A function that accepts a lane answer was called without one. That is the shape behind #2956, #2963
|
||||
and #2964: a fix adds an optional parameter, and a caller added later never passes it — so the callee
|
||||
silently falls back to the legacy literal and the board's own lanes are ignored.
|
||||
|
||||
If the call site genuinely should not pass one (identity already proven by a stronger means, a
|
||||
sentinel column, or a dead export), record it:
|
||||
|
||||
node scripts/check-lane-wiring.mjs --update-baseline
|
||||
`);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
if (fell.length > 0) {
|
||||
console.log("[check-lane-wiring] unwired call sites decreased:\n");
|
||||
console.log(fell.join("\n"));
|
||||
console.log("\nRe-record the baseline in the same commit so the allowance cannot be regrown into:\n");
|
||||
console.log(" node scripts/check-lane-wiring.mjs --update-baseline\n");
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const total = Object.values(counts).reduce((a, b) => a + b, 0);
|
||||
console.log(`[check-lane-wiring] ${total} known unwired call site(s), none added.`);
|
||||
12
scripts/lib/lane-wiring-baseline.json
Normal file
12
scripts/lib/lane-wiring-baseline.json
Normal file
@@ -0,0 +1,12 @@
|
||||
{
|
||||
"counts": {
|
||||
"packages/core/src/in-review-stall.ts": 1,
|
||||
"packages/core/src/task-merge.ts": 1,
|
||||
"packages/core/src/task-store/moves.ts": 2,
|
||||
"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/cli/src/commands/task-lifecycle.ts": 1
|
||||
}
|
||||
}
|
||||
117
scripts/lib/lane-wiring-census.mjs
Normal file
117
scripts/lib/lane-wiring-census.mjs
Normal file
@@ -0,0 +1,117 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-19:10:
|
||||
Count CALL SITES that do not pass a resolved-lane argument to a function that accepts one.
|
||||
|
||||
WHY THIS EXISTS, and why it is a census rather than a guard.
|
||||
|
||||
`unwired-lane-parameter.mjs` catches a parameter that reaches NO caller. It is deliberately satisfied
|
||||
by a mention anywhere, so PARTIAL wiring — some call sites pass the lane answer, others do not — is
|
||||
invisible to it. Three defects reached `main` through that gap in one day:
|
||||
|
||||
#2956 getInReviewStallReason wired at 0 of its 4 call sites while its two siblings were wired
|
||||
#2963 both merge entry points unwired -> "Cannot merge FN-x: task is in 'signoff', must be in
|
||||
'in-review'" — merging was impossible on a board with a renamed review lane
|
||||
#2964 merge-confirmed finalization unwired -> ALREADY-LANDED work parked `failed`
|
||||
|
||||
Each was a fix that added an optional parameter without the call-site sweep that has to follow it.
|
||||
|
||||
NOT A HARD GUARD, on purpose. Auditing the seven sites this finds showed FOUR were legitimately
|
||||
unwired: `skipColumnIdentityCheck` callers have already proven lane identity by a stronger means, a
|
||||
sentinel-column caller wants the identity check satisfied by construction, and a dead export has no
|
||||
caller to wire. A check failing on all of them would be ~57% false positives, and the sibling guard's
|
||||
header says why that is worse than a miss: it teaches people to disable the check.
|
||||
|
||||
So this ratchets like the lifecycle census: a baseline of known-unwired sites that may only shrink. A
|
||||
NEW unwired call site raises the count and fails; wiring one lowers it and re-records. The recurrence —
|
||||
adding a caller without the lane answer — is the thing caught.
|
||||
*/
|
||||
import { createRequire } from "node:module";
|
||||
import { readFileSync } from "node:fs";
|
||||
|
||||
const require = createRequire(import.meta.url);
|
||||
const ts = require("typescript");
|
||||
|
||||
/** Lane-answer argument names, kept in step with unwired-lane-parameter.mjs. */
|
||||
export const LANE_ARGUMENT_NAMES = new Set([
|
||||
"reviewColumns",
|
||||
"terminalColumns",
|
||||
"completeColumns",
|
||||
"activeColumns",
|
||||
"escalationColumns",
|
||||
"columnFlags",
|
||||
"isReviewColumn",
|
||||
"isWipColumn",
|
||||
"holdColumn",
|
||||
]);
|
||||
|
||||
function parse(file) {
|
||||
return ts.createSourceFile(file, readFileSync(file, "utf8"), ts.ScriptTarget.Latest, true);
|
||||
}
|
||||
|
||||
/**
|
||||
* Exported functions that ACCEPT a lane argument, as `name -> Set(argument names)`.
|
||||
*
|
||||
* Exported only: an internal helper's callers are all in one file and visible without a tool.
|
||||
*/
|
||||
export function findLaneAcceptingFunctions(files) {
|
||||
const accepting = new Map();
|
||||
for (const file of files) {
|
||||
const sf = parse(file);
|
||||
ts.forEachChild(sf, (node) => {
|
||||
if (!ts.isFunctionDeclaration(node) || !node.name) return;
|
||||
if (!node.modifiers?.some((m) => m.kind === ts.SyntaxKind.ExportKeyword)) return;
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-20:15 (POSITIONAL lane parameters count too):
|
||||
The first version only understood the options-bag spelling, so a function taking the lane answer
|
||||
POSITIONALLY — `isRecoverable...(task, reviewColumns)`, `isNearDuplicateCanonicalInactive(c, flags)`
|
||||
— had every one of its wired call sites reported as unwired. Six of the eight hits in
|
||||
`self-healing.ts` alone were that false positive, which would have inflated the baseline with
|
||||
sites that are already correct and taught the next reader to distrust the number.
|
||||
|
||||
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();
|
||||
const positions = new Set();
|
||||
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);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
if (names.size > 0 || positions.size > 0) accepting.set(node.name.text, { names, positions });
|
||||
});
|
||||
}
|
||||
return accepting;
|
||||
}
|
||||
|
||||
/** Call sites of those functions that pass none of the accepted lane arguments. */
|
||||
export function findUnwiredCallSites(files, accepting) {
|
||||
const unwired = [];
|
||||
for (const file of files) {
|
||||
const sf = parse(file);
|
||||
const visit = (node) => {
|
||||
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 passesPositional = [...accepted.positions].some((index) => node.arguments.length > index);
|
||||
const passes = passesOption || passesPositional;
|
||||
if (!passes) {
|
||||
const { line } = sf.getLineAndCharacterOfPosition(node.getStart(sf));
|
||||
unwired.push({ file, line: line + 1, fn: node.expression.text });
|
||||
}
|
||||
}
|
||||
}
|
||||
ts.forEachChild(node, visit);
|
||||
};
|
||||
ts.forEachChild(sf, visit);
|
||||
}
|
||||
return unwired;
|
||||
}
|
||||
Reference in New Issue
Block a user