Skip to content

fix(ssh): select a complete Node/npm toolchain for the relay - #9165

Merged
brennanb2025 merged 1 commit into
mainfrom
brennanb2025/bug-8720-triage
Jul 17, 2026
Merged

brennanb2025 merged 1 commit into
mainfrom
brennanb2025/bug-8720-triage

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Jul 17, 2026 •

Copy link
Copy Markdown
Contributor

Problem

On SSH/relay hosts, system /usr/bin/node could be selected without an accompanying npm, so the relay's npm install silently skipped the node-pty native build (lifecycle scripts blocked under nvm-managed npm) — leaving terminals broken on the remote. (Reported in #8450; the closely-related #8720 was already resolved on main via #8686.)

Fix

Adds a Node/npm toolchain probe (ssh-remote-node-toolchain-probe.ts) and requires a coherent colocated Node+npm toolchain, skipping incomplete system-Node candidates on both POSIX and Windows so a usable toolchain is chosen.

Validation

104 focused SSH tests pass (5 skipped), typecheck + targeted lint/format clean, regression-proof (tests fail with the fix reverted).

Fixes #8450

Manual validation

  • Built the relay, launched the exact PR worktree in an isolated Electron profile, and verified app identity as brennanb2025/bug-8720-triage.
  • Reproduced [Bug]: SSH relay selects system Node without npm instead of complete NVM toolchain #8450 on a throwaway Ubuntu 24.04 arm64 SSH container: non-interactive SSH exposed /usr/bin/node v18.19.1 with no npm, while an NVM install below the standard .bashrc non-interactive guard provided Node v22.23.1 + npm 10.9.8.
  • Orca skipped the incomplete system candidate, logged Found node via path probe: /root/.nvm/versions/node/v22.23.1/bin/node, installed native dependencies, and started the relay.
  • Through the rendered Electron terminal and real keyboard input, the remote PTY reported the NVM Node/npm pair and changed the bundled demo-project; backing SSH checks confirmed the proof file, Git status, the relay daemon running under the NVM Node, and successful require("node-pty") + require("@parcel/watcher").
  • Sanity-checked a normal node:22-bookworm host: Orca retained /usr/local/bin/node + /usr/local/bin/npm, native dependencies loaded, and a real remote-terminal repository action succeeded.
  • Both live connection calls completed within 6 seconds on this machine. The resolver still uses one combined SSH exec per candidate and adds no polling, caching, or terminal hot-path work; Electron reported zero console errors.
  • Cleanup covered both remote proof files/repos, relay/container state, the isolated Electron profile, and temporary keys/images.

Reliability invariant: an SSH relay bootstrap must reject a Node candidate without a colocated runnable npm, then preserve a native-backed remote PTY that accepts input and renders output. The oracle was visible terminal output plus remote process, file, Git, and native-module checks; this live test-orca-ssh evidence covers a macOS client against Linux arm64, while Linux/Windows desktop clients, WSL, and a Windows SSH target remain unexercised live (focused automated PowerShell coverage remains in the PR).

Evidence

Broken-system-Node host: the same terminal shows the original non-interactive /usr/bin/node + missing npm state, followed by Orca's selected NVM Node/npm pair and a successful remote repo action.

PR #9165 broken host selects NVM Node and npm

Normal host: the relay retains the standard colocated /usr/local/bin/node + npm toolchain and the remote repo action succeeds.

PR #9165 normal host regression check

@coderabbitai

coderabbitai Bot commented Jul 17, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Remote Node resolution now validates both Node and npm versions for POSIX and Windows candidates. New probe builders configure the appropriate PATH, execute Node and npm version commands, and parse marker-based output with legacy Node-only compatibility. Resolver tests cover rejection of an incomplete system Node installation and selection of a complete NVM toolchain.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description lacks required template sections like Screenshots, AI Review Report, Security Audit, and Notes. Add the missing template sections and the required testing checklist details, plus a review summary and security/platform notes.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The change matches #8450 by requiring a complete Node/npm pair and skipping incomplete system Node candidates until a usable toolchain is found.
Out of Scope Changes check ✅ Passed The changes stay focused on SSH Node/npm toolchain detection and tests, with no unrelated functionality added.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly describes the main SSH relay change: requiring a complete Node/npm toolchain.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

@brennanb2025
brennanb2025 merged commit f544820 into main Jul 17, 2026
2 checks passed
brennanb2025 added a commit that referenced this pull request Jul 18, 2026
… (#9255)

* fix(ssh): probe npm via prepended PATH, not colocated with node (#9165)

The remote Node/npm toolchain gate invoked npm by its absolute path
<nodeBinDir>/npm (POSIX) / npm.cmd (Windows, behind a Test-Path
colocation check). But deploy (commandWithNodePath) runs bare `npm`
with nodeBinDir merely prepended to PATH, so npm can resolve from
anywhere on PATH.

A host whose only resolvable node has npm elsewhere on PATH (e.g. node
symlinked into a dir without npm) deployed fine on v1.4.144, but after
upgrade the candidate is rejected with no fallback → SSH/relay
connection fails to establish.

Make the probe resolve npm exactly the way deploy does — bare
`npm --version` under the same prepended PATH — so it still confirms npm
is runnable (the #8450 concern) without requiring colocation. Windows
now prepends the backslash-form dir (matching deploy) so bare-command
PATH lookup resolves reliably.

* test(ssh): cover split Node npm PATH resolution
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
…lyai#9165) (stablyai#9255)

* fix(ssh): probe npm via prepended PATH, not colocated with node (stablyai#9165)

The remote Node/npm toolchain gate invoked npm by its absolute path
<nodeBinDir>/npm (POSIX) / npm.cmd (Windows, behind a Test-Path
colocation check). But deploy (commandWithNodePath) runs bare `npm`
with nodeBinDir merely prepended to PATH, so npm can resolve from
anywhere on PATH.

A host whose only resolvable node has npm elsewhere on PATH (e.g. node
symlinked into a dir without npm) deployed fine on v1.4.144, but after
upgrade the candidate is rejected with no fallback → SSH/relay
connection fails to establish.

Make the probe resolve npm exactly the way deploy does — bare
`npm --version` under the same prepended PATH — so it still confirms npm
is runnable (the stablyai#8450 concern) without requiring colocation. Windows
now prepends the backslash-form dir (matching deploy) so bare-command
PATH lookup resolves reliably.

* test(ssh): cover split Node npm PATH resolution
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]: SSH relay selects system Node without npm instead of complete NVM toolchain

1 participant