fix(scripts): route every entry guard through one symlink-correct predicate, and close the class - #10275
Merged
Merged
Conversation
…dicate
The `invokedDirectly` guard was hand-typed in ELEVEN distinct spellings
across 33 files in `scripts/` (the card estimated ~8), and nine of them
were wrong in the same invisible direction: node resolves symlinks for
the module graph but leaves `process.argv[1]` as the caller typed it, so
a script reached through a symlink compared two different paths, answered
false, and did nothing -- exit 0, no output.
Measured on origin/main, 31 of 33 went inert through a symlink. The one
that matters most is the governed-surface register:
scripts/pm/check-governed-merges.mjs --test AGENTS.md
direct : exit=3, "GOVERNED -- no seat arms auto-merge"
symlink : exit=0, no output
and EXIT_TEST_NOT_GOVERNED is 0, so through a symlink the register's
"this PR is GOVERNED" answer and its "NOT governed, ordinary queue
landing applies" clearance are the same exit code.
- adds `scripts/invoked-as.mjs` -- one predicate, pinned by a self-test
that drives a real probe through a real symlink, a differently-named
symlink, a percent-encoding path, `node <dir>`, and both import
directions. Aligned leg-for-leg with the sibling predicate in
`packages/cli/src/utils/invocation.ts`.
- rewrites all 33 sites to `isEntrypoint(import.meta.url)`.
- adds `check:entry-guard`, the class-closing gate: only `invoked-as.mjs`
may read `process.argv[1]`, so a twelfth spelling cannot be typed.
Three self-tests copy their own source into a synthetic checkout and
spawn it there; they now carry the sibling module too.
Part of #10086
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
…t a file list CI on f9a72c2 failed Test Core (2/3): 3 tests red in template-version-stamps.test.ts with Cannot find module './invoked-as.mjs' imported from /tmp/sync-template-versions-.../scripts/sync-template-versions.mjs The fixture builds a synthetic checkout and copied exactly ONE named file, so the sibling import this PR added to sync-template-versions.mjs was never materialised. Nothing in the test mentioned the sibling, so nothing here had to be edited for it to break. The first round's census covered the copy-into-a-fixture consumers INSIDE scripts/ (the --self-test paths) and missed this one: a vitest suite under packages/, a population `dispatch-gates.mjs` does not reach because it derives repo gate families and runs no package test suite. Re-derived the full population three ways. Exactly one broken consumer; packages/spec's two fixture builders symlink the WHOLE root scripts/ dir and are safe by construction, which is the property this fix adopts. The fixture now derives the closure -- copy the script, then every relative import it makes, transitively -- so the next sibling import travels on its own. Copying all of scripts/ (6 MB, 207 files) was rejected as disproportionate for a two-file closure, and a directory symlink is not available here: node resolves symlinks for the module graph, so the script would self-locate to the real checkout instead of the fixture, which is what the copy exists to prevent. Also declares scripts/invoked-as.mjs as a cross-package input for create-objectstack -- the fixture derives the path rather than quoting it, so the flat literal collector cannot see the read, but a change to the sibling really does break this test. Part of #10086 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
Completes the declaration the previous commit started. `check:cross-package-
test-inputs` refused it:
turbo.json "create-objectstack#test" inputs are missing the declared glob(s):
$TURBO_ROOT$/scripts/invoked-as.mjs
and the refusal is the substantive half. template-version-stamps.test.ts now
copies the stamper's whole import closure into its fixture, so invoked-as.mjs
is a real input; without it in turbo's inputs the cache would not invalidate
on a change to the sibling, and the test could go red on main while every PR
reported green (#7802) -- which is the exact shape that put this PR into a
patch round.
Part of #10086
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. |
os-zhuang
marked this pull request as ready for review
August 20, 2026 15:45
os-zhuang
enabled auto-merge
August 20, 2026 15:45
This was referenced Aug 20, 2026
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 free
to 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.
Fixes #10086
The measurement, re-derived
The card estimated "~8 spellings". The census on
origin/main@923c4247is 11 distinct spellings across 33 files — re-derived, not taken from the card, and the roster differs too (the card listed ~20 files and named 1 basename-matcher; there are 3).Every spelling was probed as a synthetic module invoked four ways.
INERT= exit 0, no output:#resolve(argv[1] ?? '') === resolve(fileURLToPath(...))resolveon the rightimport.meta.url.endsWith(argv[1].split('/').pop())import.meta.url === pathToFileURL(argv[1] ?? '').hrefcheck-governed-merges.mjs's variantinvokedAs(...)— already fixed by #10084new URL(import.meta.url).pathname === argv[1]resolve(argv[1] ?? '') === fileURLToPath(...)import.meta.url === pathToFileURL(argv[1]).hrefargv[1].endsWith('qa-rollup.mjs')import.meta.url === new URL(`file://${argv[1]}`).hrefThree of the card's premises moved, and each changes what the fix has to do:
#— but a space is fine, becauseURLnormalises that one. So the encoding failure is sporadic in exactly the way that resists attribution.qa-rollup.mjswas never symlink-inert at all — its defect is import-firing only.Why this one mattered
scripts/pm/check-governed-merges.mjsis the audit half of the governed-surface regime, where human merge is the review record. Its--testmode is the pre-arm predicate a seat runs before arming auto-merge, andEXIT_TEST_NOT_GOVERNEDis 0. Reproduced onorigin/main:The same invocation, run directly, exits 3 and prints
⛔ GOVERNED — a human merge is the review record for this PR. So through a symlink the register's "this PR is GOVERNED, no seat arms auto-merge" answer and its "NOT governed, ordinary queue landing applies" clearance are the same exit code. After this change:What this does
1. One predicate —
scripts/invoked-as.mjs. ExportsisEntrypoint(import.meta.url), which takes one argument and readsprocess.argvitself, so a call site has no comparison left to spell wrongly.invokedAs(entryArg, selfPath)is the testable core beneath it (hoisted fromdispatch-gates.mjs, whose export is preserved as a re-export — nothing else imported it).2. All 33 sites rewritten to
if (isEntrypoint(import.meta.url)) { ... }, and 13 now-orphaned imports removed.3.
check:entry-guard— the class-closing gate. Onlyinvoked-as.mjsmay readprocess.argv[1];require.main/import.meta.main/process.mainModuleare rejected as second idioms; andisEntrypointmust be called on the caller's own url. Comments and string/template/regex literals are masked first viajs-comment-mask.mjs, so theprocess.argv[1]insiderun-with-stall-guard.mjs's child-process payload is excluded structurally rather than by an allowlist.Following the #10196 precedent, the sweep alone was judged insufficient: nothing stopped a twelfth spelling, and the next one would be just as invisible.
Why a spelling gate rather than a behavioural sweep. Running every
scripts/**entry point and asserting it produced output was rejected on measurement: several of these scripts have real side effects (release-github-releases, thesync-*pair,objectui-changeset-digest), and "produced output" is not decidable for an arbitrary tool — a quiet-on-success script is legitimate, so the assertion would be per-script, which is the same hand-wiring the gate replaces. The behavioural evidence therefore lives once, at the predicate, and enforcement covers the 33 callers and the 34th.4. Aligned with the sibling predicate.
packages/cli/src/utils/invocation.tsalready exportsisProcessEntryfor the same reason (its header cites this card). It carries a directory leg —node <dir>gives the entry argument directory resolution — that thescripts/predicate lacked. Rather than ship two predicates answering this question differently, which is the defect being closed,invokedAsnow carries the same two legs and both headers say to change them together. The duplication itself is structural and documented:scripts/runs as plain.mjsagainst a possibly-unbuilt tree, so importing from a package would trade this bug for a worse one.Verification
Reproduction and repair, across all 33 files. Each invoked
--self-testdirectly, through a same-named symlink, and through a differently-named symlink, onorigin/main(a second worktree) and on this branch:origin/main@ 923c424The 2 non-inert on main are exactly the two the census predicts:
dispatch-gates.mjs(already fixed) andqa-rollup.mjs(basename matcher). After the change all three invocation paths produce identical output for every script — the invocation path no longer changes behaviour, which is the actual contract.Anti-vacuity control. The "0 remain" claim rests on the new gate's own scanner run against the unmodified
origin/maintree — same gate code, both trees:The gate's self-test additionally drives all 11 measured spellings as fixture sources and asserts each is rejected, so its 0 is not a grep for one shape.
Ablation (both legs of the predicate).
invoked-as.mjsis loaded from source by node — nodist/in its resolution path — so there is no rebuild leg; each mutation was confirmed on disk by counting the removed text (→ 0) and the injected marker (→ 1) before running, and each restore by the reverse plus acmpagainst the pre-ablation snapshot (byte-identical, both times).realpathcomparison withreturn false→ 2 of 11 cases red, both symlink legs, reported as{"out":"","status":0}— literally the defect shape.[entry]→ 1 of 11 red, thenode <dir>case.Fixture regressions found and repaired. Three self-tests copy their own source into a synthetic checkout and spawn it there, so a newly-imported sibling broke them — a real cost of the shared-module approach, caught by running the suites rather than by reading.
sync-template-versions.mjs,check-adr-0087-registration.mjsandobjectui-changeset-digest.mjs(5 fixture sites across the three) now carryinvoked-as.mjswith the copy. All three were red mid-change and are green now.Gates.
node scripts/pm/dispatch-gates.mjswith no path args, derived atf9a72c26(the final commit), named 43 families; all 43 were run at that sha. 37 exit 0, including:The 6 non-zero are environmental, each confirmed rather than assumed:
check-partof-closing-keyword,check-single-claim-paths,check-half-states(bare live-mode) — each prints its ownNOT WIRED — … judged nothing … This is a wiring or usage failure, NOT a verdictfor absentPR_BODY/PR_NUMBER. Theirpnpm check:*self-test wrappers all pass.check-prerelease-pin-watch— a standing board condition (act on #3653), unrelated to this diff.check:published-readme-exportsandcheck:type-check-debt— both demand a built workspace (whose type entry packages/…/dist/index.d.ts does not exist. Build first, and55 workspace dependenc(ies) … have no built type entry point on disk). Both fail identically on unmodifiedorigin/main, and this diff touches 0 files underpackages/. lint.yml builds before these steps.ESLint over
scripts/:VERDICT command-exit 0.Out of scope
skip-changeset: the diff isscripts/**, thepackage.jsonscripts block, and onelint.ymlstep — nothing published changes.Filed rather than fixed here:
scripts/, where #10086's class-closing gate cannot see them — three are thefile://${argv[1]}template that also breaks on percent-encoding #10269 — four more symlink-blind guards outsidescripts/, where this gate's population cannot reach them (packages/objectql/scripts/dry-run-hash-compat.ts, twopackages/core/examples/**, oneexamples/embed-objectql/**). Widening the gate is a design question, not a mechanical change:packages/**cannot importscripts/invoked-as.mjs, so the gate would have to acceptisProcessEntryas a second legal spelling.scripts/pm/git-history.mjs's--self-testruns nowhere in CI — the sixth instance of a class fixed five times one at a time #10196 — commented with a measurement rather than a new card:git-history.mjs --self-testis currently red (3 cases), byte-identically on unmodifiedorigin/main. Pre-existing and untouched by this change; it sharpens that card's argument that an unrun self-test hides real breakage.Fenced files checked, no intersection:
scripts/publish-smoke.sh(#10212),scripts/check-test-completeness.mjs+.github/workflows/ci.yml(PR #10205),content/docs/deployment/**(#10229) — all carry zero occurrences ofprocess.argv[1]. Nothing held by another dispatch was touched.Patch round — the fixture-copy population my first census missed
CI on
f9a72c26failedTest Core (2/3)with 3 tests red inpackages/create-objectstack/src/template-version-stamps.test.ts:Cannot find module './invoked-as.mjs' imported from /tmp/sync-template-versions-…/scripts/sync-template-versions.mjs.Reproduced locally before changing anything, matching CI exactly:
Why the first round missed it, stated plainly. I enumerated the consumers that copy a script into a synthetic tree inside
scripts/— the--self-testpaths — and fixed those 5 sites. I never asked the wider question: what else in the repo materialises a copy of ascripts/*.mjs? The answer included a vitest suite underpackages/, a populationnode scripts/pm/dispatch-gates.mjsdoes not reach, because it derives repo gate families and never runspackages/create-objectstack's tests. Nothing I ran in round one executed this file.The complete census, re-derived
Every materialisation (copy or symlink) of the repo-root
scripts/tree, or of a file inside it, outsidescripts/itself — derived three independent ways (basename co-occurrence with a write verb; reads of a root-anchoredscripts/path feeding a write; andmkdir/cpSync/symlinkSynctargeting a syntheticscriptsdirectory):packages/create-objectstack/src/template-version-stamps.test.tspackages/spec/scripts/dist-freshness.test.tssymlinkSync(REPO_ROOT/scripts → fixture/scripts)packages/spec/scripts/dist-freshness-adoption.test.tspackages/spec/scripts/openapi-self-consistency.test.tspackages/spec/scripts, not rootPlus the 5 fixture sites in 3 files inside
scripts/already repaired inf9a72c26. Four furtherpackages/specfiles import a root script in place (check-regen-pending.mjs), where the sibling resolves normally.So: one broken consumer, and the two safe ones are safe precisely because they take the whole directory rather than a hand-picked file.
The shape, and why
A second hand-listed sibling would have been the same defect one turn later, so the fixture now derives the closure instead of naming files: it copies the script into the fixture at its repo-relative position, then does the same for every relative import it makes, transitively. The next sibling import travels on its own.
Rejected alternatives, both on measurement:
scripts/tree (what thespecfixtures effectively do via symlink) — correct, but 6 MB and 207 files for a closure that is currently two.dirname(dirname(import.meta.url))).scripts/invoked-as.mjsis also now declared as a cross-package input forpackages/create-objectstackincheck-cross-package-test-inputs.mjs. The fixture derives the path rather than quoting it, so that gate's flat literal collector cannot see the read — but a change to the sibling really does break this test, and the declaration is what keeps the trigger radius honest.Verification
Red → green on the named file, counts quoted from the runs themselves: before
Tests 3 failed | 78 passed (81); afterTest Files 7 passed (7)/Tests 81 passed (81). Verbose run confirms the file executed and names all 8 cases green, including the exact three that were red.Ablation: replacing the closure walk with a no-op reproduces the CI failure precisely —
Tests 3 failed | 5 passed (8), sameERR_MODULE_NOT_FOUND. Mutation and restore each confirmed on disk (removed text → 0, injected marker → 1, then the reverse plus a byte-identicalcmp).Every package the census implicates was run, not just the failing one:
create-objectstackand@objectstack/spec.packages/spec(the two whole-directory symlink fixtures, plus four in-place importers ofcheck-regen-pending.mjs, which now pulls the sibling):Test Files 415 passed (415)/Tests 11049 passed (11049), zeroERR_MODULE_NOT_FOUNDorinvoked-ashits.A second finding the re-run produced
Declaring
scripts/invoked-as.mjsas a cross-package input madecheck:cross-package-test-inputsgo red, and the refusal was the substantive half:Without that input, turbo's cache would not invalidate when the sibling changes, so this test could go red on
mainwhile every PR reported green — issue #7802, and the exact shape that put this PR into a patch round. Added; the gate now readsOK: 12 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.Gates, re-derived at the final commit
node scripts/pm/dispatch-gates.mjswith no path args atd3d528e9names 45 families — up from 43, because the diff now reachespackages/create-objectstack/src/**. The two additions (check:slot-lookup,check-affected-docs.mjs) were run and pass; this is the "gates no path derivation predicts" case the dispatch warned about, which is why the union was re-derived rather than reused.All 45 run at
d3d528e9: 39 pass, and the 6 non-zero are the same environmental set as the first round (4 print their ownNOT WIRED … NOT a verdictfor absentPR_BODY/PR_NUMBER;check:published-readme-exportsandcheck:type-check-debtdemand a built workspace and fail identically on unmodifiedorigin/main). ESLint overscripts/and the changed test: clean.Generated by Claude Code
Generated by Claude Code