Uh oh!
There was an error while loading. Please reload this page.
refactor(tooling): route all 29 scripts/ entry guards through isEntrypoint - #6145
Merged
Conversation
…point The second half of objectui#6092. PR #6133 landed the gate and the shrink-only baseline it measured; this converts every site the baseline named and empties it. Nine distinct hand-typed spellings across 28 .mjs files became one: import { isEntrypoint } from './invoked-as.mjs'; if (isEntrypoint(import.meta.url)) { ... } Twenty-eight of those spellings were WRONG, in the direction nothing in CI can see: node resolves symlinks for the module graph but leaves process.argv[1] as the caller typed it, so a gate reached through a symlink compared two different paths, answered false, and did nothing -- exit 0, no output, which a wrapper holding result.status reads as a pass. One spelling (check-node-esm-load.mjs) went inert with no symlink at all, percent-encoding apart from argv[1] in any directory whose name needs encoding. scripts/shadcn-sync.js is the twenty-ninth and is different in kind: its invokedAsCli() already compared through realpathSync, so it was already correct. Its conversion is a SIMPLIFICATION, not a fix, and the call site now says so. KNOWN_HAND_TYPED_GUARDS is empty. It stays, because the reconciliation it feeds is the live rule: with no lines left, any scripts/ file that hand-types a guard fails as FRESH and names itself. The gate's header is rewritten to the swept state rather than left describing a tree that no longer exists -- the ported-prose defect objectui#6078 recorded. check-doc-component-types.test.ts pinned "needs no install" by requiring every import in that gate to start with `node:`. A relative import of a builtins-only local module keeps that claim true but not that spelling, so the assertion now walks the whole static import graph and requires every leaf to be a builtin. That is the stronger form of the same claim, not a loosened one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019b5UBNMtTzKbVtZZGvFuxe
… gate The KNOWN_IMPORT_UNSAFE comment said the entry had "one remedy". It does not: moving check-lucide-icon-record-names.mjs's two top-level loops behind the guard turns rule 2 green and breaks the module's importers. Measured, not reasoned -- 5 of 25 cases in the importing suite fail, and describeName() starts printing a WRONG diagnosis for a real violation rather than merely failing. So the line stays and the comment now says why, including the lazy-build shape a future card should consider. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019b5UBNMtTzKbVtZZGvFuxe
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019b5UBNMtTzKbVtZZGvFuxe
This was referenced Aug 24, 2026
yinlianghui-tw
marked this pull request as ready for review
August 24, 2026 21:33
This was referenced Aug 24, 2026
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #6092
PR 2 of 2. PR #6133 landed the gate and the shrink-only baseline it measured; this converts every one of the 29 sites that baseline named and empties it. The card's other open item —
check-lucide-icon-record-names.mjs's unguarded top-level loops — is reported, not restructured, on the dispatch order's own STOP condition. Details below.Everything here was measured at final commit
06ad4dff0, working tree clean apart from an untracked throwaway harness directory.Re-measurement at claim
mainmoved by six merges since PR 1's figures (#6128, #6130, #6133, #6134, #6136, #6137). Branched frombfdb9f906.133e2ea1ebfdb9f906process.argv[1]occurrencesisEntrypointimporterscheck-entry-guard.mjsandjs-comment-mask.mjs, both landed by PR 1 itselfThe gate's own verdict line at the branch point reproduced PR 1's numbers exactly:
and at the end of this PR:
29 files → 0. 54 occurrences → 0.
KNOWN_HAND_TYPED_GUARDSis empty;--list's hand-typed section prints nothing. Only threescripts/files still contain the stringprocess.argv[1]at all: the gate (skipped — it quotes the idioms it bans),invoked-as.mjs(the one module allowed to read it), andjs-comment-mask.mjs, whose 8 occurrences are all inside literals in its own corpus.Per-lane both-leg verification, and the harness's positive control
⛔ Not a bulk sed. A two-leg harness ran every one of the 29 before and after, each with its own argv chosen so the run crosses the guard without doing anything destructive (
--checkfor the two writers,--helpforshadcn-sync.js,--self-testforcheck-cross-repo-closer-outcome.mjs,--no-buildforcheck-published-dist-tooling.mjs, and so on).Result: 29/29 dispatched on the CLI leg and inert on the import leg. 27 of 29 produced byte-identical CLI output before and after (same exit code, same sha256 of stdout+stderr).
The two that differ, both explained and neither caused by the conversion:
check-changeset-presence.mjs, 296 → 297 bytes: it reports on the branch's own diff."0 file(s) changed"→"30 file(s) changed". A lane whose output was identical here would mean it had stopped reading the tree.check-i18n-dead-keys.mjs, 49115 → 28719 bytes: its stdout is truncated nondeterministically when it is a pipe (the script ends inprocess.exit, so a large piped write is not flushed). Proved by running the same post-conversion tree three ways: 49115 bytes through a file redirect, 28719 and 32813 bytes through the harness's pipe. Byte count is not a stable signature for this lane in either direction; its stable signature — exit code, dispatch, and first lines — is unchanged.⛔ The harness reports red, and here is the proof
A harness that has never reported red is not a reading. Two deliberate mis-conversions of
check-skills-paths.mjs, each confirmed on disk by a before/after occurrence count, each restored bytrap … EXIT INT TERMusinggit checkout HEAD --(the fix was committed first):!isEntrypoint(…))exit=0, 0 bytes— the silent-success signature itselfif (true))exit=0, 97 bytes(dispatches)Both directions caught, and the inverted case reproduces the card's whole thesis: a converted-then-broken gate goes silently inert while its own work reappears inside anyone who imports it.
The conversion is a real behaviour change, in the good direction
Measured on this branch through a real symlink, on
check-skills-paths.mjs— the gate #6078 used for its demonstration. Restore leggit checkout HEAD --, comparison leggit checkout bfdb9f906 --:isEntrypoint)exit=0, 96 bytes, prints its verdictexit=0, 96 bytes, same verdictpath.resolve(argv[1]) === fileURLToPath(import.meta.url))exit=0, 96 bytesexit=0, **0 bytes**— silently inertNothing relies on the inert behaviour. How that was checked, rather than assumed:
git ls-files -s | awk '$1=="120000"'— the tree contains no tracked symlinks..github/workflows/**andpackage.json, is the literal commandnodefollowed by the plain relative path of the file, run from the repo root — no symlink, no directory argument. The full list was enumerated, not sampled.scripts/contains noindex.mjs/index.js, so the case where node is handed a DIRECTORY and resolves its index — which the predicate additionally handles — cannot arise here either.scripts/__tests__suite — which spawns these scripts from 56 sites — is green.So the only situation in which old and new answer differently is one no current caller can reach; the change is latent-hazard removal, exactly as the card framed it.
The pre-install path, verified rather than reasoned about
PR 1 wired the gate into
lint.ymlbeforepnpm install. A parse of every workflow found 8 steps that invoke a converted script before anypnpm installin the same job:changeset-guard.yml,changeset-presence.yml,ci.yml(×2),control-bytes.yml,doc-component-types.yml,docs-links.yml,skills-paths.yml.All 29 converted files sit at
scripts/depth 0, so./invoked-as.mjsis right for every one — but that was checked, not assumed. Each of the 8 was loaded under a resolver hook that throws on any specifier that would neednode_modules: all 8 exit 0.The hook is not vacuous, and the depth check is not vacuous:
check-doc-snippet-types.mjs(a post-install lane):Error: NEEDS_NODE_MODULES: typescript, exit 1.'../invoked-as.mjs'fromscripts/:ERR_MODULE_NOT_FOUND. A wrong relative depth is a hard load failure, never a silent one.One test had to be strengthened, and it is stronger, not looser
scripts/__tests__/check-doc-component-types.test.ts'sneeds no installcase asserted that every import in that gate starts withnode:. The conversion adds./invoked-as.mjs— install-free, but not that spelling — so the test failed. This is the trap working.Loosening the predicate to "builtin or relative" would have been the wrong repair: it would let a relative import that does pull a package in later land unnoticed. Instead the assertion now walks the whole static import graph and requires every leaf to be a builtin, which keeps the original claim ("this needs no
node_modules") literally true and extends it to every module the gate reaches.Positive control, mutation confirmed on disk and restored by trap: adding
import ts from 'typescript'toscripts/invoked-as.mjs— one hop away from the gate, invisible to the old single-file assertion — reddens it:check-lucide-icon-record-names.mjs— STOP, per the dispatch orderThe order ruled: "⛔ If those loops feed module-level exports that importers actually read, STOP and report rather than restructuring the module."They do, and the measurement is worse than the condition anticipated.
The two top-level
forloops (now:243/:245, one line lower after the added import) buildkeyByComponentandkebabByKey. Those are module-private, but they are read by the exportedliveSpellingForanddescribeName, which importers really call —scripts/__tests__/check-lucide-icon-record-names.test.tscalls both.Ablation on this branch, mutation confirmed on disk (top-level loop lines 1 → 0, indented-behind-guard 1), restored by trap:
KNOWN_IMPORT_UNSAFEentry as STALE — the baseline is satisfied;describeName('BarChart3')stops sayingwrite `chart-column`and says "no live key names the same glyph" instead — a wrong diagnosis for a real violation, printed by a gate that still exits 1. That is a worse outcome than the unguarded loops, not a better one.So the loops are left alone, the
KNOWN_IMPORT_UNSAFEentry stays at one, and the gate's comment now records the measurement instead of claiming the entry has "one remedy" (it does not — it needs a judgement, which is precisely what that comment says a debt line must not be). A lazy build of the two maps insideliveSpellingForwould satisfy both sides and is noted there as the shape a follow-up card should consider. Its guard spelling is converted with the other 28; only the import-safety restructuring is deferred.shadcn-sync.js— a simplification, kept visibly distinctIt hand-typed the correct two-leg shape (
realpathSync(resolved) === __filename) and the gate never flagged it. Converting it deletes an 11-lineinvokedAsCli()and therealpathSyncimport it existed for. The call site now carries a comment saying, in the file itself, that this one was a simplification rather than a fix — and that the shared predicate additionally handles the node-given-a-directory case the hand-typed copy never had.The gate's header, rewritten to the swept state
PR 1's own lesson — a ported header describing a tree that no longer exists is the defect #6078 recorded — applies to this PR too. Every claim in
check-entry-guard.mjs's header was re-checked against the post-sweep tree, and four had gone false:reconcileGuards(and pinned by the self-test) because that is the state a re-added line would have to pass through.shadcn-sync.jsparagraph — its distinction now lives at the call site.shadcn-sync.js:1046as the file that "really does write one in prose" — and that docblock is exactly what this PR deletes. It now cites the measured survivor,js-comment-mask.mjs's 8 in-literal occurrences, which would read as a 30th hand-typed guard if the masking ever regressed.One self-test case changed shape: the loop over
CORRECT_SHAPE_BUT_HAND_TYPEDwas data-driven and becomes vacuous once that set is empty, so it is replaced by an explicit assertion that an empty baseline still calls a real guard FRESH. A test reading the map's size would have passed on an empty map that had also stopped being consulted; this one asserts the reconciliation. Count is unchanged at 63.Verification, each quoting its own verdict line
All at final commit
06ad4dff0. Exit codes captured before any pipe.Plus: the remaining 18 converted gates all ran as the harness's CLI leg above, with their verdicts captured before and after.
No lint narrowing was needed.
pnpm lint:root(eslint .over everything outsidepackages/examples/apps/docs, which is where this whole diff lives) ran the full population in under seven seconds. Its 28 warnings are all pre-existing:eslint --format jsonover the 31 changed lintable files reports exactly 31 file objects witherrorCount=0andwarningCount=0each, so none of the 28 is in this diff.pnpm lint(turbo run lint) is per-package and this diff touches no package source.Control bytes:
grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]'over every changed file returns no matches, andcheck-control-bytes.mjsis green above.Not done, and not owed by this seat: CI convergence. The report lands at draft-PR time per the dispatch contract; the PM verifies the real gate jobs.
Generated by Claude Code