fix(cli): show SSH port-forward hint for remote dashboard access (#5925)#5929
Open
atulya-singh wants to merge 2 commits into
Open
fix(cli): show SSH port-forward hint for remote dashboard access (#5925)#5929atulya-singh wants to merge 2 commits into
atulya-singh wants to merge 2 commits into
Conversation
…DIA#5925) Signed-off-by: Atulya Singh <atulyarajsingh@gmail.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds SSH-session detection and copy-pastable ChangesSSH Port-Forward Hint
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/lib/dashboard-url-command.test.ts (1)
88-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a non-quiet assertion for the
session/nonebranch.These cases only exercise the tokenized URL path.
runDashboardUrlCommandnow prints the SSH hint in the plain-URL branch too, but the visiblesessioncoverage is still quiet-only, so a regression there would slip through.🤖 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 `@src/lib/dashboard-url-command.test.ts` around lines 88 - 138, The test coverage for runDashboardUrlCommand is missing a non-quiet assertion for the plain-URL path when no tokenized session is used, so add a case that exercises the session/none branch with quiet false and verifies the SSH hint is printed there too. Update the dashboard URL command tests around runDashboardUrlCommand and makeSinks to assert the Remote access SSH hint appears in the non-quiet plain-URL output, while keeping the quiet-mode behavior unchanged.
🤖 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 `@docs/deployment/deploy-to-remote-gpu.mdx`:
- Line 154: Split the combined sentence in the deployment guide into two
separate source lines to follow the one-sentence-per-line MDX rule. Update the
surrounding prose in the deployment section so the first sentence about running
nemoclaw over SSH and the command output stands alone, and the second sentence
about opening the dashboard URL on your workstation is on its own line.
In `@src/lib/onboard/dashboard.ts`:
- Around line 496-499: Pass env explicitly into buildSshForwardHintLines from
this OnboardDashboard flow instead of letting it read process.env implicitly.
Thread env through OnboardDashboardDeps here the same way dashboard-url already
does, and update the call site in the dashboard summary rendering so the SSH
hint stays deterministic and testable. Also make sure any related SSH hint or
detection helpers use the passed env argument rather than ambient shell state.
In `@src/lib/onboard/ssh-forward-hint.ts`:
- Around line 27-37: The helper serverAddressFromSshConnection should not derive
the SSH destination from SSH_CONNECTION, since it only exposes the remote socket
address and loses the original host, port, aliases, and ProxyJump details.
Update the ssh-forward-hint flow to return a placeholder unless the caller can
pass the original destination string explicitly, and keep
serverAddressFromSshConnection limited to fallback behavior rather than
constructing the ssh -L example target.
---
Nitpick comments:
In `@src/lib/dashboard-url-command.test.ts`:
- Around line 88-138: The test coverage for runDashboardUrlCommand is missing a
non-quiet assertion for the plain-URL path when no tokenized session is used, so
add a case that exercises the session/none branch with quiet false and verifies
the SSH hint is printed there too. Update the dashboard URL command tests around
runDashboardUrlCommand and makeSinks to assert the Remote access SSH hint
appears in the non-quiet plain-URL output, while keeping the quiet-mode behavior
unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e1935265-b6a6-458e-8355-ff9cab336012
📒 Files selected for processing (7)
docs/deployment/deploy-to-remote-gpu.mdxdocs/get-started/quickstart.mdxsrc/lib/dashboard-url-command.test.tssrc/lib/dashboard-url-command.tssrc/lib/onboard/dashboard.tssrc/lib/onboard/ssh-forward-hint.test.tssrc/lib/onboard/ssh-forward-hint.ts
Comment on lines
+27
to
+37
| * Resolve the server (this host) address from `SSH_CONNECTION`, whose format is | ||
| * `<client-ip> <client-port> <server-ip> <server-port>`. The third field is the | ||
| * address the operator SSH'd into, which is exactly what belongs in the | ||
| * `ssh -L` example. Returns null when unset or malformed so callers can fall | ||
| * back to a placeholder. | ||
| */ | ||
| function serverAddressFromSshConnection(value: string | undefined): string | null { | ||
| if (!value) return null; | ||
| const parts = value.trim().split(/\s+/).filter(Boolean); | ||
| const serverIp = parts[2]; | ||
| return serverIp && serverIp.length > 0 ? serverIp : null; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not turn SSH_CONNECTION into the SSH destination.
SSH_CONNECTION only gives you the remote socket address. It does not preserve the original host token, -p port, ProxyJump, or any SSH config alias, so the generated ssh -L ... ${user}@${host} command is not reliably copy-pastable for the very sessions this feature targets. Use a placeholder unless the caller can provide the original destination string explicitly.
Also applies to: 89-97
🤖 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 `@src/lib/onboard/ssh-forward-hint.ts` around lines 27 - 37, The helper
serverAddressFromSshConnection should not derive the SSH destination from
SSH_CONNECTION, since it only exposes the remote socket address and loses the
original host, port, aliases, and ProxyJump details. Update the ssh-forward-hint
flow to return a placeholder unless the caller can pass the original destination
string explicitly, and keep serverAddressFromSshConnection limited to fallback
behavior rather than constructing the ssh -L example target.
Signed-off-by: Atulya Singh <atulyarajsingh@gmail.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
When NemoClaw runs inside an SSH session, the dashboard URL binds to
127.0.0.1on the remote host and is unreachable from the operator's workstation browser without a port forward. The post-onboard summary anddashboard-urloutput now print a copy-pastablessh -L <port>:127.0.0.1:<port> <user>@<host>example, and the Quickstart and remote-GPU deploy docs cross-reference the same command.Related Issue
Fixes #5925
Changes
src/lib/onboard/ssh-forward-hint.ts:isSshSession()detectsSSH_CONNECTION/SSH_CLIENT/SSH_TTY.buildSshForwardHintLines()derives the host from theSSH_CONNECTIONserver-IP field and the user fromUSER/LOGNAME, returning a copy-pastablessh -Lblock. Returnsnulloutside an SSH session or when the dashboard already binds a routable address (WSL fallback,NEMOCLAW_DASHBOARD_BIND=0.0.0.0). Unsafe usernames fall back to<user>; an unknown host falls back to<host>.src/lib/dashboard-url-command.ts: append the hint to non-quiet output (both token and session-auth paths); suppressed under--quietso scripted consumers still get exactly one line.src/lib/onboard/dashboard.ts:printDashboardshows the hint in the post-onboard "is ready" summary (covers OpenClaw and agent dashboards).docs/get-started/quickstart.mdx, and a copy-pastablessh -Lcommand indocs/deployment/deploy-to-remote-gpu.mdx(Remote Dashboard Access).ssh-forward-hint.test.ts(8 cases) plus 3 newdashboard-url-command.test.tscases (hint over SSH, absent without SSH, suppressed under--quiet).The hint only ever references loopback forwarding; it does not suggest binding to
0.0.0.0, and it points to the already-printed dashboard URL rather than reprinting the tokenized URL, so it does not duplicate the secret token.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Atulya Singh atulyarajsingh@gmail.com
Summary by CodeRabbit
New Features
ssh -Linstructions when needed.Bug Fixes
Documentation
ssh -Lexamples.Tests