Repository navigation
perf(cli): enable Node module compile cache - #2092
Conversation
Warm CLI invocations spend substantial time compiling the bundled ESM entrypoint. Enable Node's optional on-disk compile cache only in the process that imports the bundle, while preserving early Node 22 compatibility and making cache failures non-fatal. Add deterministic launcher coverage, packaging checks, and a reproducible benchmark procedure so the startup benefit can be measured without flaky CI thresholds.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (6)**/*📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
⚙️ CodeRabbit configuration file
Files:
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}⚙️ CodeRabbit configuration file
Files:
.github/**⚙️ CodeRabbit configuration file
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}⚙️ CodeRabbit configuration file
Files:
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}⚙️ CodeRabbit configuration file
Files:
🪛 ast-grep (0.45.0)scripts/openclaude-bin-compile-cache.test.ts[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec. (detect-child-process-typescript) 🪛 zizmor (1.29.0).github/workflows/pr-checks.yml[info] 57-57: workflow or action definition without a name (anonymous-definition): this job (anonymous-definition) 🔇 Additional comments (4)
📝 WalkthroughWalkthroughThe launcher now conditionally enables Node’s compile cache before loading the CLI. The PR adds behavior tests, startup benchmarking, Node version coverage, compatibility checks, README instructions, and tarball validation. ChangesCompile-cache launcher support
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-checks.yml:
- Line 17: Add Node version 22.8.0 to the node-version matrix to cover the
NODE_DISABLE_COMPILE_CACHE boundary, retain 22.0.0 as the floor-launcher entry,
and quote the existing bare 22 value for consistent matrix formatting.
In `@scripts/benchmark-openclaude-startup.mjs`:
- Around line 36-38: Update the error message in
scripts/benchmark-openclaude-startup.mjs around the enableCompileCache check to
state the required runtime as Node >=22.8.0. Also update the benchmark
instructions in README.md around the referenced section to add the Node >=22.8.0
caveat while preserving the broader Node >=22.0.0 source-build requirement.
- Around line 69-78: Update sample so childEnv(tempRoot, cacheMode) is evaluated
before performance.now() starts timing, then pass the precomputed environment
into spawnSync. Keep the measured window limited to process startup and version
execution.
- Around line 165-168: Guard the gitCommit metadata capture around spawnSync so
missing git or a failed process does not call trim on null stdout. Safely derive
the commit value from the returned stdout, using an appropriate fallback when
unavailable, while preserving the existing successful rev-parse behavior.
In `@scripts/openclaude-bin-compile-cache.test.ts`:
- Around line 173-183: The test NODE_DISABLE_COMPILE_CACHE remains authoritative
must verify cache suppression, not only normal output. When
nodeSupportsCompileCache() is true, provide a temporary NODE_COMPILE_CACHE
directory in the launcher environment, run the existing version assertion, and
confirm the directory remains empty; retain cleanup and skip the cache assertion
when compile-cache support is unavailable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ec995140-751e-47ba-bf0a-b17b37d7b634
📒 Files selected for processing (10)
.github/workflows/pr-checks.ymlREADME.mdbin/node-compile-cache.mjsbin/openclaudepackage.jsonscripts/benchmark-openclaude-startup.mjsscripts/fixtures/instrument-node-compile-cache.mjsscripts/openclaude-bin-compile-cache.test.tsscripts/openclaude-bin-heap.test.tsscripts/verify-clean-install.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (6)
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
scripts/fixtures/instrument-node-compile-cache.mjsscripts/verify-clean-install.tsscripts/benchmark-openclaude-startup.mjspackage.jsonREADME.mdbin/node-compile-cache.mjsscripts/openclaude-bin-heap.test.tsbin/openclaudescripts/openclaude-bin-compile-cache.test.ts
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
scripts/fixtures/instrument-node-compile-cache.mjsscripts/verify-clean-install.tsscripts/benchmark-openclaude-startup.mjspackage.jsonREADME.mdbin/node-compile-cache.mjsscripts/openclaude-bin-heap.test.tsbin/openclaudescripts/openclaude-bin-compile-cache.test.ts
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}
⚙️ CodeRabbit configuration file
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.
Files:
scripts/fixtures/instrument-node-compile-cache.mjsscripts/verify-clean-install.tsscripts/benchmark-openclaude-startup.mjspackage.jsonbin/node-compile-cache.mjsscripts/openclaude-bin-heap.test.tsbin/openclaudescripts/openclaude-bin-compile-cache.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
scripts/verify-clean-install.tsscripts/openclaude-bin-heap.test.tsscripts/openclaude-bin-compile-cache.test.ts
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
README.md
.github/**
⚙️ CodeRabbit configuration file
.github/**: Review CI and release workflow changes for token permissions, third-party actions, pull_request_target usage, artifact upload/download behavior, shell injection, and whether checks still run on the actual PR head. Block on broadened write permissions or unpinned/excessively trusted external execution unless clearly justified.
Files:
.github/workflows/pr-checks.yml
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
scripts/openclaude-bin-heap.test.tsscripts/openclaude-bin-compile-cache.test.ts
🪛 ast-grep (0.45.0)
scripts/openclaude-bin-compile-cache.test.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🔇 Additional comments (13)
scripts/benchmark-openclaude-startup.mjs (5)
1-19: LGTM!
45-67: LGTM!
95-117: LGTM!
119-159: LGTM!
170-202: LGTM!package.json (1)
30-30: LGTM!.github/workflows/pr-checks.yml (1)
41-45: 🗄️ Data Integrity & IntegrationLauncher compatibility step already has
dist/cli.mjs.
bun run checkrunsbun run smoke, andsmokerunsbun run buildbeforenode dist/cli.mjs --version, so the launcher bundle exists before Line 41.bin/node-compile-cache.mjs (1)
1-17: LGTM!bin/openclaude (1)
111-111: 📐 Maintainability & Code QualityRecord the required validation before merge.
The supplied PR context has no results for build, typecheck, smoke, or compile-cache launcher tests. Run the narrowest relevant checks with
bunand list the exact commands and results in the PR.As per coding guidelines, “Run the relevant TypeScript validation checks for changed code, including
bun run typecheckand, when applicable,bun run typecheck:type-tests.” As per path instructions, “Validate with the narrowest relevant checks, including build, typecheck, smoke tests, and the compile-cache/launcher tests, and list exact commands in the PR.”Sources: Coding guidelines, Path instructions
scripts/openclaude-bin-heap.test.ts (1)
6-6: LGTM!Also applies to: 15-22, 34-39
scripts/verify-clean-install.ts (1)
301-301: LGTM!scripts/fixtures/instrument-node-compile-cache.mjs (1)
1-24: LGTM!scripts/openclaude-bin-compile-cache.test.ts (1)
1-171: LGTM!Also applies to: 185-201
| strategy: | ||
| matrix: | ||
| node-version: [22, "24.11.x"] | ||
| node-version: ["22.0.0", 22, "24.11.x"] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🌐 Web query:
Node.js 22.8.0 release notes NODE_DISABLE_COMPILE_CACHE environment variable added
💡 Result:
The NODE_DISABLE_COMPILE_CACHE environment variable was introduced in Node.js 22.8.0 [1][2]. This variable allows users to explicitly disable the module compile cache, which can be useful in scenarios where the cache might cause unexpected behavior, such as impacting the precision of test coverage or other tooling requirements [3][4]. Setting NODE_DISABLE_COMPILE_CACHE=1 will prevent the compile cache from being enabled [3][4]. The 22.8.0 release also introduced the module.enableCompileCache JavaScript API, providing a built-in, performant alternative to third-party packages like v8-compile-cache for enabling on-disk code caching of modules [1][5].
Citations:
- 1: https://nodejs.org/en/blog/release/v22.8.0
- 2: nodejs/node@v22.7.0...v22.8.0
- 3: https://nodejs.org/api/module.html
- 4: https://nodejs.org/download/release/v22.11.0/docs/api/module.html
- 5: v22.8.0 proposal nodejs/node#54560
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow matrix and commands =="
sed -n '1,70p' .github/workflows/pr-checks.yml
echo
echo "== referenced compile-cache test section =="
fd -a 'openclaude-bin-compile-cache.test.ts' . | while read -r f; do
echo "--- $f"
wc -l "$f"
sed -n '140,200p' "$f"
done
echo
echo "== all NODE_DISABLE_COMPILE_CACHE references =="
rg -n "NODE_DISABLE_COMPILE_CACHE|enableCompileCache|compileCache|compile-cache" -S .Repository: Gitlawb/openclaude
Length of output: 8792
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== node-compile-cache implementation =="
cat -n bin/node-compile-cache.mjs
echo
echo "== launcher env/env var handling around NODE_DISABLE_COMPILE_CACHE =="
fd -a 'openclaude-bin-compile-cache.test.ts|node-compile-cache.mjs' . | xargs -r sed -n '1,80p'
echo
echo "== package scripts =="
python3 - <<'PY'
import json
from pathlib import Path
p=Path('package.json')
if p.exists():
data=json.loads(p.read_text())
for k,v in data.get('scripts',{}).items():
if 'test' in k or 'check' in k or 'script' in k:
if 'openclaude-bin-compile-cache' in v or 'compile-cache' in v or 'NODE_DISABLE' in v:
print(k, '=>', v)
PYRepository: Gitlawb/openclaude
Length of output: 3569
Add a Node 22.8.0 matrix entry if the disable path needs coverage.
NODE_DISABLE_COMPILE_CACHE is only defined from Node v22.8.0, so the 22.0.0 leg does not exercise the override checked by NODE_DISABLE_COMPILE_CACHE remains authoritative. Keep 22.0.0 as the floor-launcher leg, and add 22.8.0 if you want the boundary case. Quote the bare 22 value for matrix entry consistency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pr-checks.yml at line 17, Add Node version 22.8.0 to the
node-version matrix to cover the NODE_DISABLE_COMPILE_CACHE boundary, retain
22.0.0 as the floor-launcher entry, and quote the existing bare 22 value for
consistent matrix formatting.
There was a problem hiding this comment.
Update
No new commit is needed for this outdated thread: the current workflow separates the Node 22.0.0 launcher floor from the active Node 22 validation that exercises the compile-cache disable path.
Not changed
- Exact Node 22.8.0 full-suite matrix entry — The active Node 22 matrix leg already runs both normal startup and NODE_DISABLE_COMPILE_CACHE=1 on a compile-cache-capable runtime, while launcher-node-floor verifies the shipped entrypoint on Node 22.0.0 without invoking incompatible development tooling. A fresh exact Node 22.8.0 run of scripts/openclaude-bin-compile-cache.test.ts passed all 11 tests, including cache suppression, so another full-suite matrix leg would duplicate coverage without closing a behavioral gap.
Fresh validation also passed the 19 focused launcher/package tests, both TypeScript gates, the build, workflow YAML parsing, and Node 22.0.0 launcher execution with and without NODE_DISABLE_COMPILE_CACHE=1.
The full validation suite depends on knip and oxc-parser behavior unavailable in Node 22.0.0. Keep full CI on the active Node 22 line and exercise the declared runtime floor in a dedicated build-and-launch job.
Keep environment setup outside the timed process window, document the API's Node 22.8 floor, and preserve completed benchmark results when git metadata is unavailable.
Pair NODE_DISABLE_COMPILE_CACHE with a temporary cache directory and assert that supported Node releases leave it empty while preserving normal launcher output.
UpdateFixed the minimum-Node CI mismatch and addressed every open benchmark and compile-cache review point. The full suite remains on the active Node 22/24 toolchain, while a dedicated job now verifies that the built launcher runs on the declared Node 22.0.0 floor. Addressed
Validation included the 19 focused launcher/package tests, exact Node 22.0.0 build and launcher execution, exact Node 22.8.0 compile-cache tests, workflow YAML parsing, typecheck, package install verification, and the full suite. The full suite retained the same 41 local baseline failures with no new failures. |
3 participants
Summary
dist/cli.mjsnode:modulenamespace so the declared Node 22.0.0 floor remains supportedNODE_COMPILE_CACHEandNODE_DISABLE_COMPILE_CACHEImpact
bun run benchmark:startuprecords cold, warm-up, disabled, and repeated warm timings without adding a performance threshold to CIOn a Ryzen 7 2700X running Manjaro Linux with a 22,151,983-byte bundle, 30 separate-process warm samples showed:
Each cold result used 10 isolated empty-cache processes. Warm measurements discarded the cache-populating run, reported the first warm-up separately, and interleaved enabled and disabled samples.
Testing
bun run buildbun run smokebun run checkbun test scripts/openclaude-bin-compile-cache.test.ts scripts/openclaude-bin-heap.test.ts scripts/verify-clean-install.test.ts scripts/verify-import-specifiers.test.ts— 19 passbun run typecheckbun run typecheck:type-testsbun run test:provider— 1479 passnpm run test:provider-recommendation— 129 passbun run install:verify— cold and upgrade installs pass with silent launcher outputNODE_DISABLE_COMPILE_CACHE=1bun run security:pr-scan -- --base upstream/main --head HEADThe local full check completed with 7805 passing, 2 skipped, and the same 41 environment-sensitive failures present on the unchanged base checkout; the change introduced no additional failures.
Notes
CONTRIBUTING.mdandAGENTS.mdmodule.enableCompileCachecontinue without caching; no custom cache location or explicit flush is introducedbin/openclaudefor the heap/cache/import ordering, thenbin/node-compile-cache.mjsfor the compatibility and failure behaviorSummary by CodeRabbit