perf+fix(dep-check): drop first-party floor legs, batch the builds (0.10.0) - #50
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.
usetheodev
changed the title
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.
Fixes usetheokit/theokit-sdk#569, which has blocked that repo's
5.0.1release twice.The defect
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 —
destroying the
workspace:protocol guarantee, since pnpm "will refuse to resolve to anythingother than a local workspace package" only while that protocol is in the spec. With
linkWorkspacePackagesdefaulting tofalse, the range resolves from the registry, and thepublished tarball is installed beside the local copy.
Measured on
theokit-sdk, same checkout and machine:9.8×.
tsup's DTS worker walks 14 MB of published.d.tsinstead of the workspace source.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. The leg was validating a package against its own
published output.
Why the predicate is not
isSiblingMy 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. This check's primary case is
theokit-plugins, where fourteen packages declaretheokit >=0.50.1andtheokitis a differentrepository. 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
theokittheokit-pluginstheokit, external)theokit-gatewaystheokit-tuitheokit-sdktheokit-pluginsis the proof: the leg the docblock names as the reason this check exists survives.Failure modes handled
manifests must not silently drop every floor and report a green check that ran nothing.
::notice::with the reason. A floor nobodyexercises must not read as a floor that passed.
Prior art
uv and Cargo draw this same line:
--resolution lowest-directand-Z direct-minimal-versionslower the direct edges you do not control.
uv docs
Tests
packages/dep-check/test/checks.test.mjs, written RED first — 4 new, and the 65 existing ones keptpassing on the second attempt (the first attempt's 2 failures are what redirected the design).
Package total: 103/103.
Release
packages/dep-check/package.jsonand thedefault:pin indep-check.ymlare bumped togetherto
0.9.5, which the release gate requires —0.5.0and0.2.0each published green while the pinstayed behind, so no consumer received them.
Moving
v1is the separate step (#13) and is not done here.