fix(status): point SSH operators to dashboard-url for remote access - #8528
Conversation
`nemoclaw <sandbox> status` gave no remote-access guidance when run over SSH, unlike `dashboard-url` and the post-onboard block that print an `ssh -L` port-forward example (#5925). A remote operator checking status was left without a pointer to reach the loopback-only dashboard. Extend the #5925 guidance to the status surface: when the gateway is running and the CLI is in an SSH session, print a pointer to `nemoclaw <sandbox> dashboard-url`, which renders the copy-pastable port-forward block. Scoped to the running gateway-runtime path, so terminal-runtime sandboxes (no dashboard) are unaffected. Closes #8465 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Jason Ma <jama@nvidia.com>
📝 WalkthroughWalkthroughSandbox status now directs SSH users to ChangesSSH status guidance
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant SSHSession
participant SandboxStatus
participant DashboardAccess
SSHSession->>SandboxStatus: run status
SandboxStatus->>DashboardAccess: check gateway and dashboard access state
DashboardAccess-->>SandboxStatus: return forwarding guidance when required
SandboxStatus-->>SSHSession: print dashboard-url command
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit eae84db in the TypeScript / code-coverage/cliThe overall coverage in commit eae84db in the Show a code coverage summary of the most impacted files.
Updated |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/actions/sandbox/status-flow.test.ts (1)
678-703: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the SSH and stopped-gateway boundary.
The tests cover SSH with a running gateway and non-SSH with a running gateway. They do not prove that an SSH session omits the hint when the gateway is stopped. Add a public-boundary case with
gatewayRunning: falseand assert thatRemote access: runis absent.Suggested test
+ it("omits dashboard guidance over SSH when the gateway is stopped (`#8465`)", async () => { + vi.stubEnv("SSH_CONNECTION", "203.0.113.9 51000 198.51.100.2 22"); + const harness = createStatusFlowHarness({ gatewayRunning: false }); + + await expect(harness.showSandboxStatus("alpha")).resolves.toBeUndefined(); + + const output = harness.logSpy.mock.calls.map((call) => String(call[0])).join("\n"); + expect(output).toContain("not running"); + expect(output).not.toContain("Remote access: run"); + });As per path instructions, tests must verify observable behavior through the public boundary.
As per coding guidelines, run tests targeted to changed behavior once per relevant change set.🤖 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/actions/sandbox/status-flow.test.ts` around lines 678 - 703, Add a test alongside the existing SSH cases that stubs an SSH session, creates the status-flow harness with gatewayRunning: false, and invokes showSandboxStatus("alpha") through the public boundary. Assert the call resolves successfully and the captured output omits “Remote access: run”, preserving the stopped-gateway behavior.Sources: Coding guidelines, Path instructions
🤖 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 `@src/lib/actions/sandbox/status-text.ts`:
- Around line 386-391: Update printDashboardRemoteAccessHint to shell-quote
sandboxName when constructing the copy-pastable dashboard-url command. Reuse the
existing shell-quoting helper already used by the status flow, preserving the
current hint text and command structure.
---
Nitpick comments:
In `@src/lib/actions/sandbox/status-flow.test.ts`:
- Around line 678-703: Add a test alongside the existing SSH cases that stubs an
SSH session, creates the status-flow harness with gatewayRunning: false, and
invokes showSandboxStatus("alpha") through the public boundary. Assert the call
resolves successfully and the captured output omits “Remote access: run”,
preserving the stopped-gateway behavior.
🪄 Autofix
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: 86028052-b630-4bf2-bb53-e33252e41b3a
📒 Files selected for processing (4)
docs/deployment/deploy-to-headless-server.mdxsrc/lib/actions/sandbox/status-flow.test.tssrc/lib/actions/sandbox/status-text.tstest/support/status-flow-test-harness.ts
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: 1 warning · 0 suggestionsWarningsWarnings do not block.
|
cv
left a comment
There was a problem hiding this comment.
Two correctness changes are required. Shell-quote sandboxName in the displayed command so valid names with spaces or metacharacters remain copy-pastable. Also do not promise SSH port-forward instructions solely from SSH-session state: dashboard-url omits those instructions for a routable non-loopback dashboard. Reuse the dashboard access applicability decision or make the text accurate for both bindings, and add tests for a quoted sandbox name and a remotely bound dashboard. Then fix the failing Fern preview and refresh the branch onto current main.
|
🌿 Preview your docs: https://nvidia-preview-pr-8528.docs.buildwithfern.com/nemoclaw |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Sensitive-path security reviewResult: PASS for branch revision
Required CI and independent maintainer approval remain separate merge requirements. |
|
Addressed the requested changes in the refreshed branch revision:
Local validation passes: 39 focused tests, CLI type checking, the complete changed-file hook suite, repository architecture checks, and docs with 0 errors. The documentation receipt and security review are current. Fresh required CI and independent re-review remain pending. |
cv
left a comment
There was a problem hiding this comment.
The refreshed diff addresses both requested correctness changes: sandbox names are shell-quoted, and the status hint uses the shared loopback-forward applicability decision. Tests cover quoted names, routable URLs, prepared remote binds, stopped gateways, and non-SSH sessions. All five required checks pass on this head, with no unresolved correctness or security finding.
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Summary
nemoclaw <sandbox> statuspreviously gave no remote-access guidance for a loopback-only dashboard when an operator ran it over SSH. For OpenClaw and Hermes, status now points tonemoclaw <sandbox> dashboard-urlonly when the gateway is running and the shared dashboard-access decision determines that a port forward is needed. The displayed command shell-quotes the sandbox name so valid names with spaces or metacharacters remain one argument.Related Issue
Closes #8465
Changes
src/lib/actions/sandbox/status-text.ts: route the status hint throughbuildSshForwardHintLines, using the persisted dashboard port and the routable-dashboard state. Omit the hint outside SSH, for a stopped gateway, for a routable dashboard, or when the dashboard port is unavailable. Render the sandbox name with the existing shell-quoting helper.docs/deployment/deploy-to-headless-server.mdx: document the precise SSH, running-gateway, and loopback conditions for OpenClaw and Hermes. Deep Agents correctly omits browser-dashboard guidance.test/support/status-flow-test-harness.ts: expose the gateway-running control and remote-bind registry state needed to exercise the public status path.src/lib/actions/sandbox/status-flow.test.ts: cover SSH guidance, a shell-quoted sandbox name, a routable dashboard URL, a prepared remote bind, a stopped gateway, and a non-SSH session.Type of Change
Quality Gates
Documentation Writer Review
docs-updatednpm run docspasses with 0 errors and 2 unrelated Fern warnings.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHub — all four commits are verified.pre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailable — normal hooks and the full changed-file hook suite passed.npx vitest run --project cli src/lib/actions/sandbox/status-flow.test.tspasses 39 tests;npm run typecheck:clipasses.npm run docsbuilds without warnings (doc changes only) — passes with 0 errors and 2 existing Fern warnings unrelated to this page.Signed-off-by: Jason Ma jama@nvidia.com
Summary by CodeRabbit
New Features
dashboard-urlcommand.Documentation
Tests