fix(status): point SSH operators to dashboard-url for remote access - #8516
fix(status): point SSH operators to dashboard-url for remote access#8516jason-ma-nv wants to merge 1 commit into
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>
📝 WalkthroughWalkthrough
ChangesSSH status guidance
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Status
participant GatewayCheck
participant SSHSession
participant RemoteAccessHint
Status->>GatewayCheck: Check whether the gateway is running
Status->>SSHSession: Check SSH environment variables
Status->>RemoteAccessHint: Print dashboard-url guidance when both checks pass
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 12bfa00 in the TypeScript / code-coverage/cliThe overall coverage in commit 12bfa00 in the Show a code coverage summary of the most impacted files.
Updated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/actions/sandbox/status-flow.test.ts (1)
678-690: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd negative coverage for the gateway-state gate.
The tests cover
gatewayRunning: truewith SSH and without SSH. They do not prove that an SSH session withgatewayRunning: falseomits the hint. Add this case throughshowSandboxStatus. Also cover an SSH terminal-runtime sandbox if no existing test proves that terminal-runtime output remains unchanged.As per path instructions, review tests for behavioral confidence rather than implementation lock-in.
Also applies to: 692-703
🤖 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 - 690, Extend the status-flow tests around showSandboxStatus with an SSH case where gatewayRunning is false, and assert the remote-access dashboard-url hint is omitted while normal status output remains valid. Also add coverage for an SSH terminal-runtime sandbox if existing tests do not already verify its output remains unchanged; assert user-visible behavior rather than internal implementation details.Source: 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 382-400: Update the CLI documentation for the user-visible
`nemoclaw <sandbox> status` output introduced by
`printDashboardRemoteAccessHint`, including the SSH remote-access guidance and
`dashboard-url` command. Validate the documentation using the project’s
established process and obtain the required independent documentation-writer
review before handoff.
- Around line 382-400: Update printDashboardRemoteAccessHint to shell-quote
sandboxName when constructing the dashboard-url command, reusing the existing
shell-quoting helper used by the status flow tests. Preserve the current
SSH-only hint and message while ensuring names containing spaces or apostrophes
remain valid copy-pastable commands.
---
Nitpick comments:
In `@src/lib/actions/sandbox/status-flow.test.ts`:
- Around line 678-690: Extend the status-flow tests around showSandboxStatus
with an SSH case where gatewayRunning is false, and assert the remote-access
dashboard-url hint is omitted while normal status output remains valid. Also add
coverage for an SSH terminal-runtime sandbox if existing tests do not already
verify its output remains unchanged; assert user-visible behavior rather than
internal implementation details.
🪄 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: 78742a5d-3f28-4df4-b3c8-931196479229
📒 Files selected for processing (3)
src/lib/actions/sandbox/status-flow.test.tssrc/lib/actions/sandbox/status-text.tstest/support/status-flow-test-harness.ts
| // On an SSH session the loopback-only dashboard needs a port forward to reach | ||
| // from the operator's workstation. `status` does not print the dashboard URL, | ||
| // so point to `dashboard-url`, which renders the copy-pastable `ssh -L` block. | ||
| // This extends the #5925 remote-access guidance to the status surface (#8465). | ||
| function printDashboardRemoteAccessHint(sandboxName: string): void { | ||
| if (!isSshSession()) return; | ||
| console.log( | ||
| ` Remote access: run \`${CLI_NAME} ${sandboxName} dashboard-url\` for SSH port-forward instructions.`, | ||
| ); | ||
| } | ||
|
|
||
| async function printGatewayProcessStatus(context: SandboxStatusTextContext): Promise<void> { | ||
| const { sandboxName, statusAgent } = context; | ||
| const running = await isSandboxGatewayRunningForStatus(sandboxName); | ||
| if (running === null) return; | ||
| const agentName = statusAgent.agentDisplayName; | ||
| if (running) { | ||
| console.log(` ${agentName}: ${G}running${R}`); | ||
| printDashboardRemoteAccessHint(sandboxName); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the required documentation update before handoff.
This PR changes the user-visible output of nemoclaw <sandbox> status. Update the affected CLI documentation, validate it, and obtain the required independent documentation-writer review.
As per coding guidelines, user-visible behavior changes require updated, validated documentation and independent documentation-writer review before final handoff.
🤖 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-text.ts` around lines 382 - 400, Update the
CLI documentation for the user-visible `nemoclaw <sandbox> status` output
introduced by `printDashboardRemoteAccessHint`, including the SSH remote-access
guidance and `dashboard-url` command. Validate the documentation using the
project’s established process and obtain the required independent
documentation-writer review before handoff.
Source: Coding guidelines
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Quote sandboxName in the copy-pastable command.
Line [389] inserts sandboxName without shell quoting. A name such as alpha's box or a name with spaces produces an invalid command or multiple arguments. The existing src/lib/actions/sandbox/status-flow.test.ts exercises such names and expects shell-quoted commands at Line [70] through Line [72]. Reuse the existing shell-quoting helper here.
Proposed fix
- ` Remote access: run \`${CLI_NAME} ${sandboxName} dashboard-url\` for SSH port-forward instructions.`,
+ ` Remote access: run \`${CLI_NAME} ${shellQuote(sandboxName)} dashboard-url\` for SSH port-forward instructions.`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // On an SSH session the loopback-only dashboard needs a port forward to reach | |
| // from the operator's workstation. `status` does not print the dashboard URL, | |
| // so point to `dashboard-url`, which renders the copy-pastable `ssh -L` block. | |
| // This extends the #5925 remote-access guidance to the status surface (#8465). | |
| function printDashboardRemoteAccessHint(sandboxName: string): void { | |
| if (!isSshSession()) return; | |
| console.log( | |
| ` Remote access: run \`${CLI_NAME} ${sandboxName} dashboard-url\` for SSH port-forward instructions.`, | |
| ); | |
| } | |
| async function printGatewayProcessStatus(context: SandboxStatusTextContext): Promise<void> { | |
| const { sandboxName, statusAgent } = context; | |
| const running = await isSandboxGatewayRunningForStatus(sandboxName); | |
| if (running === null) return; | |
| const agentName = statusAgent.agentDisplayName; | |
| if (running) { | |
| console.log(` ${agentName}: ${G}running${R}`); | |
| printDashboardRemoteAccessHint(sandboxName); | |
| // On an SSH session the loopback-only dashboard needs a port forward to reach | |
| // from the operator's workstation. `status` does not print the dashboard URL, | |
| // so point to `dashboard-url`, which renders the copy-pastable `ssh -L` block. | |
| // This extends the `#5925` remote-access guidance to the status surface (`#8465`). | |
| function printDashboardRemoteAccessHint(sandboxName: string): void { | |
| if (!isSshSession()) return; | |
| console.log( | |
| ` Remote access: run \`${CLI_NAME} ${shellQuote(sandboxName)} dashboard-url\` for SSH port-forward instructions.`, | |
| ); | |
| } | |
| async function printGatewayProcessStatus(context: SandboxStatusTextContext): Promise<void> { | |
| const { sandboxName, statusAgent } = context; | |
| const running = await isSandboxGatewayRunningForStatus(sandboxName); | |
| if (running === null) return; | |
| const agentName = statusAgent.agentDisplayName; | |
| if (running) { | |
| console.log(` ${agentName}: ${G}running${R}`); | |
| printDashboardRemoteAccessHint(sandboxName); |
🤖 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-text.ts` around lines 382 - 400, Update
printDashboardRemoteAccessHint to shell-quote sandboxName when constructing the
dashboard-url command, reusing the existing shell-quoting helper used by the
status flow tests. Preserve the current SSH-only hint and message while ensuring
names containing spaces or apostrophes remain valid copy-pastable commands.
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 2 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.
|
|
Superseded by #8528. This PR's branch fell behind main and I needed to add the CodeRabbit-requested documentation update ( |
1 participant
Summary
nemoclaw <sandbox> statusgave no remote-access guidance when run over SSH, so an operator on a remote host was left without a pointer to reach the loopback-only dashboard — unlikedashboard-urland the post-onboard block, which print anssh -Lport-forward example (#5925). When the gateway is running and the CLI is in an SSH session, status now prints a pointer tonemoclaw <sandbox> dashboard-url, which renders the copy-pastable port-forward block.Related Issue
Closes #8465
Changes
src/lib/actions/sandbox/status-text.ts: addprintDashboardRemoteAccessHint, called in the running-gateway branch ofprintGatewayProcessStatus. It reuses the existingisSshSessionhelper fromssh-forward-hint.ts([All Platforms][CLI&UX]dashboard-urland post-onboard output print only the loopback URL — no copy-pastable SSH port-forward example for remote-SSH'd users #5925) and, only inside an SSH session, prints one line pointing tonemoclaw <sandbox> dashboard-url. Scoped to the gateway-runtime running path, so terminal-runtime sandboxes (which have no browser dashboard) are unaffected. This is a display-only status-output addition; it introduces no new abstraction, flag, or lifecycle behavior.test/support/status-flow-test-harness.ts: add an optionalgatewayRunningoption (defaultfalse, preserving every existing test) and expose theisSandboxGatewayRunningForStatusspy, so a test can drive the real running-gateway rendering path end-to-end.src/lib/actions/sandbox/status-flow.test.ts: add two tests — the pointer appears for a running gateway inside an SSH session, and is omitted when the session is not over SSH (SSH env controlled viavi.stubEnvso the result is deterministic even when the verifying host itself runs over SSH).Type of Change
Quality Gates
dashboard-urlcommand; no new command, flag, or documented contract, and the remote-access/port-forward guidance it points to is already documented.actions/sandbox/(status output rendering); it adds an SSH-gated guidance line and alters no sandbox lifecycle, credential, policy, or security behavior. Deferred to normal CODEOWNERS review.Documentation Writer Review
no-docs-needednemoclaw <sandbox> dashboard-urlcommand; it introduces no new command, flag, configuration, or documented contract.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpx vitest run --project cli src/lib/actions/sandbox/status-flow.test.tson a fresh clone of this branch on an Ubuntu host (Node v22.23.1,npm ci+ plugin build) → 35 passed. Two-way check: reverting onlystatus-text.tstoorigin/main(keeping the tests) fails exactly the new positive test (pointer absent = bug reproduced); the negative test still passes.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Jason Ma jama@nvidia.com
Summary by CodeRabbit
New Features
Tests