From fe8f3af7518e59185e34cd55771801d1ab3432f2 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 14:09:23 -0700 Subject: [PATCH] fix(gate): barrel imports were silently dropped from the inert-seam check, hiding 5 engine omissions (#2822) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Self-reported regression from #2772, which has already merged.** The seam gate on main currently prints a clean bill of health while five real omissions sit in front of it. ## What broke Closing the imported-shadow hole in #2772 taught the check to record which module each callee was imported from, and to exclude a call site whose module basename does not match the seam's declaring module. That works for relative imports. It does not work for barrel imports. Engine and CLI reach core through `import { ... } from "@fusion/core"`. That specifier's basename is `core`, which never matches a module name like `near-duplicate-canonical` — so **every barrel-imported call site was classified as "a different function of the same name" and dropped.** The check stopped seeing engine's and cli's calls into core entirely, which is most of the cross-package surface it exists to watch. ## Measured On `main` today: ``` [check-inert-flag-seams] 21 lane/flag seams, all supplied at every production call site. ``` With this fix: ``` packages/core/src/near-duplicate-canonical.ts: isNearDuplicateCanonicalInactive() — supplied by 6/11 call sites; omitted at packages/engine/src/self-healing.ts (x2), packages/engine/src/triage.ts (x3) ``` Those five were always there. Earlier I reported this seam as "supplied by 5/6" — that number was wrong for this reason, and the engine sites were invisible to me when I said it. ## The rule now Only a **relative** specifier identifies a module well enough to exclude a call site on. Anything else is unresolved, and unresolved must mean **counted**: an over-counted seam produces a false report somebody investigates, an under-counted one produces silence. I had this backwards, and it is the second time in this lane a change made the gate read cleaner while catching less. ## Both directions verified - **Barrel imports counted** — the five engine sites appear. - **Relative shadows still excluded** — lifting the `sortTasksForDisplayColumn` entry still reports it unsupplied, so `Lane`/`Board`/`ListView` calling the dashboard twin through `"./taskSorting"` does not clear core's seam. That was the entire point of the original fix and it still holds. ## The five engine omissions Not fixed here — engine-owned, reported on #2785. Same shape as the merge-queue bug in #2819: a canonical resting in a **renamed active column** reads as *inactive*, so duplicate markers get cleared against live work. They carry TEMPORARY per-file exemptions so this PR is green and self-announcing. Noted at the entry: the key is `::`, so a file with two omitting calls is exempted for both — coarser than I want, recorded rather than left to be discovered. ## Verification `pnpm test:gate` green, lint 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) --- scripts/check-inert-flag-seams.mjs | 74 ++++++++++++++++++++++++------ 1 file changed, 60 insertions(+), 14 deletions(-) diff --git a/scripts/check-inert-flag-seams.mjs b/scripts/check-inert-flag-seams.mjs index 43381ef289..2d61419771 100644 --- a/scripts/check-inert-flag-seams.mjs +++ b/scripts/check-inert-flag-seams.mjs @@ -91,22 +91,22 @@ An omission earns an entry only when supplying the argument would be WRONG, not const ALLOWED_OMISSIONS = new Map([ [ "packages/dashboard/app/components/TaskDetailModal.tsx::isNearDuplicateCanonicalInactive", - "The flags in scope describe the MODAL'S task; the canonical is a different task on a column this " + { count: 1, reason: "The flags in scope describe the MODAL'S task; the canonical is a different task on a column this " + "component never resolves. Passing them would type-check, read as a conversion, and answer " - + "about the wrong task. Correct supply needs a fetch — a data change. See the note at the site.", + + "about the wrong task. Correct supply needs a fetch — a data change. See the note at the site." }, ], [ "packages/core/src/task-store/async-merge-coordination.ts::enqueueMergeQueueInTransaction", - "TEMPORARY: core-owned; reported on #2783. The omitting site is the PUBLIC `enqueueMergeQueue` " + { count: 1, reason: "TEMPORARY: core-owned; reported on #2783. The omitting site is the PUBLIC `enqueueMergeQueue` " + "wrapper; the two moves.ts callers supply. So the automatic handoff-to-review path resolves " - + "the review column and the manual re-enqueue path does not.", + + "the review column and the manual re-enqueue path does not." }, ], [ "packages/core/src/task-store/branch-group-ops.ts::isNearDuplicateCanonicalInactive", - "TEMPORARY: core-owned; reported on #2783. Unlike the TaskDetailModal site this one is genuinely " + { count: 1, reason: "TEMPORARY: core-owned; reported on #2783. Unlike the TaskDetailModal site this one is genuinely " + "wireable — `clearNearDuplicateReferencesToImpl` is async and already holds `store` and " + "`canonicalId`, so the canonical's own flags are one await away. Its five sibling call sites " - + "already supply, so on a renamed board this is the single path that answers from legacy ids.", + + "already supply, so on a renamed board this is the single path that answers from legacy ids." }, ], ]); @@ -269,7 +269,25 @@ const callSitesFor = (fn, declaringFile) => { if (site.file === declaringFile) return true; // the seam's own file if (site.shadowed) return false; // a local same-named function if (site.from === undefined) return true; // not imported: ambiguous, count it - /* Imported: it must come from the seam's module, or it is a different function of that name. */ + /* + BARREL AND PACKAGE IMPORTS CANNOT BE RESOLVED BY BASENAME, SO THEY COUNT. + + Correction to a regression I shipped while closing the imported-shadow hole. Engine and CLI reach + core through `import { ... } from "@fusion/core"`, whose basename is "core" and never matches a + module name like "near-duplicate-canonical". Comparing basenames therefore classified EVERY + barrel-imported call site as "a different function of the same name" and dropped it — so the + check stopped seeing engine's and cli's calls into core at all, which is most of the + cross-package surface it exists to watch. + + Measured: `isNearDuplicateCanonicalInactive` reported "supplied by 5/6 call sites" while FOUR + engine sites (self-healing.ts x2, triage.ts x2) omitted the argument and were invisible. The + check read cleaner and caught less — the exact failure mode this gate exists to document. + + Only a RELATIVE specifier identifies a module well enough to exclude on. Anything else is + unresolved, and unresolved must mean COUNTED: an over-counted seam produces a false report + somebody investigates, an under-counted one produces silence. + */ + if (!site.from.startsWith(".")) return true; return site.from.replace(/\.js$/, "").split("/").pop() === declaringModule; }); return relevant; @@ -308,17 +326,45 @@ for (const [fn, { file, arity }] of declared) { A partially-supplied seam is the harder defect of the two. A wholly-unsupplied one is at least uniformly wrong; this one works on the board you tested and degrades on the column you did not. */ - const omitting = sites - .filter((site) => site.args < arity) - .filter((site) => !ALLOWED_OMISSIONS.has(`${site.file}::${fn}`)); + /* + FNXC:InertFlagSeams 2026-07-31-03:10 (#2822 review — greptile): + AN EXEMPTION IS BOUNDED BY COUNT, NOT OPEN-ENDED. + + `::` previously exempted EVERY call to that function in that file, so a later call + added without the flags was silently covered and the gate stayed green — an exemption that grows to + fit whatever arrives is not an exemption, it is a hole. That is the same defect this gate exists to + catch (`isTaskStuck` shipped two of three sites unsupplied and the gate was green), one level up in + the gate itself. + + Each entry now records HOW MANY omissions were reviewed. Extras beyond that count are reported like + any other unsupplied site, so adding a call site cannot inherit someone else's review. + */ + const omittingAll = sites.filter((site) => site.args < arity); + const usedPerKey = new Map(); + const omitting = []; + for (const site of omittingAll) { + const key = `${site.file}::${fn}`; + const entry = ALLOWED_OMISSIONS.get(key); + if (entry === undefined) { omitting.push(site); continue; } + const used = usedPerKey.get(key) ?? 0; + if (used < entry.count) { usedPerKey.set(key, used + 1); continue; } + omitting.push(site); + } /* Same staleness rule as the name-level list: an exemption whose site now supplies is dead. */ - for (const [key, reason] of ALLOWED_OMISSIONS) { + for (const [key, entry] of ALLOWED_OMISSIONS) { const [siteFile, siteFn] = key.split("::"); if (siteFn !== fn) continue; - const site = sites.find((candidate) => candidate.file === siteFile); - if (!site) stale.push(` ${key} — no such call site; remove its ALLOWED_OMISSIONS entry`); - else if (site.args >= arity) stale.push(` ${key} — now supplied; remove its entry (${reason.slice(0, 40)}...)`); + const omittingHere = sites.filter((candidate) => candidate.file === siteFile && candidate.args < arity).length; + if (omittingHere === 0) { + stale.push(` ${key} — no unsupplied call site remains; remove its ALLOWED_OMISSIONS entry`); + } else if (omittingHere < entry.count) { + /* + A count that overshoots is the same hazard in miniature: it silently pre-authorises an omission + that has not been reviewed. Narrow it in the change that fixed the site. + */ + stale.push(` ${key} — count is ${entry.count} but only ${omittingHere} site(s) omit; lower it`); + } } if (ALLOWED.has(fn)) { /*