gate: follow sync-lane sources across module boundaries (11 inert guards were invisible) (#3079)
## Eleven inert guards were invisible to the check that exists to find them #3062 shipped this check collecting sync-lane sources **per file**, and recorded the module-boundary gap as a known limit. That limit has stopped being theoretical. `resolvePlannerLanes` is defined in `replan-target.ts` and consumed in `triage.ts` and `executor.ts`. `triage.ts:723` already reads: ```ts if (task.column === disposeLanes.hold || task.column === disposeLanes.intake || task.column === "in-progress") return; ``` A sync-resolved pair sitting directly beside a literal. That is precisely the site a fleet worker reaches for next — the "conversion" is a one-word edit, it drops the census by one, and it changes nothing, because `resolvePlannerLanes` resolves through `store.resolveTaskWorkflowIrSync` and always describes the default board. **The shipped check could not have seen it.** ## What changed Sources are collected in a **first pass over the whole tree**; consumption is counted in a **second pass** against that repo-wide set. | file | guards now visible | |---|---| | `packages/engine/src/scheduler.ts` | 18 | | `packages/engine/src/triage.ts` | **7 — previously invisible** | | `packages/engine/src/executor.ts` | **4 — previously invisible** | **Baseline 20 → 29. The rise is detection, not regression.** Those eleven guards were already inert; nothing in the tree got worse and no production file is touched by this PR. Flagging that explicitly because the check's own failure text says *"do NOT re-record the baseline to clear this"* — that rule is about a code change, and this is a detector widening. The distinction matters and I would rather state it than have it inferred. ## Mutation evidence, on the motivating site Converting `triage.ts`'s literal to `disposeLanes.wip` — the exact inert edit this widening exists to prevent: | | result | |---|---| | check as shipped on main | **exit 0** — missed entirely | | this PR | **exit 1**, `triage.ts: 7 → 8` | `scheduler.ts` restored clean after every run. ## Third widening — and the honest read on that This is the third hole found in this check, each discovered the same way: 1. **#3062**: matched only the local-variable spelling; the inline `resolveX(...).review` walked past. 2. **#3068**: matched only comparisons; `parked.terminal.has(to)` walked past — and worse, made the count *fall*, inviting a re-record that would have retired live sites. 3. **here**: matched only same-file sources; a cross-module helper walked past. Every time, the check was correct about the shape in front of it and blind to a trivially different spelling of the same defect. I said in #3068 that the durable fix is to key on **the source** rather than enumerate consuming syntax; this closes the module-boundary half of that. The remaining half is value-level dataflow (a sync-resolved id assigned through an intermediate, passed as an argument, or returned), which needs a type-aware pass and is a larger change than a gate script should carry casually. I am not claiming this version is complete. It is measurably better than the shipped one on a site that exists in the tree today. ## Census before / after ``` before: COLUMN guards (the backlog): 84 after: COLUMN guards (the backlog): 84 ``` Unchanged — this converts nothing. It makes eleven already-inert guards countable so the next conversion of them fails loudly instead of reading as progress. ## Verification `test:gate` exit 0 · `pnpm check:inert-sync-lanes` exit 0 · lifecycle-column census exit 0 · `pnpm lint` clean. Gate script + baseline only. Still open and unrelated: **#3073** re-greens the archived-gate parity ratchet, which is red on `main` right now and sits outside the merge gate.
This commit is contained in:
@@ -33,17 +33,35 @@ WHAT IT CHECKS. Per file: functions whose body reaches `resolveTaskWorkflowIrSyn
|
||||
column ids ("sync lane sources"), the locals assigned from them, and the `===`/`!==` guards that
|
||||
consume those locals' role fields. The per-file count is baselined and a RISE fails.
|
||||
|
||||
SCHEDULER 20 -> 18, AND WHY THAT IS NOT A RETIREMENT (2026-07-31-20:30). #3065 replaced three
|
||||
`to === parked.complete || to === parked.archived` pairs with three `parked.terminal.has(to)` calls.
|
||||
Six comparison sites became three membership sites, so the COUNT fell while the guards themselves are
|
||||
unchanged and all three remain counted (#3068 taught this check to see `.has`). Verified before
|
||||
re-recording, because from inside this check a fall caused by denser encoding is indistinguishable
|
||||
from a fall caused by a gate being deleted — and only the second is a defect. That check is the
|
||||
standing obligation attached to every `--update-baseline`.
|
||||
|
||||
WHY A RISE AND NOT ZERO. Existing sync-resolved guards are real, deliberate, and documented — the
|
||||
scheduler's listeners genuinely cannot await today, and their authors said so. Demanding zero would
|
||||
force either a revert or an exemption marker on day one. What must not happen is MORE literals
|
||||
quietly becoming inert-resolved, which is precisely the fleet-phase failure. A drop is welcome and
|
||||
re-records with `--update-baseline`.
|
||||
|
||||
LIMITS, STATED SO NOBODY OVER-TRUSTS IT. Sources are matched within a file by function NAME, so a
|
||||
helper imported from another module is not followed — this finds the dominant local-helper shape
|
||||
(`resolveTaskParkedColumnsSync`, `resolvePlannerLanes`) and will miss a cross-module one. It proves a
|
||||
guard consumes a sync-resolved answer, not that the answer is wrong for every caller. Treat a report
|
||||
as a pointer to investigate. Tests are excluded.
|
||||
CROSS-MODULE SOURCES ARE FOLLOWED (2026-07-31-19:05). The first two versions collected sync-lane
|
||||
sources PER FILE, so a helper defined in one module and consumed in another was invisible. That limit
|
||||
stopped being theoretical: `resolvePlannerLanes` lives in `replan-target.ts` and is consumed in
|
||||
`triage.ts` and `executor.ts`, and `triage.ts:723` already reads
|
||||
`task.column === disposeLanes.hold || ... || task.column === "in-progress"` — a sync-resolved pair
|
||||
sitting beside a literal, i.e. a site a fleet worker would reach for next, whose "conversion" would be
|
||||
inert and which this check could not have seen.
|
||||
|
||||
So sources are now collected in a FIRST PASS over the whole tree, and consumption is counted in a
|
||||
second pass against that repo-wide set.
|
||||
|
||||
LIMITS, STATED SO NOBODY OVER-TRUSTS IT. Sources are still matched by function NAME, not by resolved
|
||||
symbol, so two unrelated functions sharing a name are conflated — the same caveat
|
||||
`check-inert-flag-seams.mjs` records. It proves a guard consumes a sync-resolved answer, not that the
|
||||
answer is wrong for every caller. Treat a report as a pointer to investigate. Tests are excluded.
|
||||
|
||||
TO CLEAR A FAILURE: resolve asynchronously (thread the lane in from a caller that has already awaited
|
||||
a store read), or carry the resolved lanes on the event payload so no listener resolves at all. Do NOT
|
||||
@@ -178,14 +196,24 @@ function countInertGuards(sf, locals, sources) {
|
||||
return hits;
|
||||
}
|
||||
|
||||
const byFile = {};
|
||||
const detail = {};
|
||||
for (const file of sourceFiles(join(REPO, "packages"))) {
|
||||
const files = sourceFiles(join(REPO, "packages"));
|
||||
|
||||
/* PASS 1 — every sync-lane source in the tree, so a helper consumed across a module boundary counts. */
|
||||
const sources = new Set();
|
||||
for (const file of files) {
|
||||
const text = readFileSync(file, "utf8");
|
||||
if (!text.includes(SYNC_IR_READER)) continue;
|
||||
const sf = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true);
|
||||
const sources = syncLaneSources(sf);
|
||||
if (sources.size === 0) continue;
|
||||
for (const name of syncLaneSources(sf)) sources.add(name);
|
||||
}
|
||||
|
||||
/* PASS 2 — guards consuming any of them, anywhere. */
|
||||
const byFile = {};
|
||||
const detail = {};
|
||||
for (const file of files) {
|
||||
const text = readFileSync(file, "utf8");
|
||||
if (![...sources].some((n) => text.includes(n))) continue;
|
||||
const sf = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true);
|
||||
const locals = syncLaneLocals(sf, sources);
|
||||
const hits = countInertGuards(sf, locals, sources);
|
||||
if (hits.length === 0) continue;
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
{
|
||||
"total": 20,
|
||||
"total": 29,
|
||||
"byFile": {
|
||||
"packages/engine/src/scheduler.ts": 20
|
||||
"packages/engine/src/executor.ts": 4,
|
||||
"packages/engine/src/scheduler.ts": 18,
|
||||
"packages/engine/src/triage.ts": 7
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user