refactor(cli): expose explicit main entrypoint - #2901
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. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe CLI entry point is refactored by extracting the top-level async IIFE into a named, exported ChangesCLI Entry Point Restructuring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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)
Comment |
## Summary Remove the remaining legacy dispatch fallback targets for policy-add, skill, and snapshot edge paths. These paths now route through hidden/raw oclif adapters that preserve the existing command-action behavior without using the legacy dispatch variant. ## Stack Navigation - Position: 10 of 60 - Previous PR: [#2898 — refactor(cli): remove runtime bridge](#2898) - Next PR: [#2901 — refactor(cli): expose explicit main entrypoint](#2901) ## Changes - Added raw oclif adapters for policy-add missing-value handling, skill usage/unknown subcommands, and snapshot usage/unknown subcommands. - Registered the raw adapters in the oclif command map. - Removed `LegacyDispatch` and the `legacy` handling branch from `src/nemoclaw.ts`. - Added dispatch tests covering the newly routed policy, skill, and snapshot fallback paths. ## 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 * **Refactoring** * Streamlined sandbox command routing so help and unknown subcommands are forwarded consistently and original arguments are preserved. * Unified behavior for policy, skill, and snapshot commands to improve predictability. * **New Features** * Sandbox commands now accept raw argument forms and validate missing sandbox names with clearer exit behavior. * **Tests** * Added coverage for the updated dispatch and routing behaviors. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
prekshivyas
left a comment
There was a problem hiding this comment.
LGTM after rebase. Tiny clean refactor — 1 file / +8 / -5.
- Top-level argv destructure moves into the new
main(argv = process.argv.slice(2)). - Anonymous IIFE replaced with the named
async function main. exports.mainPromise = main()preserves the load-time side-effect invocation byte-equivalent to the old(async () => {...})().- New
exports.mainenables in-process harnesses to call with explicit argv. Comment correctly directs new callers tomain(argv)over the legacymainPromise.
No argv parsing, exit, or rejection-handling deltas. bin/nemoclaw.js untouched, package.json bin mapping untouched — public CLI surface identical.
CI: pr.yaml fully green; pr-self-hosted build-sandbox-images/arm64 still in flight at review time but no failures. Cumulative ~16-orphan debt in src/nemoclaw.ts unchanged here, still pending the dedicated cleanup PR.
## Summary Add oclif examples to low-risk global and utility commands so generated help has concrete usage snippets. This starts the oclif UX pass without changing command behavior. ## Stack Navigation - Position: 12 of 60 - Previous PR: [#2901 — refactor(cli): expose explicit main entrypoint](#2901) - Next PR: [#2905 — refactor(cli): validate logs flags with oclif](#2905) ## Changes - Added examples for `list`, global `status`, `gateway-token`, `credentials`, `credentials list`, and `credentials reset`. - Added examples for `backup-all`, `upgrade-sandboxes`, and `gc`. - Added examples for `uninstall`, `tunnel start`, `tunnel stop`, and deprecated `start`/`stop` aliases. - Added a help flag declaration to the uninstall oclif adapter to align its metadata with other utility 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 - [ ] 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 for comprehensive health diagnostics across sandbox infrastructure, gateway, and services * Introduced automated sandbox process recovery and health probing capabilities * Enhanced sandbox lifecycle management with improved destroy, rebuild, and skill installation workflows * **Documentation** * Expanded CLI usage examples across multiple commands for improved discoverability and guidance <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…aging (NVIDIA#1691) When a sandbox is created without enabling Telegram (or Discord, or Slack) during `nemoclaw onboard`, applying the matching policy preset via `nemoclaw <name> policy-add` only opens network egress to the channel API. The bot token, channel configuration, and in-sandbox bridge are wired up at onboard time, so users who apply the preset after onboarding without having enabled the channel can reasonably believe they have enabled messaging when only the firewall has been widened. Add `getMessagingPresetWarning()` in `src/lib/policies.ts` and surface it in `addSandboxPolicy()` (now in `src/lib/policy-channel-actions.ts` after the recent CLI dispatch refactor) before the apply confirmation so users see, for example, that the `telegram` preset alone does not enable Telegram bots and that re-running `nemoclaw onboard` with Telegram selected is the path to actually enabling the channel. This is a rebase of an earlier branch onto current main: - Hook moved from the legacy `src/nemoclaw.ts` dispatcher to the new `src/lib/policy-channel-actions.ts:addSandboxPolicy` after NVIDIA#2899 / NVIDIA#2901 / NVIDIA#2907 extracted dispatch. - `getMessagingPresetWarning` got an explicit TS signature (`presetName: string): string | null`) to match the rest of `src/lib/policies.ts`. - Replaced the em dash in the warning message with a period for consistency with project style. Originally three commits (warning logic + ordering assertion + tmpDir cleanup) on the prior branch; consolidated here because the rebase needed the dispatcher hook ported to a new file. Closes NVIDIA#1691 Re-ran `npx vitest run test/policies.test.ts` after rebase: 120/120 pass. Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
Summary
Introduce an explicit
main(argv)entrypoint for the CLI module. The existingmainPromiseexport remains as a thin compatibility shim for in-process test harnesses that still require and await the CLI module.Stack Navigation
Changes
main(argv = process.argv.slice(2)).mainfor new in-process harnesses and future launcher cleanup.mainPromise = main()compatibility with a comment directing new callers tomain(argv).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