Skip to content

refactor(cli): extract sandbox live state helpers - #2886

Merged
cv merged 4 commits into
mainfrom
refactor/oclif-extract-sandbox-live-state
May 4, 2026
Merged

refactor(cli): extract sandbox live state helpers#2886
cv merged 4 commits into
mainfrom
refactor/oclif-extract-sandbox-live-state

Conversation

@cv

@cv cv commented May 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

Extract shared sandbox gateway-state and liveness helpers from src/nemoclaw.ts into reusable CLI library modules. This reduces the legacy entrypoint surface before moving connect, status, skill install, and lifecycle actions out of the runtime bridge.

Stack Navigation

Changes

  • Added src/lib/sandbox-gateway-state-action.ts for sandbox liveness, gateway reconciliation, live-policy merging, and lifecycle guidance helpers.
  • Added src/lib/terminal-style.ts for shared CLI color constants.
  • Updated src/nemoclaw.ts to import the extracted sandbox gateway-state helpers instead of defining them inline.
  • Updated src/lib/share-command-deps.ts to depend on extracted helpers instead of requiring ../nemoclaw.
  • Removed stale sandboxLogs from NemoClawRuntimeBridge.

Type of Change

  • 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

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • 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 (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • New Features

    • Enhanced sandbox diagnostics with live-policy display and guided recovery steps.
  • Bug Fixes

    • More accurate sandbox presence/status detection, identity-drift handling, and gateway reconnection.
  • Chores

    • Refactored sandbox/gateway wiring and share-command startup for reliability.
    • Unified terminal color handling.
    • Removed sandbox log streaming (log-following is no longer available).

@cv cv self-assigned this May 2, 2026
@copy-pr-bot

copy-pr-bot Bot commented May 2, 2026

Copy link
Copy Markdown

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.

@cv
cv marked this pull request as ready for review May 2, 2026 20:41
@coderabbitai

coderabbitai Bot commented May 2, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Moves sandbox gateway reconciliation and lifecycle handling into a new module, adds terminal color utilities, removes an unused runtime-bridge method, updates wiring to use the new helpers, and adjusts a test Dockerfile extraction marker.

Changes

Sandbox Gateway State & CLI Wiring

Layer / File(s) Summary
Interface Cleanup
src/lib/nemoclaw-runtime-bridge.ts
Removed sandboxLogs(sandboxName: string, follow: boolean): void from NemoClawRuntimeBridge.
Terminal Styling
src/lib/terminal-style.ts
New module that computes color/truecolor availability and exports ANSI constants G, B, D, R, RD, YW (or empty strings).
Core State Management
src/lib/sandbox-gateway-state-action.ts
New comprehensive sandbox-gateway module: mergeLivePolicyIntoSandboxOutput, getSandboxGatewayState, getSandboxGatewayStateForStatus, reconcileMissingAgainstNamedGateway, getReconciledSandboxGatewayState. Classifies states (present, missing, gateway_error, unknown_error, status_probe_timeout, etc.) and merges live policy into output when valid.
Diagnostics & Recovery
src/lib/sandbox-gateway-state-action.ts
Adds printWrongGatewayActiveGuidance, printGatewayLifecycleHint, and ensureLiveSandboxOrExit to emit guidance, recover gateway runtime, prune SSH known_hosts on identity drift, remove stale registry entries, and exit with lifecycle-specific messages when necessary.
Dependency Wiring
src/lib/share-command-deps.ts
buildShareCommandDeps now requires ./openshell-runtime for capture, uses ensureLiveSandboxOrExit from ./sandbox-gateway-state-action, and sources color codes from ./terminal-style.
Module Integration
src/nemoclaw.ts
Removed in-file gateway reconciliation and local exports; imports ensureLiveSandboxOrExit, getReconciledSandboxGatewayState, getSandboxGatewayStateForStatus, printGatewayLifecycleHint, printWrongGatewayActiveGuidance from the new module; reduced timeout imports to OPENSHELL_PROBE_TIMEOUT_MS.
Tests
test/sandbox-provisioning.test.ts
Adjusted dockerRunCommandBetween end marker from # Install OpenClaw CLI + PyYAML to # System-wide proxy hooks. affecting which Dockerfile RUN block is extracted.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as NemoClaw CLI
    participant Module as sandbox-gateway-state-action
    participant Gateway as OpenShell Gateway
    participant SSH as SSH / known_hosts & Registry

    CLI->>Module: getReconciledSandboxGatewayState(sandbox)
    Module->>Gateway: sandbox get / status probe (sync or async)
    alt present
        Module-->>CLI: present (with merged live policy if available)
    else missing
        Module->>Gateway: openshell gateway select nemoclaw (attempt recovery)
        Gateway-->>Module: new state
        alt recovered
            Module-->>CLI: recoveredGateway (present)
        else still missing
            Module->>SSH: remove stale registry / prune known_hosts
            Module-->>CLI: exit with remediation instructions
        end
    else gateway_error
        Module->>Gateway: recoverNamedGatewayRuntime (restart/recover)
        Module-->>CLI: gateway_unreachable_after_restart or gateway_missing_after_restart
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐰 I hopped through code to mend a gate,
Moved the logic, tuned the terminal’s slate.
Policies stitched and SSH paths pruned,
Colors return where prompts once loomed.
Sandboxes sing — the CLI’s in shape!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'refactor(cli): extract sandbox live state helpers' directly and accurately summarizes the main change: extracting sandbox gateway-state and liveness helpers from src/nemoclaw.ts into reusable library modules.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/oclif-extract-sandbox-live-state

Comment @coderabbitai help to get the list of available commands and usage tips.

@cv cv added the v0.0.34 label May 4, 2026

@prekshivyas prekshivyas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as a lift-and-shift. Verified mechanically against main: no behavior change, no exit-code/error-message drift, no zombies in nemoclaw.ts, all 15 PR checks green, no tests deleted. The share-command-deps.ts cleanup (drops require("../nemoclaw")) and the getSshConfig status: number | null typing fix are both nice incidental wins.

Two non-blocking asks for the description so future bisects don't get confused:

  • Mention the test/sandbox-provisioning.test.ts marker change. It came in via the merge(main): resolve sandbox live state conflicts commit, but Dockerfile.base is byte-identical on main and the PR branch — there was no actual conflict, so the change reads as out-of-scope without an explanation.
  • Note that src/lib/terminal-style.ts is currently a duplicate of the inline color block at src/nemoclaw.ts:14-22 and link the follow-up PR that migrates the inline copy.

@cv
cv merged commit 73c6a91 into main May 4, 2026
18 checks passed
@prekshivyas
prekshivyas deleted the refactor/oclif-extract-sandbox-live-state branch May 4, 2026 17:34
cv added a commit that referenced this pull request May 4, 2026
## Summary
Extract the sandbox skill install command implementation from
`src/nemoclaw.ts` into a dedicated action module. This removes
`sandboxSkillInstall` from the transitional runtime bridge while
preserving the existing oclif wrapper and legacy fallback behavior.

## Stack Navigation
- Position: 2 of 60
- Previous PR: [#2886 — refactor(cli): extract sandbox live state
helpers](#2886)
- Next PR: [#2891 — refactor(cli): extract sandbox connect
action](#2891)

## Changes
- Added `src/lib/sandbox-skill-install-action.ts` for skill install
usage, plugin-shape detection, validation, upload, post-install, and
verification flow.
- Updated `src/lib/sandbox-runtime-actions.ts` and the legacy skill
fallback to call the extracted action.
- Removed `sandboxSkillInstall` 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>

---------

Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
@wscurran wscurran added the refactor PR restructures code without intended behavior change label Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants