Skip to content

refactor(cli): harden oclif bridge - #2774

Merged
cv merged 2 commits into
mainfrom
refactor/oclif-bridge-cleanup
Apr 30, 2026
Merged

cv merged 2 commits into
mainfrom
refactor/oclif-bridge-cleanup

Conversation

@cv

@cv cv commented Apr 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Hardens the oclif bridge introduced for nemoclaw list so command help uses the active launcher branding, including the nemohermes alias. It also centralizes legacy dispatch through a runOclif() helper and relaxes uninstall helper test timeouts that were flaky under full-suite load.

Changes

  • Set the oclif config bin to the active CLI_NAME and invoke command classes with that config so alias help renders correctly.
  • Add a shared runOclif() delegation helper in src/nemoclaw.ts and route list through it.
  • Add CLI coverage for nemohermes list --help branding.
  • Increase timeouts for uninstall helper tests that can exceed Vitest's default when the full CLI suite runs concurrently.

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)

AI Disclosure

  • AI-assisted — tool: OpenAI Codex

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

Summary by CodeRabbit

  • Tests

    • Added validation for the nemohermes command alias to ensure correct branding in CLI output.
    • Adjusted execution timeouts for uninstall operation tests.
  • Refactor

    • Optimized command execution flow for improved reliability.
    • Improved sandbox listing implementation through centralized configuration.

cv added 2 commits April 30, 2026 13:51
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv self-assigned this Apr 30, 2026
@coderabbitai

coderabbitai Bot commented Apr 30, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f91f286e-ab09-4f2a-b393-a0a157279969

📥 Commits

Reviewing files that changed from the base of the PR and between cad2e10 and 5e997cb.

📒 Files selected for processing (4)
  • src/lib/oclif-runner.ts
  • src/nemoclaw.ts
  • test/cli.test.ts
  • test/uninstall.test.ts

📝 Walkthrough

Walkthrough

The oclif command runner is refactored to explicitly manage command execution by setting the bin value from CLI_NAME, finding and loading commands manually, and executing prerun and postrun hooks around command instances. A new test validates the nemohermes command alias works correctly. Test timeouts are standardized to 60 seconds.

Changes

Cohort / File(s) Summary
Command Runner Refactoring
src/lib/oclif-runner.ts, src/nemoclaw.ts
Core runner logic now sets bin from CLI_NAME and explicitly handles oclif prerun/postrun hooks. A new runOclif helper centralizes shared configuration in nemoclaw.ts.
Test Coverage Expansion
test/cli.test.ts
New test validates that the nemohermes command alias displays correct branding in CLI output and does not show nemoclaw branding.
Test Configuration
test/uninstall.test.ts
Uninstall helper tests updated with explicit 60-second timeouts to address performance variability.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Command hooks dance in sequence true,
Pre and post, the runner's new view,
Nemohermes alias finds its place,
Tests keep pace with proper grace!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor(cli): harden oclif bridge' directly matches the main objective of hardening the oclif bridge to ensure command help uses active launcher branding.
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.

✏️ 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-bridge-cleanup

Review rate limit: 9/10 reviews remaining, refill in 6 minutes.

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

@cv cv added the v0.0.32 label Apr 30, 2026
@cv
cv enabled auto-merge (squash) April 30, 2026 21:29

@ericksoa ericksoa left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed locally at 5e997cb. Focused validation passed: npm run build:cli; npx vitest run test/cli.test.ts test/nemohermes-alias.test.ts test/uninstall.test.ts.

@cv
cv merged commit 0e641f9 into main Apr 30, 2026
18 of 19 checks passed
cv added a commit that referenced this pull request May 1, 2026
## Summary
Draft stacked PR on top of #2774. Migrates the global `status` and
tunnel service commands onto oclif wrappers while keeping the public
legacy command shapes intact.

## Changes
- Add oclif command classes for `status`, `tunnel start`, `tunnel stop`,
and deprecated `start`/`stop` aliases.
- Move global status dependency wiring into
`src/lib/status-command-deps.ts` so the oclif wrapper can reuse
`showStatusCommand()` without the legacy dispatcher owning status
internals.
- Route legacy global dispatch for `status`, `tunnel`, `start`, and
`stop` through `runOclif()`.
- Add CLI help coverage for the migrated status and tunnel commands.

## 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)

## AI Disclosure
- [x] AI-assisted — tool: OpenAI Codex

