Skip to content

fix(process): complete Node.js 26.5.0 parity#6854

Merged
proggeramlug merged 2 commits into
mainfrom
fix/process-node26-parity
Jul 27, 2026
Merged

fix(process): complete Node.js 26.5.0 parity#6854
proggeramlug merged 2 commits into
mainfrom
fix/process-node26-parity

Conversation

@TheHypnoo

@TheHypnoo TheHypnoo commented Jul 26, 2026

Copy link
Copy Markdown
Member

Summary

Completes the Node.js 26.5.0 node:process parity suite and prevents process.execPath -e from recursively relaunching Perry's AOT binary.

Changes

  • Route _getActiveHandles and _getActiveRequests through native process dispatch.
  • Detect direct and extracted process.getBuiltinModule calls in optimized builds.
  • Retain dynamic builtin namespace data and include that gate in optimized archive freshness.
  • Route self process.execPath eval invocations through the configurable Node interpreter.
  • Add focused regression tests for each root cause.

Related issue

Closes #6781

Test plan

  • cargo build --release clean
  • Workspace-wide test command not run; the issue constraints forbid cargo test --workspace
  • Added or updated focused #[test] coverage in the affected crates
  • No documentation update required; the public API shape did not change
  • cargo check -p perry -p perry-hir -p perry-runtime
  • cargo test -p perry-runtime --lib self_exec_node_cli_eval_modes_use_node_interpreter
  • cargo test -p perry --bin perry get_builtin_module_gate_detects_direct_and_extracted_calls
  • cargo test -p perry-hir --lib process_active_array_helpers_use_native_dispatch
  • ./run_parity_tests.sh --suite node-suite --module process

Final process parity: 103 passed, 0 failed, 0 compile failures, 0 crashes, 0 Node failures, 0 skipped.

Screenshots / output

Parity Pass:   103
Parity Fail:   0
Compile Fail:  0
Crashed:       0
Skipped:       0
Parity Rate:   100.0%

Checklist

  • I have NOT bumped the workspace version or edited CLAUDE.md / CHANGELOG.md
  • My commits follow the loose feat: / fix: / docs: / chore: prefix convention used in the log
  • I've read CONTRIBUTING.md and agree to the Code of Conduct
  • No Co-Authored-By trailers or attribution added

Summary by CodeRabbit

  • New Features
    • Support lowered calls for process._getActiveHandles() and process._getActiveRequests().
    • Improved handling of extracted process.getBuiltinModule for loading built-in modules.
    • Added compatibility for eval/print-style command execution by switching to the Node interpreter when needed.
  • Bug Fixes
    • Ensured HTTP/2 constants and http2 exports remain available when triggered via process.getBuiltinModule.
  • Tests
    • Added regression coverage for dynamically loading http2 and verifying key exports.

@TheHypnoo TheHypnoo added bug Confirmed defect or regression parity Compatibility gap with Node.js, ECMAScript, or the supported ecosystem rust Pull requests that update rust code labels Jul 26, 2026
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The changes add lowering for internal process methods, route eval-style child process launches through Node, and track process.getBuiltinModule usage so optimized runtimes retain required HTTP/2 namespace data.

Changes

Process method lowering

Layer / File(s) Summary
Active process method dispatch
crates/perry-hir/src/lower/expr_call/native_module.rs
_getActiveHandles and _getActiveRequests now lower to native process calls, with unit coverage for matching and non-matching methods.

Child process eval execution

Layer / File(s) Summary
Eval-aware command construction
crates/perry-runtime/src/child_process/options.rs
Eval-style self-relaunches select the Node interpreter using PERRY_FORK_EXECPATH or node, with command construction and detection tests.

Builtin module feature gating

Layer / File(s) Summary
getBuiltinModule compilation tracking
crates/perry/src/commands/compile/types.rs, crates/perry/src/commands/compile/collect_modules/feature_detect.rs
Compilation state records getBuiltinModule usage, and HIR scanning recognizes direct and extracted calls.
HTTP/2 constants cache and feature gates
crates/perry/src/commands/compile/optimized_libs/freshness.rs, crates/perry/src/commands/compile/optimized_libs/tests.rs
HTTP/2 constants are included when either http2 is imported or getBuiltinModule is used, covering cache keys and cross-features.
Dynamic builtin namespace validation
crates/perry/tests/createrequire_builtin_modules.rs
A regression test verifies extracted getBuiltinModule access exposes expected HTTP/2 namespace keys.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Source
  participant HIRFeatureDetection
  participant CompilationContext
  participant OptimizedLibraries
  Source->>HIRFeatureDetection: scan getBuiltinModule usage
  HIRFeatureDetection->>CompilationContext: set uses_get_builtin_module
  CompilationContext->>OptimizedLibraries: evaluate HTTP/2 constants requirement
  OptimizedLibraries-->>CompilationContext: update cache key and cross-features
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR appears to resolve the linked node:process parity failures in #6781 and reports the process suite passing.
Out of Scope Changes check ✅ Passed The changes stay within the process parity and eval-launch fix scope, with matching tests and no obvious unrelated edits.
Title check ✅ Passed The title is concise and clearly summarizes the main change: completing Node.js 26.5.0 process parity.
Description check ✅ Passed The description matches the template and includes summary, changes, related issue, test plan, screenshots/output, and checklist.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/process-node26-parity

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/perry-runtime/src/child_process/options.rs (1)

