fix(devices): Test connection says when Node or npm is missing on an SSH host#33198

open2
main
GitHub fork: fix/ssh-device-probe-error
opened3d agobyWwatashiwaaniket
View on GitHubTwo-way sync enabledPR last imported 18h ago

Problem

Test connection on a Machine over SSH host whose Node is only on the interactive PATH (mise, nvm, and similar) shows Device probe host failed: Could not communicate with device support. Try refreshing devices. Refreshing cannot fix a missing executable on the other machine. The server already knows the cause: the bootstrap prints Node is missing from the non-interactive SSH PATH. Test connection drops it.

readiness and agentReadinessIfSupported already turn a probe failure into a DeviceHostUnavailableError with sshProbeFailureMessage. testHost does that only for Windows hosts, which #30941 added. Every other probe failure still becomes DeviceOperationError { reason: "request_failed" }.

Change

  • testHost treats every DeviceHostError from the probe step the way readiness does. Windows hosts keep their message. Failures in later steps, such as decoding the probe result, keep the generic error.
  • sshProbeFailureMessage also recognizes the two other prerequisite lines the probe script prints: Node older than 22, and npm missing. Each one gets its own message. As before, the user sees only these known lines from stderr. Other stderr stays in the server log, because it can carry private diagnostics.

Test connection now shows, per environment:

text
1MacBook Pro is unavailable: Connected over SSH, but Node is not on this host's PATH for non-interactive SSH commands.

An unreachable host or failed authentication now reads Could not connect to this host over SSH., which is the same message readiness already gives.

Scope and approval

Fixes #32908. Server only. The web and mobile settings already show the error's message, so no client changes are needed.

Not included: the issue's other ideas, which are a Node path override, version-manager discovery, and a stderr details panel. Those change behavior or the UI and need their own decision.

Verification

Failing test first. The Windows-only Test connection wire test in DeviceService.test.ts is now a table. It adds missing Node, Node older than 22, missing npm, and an unreachable host. All four fail on main with DeviceOperationError { reason: "request_failed" } and pass here. Every case still asserts that the private stderr line does not reach the encoded message.

Runtime transcript: real subprocesses through DeviceService.testHost, using the real ssh → sh -c → bootstrap script path. An ssh shim on PATH answers ssh -G with a non-local address. It then runs the remote command locally, the way sshd would, with env -i PATH=/usr/bin:/bin and Homebrew dirs removed. That simulates a host whose Node exists only in interactive shells. The decoded wire error:

text
1main: DeviceOperationError: Device probe host failed: Could not communicate with device support. Try refreshing devices.
2branch: DeviceHostUnavailableError: MacBook Pro is unavailable: Connected over SSH, but Node is not on this host's PATH for non-interactive SSH commands.

vp test run src/device/DeviceService.test.ts src/device/SshDeviceHost.test.ts: 77 passed. vp run check:changed: format, lint, knip, graphql, typecheck (3 packages) and tests pass.

Not checked: a real remote macOS host over SSH, a real host with Node older than 22 or without npm (only the unit table covers those), and the Settings UI. The Settings UI shows the message unchanged.

Claude Opus 5.5 in T3 Code through the Antigravity harness.

Checks

21 successful · 10 skipped
All checks passed
f45d8e5

Added 1 commit

  • fix(devices): Test connection says when Node or npm is missing on an SSH hoste8252c3
Synced from GitHub

Approvability

Verdict: Approved at e8252c3

Macroscope's review found this PR approvable — Test connection now provides specific guidance when an SSH host lacks npm or a supported Node version. The change is limited to error reporting and retains fixed messages rather than exposing arbitrary remote stderr.

You can add or adjust custom eligibility rules. Learn more.

Synced from GitHub

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The SSH probe formatter now maps missing Node, outdated Node, and missing npm diagnostics to specific guidance. It applies to probe errors beyond Windows SSH failures. Tests cover these cases and verify serialized error details.

Changes: SSH probe diagnostics

Layer / File(s)Summary
Map and format probe diagnostics
apps/server/src/device/DeviceService.ts
The formatter maps exact stderr diagnostics for missing Node, Node below version 22, and missing npm. For exit code 1, it returns guidance for the first matching diagnostic. Probe errors use the formatter for any DeviceHostError at the probe step.
Test probe diagnostic guidance
apps/server/src/device/DeviceService.test.ts
The parameterized test covers Windows failures, missing Node, outdated Node, missing npm, and an unreachable host. It checks each case’s reason in the serialized error.

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

Suggested reviewers: shivamhwp, juliusmarminge

Merge Risk: External image Minimal · up to f45d8

The change provides specific connection guidance for the reported SSH prerequisites, with no identified issue requiring a fix before merge.

Pre-merge checks: Passed 4