---
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 a new status command to display sandbox/service/messaging health
and recent gateway logs
* Added tunnel:start and tunnel:stop commands to manage public-URL
tunnels

* **Deprecated**
* Top-level start and stop remain as deprecated aliases for
tunnel:start/tunnel:stop

* **Tests**
* Added help-mode and unit tests for status, tunnel commands, deprecated
aliases, runner behavior, sharing, and status deps

* **Refactor**
* CLI now delegates to the oclif runner and preserves branded
help/output; top-level CLI dispatch forwards to oclif commands
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

<!-- STACK-CONTEXT:START -->
## Stack position

> [!NOTE]
> This PR is part of a stacked refactor. Please review/merge in stack
order.
> The current PR is highlighted below.

This is PR **#2775** in the stacked CLI oclif refactor series.

- Merge after: **`main`**
- Next PR: **#2776**
- Stack position: **1 / 37**

## Full stack

Merge from top to bottom.

| Order | PR | Merge after | Title |
|---:|---|---|---|
| 1 | **#2775** | `main` | **refactor(cli): migrate status and tunnel
commands to oclif** |
| 2 | #2776 | #2775 | refactor(cli): migrate debug uninstall and
gateway-token to oclif |
| 3 | #2786 | #2776 | refactor(cli): migrate credentials commands to
oclif |
| 4 | #2787 | #2786 | refactor(cli): migrate sandbox inspection commands
to oclif |
| 5 | #2788 | #2787 | refactor(cli): migrate maintenance commands to
oclif |
| 6 | #2796 | #2788 | refactor(cli): migrate sandbox logs command to
oclif |
| 7 | #2797 | #2796 | refactor(cli): migrate skill install command to
oclif |
| 8 | #2798 | #2797 | refactor(cli): migrate snapshot list and create to
oclif |
| 9 | #2802 | #2798 | refactor(cli): migrate shields commands to oclif |
| 10 | #2803 | #2802 | refactor(cli): migrate channels mutation commands
to oclif |
| 11 | #2804 | #2803 | refactor(cli): migrate policy mutation commands
to oclif |
| 12 | #2805 | #2804 | refactor(cli): migrate snapshot restore command
to oclif |
| 13 | #2806 | #2805 | refactor(cli): migrate destroy command to oclif |
| 14 | #2807 | #2806 | refactor(cli): migrate rebuild command to oclif |
| 15 | #2808 | #2807 | refactor(cli): migrate connect command to oclif |
| 16 | #2809 | #2808 | refactor(cli): migrate deploy command to oclif |
| 17 | #2810 | #2809 | refactor(cli): migrate onboard aliases to oclif |
| 18 | #2811 | #2810 | refactor(cli): migrate help and version commands
to oclif |
| 19 | #2814 | #2811 | refactor(cli): centralize legacy oclif dispatch |
| 20 | #2815 | #2814 | refactor(cli): drop trivial oclif wrapper helpers
|
| 21 | #2816 | #2815 | refactor(cli): centralize sandbox runtime bridge
|
| 22 | #2817 | #2816 | refactor(cli): centralize policy and channels
runtime bridge |
| 23 | #2818 | #2817 | refactor(cli): centralize global runtime bridge |
| 24 | #2819 | #2818 | refactor(cli): collapse runtime bridge exports |
| 25 | #2826 | #2819 | refactor(cli): introduce sandbox runtime action
facade |
| 26 | #2827 | #2826 | refactor(cli): introduce policy and channels
action facade |
| 27 | #2828 | #2827 | refactor(cli): introduce global command action
facade |
| 28 | #2830 | #2828 | refactor(cli): extract root help action |
| 29 | #2831 | #2830 | refactor(cli): extract deploy action |
| 30 | #2832 | #2831 | refactor(cli): extract onboard actions |
| 31 | #2835 | #2832 | refactor(cli): extract openshell runtime helpers
|
| 32 | #2836 | #2835 | refactor(cli): extract sandbox logs action |
| 33 | #2837 | #2836 | refactor(cli): extract maintenance actions |
| 34 | #2838 | #2837 | refactor(cli): extract snapshot actions |
| 35 | #2839 | #2838 | refactor(cli): extract policy and channels
actions |
| 36 | #2840 | #2839 | refactor(cli): extract credentials and list
runtime bits |
| 37 | #2841 | #2840 | refactor(cli): shrink runtime bridge |
<!-- STACK-CONTEXT:END -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
@cv
cv deleted the refactor/oclif-bridge-cleanup branch May 27, 2026 21:17
@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