refactor(cli): preserve display registry and drop test bridges#3838
Conversation
|
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 (12)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughRemoves runtime-bridge indirection from sandbox CLI commands, centralizes public display metadata in PUBLIC_DISPLAY_LAYOUT, converts several action helpers to options-based signatures, updates commands to call action functions directly, and refactors related tests to use Vitest module mocks. ChangesRuntime bridge removal and centralized display
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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
|
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. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/commands/sandbox/channels/stop.ts (1)
23-26: Run the channel stop/start lifecycle E2E before merge.This refactor touches the command path that influences disabled-channel persistence and credential reattachment across rebuild, so run
channels-stop-start-e2eon this branch.As per coding guidelines:
src/commands/sandbox/channels/**changes affect stop/start persistence across rebuild and recommendchannels-stop-start-e2e.🤖 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/commands/sandbox/channels/stop.ts` around lines 23 - 26, This change touches the sandbox channel stop/start path (ChannelsStopCommand -> stopSandboxChannel with channelMutationOptions) which can affect disabled-channel persistence and credential reattachment across rebuilds; before merging, run the channels-stop-start-e2e suite (locally and/or in CI) against this branch, verify the test passes, inspect and fix any failures related to disabled-channel persistence or credential reattachment during rebuilds, then re-run the e2e until green and include test results with the PR.
🤖 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.
Nitpick comments:
In `@src/commands/sandbox/channels/stop.ts`:
- Around line 23-26: This change touches the sandbox channel stop/start path
(ChannelsStopCommand -> stopSandboxChannel with channelMutationOptions) which
can affect disabled-channel persistence and credential reattachment across
rebuilds; before merging, run the channels-stop-start-e2e suite (locally and/or
in CI) against this branch, verify the test passes, inspect and fix any failures
related to disabled-channel persistence or credential reattachment during
rebuilds, then re-run the e2e until green and include test results with the PR.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f19c7c01-a410-4928-9e16-3add9d13a0b2
📒 Files selected for processing (19)
src/commands/sandbox/channels/add.tssrc/commands/sandbox/channels/list.tssrc/commands/sandbox/channels/mutate.test.tssrc/commands/sandbox/channels/remove.tssrc/commands/sandbox/channels/start.tssrc/commands/sandbox/channels/stop.tssrc/commands/sandbox/policy/add.tssrc/commands/sandbox/policy/list.tssrc/commands/sandbox/policy/mutate.test.tssrc/commands/sandbox/policy/remove.tssrc/commands/sandbox/skill/install.tssrc/commands/sandbox/snapshot/create.tssrc/commands/sandbox/snapshot/list.tssrc/commands/sandbox/snapshot/restore.tssrc/lib/cli/public-display-defaults.tssrc/lib/domain/policy-channel.test.tssrc/lib/domain/policy-channel.tssrc/lib/sandbox/channels-command-support.tssrc/lib/sandbox/policy-command-support.ts
💤 Files with no reviewable changes (8)
- src/commands/sandbox/channels/list.ts
- src/commands/sandbox/policy/list.ts
- src/commands/sandbox/skill/install.ts
- src/commands/sandbox/snapshot/list.ts
- src/lib/sandbox/policy-command-support.ts
- src/commands/sandbox/snapshot/create.ts
- src/commands/sandbox/snapshot/restore.ts
- src/lib/sandbox/channels-command-support.ts
Selective E2E Results — ✅ All requested jobs passedRun: 26131087607
|
Selective E2E Results — ❌ Some jobs failedRun: 26132321509
|
Selective E2E Results — ✅ All requested jobs passedRun: 26133966852
|
Selective E2E Results — ❌ Some jobs failedRun: 26134670231
|
Selective E2E Results — ❌ Some jobs failedRun: 26135123825
|
Selective E2E Results — ❌ Some jobs failedRun: 26147832984
|
Selective E2E Results — ✅ All requested jobs passedRun: 26147832984
|
## Summary Refreshes NemoClaw release notes for v0.0.47 and v0.0.48, then regenerates the corresponding user-skill references so agent-facing docs match the source pages. Preview: https://nvidia-preview-docs-release-notes-47-48.docs.buildwithfern.com/nemoclaw/about/release-notes ## Changes - Adds explicit v0.0.47 and v0.0.48 sections to `docs/about/release-notes.mdx`. - Documents follow-up WSL Ollama, sandbox image, share mount, and troubleshooting updates from recent release changes. - Regenerates `nemoclaw-user-*` skill references from the Fern MDX source docs. ## Source Summary - #4003 -> `docs/about/release-notes.mdx`: Notes the messaging manifest registry work as part of v0.0.48 release coverage. - #3984 -> `docs/about/release-notes.mdx`: Captures Hermes messaging policy scoping in the v0.0.48 release notes. - #3963 -> `docs/about/release-notes.mdx`: Captures DGX Spark Hermes GPU recreation startup recovery in the v0.0.48 release notes. - #3961 -> `docs/about/release-notes.mdx`: Captures Discord loopback proxy routing in the v0.0.48 release notes. - #3940 -> `docs/about/release-notes.mdx`: Captures installer prompt clarification and express-install behavior in the v0.0.48 release notes. - #3946 -> `docs/about/release-notes.mdx`: Carries forward the Homebrew preinstall clarification in release coverage. - #3937 -> `docs/about/release-notes.mdx`: Carries forward the dashboard URL command and post-install next steps coverage. - #3921 -> `docs/about/release-notes.mdx`: Carries forward managed vLLM default behavior for DGX Spark and DGX Station. - #3931 -> `docs/about/release-notes.mdx`, `docs/reference/architecture.mdx`: Documents the sandbox `python` to `python3` compatibility symlink. - #1485 -> `docs/about/release-notes.mdx`, `docs/reference/architecture.mdx`: Documents the sandbox image Docker health check. - #3784 -> `docs/about/release-notes.mdx`: Captures VM-driver snapshot health-check reliability in release notes. - #3917 -> `docs/about/release-notes.mdx`: Captures package-based workspace template resolution in release notes. - #3170 -> `docs/about/release-notes.mdx`: Captures installer checksum compatibility from preferring `sha256sum`. - #3898 -> `docs/about/release-notes.mdx`: Adds v0.0.47 release coverage for messaging provider scenario validation. - #3897 -> `docs/about/release-notes.mdx`: Adds v0.0.47 release coverage for baseline onboarding scenario validation. - #3834 -> `docs/about/release-notes.mdx`: Adds v0.0.47 release coverage for PR review advisor automation. - #3838 -> `docs/about/release-notes.mdx`: Adds v0.0.47 release coverage for CLI display registry refactoring. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [x] 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) `make docs` was attempted but could not complete because `npx fern-api` failed with `403 Forbidden` from `https://registry.npmjs.org/fern-api` in this environment. Pre-commit and pre-push hooks passed after refreshing the local CLI build output with `npm run build:cli`; no build artifacts were committed. --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added WSL onboarding notes for Windows-host Ollama detection, restart guidance, and PowerShell checks. * Clarified express-install behavior (non-interactive, sudo prompts) and default sandbox policy selection. * Added Windows preparation guidance when installer tooling is missing (winget/App Installer or Docker Desktop). * Expanded sandbox docs with Docker health checks, Homebrew/python compatibility helpers, share-mount path validation, Discord troubleshooting, and new v0.0.48/v0.0.47 release notes. * **Chores** * Improved docs preview workflow error handling. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4007?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Keeps the public sandbox-first display mapping centralized in
public-display-defaults.tsafter the previous CLI adapter cleanup. It also removes remaining mutation/log bridge helpers, trims legacy argv parser tests, and renames the direct oclif command-id runner so it no longer reads like legacy compatibility code.Changes
publicDisplaymove so public UX mappings remain centralized insrc/lib/cli/public-display-defaults.ts.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
Release Notes
Refactor
Tests
Chores