You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(codex): keep npm Node cache within inspection lifetime - #7562
Keep Node's compile cache inside the temporary directory owned by Codex CLI update metadata inspection. npm can enable this cache itself after the ambient NODE_COMPILE_CACHE value has been removed, leaving files in the user's temporary directory after inspection finishes.
Assign an absolute cache path inside the existing isolation directory and remove it with the rest of the inspection state. The public API and update/install behavior stay unchanged.
Add a regression that starts real npm through the existing resolver spawn seam and checks the cache lifetime, empty temporary/home directories, and cleanup. Update the isolation contract documentation.
Verification
Bun 1.4.3 on Windows: the two focused regressions failed before the fix and passed afterward.
bun test tests/codex-integration/codex-cli-update-plan.test.ts: 54 passed, 1 POSIX-only test skipped, 0 failed.
The regression uses real npm startup with fixed --version --offline arguments; registry replies remain synthetic. It verifies cache output and cleanup, not registry authenticity or a complete installation.
The full suite was not repeated locally because several isolated worktrees share this host. Focused behavior and static checks passed. No installed CLI, active proxy, credentials, or user configuration were changed.
Upstream CI completed successfully at c129c1e164a9b97030f127182d084befda2525c7: all four Ubuntu test shards and the aggregate check passed (14 successful jobs, 9 conditional skips). The macOS and Windows full test matrices were not run.
Checklist
Scope stays focused and avoids unrelated cleanup.
Docs or release notes were updated when needed.
Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.
Summary by CodeRabbit
Bug Fixes
The CLI update process now keeps Node’s compile cache within its isolated temporary workspace, including when an external cache location is configured. This helps prevent update-related cache files from being left in other directories.
The update process continues to clean up its temporary files on a best-effort basis.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📝 Walkthrough
The npm update environment now sets NODE_COMPILE_CACHE to a directory inside its owned temporary root. Integration tests check this setting and verify that npm startup does not leave cache files in the ambient cache path, temporary directory, or home directory.
Changes: npm cache isolation
Layer / File(s)
Summary
Redirect and verify Node compile cache src/codex/cli-update-plan.ts, tests/codex-integration/codex-cli-update-plan.test.ts, structure/codex-cli-update.md
At lines 318–322, createNpmConfigIsolation sets NODE_COMPILE_CACHE to node-compile-cache inside the isolated root. Tests check the environment setting and run npm with an ambient compile-cache path to verify cleanup. The documentation describes the npm and Node cache isolation.
No actionable merge-blocking risk is identified; normal checks can proceed.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely describes the main change: keeping npm's Node compile cache within the Codex inspection lifetime. It matches the implementation, tests, documentation, and stated object…
Docstring Coverage
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 …
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title clearly and concisely describes the main change: keeping npm's Node compile cache within the Codex inspection lifetime. It matches the implementation, tests, documentation, and stated objectives.
Full details: Docstring Coverage
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches
🧪 Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Superseded by #7585, the upstream-hosted parent for the CLI-update stack. All three-file changes from c129c1e164a9b97030f127182d084befda2525c7 are preserved at d5e5b4158a713c4bf805f50826b24cf2698a942b (55 additions, 5 deletions; identical stable patch ID 699d364878085768767b6302b6c0ad7c2b813b27). CI for the new head passed. The original branch and PR history are retained.
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
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.
Summary
NODE_COMPILE_CACHEvalue has been removed, leaving files in the user's temporary directory after inspection finishes.Verification
bun test tests/codex-integration/codex-cli-update-plan.test.ts: 54 passed, 1 POSIX-only test skipped, 0 failed.--noEmit, privacy scan, structure check, file-size ratchet, andgit diff --check: passed.--version --offlinearguments; registry replies remain synthetic. It verifies cache output and cleanup, not registry authenticity or a complete installation.c129c1e164a9b97030f127182d084befda2525c7: all four Ubuntu test shards and the aggregate check passed (14 successful jobs, 9 conditional skips). The macOS and Windows full test matrices were not run.Checklist
Summary by CodeRabbit