feat(setup): accept registry-url — the passthrough that kept every release workflow off this action - #54
Merged
Merged
Conversation
usetheokit/theokit-sdk#569. The floor leg pins the dep under the package manager's override field and reinstalls. When the dep is a package of THIS workspace, that override rewrites its spec into a plain semver range, which destroys the `workspace:` protocol guarantee -- pnpm 'will refuse to resolve to anything other than a local workspace package' only while that protocol is in the spec. With linkWorkspacePackages defaulting to false, the range resolves from the registry and the published tarball is installed BESIDE the local copy. Measured on theokit-sdk, same checkout and machine: without the override 452 MB peak, 6.6s with it 4,432 MB peak, OOM on the runner 9.8x. tsup's DTS worker walks 14 MB of published .d.ts instead of the workspace source. It blocked the 5.0.1 release twice before anyone looked at why. It is also the wrong question. A floor is a claim about what a CONSUMER resolves, and a consumer never has this workspace's copy -- so the leg was validating a package against its own published output. WHY NOT isSibling. My first attempt excluded every ecosystem sibling and broke two existing tests, which was the code telling me I was deleting the feature rather than fixing it: its primary case is theokit-plugins, where fourteen packages declare `theokit >=0.50.1` and theokit is a DIFFERENT repository. Overriding that duplicates nothing and must keep running. The predicate is 'the dep is published by this very workspace', not 'the dep is ours'. Measured across the ecosystem after the change: theokit 1 leg kept, 0 skipped theokit-plugins 1 leg kept (theokit, external), 1 skipped theokit-gateways 0 kept, 1 skipped theokit-tui unchanged theokit-plugins is the proof: the leg the docblock names as the reason this check exists survives. An empty workspace list fails OPEN -- a caller that could not read the manifests must not silently drop every floor and report a green check that ran nothing. Every dropped gap is announced via ::notice::, never silently. The uv and Cargo ecosystems draw this same line: --resolution lowest-direct and -Z direct-minimal-versions lower the direct edges you do not control.
Bumps the manifest and the pin in dep-check.yml together, which the release gate requires -- 0.5.0 and 0.2.0 each published green while the pin stayed behind, so no consumer received the change. Carries the fix for usetheokit/theokit-sdk#569: floor legs whose dep this workspace publishes are dropped, because pinning one installs its own tarball beside the local copy (452 MB -> 4,432 MB peak, measured) and asks a question a consumer never asks. Eleven repositories consume this gate. Moving v1 is the separate step.
vitest-dev/vitest sets it repo-wide and documents ~15-20s per job -- the only hard number anyone published for an install optimisation. Measured on theokit-sdk, pnpm 10.34.1, two runs each against a warm store: default 2.78s, 2.54s TRUST_LOCKFILE 2.52s, 2.54s Nothing. Their saving is real and ours is not, most plausibly because every job here restores a warm store through the cache: pnpm above. Recorded rather than left silent, following the precedent of the turbo-cache note in theokit-sdk's ci.yml: configuration that does nothing is debt wearing an optimisation's clothes, and the next person to read the vitest number should find this instead of re-deriving it. It would have traded a supply-chain check for zero measured seconds.
…kage The leg builds every package that claims the floor. Doing it one --filter at a time re-plans the task graph once per package and rebuilds the shared dependencies each round. Measured on theokit-plugins, whose leg claims 10 packages, cold cache (dist and turbo cleared), two rounds each: loop (10 invocations) 35.2s, 39.6s batched (1 invocation) 23.4s, 24.0s -34% and -39%, producing the same 10 dist/ directories and the same 20 build successes. Two things this deliberately does NOT do: - batchedWithDeps returns null for npm and yarn rather than guessing. npm has no '...' equivalent at all, and yarn's 'yarn workspace <pkg> run' takes exactly one name -- batching there needs 'workspaces foreach', different semantics nobody here has measured. Those callers keep the loop, correct if slower. - an empty package list returns null, because 'pnpm run build' with no filter builds the WHOLE workspace, which is the defect the per-package filter exists to prevent. --package is now repeatable, and that needed parseArgs 'multiple: true'. Without it the parser keeps only the LAST occurrence, so the repeated flag would have silently built one package and reported the whole leg green -- caught by running the command with three packages and reading the output, not by the tests. Bumps to 0.10.0 with the workflow pin, together, as the release gate requires.
The obvious move is `cache: pnpm` on its Setup Node, and it would be dead configuration. This job only runs when base_ref == 'main' -- a pull request, never a push. A run restores a cache from its own branch or from the DEFAULT branch, so a cache only ever written on release pull requests is one no other run can reach: every leg a guaranteed miss, paying the save cost forever. cloudflare/workers-sdk hit this exact shape and wrote it down: actions/cache saves from a post step declaring post-if: success(), so a failing job never saves either -- and this job has been failing. What it was worth, measured on run 101194428887: install 26s and reinstall 10s of a 319s run, about 8%. The two changes that matter took the other 92%. Doing it properly needs cache/restore + explicit cache/save plus a job on push: [main] to populate what the pull requests read. New machinery for 8%, recorded rather than built.
…ecome one Measured 2026-09-05 across the ten consumers: nine held byte-identical copies of the 74-line gate and one held a variant that had learned two things the others never received. A fix in one copy reached one repository. The shared version is the union of both: the nine contributed the diagnostics that cite git-safety.md and name the next step, theokit-sdk contributed timeout-minutes and the changesets-bot exemption that cost #535 on 2026-09-03. concurrency was deliberately not carried over, and the file says why, so its absence does not read as an oversight. actionlint and zizmor clean, both verified against a positive control.
…off this action Measured 2026-09-05 across the ten consumers: 22 workflows set up Node and pnpm by hand and exactly one file used this action. The largest group of duplicates is the nine release.yml, and seven of them pass registry-url -- so adopting the shared setup meant losing npm authentication. Everything else was already equivalent, including the part most likely to be lost in a hand rewrite: cache-dependency-path already lists package.json beside pnpm-lock.yaml, which is the fix for a pnpm major bump restoring a store the new pnpm reads as inconsistent. The action's own CI job now runs it with a registry and asserts that setup-node wrote an npmrc naming the registry and binding _authToken. It reads the file rather than npm config get registry, which would go green on a registry that came from somewhere else.
# Conflicts: # CHANGELOG.md
usetheodev
pushed a commit
that referenced
this pull request
Sep 5, 2026
…ful config The input added in #54 was justified by 'seven of the nine release.yml pass registry-url'. That number came from a grep that counted the word wherever it appeared, and where it appears is inside a comment in seven of those workflows explaining why the setting is deliberately absent: NO registry-url. It looks harmless and is not: setup-node writes //registry.npmjs.org/:_authToken=${NODE_AUTH_TOKEN} into an npmrc UNCONDITIONALLY and points NPM_CONFIG_USERCONFIG at it, then exports NODE_AUTH_TOKEN only if the caller supplied one -- which, under OIDC, nobody does. Matching the YAML key instead of the string: zero of nine use it, and all nine publish through actions/npm-oidc. The input served nobody and its docstring invited the exact configuration this ecosystem had already found and documented as harmful. The reasoning now lives beside the setup-node call so the absence reads as a decision, and the CI job asserts the opposite property: the action writes no _authToken npmrc on a caller's behalf. The finding that prompted #54 survives: 22 workflows set up Node and pnpm by hand and one file uses this action. The blocker was imagined; the duplication is real and the action was a drop-in all along.
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
2 participants
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.
Continues the pipeline standardisation from #51, and this one is the bigger number.
The measurement
Across the ten consumers, counting workflows that set up Node and pnpm:
Twenty hand-rolled setups is the largest duplication in the ecosystem, and it is not there because
anyone preferred it. The largest single group is the nine
release.yml, and seven of them passregistry-url— which this action did not accept. Adopting the shared setup meant losing npmauthentication, so nobody did.
One missing passthrough was holding the whole adoption.
What this changes
registry-url, forwarded toactions/setup-node, default empty (which writes no.npmrcat all,so nothing changes for the twenty non-publishing callers).
Everything else was already equivalent, including the part most likely to be dropped in a hand
rewrite: the action's
cache-dependency-pathalready listspackage.jsonbesidepnpm-lock.yaml, which is the fix for a pnpm major bump restoring a store the new pnpm reads asinconsistent (
ERR_PNPM_MISSING_PACKAGE_INDEX_FILE).Exercised, not asserted
The action's own CI job now runs it with a registry and checks that
setup-nodewrote an.npmrcnaming the registry and binding
_authToken.It reads the file rather than
npm config get registry, because the latter reads theeffective config and would go green on a registry that came from somewhere else — the claim
under test is that this action caused it.
What this does NOT do
Convert the twenty callers. That is per-repository work and the
release.ymlones touchpublishing, so they get converted and watched one at a time rather than in a sweep. This removes
the reason they could not be.