Repository navigation
feat(runtime): require Node 24 for servers and remote hosts - #26171
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📥 CommitsReviewing files that changed from the base of the PR and between 798acfbb65c31ce264b7fa6626487099007d094f and 9a78990e6e4e1d060ad40368789c21ebfd0bbec3. 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe Orcad build now produces a Node 24 server entry and a separate compatibility launcher. Local and SSH launch paths select the appropriate entry, and build identity uses the launcher for split-entry installations. SSH host-runtime checks and relay bundles now require Node 24 or newer, with pinned Node selected by default. Managed-hook CI, runtime tests, E2E setup, reliability-gate records, and SSH documentation reflect the updated runtime. Priority: ➖ Normal Merge Risk: 🔵 Low · up to A WSL benchmark can reject an environment that has a usable Node 24, producing a false negative in benchmark validation. The issue is limited to that workflow and does not block the runtime path. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 47.06% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 51 functions across 58 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
eb41bd7 to
99cf546
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
4af01711-6f7a-484c-bf0b-8b8276489510
📥 Commits
Reviewing files that changed from the base of the PR and between 00984cc and 99cf5463ff8dbf20fc4ca10e78062676d38237bb.
📒 Files selected for processing (98)
.github/workflows/e2e.yml.github/workflows/pr.ymlconfig/reliability-gates.jsoncconfig/scripts/build-orcad-prebuilds.mjsconfig/scripts/build-orcad.mjsconfig/scripts/build-relay.mjsconfig/scripts/check-runtime-launcher-protocol-ratchet.mjsconfig/scripts/node-server-change-scope.mjsconfig/scripts/node-server-test-paths.mjsconfig/scripts/orcad-entry-build.mjsconfig/scripts/orcad-prebuild-slot-contents.mjsconfig/scripts/orcad-prebuild-slot-contents.test.mjsconfig/scripts/orcad-terminal-smoke-change-scope.mjsconfig/scripts/pr-auxiliary-preflight-admission.test.mjsconfig/scripts/pr-code-change-scope.mjsconfig/scripts/pr-code-change-scope.test.mjsconfig/scripts/pr-workflow-parallelism.test.mjsconfig/scripts/profile-state-worker-smoke.mjsconfig/scripts/run-node-server-tests.mjsconfig/scripts/runtime-serve-terminal-smoke.mjsconfig/scripts/smoke-managed-hook-runtime-node24.mjsconfig/scripts/wsl-hook-relay-reattach-benchmark.mjsdocs/reference/orcad-operations.mddocs/site/content/docs/ssh.mdxsrc/main/agent-hooks/wsl-hook-relay-deps.tssrc/main/agent-hooks/wsl-hook-relay-launch.tssrc/main/agent-hooks/wsl-hook-relay-manager.tssrc/main/agent-hooks/wsl-relay-runtime-policy.test.tssrc/main/ai-vault-search/session-search-in-process-service.tssrc/main/ai-vault-search/session-search-sqlite-support.tssrc/main/browser/wsl-browser-network-relay-launch.test.tssrc/main/browser/wsl-browser-network-relay-launch.tssrc/main/ipc/runtime-watcher-disposal-owners.tssrc/main/orcad/launcher.tssrc/main/orcad/main-preflight-order.test.tssrc/main/orcad/main.tssrc/main/orcad/orcad-bundled-runtime.test.tssrc/main/orcad/orcad-bundled-runtime.tssrc/main/orcad/orcad-health.tssrc/main/orcad/orcad-launcher-isolation.integration.test.tssrc/main/orcad/orcad-local-serve-selection.test.tssrc/main/orcad/orcad-local-serve-selection.tssrc/main/orcad/orcad-managed-stop.test.tssrc/main/orcad/orcad-profile-preflight.test.tssrc/main/orcad/orcad-profile-preflight.tssrc/main/persistence/profile-state/profile-state-database.tssrc/main/runtime/mobile-session-terminal-retirement.tssrc/main/skills/skill-delete/recovery.tssrc/main/skills/skill-delete/staging.tssrc/main/sqlite/sync-database.tssrc/main/ssh/orcad-activation-gate.tssrc/main/ssh/orcad-local-build-hash.tssrc/main/ssh/orcad-remote-build-hash.integration.test.tssrc/main/ssh/orcad-remote-build-hash.tssrc/main/ssh/orcad-remote-launch-windows.tssrc/main/ssh/orcad-remote-launch.tssrc/main/ssh/orcad-remote-preflight.tssrc/main/ssh/orcad-remote-runtime-control.tssrc/main/ssh/orcad-remote-runtime.tssrc/main/ssh/orcad-remote-shell-commands.integration.test.tssrc/main/ssh/orcad-remote-windows-commands.test.tssrc/main/ssh/orcad-remote-windows-scripts.test.tssrc/main/ssh/orcad-windows-host-script.tssrc/main/ssh/relay-socket-path-limit.test.tssrc/main/ssh/ssh-relay-cross-version-isolation.test.tssrc/main/ssh/ssh-relay-deploy-incumbent-verdict.test.tssrc/main/ssh/ssh-relay-deploy-pinned-node.test.tssrc/main/ssh/ssh-relay-deploy-staged-upload.test.tssrc/main/ssh/ssh-relay-deploy-windows-pipe.test.tssrc/main/ssh/ssh-relay-deploy.test.tssrc/main/ssh/ssh-relay-gc-retry.test.tssrc/main/ssh/ssh-relay-host-node-addons.test.tssrc/main/ssh/ssh-relay-host-node-addons.tssrc/main/ssh/ssh-relay-native-deps-install-fixture.tssrc/main/ssh/ssh-relay-pinned-node-install.test.tssrc/main/ssh/ssh-relay-pinned-node.test.tssrc/main/ssh/ssh-relay-pinned-node.tssrc/main/ssh/ssh-relay-runtime-ladder.test.tssrc/main/ssh/ssh-relay-runtime-ladder.tssrc/main/ssh/ssh-relay-runtime-resolution.test.tssrc/main/ssh/ssh-relay-sftp-namespace-install.test.tssrc/main/ssh/ssh-remote-node-addon-resolution.test.tssrc/main/ssh/ssh-remote-node-addon-resolution.tssrc/main/ssh/ssh-remote-node-install-guidance.tssrc/main/ssh/ssh-remote-node-resolution.test.tssrc/main/ssh/ssh-remote-node-resolution.tssrc/main/ssh/ssh-remote-node-toolchain-probe.test.tssrc/main/ssh/ssh-remote-node-toolchain-probe.tssrc/relay/fs-stream-registry.tssrc/relay/git-response-stream.tssrc/relay/pty-handler.tssrc/relay/relay-agent-process-lifetime.tssrc/shared/grouped-by.tssrc/shared/orcad-artifacts.tssrc/shared/orcad-build-identity.test.tssrc/shared/orcad-build-identity.tssrc/shared/ssh-types.tstests/e2e/fixtures/docker-ssh-relay/Dockerfile
💤 Files with no reviewable changes (9)
- src/relay/pty-handler.ts
- src/main/runtime/mobile-session-terminal-retirement.ts
- src/main/skills/skill-delete/staging.ts
- src/relay/git-response-stream.ts
- src/shared/grouped-by.ts
- src/relay/relay-agent-process-lifetime.ts
- src/relay/fs-stream-registry.ts
- src/main/ipc/runtime-watcher-disposal-owners.ts
- src/main/ai-vault-search/session-search-sqlite-support.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
| @@ -272,8 +272,8 @@ async function main() { | |||
| ]), | |||
| { allowFailure: true } | |||
| ) | |||
| if (nodeVersion.status !== 0 || Number(nodeVersion.stdout.trim().split('.')[0]) < 18) { | |||
| throw new Error(`WSL distro '${distro}' needs Node.js 18 or newer to run Orca's relay`) | |||
| if (nodeVersion.status !== 0 || Number(nodeVersion.stdout.trim().split('.')[0]) < 24) { | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Check all WSL Node candidates before rejecting the distro.
If node on PATH is Node 18 and $HOME/.local/bin/node is Node 24, the probe on Line 271 returns Node 18 and this condition stops the benchmark. The production launcher checks later candidates and can start the relay in that distro. Make the probe select a candidate that meets the Node 24 minimum before it reports failure.
99cf546 to
a441274
Compare
There was a problem hiding this comment.
🔇 Additional comments (14)
config/scripts/orcad-entry-build.mjs (1)
58-75: LGTM!src/main/orcad/launcher.ts (1)
10-23: LGTM!src/main/ssh/orcad-local-build-hash.ts (1)
1-9: LGTM!src/main/orcad/orcad-health.ts (1)
11-11: LGTM!Also applies to: 60-60, 85-85, 96-96
src/shared/orcad-build-identity.test.ts (1)
1-34: LGTM!src/shared/orcad-build-identity.ts (1)
1-19: LGTM!src/main/ssh/orcad-remote-launch.ts (1)
93-93: LGTM!Also applies to: 120-120
src/main/ssh/orcad-remote-build-hash.ts (1)
1-2: LGTM!Also applies to: 33-39
src/main/ssh/orcad-remote-build-hash.integration.test.ts (1)
18-42: LGTM!src/main/ssh/orcad-remote-windows-scripts.test.ts (1)
358-371: LGTM!src/main/ssh/orcad-remote-shell-commands.integration.test.ts (1)
135-137: LGTM!Also applies to: 379-402
config/scripts/pr-code-change-scope.mjs (1)
31-31: 🗄️ Data Integrity & IntegrationThe concern is refuted.
.github/workflows/pr.ymlusesmanaged_hook_node24for the path output, job, required-check list, and aggregation. The related tests also use the renamed key. No stalemanaged_hook_node18reference appears in the inspected workflow and script scopes..github/workflows/e2e.yml (1)
695-697: 🎯 Functional CorrectnessThe concern is unsubstantiated as a functional defect. The SSH journey does not invoke
nodeornpmremotely. It runs shell commands,/bin/sh,printf,test, and file checks. The symlink setup alone does not establish a required PATH contract or a failure.docs/reference/orcad-operations.md-11-11 (1)
11-11: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.Clarify the Windows launch modifier.
“Windows managed launches” is ambiguous. Separate the two launch modes and hyphenate the compound modifier.
Proposed wording
-Local and Windows managed launches start `orcad-server.js` directly. +Local launches and Windows-managed launches start `orcad-server.js` directly.Source: Linters/SAST tools
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
b4151869-981d-4207-8553-e79ba6b0766c
📥 Commits
Reviewing files that changed from the base of the PR and between 99cf5463ff8dbf20fc4ca10e78062676d38237bb and a441274.
📒 Files selected for processing (15)
.github/workflows/e2e.ymlconfig/scripts/orcad-entry-build.mjsconfig/scripts/pr-code-change-scope.mjsdocs/reference/orcad-operations.mdsrc/main/orcad/launcher.tssrc/main/orcad/orcad-health.tssrc/main/orcad/orcad-launcher-isolation.integration.test.tssrc/main/ssh/orcad-local-build-hash.tssrc/main/ssh/orcad-remote-build-hash.integration.test.tssrc/main/ssh/orcad-remote-build-hash.tssrc/main/ssh/orcad-remote-launch.tssrc/main/ssh/orcad-remote-shell-commands.integration.test.tssrc/main/ssh/orcad-remote-windows-scripts.test.tssrc/shared/orcad-build-identity.test.tssrc/shared/orcad-build-identity.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.
798acfb to
9a78990
Compare
9a78990 to
dc0feb8
Compare
nwparker
changed the title
e7c9ba0 to
245a6c7
Compare
dc0feb8 to
e775d14
Compare
nwparker
changed the title
…26823) This reverts commit 46460d4 (#26171). Pause the Node 24 requirement so existing SSH and WSL users are not surprised by a host-runtime upgrade requirement in the next release. This is a precautionary rollout hold requested to avoid near-term user disruption, not a cancellation of the Node 24 transition. Restore the prior Node 18 host minimum and companion build targets. The previously shipped bundled Node 24 runtime and compatibility launcher remain in place. Preserve the original change in a separate draft PR for reintroduction after compatibility and rollout validation.
Reapply the Node 24 server and host-runtime policy from #26171 after its temporary compatibility rollback. Keep this change in a draft PR until host compatibility, fallback behavior and rollout communication have been reviewed so existing users are not unexpectedly forced to upgrade.
ELI5
Orca servers now require Node 24 or newer. The bundled runtime meets that requirement; explicitly selected host runtimes must also meet it.
What Changed
Server code and host-runtime discovery previously accepted Node 18. They now require Node 24, with matching installation guidance. The bundled SSH runtime remains the default established on main. WSL follows the same host minimum and rebuilds cached launchers after the policy change.
Server, relay and child-process builds target Node 24. CI verifies the managed-hook runtime on Node 24, and remote fixtures and SSH documentation reflect the new default. The small compatibility launcher remains built for Node 18, allowing older host Node to start the bundled runtime.
Why
Using the runtime shipped with Orca gives server code a consistent Node 24 baseline while preserving bootstrap on older hosts. Retaining host Node 18 as a server runtime would prevent adopting that baseline.
Linked Issue
N/A — maintainer change.
Visual Proof
N/A — runtime selection and build changes have no visible UI change.
Testing
Final macOS validation after merging current main: 436 focused runtime, WSL-policy, SSH and CI-scope tests passed (one environment-gated test skipped). A separate launcher/fallback run passed all 21 tests, using a real Node 18 launcher and bundled Node 24.21.0. Full
pnpm tc,pnpm run check:code-quality:changed origin/main,pnpm run build:relay,pnpm run build:orcad, and the managed-hook Node 24 smoke passed.The merge preserves main's newer SSH fallback behavior and launcher crash regression. Windows and missing-artifact failure guidance now consistently requires Node 24; the WSL benchmark probe checks later candidates when PATH points to an older Node. A controlled shell check verifies Node 18 is skipped in favor of Node 24. After correcting a stale deployment-test expectation, all 43 affected deployment/fallback tests passed locally. Fresh CI on final commit d43bd15 passed all applicable checks, including all five test shards, Windows x64/ARM inbox and preview OpenSSH, packaged Windows runtime switching, macOS/Linux persistence, SSH watcher isolation, and cross-version wire compatibility.
Review
All three sub-agent adversarial reviews returned CLEAN for both split boundaries, followed by a CLEAN final Claude Opus 5.5 review via Orca orchestration. An independent rebase review verified that every policy change and the launcher crash fix are preserved.
Agent skill upstream boundary
Notes
The launcher is merged in #26374. This PR contains the separate Node 24 runtime policy change and preserves main’s newer SSH fallback behavior, bundled-runtime default, and launcher crash regression. No remote wire format change.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)