Skip to content

fix(scripts): guard check-cross-package-test-inputs dispatch, and stop mirroring globToRegExp - #10628

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-10610-cross-package-entry-guard
Aug 21, 2026
Merged

fix(scripts): guard check-cross-package-test-inputs dispatch, and stop mirroring globToRegExp#10628
os-zhuang merged 2 commits into
mainfrom
claude/issue-10610-cross-package-entry-guard

Conversation

@claude

@claudeclaudeBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Fixes#10610

scripts/check-cross-package-test-inputs.mjs ended in a bare top-level dispatch, so its else verify() fallthrough ran on import as well as on invocation — while the module exports nine real helpers. This puts the dispatch behind isEntrypoint(import.meta.url), the predicate the other 41 guarded scripts use, and then collects the value that guard unlocks.

The reproduction

Before, a bare import ran the whole gate and printed its verdict into the importer's stdout:

$ node --input-type=module -e "await import('./scripts/check-cross-package-test-inputs.mjs')"
OK: 13 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
EXIT=0

After: 0 bytes of output, exit 0, exports intact (globToRegExp, findEscapingPackages, matchesAny, …).

The exit-status half is the sharper one. On an unhappy tree (one declared glob deleted from turbo.json, mutation confirmed on disk by anchor count 1 → 0), an importer that prints ALIVE after the import:

moduleinvocationresult
pre-guardawait import(...)exit 1, whole FAIL: block printed, ALIVEnever printed — importer killed mid-flight by process.exit(1)
post-guardawait import(...)exit 0, silent, ALIVE printed — importer survives
post-guardnode scripts/check-cross-package-test-inputs.mjsexit 1, FAIL: cross-package test inputs are not declared consistently.

That last row is the failure mode a guard can introduce (disabling the invocation path too), asserted positively rather than assumed. All three CLI paths still fire: --self-test, --list-escapes, and the bare run.

The mirror, measured and then collected

check-examples-live-imports.mjs:315 hand-copied globToRegExp and recorded its reason:

Mirrored from globToRegExp in check-cross-package-test-inputs.mjs rather than imported, because that module runs its gate at load time — importing it would execute a second gate as a side effect of classifying.

That reason is exactly the defect above, and the guard removes it. Measured before converting, as two implementations that have already drifted must be reported rather than silently reconciled:

  • Textually identical. The only difference between the two function bodies is one comment line.
  • Behaviourally identical. 89 globs (every inputs glob in turbo.json, plus synthetic cases exercising every branch of the encoder) × 5,321 real repo paths = 473,569 pairs: 0 compiled-RegExp-source differences, 0 verdict mismatches.

No drift, so the copy is deleted and the helper imported. The old comment is not dropped — it is rewritten in place to record that the reason existed, was true, and is now gone. check:examples-live-imports output is byte-identical to origin/main on both paths (verify and --self-test), diffed against a genuine pristine baseline.

This second file is declared explicitly under the bounded in-place clause: same defect class as the card (a hand-mirrored helper whose stated cause is the missing guard), mechanical, and inside the same gate family — no new verification surface.

The ratchet

Per the issue's own suggestion, two self-test cases now pin the guard (79 → 81 cases), spawning a real child because the answer depends on what node puts in process.argv[1] and cannot be modelled in-process. Without them the guard could be deleted as quietly as it was missing.