Check nameStatusExplanation
Description checkPassedThe description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the issue, implementation, scope, test coverage, observed results, and unverified scen…
Title checkPassedThe title is concise, uses the repository's conventional format, and clearly identifies the main change: showing actionable guidance when Node or npm is missing on an SSH host.
Linked Issues checkPassed[#32908] testHost now converts every probe-step DeviceHostError to DeviceHostUnavailableError. The formatter gives distinct messages for missing Node, Node older than 22, and missing npm. It pre…
Out of Scope Changes checkPassedThe whole-PR diff changes only apps/server/src/device/DeviceService.ts and its focused test. The implementation adds prerequisite-specific SSH probe messages, routes probe failures through the exist…
Full details: Description check

Explanation

The description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the issue, implementation, scope, test coverage, observed results, and unverified scenarios.

Full details: Linked Issues check

Explanation

[#32908] testHost now converts every probe-step DeviceHostError to DeviceHostUnavailableError. The formatter gives distinct messages for missing Node, Node older than 22, and missing npm. It preserves the SSH connectivity message for unreachable hosts and the Windows message for Windows hosts. The error includes the device host label. The test table covers these cases and verifies that private SSH diagnostics do not reach the serialized message. The change does not include the issue's separate path override, version-manager discovery, or stderr panel requests.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff changes only apps/server/src/device/DeviceService.ts and its focused test. The implementation adds prerequisite-specific SSH probe messages, routes probe failures through the existing formatter, and adds regression coverage for the linked issue. No unrelated client, mobile, documentation, or behavior changes are present.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
External image Recent review info

External image Run configuration

External image Commits

Reviewing files that changed from the base of the PR and between e8252c3c3632764a5596921ab0ac0d8cde258a7d and f45d8e59c5713898f15612ef629b585f413e6507.

External image Files selected for processing: 1

  • apps/server/src/device/DeviceService.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head e8252c3c3632764a5596921ab0ac0d8cde258a7d on linux, using the dev web app in /usr/bin/google-chrome driven by the repo's playwright dependency.

started github/main (fa2f4ff2a2) in a separate worktree first, then stopped its server before starting the exact pr head. used settings → devices → add source → machine over ssh → test connection. an actual openssh server on port 22222 accepted a key-authenticated login into a minimal linux chroot. no ssh shim, mocked probe, fixture response, or demo data was used.

  • main reproduced the bug: missing node became "could not communicate with device support. try refreshing devices."
  • the pr displayed the specific missing-node guidance for the same account, target, port, key, and host label.
  • installing a real node 20.20.2 binary displayed the node 22 requirement.
  • installing real node 24.22.1 without npm displayed the missing-npm guidance.
  • adding real npm 11.19.1 made the check pass. the host saved and reopened successfully.
  • an unused ssh port displayed "could not connect to this host over ssh." restoring the port passed again, as recorded below.
  • the local-host shortcut on port 22 still displayed "already available locally."
  • added a real git repository through the app, selected codex gpt-6.1-sol, and sent a new thread. the provider read README.md and ran git status --short; the client displayed the correct heading and clean working tree.

blacksmith checks passed: 77 focused device tests, plus vp run check:changed 637507056c9e46e8bca21e557e8c61044a314563 (format, lint, knip, graphql, typecheck, 69 tests). the explicit parent ref was needed after fetching history into the shallow runner. existing pr ci reports 31 successful checks.

short runtime transcript

main: missing node produces the generic refresh error

pr: same missing-node host displays actionable guidance

pr: a real node 20 installation displays the node 22 requirement

pr: real node without npm displays the npm guidance

pr: failed ssh connection recovers after restoring the port

pr: completed real codex turn in the web client

Not verified

native ios/android and the electron shell were not exercised. this linux vm has no mobile devices, simulators, or android sdk. windows-specific guidance, a macos ssh host, actual version-manager shell initialization, cross-machine ssh networking, relay/tunnel connections, and device-tool installation/launch were not exercised. ssh verification used a real local sshd with a chroot, rather than a separate physical machine. arbitrary private-stderr suppression has automated coverage only. malformed probe-result handling was not exercised.

Added 1 commit

  • Merge remote-tracking branch 'origin/main' into fix/ssh-device-probe-errorf45d8e5

Evidence

Open Settings → Devices → Add source → Machine over SSH. Enter the synthetic SSH host, click Test connection, and expand Show error. Repeat with Node absent, Node 20.20.2, and Node 24.22.1 without npm.

Environment on both sides: real web client in Chromium on Linux, 1280×800, light theme, fresh isolated database, synthetic project, and real passwordless OpenSSH with an isolated filesystem. No RPC or error-response mocks. Stills were captured after matching screenshots 500 ms apart; all captures were visually inspected.

Scenario driver · SSH fixture switcher · SSH fixture setup

Base displays the generic communication/refresh error in all three cases. Head names the host and gives distinct missing-Node, old-Node, and missing-npm explanations. Incidental SSH stderr is absent from the displayed messages.

AI-assisted; verified with the tests and tools listed above.

Not shown:

  • The original macOS/mise setup and Electron desktop client were not exercised. The proof uses the real web client and an isolated Linux SSH host producing the same three prerequisite failures.