refactor(cli): remove runtime bridge - #2898
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe PR refactors CLI runtime dependency injection by introducing a test-only runtime hooks mechanism in ChangesTest-Only Runtime Hooks Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
Comment |
## Summary Extract upgrade-sandboxes orchestration from `src/nemoclaw.ts` into a dedicated action module. This removes the final command action from the transitional runtime bridge while preserving stale-version detection and rebuild-loop behavior. ## Stack Navigation - Position: 8 of 60 - Previous PR: [#2896 — refactor(cli): extract sandbox rebuild action](#2896) - Next PR: [#2898 — refactor(cli): remove runtime bridge](#2898) ## Changes - Added `src/lib/upgrade-sandboxes-action.ts` for stale/unknown version classification and optional rebuild orchestration. - Updated `src/lib/global-cli-actions.ts` to call the extracted upgrade action. - Removed `upgradeSandboxes` from `src/nemoclaw.ts` and emptied the `NemoClawRuntimeBridge` action surface. - Removed the remaining runtime-bridge dependency from `src/lib/sandbox-runtime-actions.ts`. ## 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 * **Refactor** * Internal reorganization of the sandbox upgrade system to improve code modularity and maintainability. The upgrade functionality, including sandbox stale detection and batch rebuild operations with `--check`, `--auto`, and `--yes` flag support, remains available as before. <!-- 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. Clean removal of the now-empty runtime bridge.
src/lib/nemoclaw-runtime-bridge.tsdeleted; the singleglobal-cli-actions.tsconsumer rewiresrecoverNamedGatewayRuntimeandrunOpenshellProviderCommandthrough a localruntimeHooksobject with production fall-through to the real implementations.- Test refactor in
test/credentials-cli-command.test.tsswapsrequire.cache[dist/nemoclaw.js]injection for the explicitsetGlobalCliActionRuntimeHooksForTestAPI — strict improvement, no spooky cache-mutation seam. - Zero new orphans in
src/nemoclaw.ts(this PR doesn't touch it). Cumulative ~16-orphan debt from earlier stack PRs still pending the dedicated cleanup PR.
CI: pr.yaml fully green; pr-self-hosted build-sandbox-images/arm64 still in flight at review time, plus wsl-e2e. No failures so far — worth confirming green before merging.
## 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>
Summary
Remove the now-empty transitional NemoClaw runtime bridge from the oclif CLI layer. Credentials tests now inject gateway/OpenShell hooks directly through
global-cli-actionsinstead of stubbing../nemoclaw.Stack Navigation
Changes
src/lib/nemoclaw-runtime-bridge.ts.src/lib/global-cli-actions.tsfor credential command gateway recovery and provider operations.dist/nemoclaw.js.src/libcode imports or requires../nemoclaw.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
Refactor
Tests