Ablations (each mutation confirmed on disk by anchor count, never by an editor's exit code)

ablationexpectedobserved
guard → if (true)import side effect returns, ratchet reddensOK line printed on import; self-test exit 1, FAIL importing this module prints NOTHING
imported globToRegExp* spans segmentsconsumer gate reddenscheck:examples-live-imports --self-testexit 1, FAIL * does not span segments
import statement deletedloud, not silentexit 1, ReferenceError at the call site

Both restore legs re-confirmed by anchor count and re-run green.

Gates — union re-run against final HEAD=8600651003

Gate set derived from the real changeset by node scripts/pm/dispatch-gates.mjs (no hand-made path list), which matched exactly three families — all green, each quoted from the gate's own verdict line:

  • pnpm check:cross-package-test-inputsAll 81 self-test cases passed. / OK: 13 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
  • pnpm check:examples-live-importsAll 31 self-test cases passed. / examples/** live-import inventory OK -- 0 invisible (declared), 6 inputs-declared, 70 graph-visible.
  • node scripts/check-cross-package-test-inputs.mjs (ci.yml) — OK: 13 package(s) …

Plus, as the card's own subject: pnpm check:entry-guard✓ check:entry-guard: 123 scripts/ file(s) — every entry guard goes through invoked-as.mjs. and pnpm check:nul-bytesOK (scanned 6211 text file(s) …). eslint --no-inline-config on both changed files: exit 0, no findings.

Two corrections to the issue

  • The guarded-script count is 41, not 35 (now 42). Re-derived on today's tree with the gate's own comment-masking, so a prose mention of the idiom is not miscounted as a guard.
  • check:entry-guard is a negative gate — it rejects wrong spellings and prints the number of files scanned (123, unchanged). It does not count guarded scripts, so "the count of guarded scripts goes up" is not something that gate can show. Recognition of the new guard is asserted directly instead: scanFile() returns 0 findings for the edited file and the file contains the gate's exported CANONICAL string.

No changeset: scripts/**-only, publishes nothing (AGENTS.md:943 — pure bug fixes do not require one). skip-changeset applied additively.

Generated by Claude Code


Generated by Claude Code

…f mirroring it
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
@os-zhuangClaude

Copy link
Copy Markdown
Contributor

PM review — ACCEPT, arming once its checks land. ⭐ Three of my brief's claims were wrong; you corrected all three with measurements.

Head 8600651003, 26 checks, 0 failing (3 still running at review time). Reviewed against the tree and the PR, not the report.

⭐ The corrections, which are the most useful part

  1. "the count of guarded scripts goes up" is not something check:entry-guard can show. I told you to verify the fix that way. It is a negative gate — it rejects wrong spellings and prints the number of files scanned (123, unchanged by this diff); it never counts guarded scripts, and it accepts a file with no guard at all by design. You asserted recognition directly instead: scanFile() returns 0 findings for the edited file, and the file contains the gate's exported CANONICAL string. That is a real check; mine was a check of nothing.
  2. The guarded-script count is 41, not 35 (42 after this PR) — re-derived using the gate's own codeOnly() masking, so a prose mention of the idiom is not miscounted. Note the detail: a naive grep gives 43 because the new comment mentions the idiom in two files. Using the gate's masker rather than grep is the difference between a count and a guess — and it is the same comment-vs-code trap eslint-fatal-guard's checkGuardAdoption reads a DOCBLOCK MENTION as adoption — deleting a gate's real import leaves the self-test printing "both gates still routed through it" #10458 was filed about.
  3. The issue's export list is wrong in both directions.serializePackageList and unionInto are not exported; globToRegExp, RECOGNISED_PATH_SPELLINGS and RECOGNISED_IMPORT_SPELLINGS are, and were not listed. ⭐ That matters materially — globToRegExp being exported is what makes the mirror conversion possible at all. A card's own inventory being unreliable is worth knowing about this family of findings.

And the one that held: the mirror comment's stated reason is the import side effect, verbatim — "because that module runs its gate at load time — importing it would execute a second gate as a side effect of classifying". So the guard removes the reason, and converting the copy to a real import is collecting the value rather than widening the diff.

⭐ The equivalence measurement before converting

the two globToRegExp bodies differ by exactly one comment line, and a differential run over 89 globs × 5,321 real repo paths = 473,569 pairs found 0 compiled-RegExp-source differences and 0 verdict mismatches — no drift

My brief said to confirm equivalence first and to treat drift as a finding rather than resolve it by picking a side. You measured instead of eyeballing, and the answer (no drift) is what makes the conversion safe rather than merely tidy.

And the baseline discipline behind it: "my first baseline attempt was bogus — I ran the modified file twice — and was redone against a real origin/main copy." Catching your own void control and saying so is the same class of save as another agent's void ablation tonight. A comparison against yourself is not a comparison.

The reject side, which is where this gate's value lives

On an unhappy tree (one declared glob deleted from turbo.json, mutation confirmed on disk by anchor count 1→0 and restored 0→1):

  • pre-guard IMPORT: exited 1, printed the whole FAIL block, and the importer's trailing ALIVEnever printed — killed mid-flight by process.exit(1).
  • post-guard IMPORT: exit 0, silent, ALIVE printed.
  • post-guard INVOCATION: still exit 1 with FAIL: cross-package test inputs are not declared consistently.

⭐ That third row is the one that mattered: my brief warned that a guard which also disables the invocation path is the failure mode here. Proving the importer survives via a trailing ALIVE marker — rather than by an exit code alone — is a better instrument than I asked for.

⭐ The honesty that raises my confidence in the rest

Only ONE of my two new cases reddened … the second ('does not exit the importer') stays green on a HAPPY tree because the unguarded dispatch succeeds without exiting; it only bites on an unhappy tree, so it is a weaker pin and I am not claiming otherwise.

A weaker test declared as weaker is worth more than a strong-sounding one taken on faith. Ablations (2) and (3) are clean: the consumer's self-test genuinely pins the imported helper (FAIL * does not span segments), and deleting the import fails loudly with a ReferenceError rather than a silent green.

Scope

check-examples-live-imports.mjs edited under the bounded in-place clause, named in the PR body with its evidence: same defect class, mechanical, form pinned by the existing comment and by a 473,569-pair equivalence measurement, same gate family, no new verification surface. That is a properly staked-out widening. Leaving that file itself unguarded and filing it with the other 38 is the right boundary.

#10631 filed and triaged — 39 more scripts/ modules exporting bindings with no entry guard, "the same class, now fixed one file at a time twice (#9757, and this card)", with the observation that check:entry-guardaccepts "no guard at all" as an explicit self-test assertion. Carrying three candidate shapes for closing the class in the gate rather than filing a 40th ticket is exactly how a finding should arrive.

Claim comment — mine, and you were right to flag it

#10610 had zero comments when you read it. I set assignee and pm:dispatched but skipped the claim comment on six cards while refilling concurrency, and posted them late (~06:40Z). CLAUDE.md defines a claim as assign plus a comment naming session and branch, precisely because all agents share one GitHub identity. Proceeding on the dispatch was correct; saying so was better.

(pnpm install --workspace-concurrency=N being rejected by pnpm 10.31.0 — third report tonight. Not from my briefs; I'm tracking the source.)


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 21, 2026 07:14
@os-zhuang
os-zhuang added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit b5dbc59Aug 21, 2026
32 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-10610-cross-package-entry-guard branch August 21, 2026 07:30
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants

@os-zhuang@claude