327-343: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Quote the shell-invoked command string.

cp_build_command() joins program and args into one command string before passing it to the shell; if program comes from a configured PERRY_FORK_EXECPATH like C:\Program Files\nodejs\node.exe, that path is split by the shell and the self-eval launch fails. Preserve a single-token representation for the selected interpreter and build the shell command with platform-appropriate escaping/quoting.

🤖 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 `@crates/perry-runtime/src/child_process/options.rs` around lines 327 - 343,
Update cp_build_command so the shell-invoked command preserves program and
argument boundaries, especially interpreter paths containing spaces such as
PERRY_FORK_EXECPATH. Quote or escape the selected program and each argument
using platform-appropriate shell rules before joining them into the command
string, while retaining the existing shell selection behavior.
🧹 Nitpick comments (1)
crates/perry-hir/src/lower/expr_call/native_module.rs (1)

1876-1883: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the lowering result, not only the predicate.

This test would pass even if the new NativeMethodCall arm were removed. Add a focused lowering/compile assertion for both calls, or rename the test so it does not imply native-dispatch coverage.

🤖 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 `@crates/perry-hir/src/lower/expr_call/native_module.rs` around lines 1876 -
1883, Expand process_active_array_helpers_use_native_dispatch beyond predicate
checks by exercising lowering or compilation for both _getActiveHandles and
_getActiveRequests and asserting each produces the NativeMethodCall path.
Alternatively, rename the test to describe only is_process_active_array_helper
predicate behavior, without implying lowering coverage.
🤖 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 `@crates/perry-runtime/src/child_process/options.rs`:
- Around line 297-305: Update cp_should_use_node_interpreter to recognize -p,
--print, and the --print= form alongside the existing eval flags, so
self-launches using Node print mode select the Node interpreter. Extend the
existing eval-mode tests to cover all print aliases.

---

Outside diff comments:
In `@crates/perry-runtime/src/child_process/options.rs`:
- Around line 327-343: Update cp_build_command so the shell-invoked command
preserves program and argument boundaries, especially interpreter paths
containing spaces such as PERRY_FORK_EXECPATH. Quote or escape the selected
program and each argument using platform-appropriate shell rules before joining
them into the command string, while retaining the existing shell selection
behavior.

---

Nitpick comments:
In `@crates/perry-hir/src/lower/expr_call/native_module.rs`:
- Around line 1876-1883: Expand process_active_array_helpers_use_native_dispatch
beyond predicate checks by exercising lowering or compilation for both
_getActiveHandles and _getActiveRequests and asserting each produces the
NativeMethodCall path. Alternatively, rename the test to describe only
is_process_active_array_helper predicate behavior, without implying lowering
coverage.
🪄 Autofix (Beta)

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5f646265-b66d-4048-8a30-96b876a6475c

📥 Commits

Reviewing files that changed from the base of the PR and between 6899e75 and 879a810.

📒 Files selected for processing (7)
  • crates/perry-hir/src/lower/expr_call/native_module.rs
  • crates/perry-runtime/src/child_process/options.rs
  • crates/perry/src/commands/compile/collect_modules/feature_detect.rs
  • crates/perry/src/commands/compile/optimized_libs/freshness.rs
  • crates/perry/src/commands/compile/optimized_libs/tests.rs
  • crates/perry/src/commands/compile/types.rs
  • crates/perry/tests/createrequire_builtin_modules.rs

Comment thread crates/perry-runtime/src/child_process/options.rs
@proggeramlug
proggeramlug merged commit afd919d into main Jul 27, 2026
29 of 31 checks passed
@proggeramlug
proggeramlug deleted the fix/process-node26-parity branch July 27, 2026 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Confirmed defect or regression parity Compatibility gap with Node.js, ECMAScript, or the supported ecosystem rust Pull requests that update rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[parity] node:process — 4 failing node-suite tests (2026-07-22 baseline)

2 participants