Uh oh!
There was an error while loading. Please reload this page.
ci(tooling): gate the import graph of every gate that runs before pnpm install - #6159
Merged
Merged
Conversation
…m install Derives the pre-install step list from .github/workflows/ and walks each named script's whole static import graph, requiring every non-relative leaf to be a node builtin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019b5UBNMtTzKbVtZZGvFuxe
This was referenced Aug 24, 2026
…-install-import-graph # Conflicts: # content/docs/guide/ci-cd-pipeline.md # scripts/dependabot-merge-gate.mjs
yinlianghui-tw
marked this pull request as ready for review
August 24, 2026 22:47
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 24, 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 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.
Fixes#6148
3b2979f1f, after mergingorigin/main(b08b7eba9) in — never rebased, never force-pushed. The chain ahead cleared while this sat: #6145 merged 21:45, #6156 merged 22:24, and #6144 landed a new pre-install gate in between. Every number below is the post-merge reading; the pre-merge ones are kept only where the delta is the evidence. Working tree clean, exit codes captured before any pipe. Angle-bracket placeholders are written out in words throughout — GitHub's body sanitizer eats short bracketed fragments silently, which is the same class of hazard as the one this gate closes.What landed
One gate, not seven copies of a test.
scripts/check-pre-install-import-graph.mjs— derives the pre-install step list from.github/workflows/on every run, then walks each named script's whole static import graph and requires every non-relative leaf to be a node builtin..github/workflows/pre-install-import-graph.yml— unfiltered, install-free, subscribesmerge_group. Appears as Pre-Install Import Graph Check.scripts/__tests__/check-pre-install-import-graph.test.ts— 33 cases.scripts/dependabot-merge-gate.mjsREQUIRED_CONTEXTS, thecheck:pre-install-import-graphalias,MUST_SUBSCRIBE_MERGE_GROUP, and a section plus inventory row incontent/docs/guide/ci-cd-pipeline.md.Seven files, 1244 insertions, 0 deletions against
mainafter the merge. Nothing inscripts/__tests__/check-doc-component-types.test.tsand none of the 29 scripts PR #6145 owned were touched — the merge conflicted only where #6144 added its own adjacent registration lines, and both sides were kept.scripts/check-doc-component-types.mjs's ownneeds no installcase stays as a second, cheaper witness. Two witnesses on one property is not duplication here: one is a unit test on one file, one is a repo-wide gate derived from the workflows.⭐ The merge is itself the derivation's best evidence
#6144addeddoc-fence-languages.yml, a new pre-install gate, while this branch sat. The population picked it up with zero code change on this branch:A hard-coded list of eight would have merged clean and silently not covered it. That is the failure the card predicted, arriving inside the same afternoon.
The gate's own output, reconciled against the card
node scripts/check-pre-install-import-graph.mjs --list, exit 0:changeset-guard.yml:no-majorcheck-changeset-no-major.mjschangeset-presence.yml:changeset-presencecheck-changeset-presence.mjsci.yml:changeset-checkcheck-changeset-fixed.mjsci.yml:type-checkcheck-type-check-coverage.mjscontrol-bytes.yml:control-bytescheck-control-bytes.mjsdoc-component-types.yml:doc-component-typescheck-doc-component-types.mjsdocs-links.yml:docs-linkscheck-doc-links.mjsskills-paths.yml:skills-pathscheck-skills-paths.mjslint.yml:lintcheck-entry-guard.mjslint.yml:lintcheck-lint-coverage.mjsdependabot-auto-merge.yml:dependabotdependabot-merge-gate.mjshalf-state-patrol.yml:patrolpm/check-half-states.mjsdoc-fence-languages.yml:doc-fence-languagescheck-doc-fence-languages.mjspre-install-import-graph.yml:pre-install-import-graphcheck-pre-install-import-graph.mjsAll 8 of the card's rows reproduce exactly — same workflow, same job, same step index. The reconciliation is that the derivation finds three more the card's parse missed, and none of them arrived after the card was filed:
scripts/check-lint-coverage.mjs— inlint.ymlsince9e725e06a(2026-08-23). The card bracketedlint.ymlas a known case and named only one of its two pre-install script steps.scripts/pm/check-half-states.mjs— since97114f83b(2026-08-24 11:04). Not one of the 29 files Portcheck-entry-guard.mjsand convert the 29 hand-typed entry guards toisEntrypoint(nine distinct spellings, 28 with no realpath leg) #6092's PR 2 converted (it already usedisEntrypoint), so a parse scoped to converted scripts could not see it.scripts/dependabot-merge-gate.mjs— since9e725e06a. This one is the interesting miss, it is the silent direction, and it is reproducible. That job never installs, but step 4 runsgit config merge.pnpm-merge.driver "pnpm install --no-frozen-lockfile". A boundary parse that looks for the stringpnpm installanywhere in a step puts the install at step 4 and drops step 7 out of the population — the SHRINKING direction, which costs coverage silently rather than raising a false red.INSTALL_REis therefore anchored at a command position (start of line, or just after;,&&,||,|,(), so a package manager named inside an argument is not an install. The mirror case is anchored on the tool:pnpm exec playwright install chromiuminci.yml'se2ejob installs a browser, not the workspace.⭐ Three of twelve missed by a careful hand-run parse is itself the argument for the gate.
⭐ Non-vacuity: the one-hop control, re-run at the merged head — and the number grew exactly as predicted
The card hands the exact control.
import ts from 'typescript'added toscripts/invoked-as.mjs— one hop away from a gate, invisible to any own-imports-only check. Fix committed first; mutation confirmed on disk by anchoredgrep -cbefore any result was read; restored bytrap … EXIT INT TERMusinggit checkout HEAD --followed by the path.Pre-merge this reddened 3 chains. Post-merge it reddens 13, because #6145's sweep routed every converted script through
invoked-as.mjs— the first round's report predicted this in as many words, and the prediction is now measured rather than asserted:(each chain is printed on its own indented line under the offending script; collapsed to one line per row here for width). The pin test goes with it:
13 of 14, and the one exception is explained rather than shrugged at:
scripts/check-lint-coverage.mjsimports only bare builtins (from "fs","path","url") and carries no entry guard at all, so it never reachesinvoked-as.mjs. That is consistent withcheck:entry-guard's own verdict — "no entry guard outside the baseline" — and is not a gap in the sweep. Restored by trap; tree clean, gate green again.⭐ The derivation is shown to be a derivation, re-run at the merged head
A gate reporting "14 steps" while reading a constant is the defect it exists to prevent. Both legs moved a real step in
.github/workflows/lint.yml, mutation confirmed on disk by line-number readback plusgit diff --stat, restore undertrap … EXIT INT TERM.SHRINK —
Verify lint coveragemoved BELOWInstall dependencies(line 158 to 214; install 213 to 210):and the step index of the neighbour followed the move, which a stored list could not do:
GROW —
Cross-repo closer outcome contractmoved ABOVEInstall dependencies(line 259 to 213):⭐ And the grow leg is a real positive control, not just a bigger number: that script imports
yaml, a genuine package, so the gate went red on the arrangement rather than merely counting it —That is exactly the
ERR_MODULE_NOT_FOUNDthe card describes, caught at the moment the workflow is edited rather than in a CI job on some later pull request. Both legs restored; population back to 14, verdict back to green.pre-install-import-graph.ymlruns it before any install, in a job that never installs, so the gate is in its own derived population and walks its own graph on every run. Visible in the table above, asserted by the test (is in its own population), and demonstrated by the one-hop ablation, which namedcheck-pre-install-import-graph.mjsamong the reddened chains. Its graph isnode:fs,node:module,node:path,node:url, plus./invoked-as.mjsand./js-comment-mask.mjs, whose own graphs are builtins only.Its entry guard is⚠️ Both shrink-only baselines are empty on
isEntrypoint(import.meta.url)fromscripts/invoked-as.mjs— never a hand-typed one.mainnow (#6145 tookKNOWN_HAND_TYPED_GUARDSfrom 29 to 0; #6156 tookKNOWN_IMPORT_UNSAFEfrom 1 to 0), and this branch puts nothing back into either:42 files to 43 and 37 export bindings to 38 — this file, counted, guarded, and inert on import — with both baselines still at zero. The pre-merge reading in the first round of this PR (
29 file(s) … 54 occurrence(s),1 known-unsafe) is stale and superseded by the line above.Two design decisions, stated because they are the whole gate
Derived, never listed. A hard-coded list of eight breaks silently the first time someone moves a step above
pnpm install, which is the edit that needs catching. #6135 landed the same lesson an hour before this card was filed; #6144 supplied the live proof above, within the day.Static walk, not a runtime resolver hook. The hook was the right ad-hoc instrument for a one-off measurement and is the wrong one for a per-PR gate: it executes module top level, and these files spawn
git, read the whole tree and callprocess.exit. Running fourteen gates to decide whether they could run is a far larger blast radius than the question deserves.Two further choices, each with a case in this tree:
node:".check-changeset-fixed.mjs,check-type-check-coverage.mjsandcheck-lint-coverage.mjsreally do writefrom "fs". That is install-free; requiring the prefix would be a style rule wearing a gate's clothes. The old assertion was too narrow in that direction and too weak in the other.scripts/js-comment-mask.mjs.check-entry-guard.mjscarries'require("fs").writeFileSync(…)'inside a corpus string, and this gate's own self-test spells atypescriptimport inside a fixture. The import KEYWORD's offset is tested against the masks and the specifier is read from the untouched source, so quotes survive — blanking literals outright would have destroyed the specifier being read.The order put
content/docs/**out of scope.scripts/__tests__/ci-cd-pipeline-doc.test.tsrequires every workflow in.github/workflows/to have its own heading incontent/docs/guide/ci-cd-pipeline.md, withDOCUMENTATION_EXEMPTdeliberately empty — so a new workflow with no section is a red suite, not a style preference. The page therefore gets a section and an inventory row. It is a registration the repository mechanically demands, in the same family as the Dependabot classification, and not documentation work: nothing else undercontent/docs/**is touched. Flagging it rather than doing it quietly, so a reviewer does not read it as scope creep. (The merge conflict here was purely positional — #6144'sFence Languagessection landed at the same insertion point; both sections are kept, main's first.)Verification, each quoting its own verdict line
All at merged head
3b2979f1f. Exit codes captured before any pipe.No changeset is owed and none was added — the presence gate's own verdict line says so above, over the merged head's range. This diff touches no published package source.
objectuihas noskip-changesetlabel mechanism, so nothing was labelled either.No lint narrowing was needed.
pnpm lint:rootran the full population in seconds. Its 28 warnings are all pre-existing:eslint --format jsonover the four changed lintable files reports exactly four file objects, eacherrorCount=0 warningCount=0, so none of the 28 is in this diff.Vitest narrowing, declared. The root run was scoped to
scripts/__tests__/— the whole of the suite that reads any file in this diff. The package and app suites were not run: this diff contains no package source, no shared config and no shared fixture, and its only rootpackage.jsonedit is one addedcheck:alias. CI runs the four shards regardless.Control bytes:
grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]'over all seven changed files returns no matches (exit 1), andcheck-control-bytes.mjsis green above.Filed while here, not fixed here:#6160 —
MUST_SUBSCRIBE_MERGE_GROUPnames 6 workflows while the unfiltered blocking set is larger. Post-merge that gap is three, not two:doc-component-types.yml,doc-snippet-types.ymland now #6144'sdoc-fence-languages.ymlall subscribemerge_groupon disk with nothing asserting they keep doing so.Not done, and not owed by this seat: CI convergence. The PM verifies the real gate jobs.
Generated by Claude Code