From 7d75633daa71aa97208b51c6dd5cbe8fd29d17a5 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 05:03:11 -0700 Subject: [PATCH] gate: follow sync-lane sources across module boundaries (11 inert guards were invisible) (#3079) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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. --- scripts/check-inert-sync-lane-conversions.mjs | 48 +++++++++++++++---- scripts/lib/inert-sync-lane-baseline.json | 6 ++- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/scripts/check-inert-sync-lane-conversions.mjs b/scripts/check-inert-sync-lane-conversions.mjs index c3f1c26b90..c8ee882e58 100644 --- a/scripts/check-inert-sync-lane-conversions.mjs +++ b/scripts/check-inert-sync-lane-conversions.mjs @@ -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; diff --git a/scripts/lib/inert-sync-lane-baseline.json b/scripts/lib/inert-sync-lane-baseline.json index 44a84a48f0..066330a20c 100644 --- a/scripts/lib/inert-sync-lane-baseline.json +++ b/scripts/lib/inert-sync-lane-baseline.json @@ -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 } }