Skip to content

fix: require validated registry lockfile warmups before timing - #171

Open
lukekarrys wants to merge 2 commits into
mainfrom
codex/issue-159
Open

lukekarrys wants to merge 2 commits into
mainfrom
codex/issue-159

Conversation

@lukekarrys

@lukekarrys lukekarrys commented Sep 16, 2026

Copy link
Copy Markdown
Member

A timed registry-lockfile run could resolve the entire dependency graph when an ignored warmup failure or 300-second timeout left no lockfile. Warmups now run in a checked hyperfine prepare hook, validate a freshly generated npm lockfile for the selected registry, and retain a snapshot checked before every timed run. Failed warmups stop the job, report the phase/exit code in its summary, and preserve diagnostic artifacts.

Warmups have a separate 600-second default (BENCH_WARMUP_TIMEOUT) so observed 300–380-second third-party registry resolution can finish while timed installs retain the 300-second budget. Even BENCH_WARMUP=0 performs the required initial resolution. The README documents this choice and the generated logs.

Validation: 11 regression tests pass using real hyperfine, including successful slow warmup, timeout/nonzero exit, missing/invalid/stale/changed lockfiles, and separate npm/vlt warmups through the full variation. A localhost npm registry test verifies both successful timed installs fetch tarballs and no packuments. The existing 5 benchmark-data tests and Bash syntax checks also pass. A dedicated PR workflow runs scripts/registry/lockfile.test.js with hyperfine 1.19.0, keeping the native-tool dependency outside the generic scripts/*.test.js workflow; full authenticated registry coverage runs in benchmark CI.

Closes #159

@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 56 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: f0b1072c-fcf1-4a7f-b421-4b082957d601

📥 Commits

Reviewing files that changed from the base of the PR and between 2a654d4 and b831149.

📒 Files selected for processing (7)
  • .github/workflows/benchmark.yaml
  • .github/workflows/registry-lockfile-tests.yml
  • README.md
  • scripts/registry/lockfile.test.js
  • scripts/registry/prepare-lockfile.sh
  • scripts/registry/validate-lockfile.js
  • scripts/variations/registry-lockfile.sh

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Fail registry-lockfile benchmarks when warmup does not create a lockfile

2 participants