refactor(cli): extract sandbox status action - #2892
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
📝 WalkthroughWalkthroughThe PR extracts the ChangesSandbox Status Extraction
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
## Summary Extract sandbox connect and sandbox process recovery logic from `src/nemoclaw.ts` into dedicated CLI action modules. This removes `sandboxConnect` from the transitional runtime bridge while keeping existing connect, probe, status health, and rebuild post-upgrade behavior intact. ## Stack Navigation - Position: 3 of 60 - Previous PR: [#2887 — refactor(cli): extract sandbox skill install action](#2887) - Next PR: [#2892 — refactor(cli): extract sandbox status action](#2892) ## Changes - Added `src/lib/sandbox-connect-action.ts` for connect help/argument parsing, probe-only checks, inference route swapping, readiness polling, and OpenShell connection handoff. - Added `src/lib/sandbox-process-recovery-action.ts` for sandbox SSH/exec helpers, gateway process probes, and automatic gateway process recovery. - Updated `src/lib/sandbox-runtime-actions.ts` to call the extracted connect action instead of `NemoClawRuntimeBridge`. - Updated `src/nemoclaw.ts` status/rebuild paths to import the extracted process helpers. - Removed `sandboxConnect` from `src/nemoclaw.ts` and `NemoClawRuntimeBridge`. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes - [x] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added `--probe-only` flag for sandbox connection to check gateway health without full connection. * Enhanced sandbox connection with automatic gateway recovery and configurable timeout via `NEMOCLAW_CONNECT_TIMEOUT`. * Automatic dashboard port forwarding verification during sandbox connection. * **Improvements** * Better error detection and recovery for failed sandboxes during connection. * Clearer status feedback when sandbox connection is ready. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
prekshivyas
left a comment
There was a problem hiding this comment.
LGTM — clean mechanical extraction of sandboxStatus into src/lib/sandbox-status-action.ts. Output ordering, exit codes, recovery hints, and color/styling all preserved (verified RD from terminal-style is byte-identical to the local _RD). Old code path fully removed from src/nemoclaw.ts and NemoClawRuntimeBridge; no orphaned imports introduced by this PR.
Carryover nit (not blocking, from #2891): parseForwardList is still imported but unused at src/nemoclaw.ts:79 — happy for that to land in a follow-up cleanup PR in the stack.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/sandbox-status-action.ts`:
- Around line 34-35: Remove the obsolete ESLint suppression comment above the
showSandboxStatus function; delete the line "// eslint-disable-next-line
complexity" so the function declaration export async function
showSandboxStatus(sandboxName: string): Promise<void> is not preceded by any
linter suppression. Ensure no replacement Biome directive is added and leave the
function signature as-is.
🪄 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: 6f0e3ee1-ca6b-4e5d-998b-ff695c26a32d
📒 Files selected for processing (4)
src/lib/nemoclaw-runtime-bridge.tssrc/lib/sandbox-runtime-actions.tssrc/lib/sandbox-status-action.tssrc/nemoclaw.ts
💤 Files with no reviewable changes (2)
- src/lib/nemoclaw-runtime-bridge.ts
- src/nemoclaw.ts
| // eslint-disable-next-line complexity | ||
| export async function showSandboxStatus(sandboxName: string): Promise<void> { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -HI '^biome\.json$' . -x sh -c 'printf "== %s ==\n" "$1"; cat "$1"' _ {}
printf "\n== JS/TS ESLint suppression comments ==\n"
rg -n --iglob '*.{js,jsx,ts,tsx}' 'eslint-disable'Repository: NVIDIA/NemoClaw
Length of output: 3933
🏁 Script executed:
fd -HI '^sandbox-status-action\.ts$' . -x wc -lRepository: NVIDIA/NemoClaw
Length of output: 98
🏁 Script executed:
fd -HI '^sandbox-status-action\.ts$' . -x head -40Repository: NVIDIA/NemoClaw
Length of output: 1784
🏁 Script executed:
sed -n '34,260p' ./src/lib/sandbox-status-action.ts | head -100Repository: NVIDIA/NemoClaw
Length of output: 4088
🏁 Script executed:
sed -n '34,260p' ./src/lib/sandbox-status-action.ts | tail -70Repository: NVIDIA/NemoClaw
Length of output: 2794
Remove the ESLint complexity suppression comment.
The // eslint-disable-next-line complexity directive will not suppress any Biome rules, and Biome (per the coding guidelines) is the active linter for this file. This comment signals false coverage. Since the coding guidelines state that complexity hotspots are tracked separately, drop this comment entirely rather than converting it to Biome syntax.
🤖 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/sandbox-status-action.ts` around lines 34 - 35, Remove the obsolete
ESLint suppression comment above the showSandboxStatus function; delete the line
"// eslint-disable-next-line complexity" so the function declaration export
async function showSandboxStatus(sandboxName: string): Promise<void> is not
preceded by any linter suppression. Ensure no replacement Biome directive is
added and leave the function signature as-is.
## Summary Extract sandbox doctor diagnostics from `src/nemoclaw.ts` into a dedicated action module and route the command through oclif. This removes another legacy-dispatched public sandbox command while preserving the existing host, gateway, sandbox, inference, messaging, and local service diagnostics. ## Stack Navigation - Position: 5 of 60 - Previous PR: [#2892 — refactor(cli): extract sandbox status action](#2892) - Next PR: [#2895 — refactor(cli): extract sandbox destroy action](#2895) ## Changes - Added `src/lib/sandbox-doctor-action.ts` for doctor checks and report rendering. - Added `src/lib/sandbox-doctor-cli-command.ts` and registered `sandbox:doctor` in the oclif command map. - Updated legacy sandbox dispatch to route `doctor` through oclif instead of the legacy target path. - Removed doctor-specific helpers and `sandboxDoctor` from `src/nemoclaw.ts`. - Added a dispatch unit test for the new doctor route. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes - [x] `npm test` passes - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added `sandbox doctor` command to diagnose sandbox and gateway health, running comprehensive checks across host, gateway, inference, messaging, and local services. * Supports `--json` flag for structured diagnostic output with status details and summary. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Summary
Extract the sandbox status implementation from
src/nemoclaw.tsinto a dedicated action module. This removessandboxStatusfrom the transitional runtime bridge while preserving the existing status output, gateway reconciliation, process health, and NIM reporting behavior.Stack Navigation
Changes
src/lib/sandbox-status-action.tsfor sandbox status rendering, gateway lookup handling, local inference health, active session reporting, process health, and NIM status.src/lib/sandbox-runtime-actions.tsto call the extracted status action.sandboxStatusfromsrc/nemoclaw.tsandNemoClawRuntimeBridge.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
Refactoring
Improvements