feat(cli): add connection info command to reprint the connection block#7478
Open
laitingsheng wants to merge 1 commit into
Open
feat(cli): add connection info command to reprint the connection block#7478laitingsheng wants to merge 1 commit into
laitingsheng wants to merge 1 commit into
Conversation
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
📝 WalkthroughWalkthroughAdds ChangesConnection info flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant User
participant ConnectionInfoCliCommand
participant runConnectionInfoCommand
participant SandboxRegistry
participant GatewayToken
participant printDashboard
User->>ConnectionInfoCliCommand: run connection info
ConnectionInfoCliCommand->>runConnectionInfoCommand: pass sandbox name and runtime bridge
runConnectionInfoCommand->>SandboxRegistry: retrieve sandbox state
runConnectionInfoCommand->>GatewayToken: fetch token for dashboard runtime
runConnectionInfoCommand->>printDashboard: render connection block
printDashboard-->>User: display dashboard and management commands
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-7478.docs.buildwithfern.com/nemoclaw |
Contributor
PR Review Advisor — Blocking findings reportedAdvisor assessment: Blockers require maintainer review Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
Blockers
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/connection-info-command.ts (1)
45-49: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDo not silently relabel dependency failures as missing sandbox metadata.
These catch-all fallbacks turn registry/agent-loading failures into either “sandbox does not exist” or an OpenClaw-style dashboard path, hiding the actionable error.
src/lib/connection-info-command.ts#L45-L49: let unexpectedloadAgentfailures propagate; its declarednullresult already represents an unknown agent.src/lib/connection-info-command.ts#L74-L79: do not convert a registry failure into a nonexistent sandbox.src/commands/sandbox/connection/info.ts#L44-L50: remove the duplicate registry-error fallback.Based on learnings, avoid defensive catch-and-fallback handling around internal helpers without an actionable recovery path.
🤖 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/connection-info-command.ts` around lines 45 - 49, Remove catch-all registry-error fallbacks around the connection-info flow: in src/lib/connection-info-command.ts lines 45-49, let unexpected loadAgent failures propagate while preserving its null result for unknown agents; in lines 74-79, stop converting registry failures into nonexistent-sandbox results; and in src/commands/sandbox/connection/info.ts lines 44-50, remove the duplicate registry-error fallback.Source: Learnings
🤖 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/connection-info-command.test.ts`:
- Around line 14-15: Update the parent suite title in
src/lib/connection-info-command.test.ts at lines 14-15 and
src/lib/cli/connection-info-route.test.ts at lines 9-10 to include the final
issue suffix (`#7473`). In test/package-contract/cli/command-registry.test.ts at
lines 59-60 and 229-231, replace the count-only test titles with
behavior-oriented descriptions of connection-info registration and the
connection action token, respectively, and append (`#7473`) as the final suffix.
In `@src/lib/connection-info-command.ts`:
- Around line 90-92: Update the terminal-agent branch in the connection-info
command so it includes the shared management-command section before returning,
while preserving the existing terminal connection instructions. Reuse the
existing management-section helper or rendering path rather than duplicating its
content.
---
Nitpick comments:
In `@src/lib/connection-info-command.ts`:
- Around line 45-49: Remove catch-all registry-error fallbacks around the
connection-info flow: in src/lib/connection-info-command.ts lines 45-49, let
unexpected loadAgent failures propagate while preserving its null result for
unknown agents; in lines 74-79, stop converting registry failures into
nonexistent-sandbox results; and in src/commands/sandbox/connection/info.ts
lines 44-50, remove the duplicate registry-error fallback.
🪄 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: 34bb96c9-4e7b-42cd-a139-bec0a3376288
📒 Files selected for processing (8)
docs/reference/commands.mdxsrc/commands/sandbox/connection/info.tssrc/lib/cli/connection-info-route.test.tssrc/lib/cli/public-display-defaults.tssrc/lib/connection-info-command.test.tssrc/lib/connection-info-command.tssrc/lib/onboard.tstest/package-contract/cli/command-registry.test.ts
Comment on lines
+14
to
+15
| describe("connection info command", () => { | ||
| it("reprints the connection block for a running OpenClaw sandbox", () => { |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add behavior-oriented, issue-linked test titles.
Issue #7473 is known, but these new tests lack its required final suffix.
src/lib/connection-info-command.test.ts#L14-L15: suffix the parent suite with([All Platforms][CLI&UX] no command reprints the "OpenClaw is ready" connection block after onboarding #7473).src/lib/cli/connection-info-route.test.ts#L9-L10: suffix the parent suite with([All Platforms][CLI&UX] no command reprints the "OpenClaw is ready" connection block after onboarding #7473).test/package-contract/cli/command-registry.test.ts#L59-L60: rename the count-only title to describe connection-info registration and suffix([All Platforms][CLI&UX] no command reprints the "OpenClaw is ready" connection block after onboarding #7473).test/package-contract/cli/command-registry.test.ts#L229-L231: rename the count-only title to describe the connection action token and suffix([All Platforms][CLI&UX] no command reprints the "OpenClaw is ready" connection block after onboarding #7473).
As per coding guidelines, “Tests must use behavior-oriented titles and put local issue references in a final (#1234) suffix.”
📍 Affects 3 files
src/lib/connection-info-command.test.ts#L14-L15(this comment)src/lib/cli/connection-info-route.test.ts#L9-L10test/package-contract/cli/command-registry.test.ts#L59-L60test/package-contract/cli/command-registry.test.ts#L229-L231
🤖 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/connection-info-command.test.ts` around lines 14 - 15, Update the
parent suite title in src/lib/connection-info-command.test.ts at lines 14-15 and
src/lib/cli/connection-info-route.test.ts at lines 9-10 to include the final
issue suffix (`#7473`). In test/package-contract/cli/command-registry.test.ts at
lines 59-60 and 229-231, replace the count-only test titles with
behavior-oriented descriptions of connection-info registration and the
connection action token, respectively, and append (`#7473`) as the final suffix.
Source: Coding guidelines
Comment on lines
+90
to
+92
| if (agent && deps.isTerminalAgent(agent)) { | ||
| printTerminalConnectionBlock(sandboxName, agent, deps.log); | ||
| return; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Include management commands for terminal agents.
This early return emits only terminal-connect instructions, so terminal runtimes never receive the required management-command portion of the connection block. Reuse or append the shared management section before returning.
🤖 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/connection-info-command.ts` around lines 90 - 92, Update the
terminal-agent branch in the connection-info command so it includes the shared
management-command section before returning, while preserving the existing
terminal connection instructions. Reuse the existing management-section helper
or rendering path rather than duplicating its content.
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
Add a read-only
nemoclaw <name> connection infocommand that reprints the connection block onboarding shows at the end of setup — the dashboard URL, terminal connect command, and management commands — so a user who scrolls past it can recover the details without onboarding again. Before this change no supported command reprinted that block;show-connection-infofell through to theUnknown actionpath.Related Issue
Resolves #7473
Changes
sandbox:connection:infooclif command (nemoclaw <name> connection info). It routes through the existing public sandbox-action grammar and auto-registers as a valid action from its command metadata, mirroring the nestedgateway restartcommand.runConnectionInfoCommand, which resolves the sandbox registry entry (model, provider, agent) and reprints the block from live state: OpenClaw and other dashboard agents reuse the onboarding renderer, terminal-runtime agents print their connect and run commands, and a stopped OpenClaw sandbox reports an honest "not running" message instead of a half-rendered block.printDashboardfromonboardso the command reuses the exact onboarding renderer rather than duplicating it and drifting. The current and only new consumer is this command;src/lib/connection-info-command.test.tsprotects the wiring.docs/reference/commands.mdx.Type of Change
Quality Gates
printDashboardrenderer with no behaviour change; no credential, policy, gateway, or state mutation occurs. Pending maintainer sensitive-path review.Documentation Writer Review
docs-updateddocs/reference/commands.mdx(newconnection inforeference section). Writer-review findings applied: split the security warning into one instruction per sentence, and scoped the description by agent runtime (dashboard agents show the browser URL; terminal-runtime agents show connect and run commands).npm run docsreported 0 errors (2 pre-existing repo-wide warnings unrelated to this change).DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run src/lib/connection-info-command.test.ts src/lib/cli/connection-info-route.test.ts→ 8 passed;npx vitest run test/package-contract/cli/command-registry.test.ts→ passed;npm run typecheck:cli→ 0 errorsnpm 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: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
New Features
nemoclaw <name> connection infoto reprint connection details for a running sandbox.Documentation