feat(cli): typed command registry as single source of truth (#2388)#2446
Merged
Conversation
…2388) Create src/lib/command-registry.ts with: - CommandDef interface and CommandGroup union type - COMMANDS array with all 46 CLI commands (23 global + 23 sandbox) - Helper functions: globalCommands, sandboxCommands, visibleCommands, commandsByGroup, canonicalUsageList, globalCommandTokens, sandboxActionTokens - GROUP_ORDER for consistent help display ordering 11 hidden commands (5 help/version aliases + 6 shields/config) are excluded from help output and canonical list but included in dispatch derivation. Unit tests cover all invariants: counts, deduplication, ordering, token extraction.
Replace hand-written help() with registry-driven output: - Import commandsByGroup from command-registry - Iterate groups in GROUP_ORDER for consistent section ordering - Apply bold/dim styling for first-in-group and deprecated commands - Rewrite Reconfiguration section with bullet points to prevent phantom nemoclaw-prefixed lines from confusing check-docs.sh The help output visually matches the previous layout while being derived entirely from the single source of truth.
…gistry (#2388) Replace hand-maintained dispatch arrays with registry-derived values: - GLOBAL_COMMANDS = globalCommandTokens() (was new Set([...])) - sandboxActions = sandboxActionTokens() (was inline array) - Update image-cleanup.test.ts to verify gc via registry source instead of regex-matching the old Set literal
…ump (#2388) Phase 4 of the typed command registry: - Add --dump-commands flag to CLI dispatch (internal developer flag) - Replace fragile perl help-parsing pipeline in check-docs.sh with direct --dump-commands invocation - Strip <arg> and [optional] placeholders from docs headings in check-docs.sh Phase 2 for clean comparison - Add missing docs headings: onboard --from, setup, start, stop - check-docs.sh --only-cli now passes with 35 matched commands
- Remove DISABLED comment and CLOUD_EXPERIMENTAL_E2E_ENABLED variable gate - Remove SKIP_CHECK_DOCS=1 flag since parity check now passes - Job runs unconditionally for the NVIDIA/NemoClaw repository
Contributor
|
Caution Review failedPull request was closed or merged during review 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 (6)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA new typed CLI command registry centralizes command metadata and powers runtime help, token extraction, and a Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant CLI as nem oclaw CLI
participant Reg as command-registry
participant Docs as docs/reference/commands.md
participant Check as check-docs.sh
participant CI as GitHub Actions (nightly-e2e)
Dev->>Reg: add/modify CommandDef entry
CLI->>Reg: call commandsByGroup(), globalCommandTokens(), sandboxActionTokens()
CLI->>Dev: on --dump-commands emit canonicalUsageList()
Check->>CLI: run --dump-commands
Check->>Docs: parse headings and normalize
Check->>Reg: compare normalized docs vs canonicalUsageList()
CI->>Check: execute check-docs.sh during cloud-experimental-e2e
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/nightly-e2e.yaml (1)
90-102:⚠️ Potential issue | 🟠 MajorRestore the docs-skip env or remove the dedicated docs step.
This job still runs
check-docs.shin its own step below, but the main E2E step no longer setsRUN_E2E_CLOUD_EXPERIMENTAL_SKIP_CHECK_DOCS=1. That makes docs parity run twice, adds avoidable runtime, and can fail the main E2E step before the dedicated docs step gets a chance to isolate the problem.Suggested fix
- name: Run cloud-experimental E2E test env: NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }} GITHUB_TOKEN: ${{ github.token }} # Non-interactive install (expect-driven Phase 3 optional). Runner has no expect; Phase 5e TUI skips if expect is absent. RUN_E2E_CLOUD_EXPERIMENTAL_INTERACTIVE_INSTALL: "0" + RUN_E2E_CLOUD_EXPERIMENTAL_SKIP_CHECK_DOCS: "1" NEMOCLAW_NON_INTERACTIVE: "1" NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1" NEMOCLAW_RECREATE_SANDBOX: "1" NEMOCLAW_POLICY_MODE: "custom" NEMOCLAW_POLICY_PRESETS: "npm,pypi"Also applies to: 104-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/nightly-e2e.yaml around lines 90 - 102, The main E2E step is missing the RUN_E2E_CLOUD_EXPERIMENTAL_SKIP_CHECK_DOCS=1 environment variable so docs checking runs twice; either add RUN_E2E_CLOUD_EXPERIMENTAL_SKIP_CHECK_DOCS: "1" to the Run cloud-experimental E2E test step (so check-docs.sh only runs in the dedicated docs step) or remove the dedicated check-docs.sh step entirely; update the workflow where the Run cloud-experimental E2E test env is defined and/or remove the separate check-docs.sh step to ensure docs parity runs exactly once.
🧹 Nitpick comments (1)
docs/reference/commands.md (1)
617-628: Avoid maintaining twosetupsections.This adds a second
nemoclaw setupentry while the page still has### Legacy \nemoclaw setup`` later on. Keeping both descriptions in sync will reintroduce the same drift this PR is trying to remove; it would be safer to fold the legacy content into this new heading and keep a single source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/commands.md` around lines 617 - 628, There are two separate "nemoclaw setup" sections causing duplication; remove the later "Legacy `nemoclaw setup`" section and merge any unique legacy details into the current "### `nemoclaw setup`" block so there is a single canonical entry; ensure the merged block preserves the deprecation notice, the compatibility alias sentence, and any legacy usage examples or caveats previously under "Legacy `nemoclaw setup`" and delete the duplicate heading.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/command-registry.ts`:
- Around line 353-358: The registry entry for the "nemoclaw uninstall" command
is wrong: either remove the code that passes a remoteScriptUrl into
runUninstallCommand or update the registry description to match the actual
behavior; specifically, because src/nemoclaw.ts still passes remoteScriptUrl
into runUninstallCommand, change the command registry object's description (the
entry with usage "nemoclaw uninstall") to note that it will attempt a remote
fallback (or otherwise accurately describe remoteScriptUrl behavior) so
user-facing help matches the implementation.
- Around line 26-40: The CommandDef.usage field is being used both as the
canonical command ID and as the help-facing syntax, which causes help() to show
non-runnable placeholders; add an optional helpUsage (or help?) property to
CommandDef and update help() to render cmd.helpUsage ?? cmd.usage while leaving
canonicalUsageList() and any logic that expects the canonical ID using the
original usage value; locate CommandDef, canonicalUsageList(), and help() in
command-registry.ts and change types and rendering accordingly so docs/ID
generation still use usage but help output prefers helpUsage when present.
In `@test/e2e/e2e-cloud-experimental/check-docs.sh`:
- Around line 145-146: The test is capturing stderr into the canonical command
list because the invocation of "$NODE" "$CLI_JS" --dump-commands uses 2>&1;
change it to capture stdout only by removing 2>&1 and redirecting stdout into
"$_tmp/help.txt" (retain the existing LC_ALL=C sort -u pipeline), and capture
stderr separately (e.g., to a temporary stderr file or variable) so you can
print or fail with the stderr contents if the command exits non-zero; update the
invocation in check-docs.sh where "$NODE" "$CLI_JS" --dump-commands is called to
implement this separation.
---
Outside diff comments:
In @.github/workflows/nightly-e2e.yaml:
- Around line 90-102: The main E2E step is missing the
RUN_E2E_CLOUD_EXPERIMENTAL_SKIP_CHECK_DOCS=1 environment variable so docs
checking runs twice; either add RUN_E2E_CLOUD_EXPERIMENTAL_SKIP_CHECK_DOCS: "1"
to the Run cloud-experimental E2E test step (so check-docs.sh only runs in the
dedicated docs step) or remove the dedicated check-docs.sh step entirely; update
the workflow where the Run cloud-experimental E2E test env is defined and/or
remove the separate check-docs.sh step to ensure docs parity runs exactly once.
---
Nitpick comments:
In `@docs/reference/commands.md`:
- Around line 617-628: There are two separate "nemoclaw setup" sections causing
duplication; remove the later "Legacy `nemoclaw setup`" section and merge any
unique legacy details into the current "### `nemoclaw setup`" block so there is
a single canonical entry; ensure the merged block preserves the deprecation
notice, the compatibility alias sentence, and any legacy usage examples or
caveats previously under "Legacy `nemoclaw setup`" and delete the duplicate
heading.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a57f21ec-8109-4a53-8ba9-60f7dd994cc7
📒 Files selected for processing (7)
.github/workflows/nightly-e2e.yamldocs/reference/commands.mdsrc/lib/command-registry.test.tssrc/lib/command-registry.tssrc/nemoclaw.tstest/e2e/e2e-cloud-experimental/check-docs.shtest/image-cleanup.test.ts
Comment on lines
+26
to
+40
| export interface CommandDef { | ||
| /** Canonical command signature, e.g. "nemoclaw <name> snapshot create" */ | ||
| usage: string; | ||
| /** One-line description for help output */ | ||
| description: string; | ||
| /** Optional flag syntax, e.g. "[--name <label>]" */ | ||
| flags?: string; | ||
| /** Section header in help output */ | ||
| group: CommandGroup; | ||
| /** Deprecated commands show dimmed in help */ | ||
| deprecated?: boolean; | ||
| /** Hidden commands are excluded from help and canonical list but included in dispatch */ | ||
| hidden?: boolean; | ||
| /** Whether this command is global or sandbox-scoped */ | ||
| scope: "global" | "sandbox"; |
Contributor
There was a problem hiding this comment.
Split canonical command IDs from help syntax.
usage is now doing double duty for docs parity and nemoclaw help, so the help output drops required arguments for several runnable commands: nemoclaw onboard --from <Dockerfile>, nemoclaw <name> skill install <path>, nemoclaw <name> channels add <channel>, nemoclaw credentials reset <KEY>, and similar entries. The docs checker compensates by stripping placeholders from headings, but help() renders these strings verbatim, so users no longer see runnable syntax.
Suggested direction
export interface CommandDef {
- /** Canonical command signature, e.g. "nemoclaw <name> snapshot create" */
+ /** Canonical command signature used for docs parity/token derivation. */
usage: string;
+ /** User-facing syntax shown by `nemoclaw help`. Defaults to `usage`. */
+ helpUsage?: string;
/** One-line description for help output */
description: string;Then keep canonicalUsageList() on usage, but have help() render cmd.helpUsage ?? cmd.usage:
{
usage: "nemoclaw onboard --from",
+ helpUsage: "nemoclaw onboard --from <Dockerfile>",
description: "Use a custom Dockerfile for the sandbox image",
},
{
usage: "nemoclaw <name> skill install",
+ helpUsage: "nemoclaw <name> skill install <path>",
description: "Deploy a skill directory to the sandbox",
},
{
usage: "nemoclaw credentials reset",
+ helpUsage: "nemoclaw credentials reset <KEY>",
description: "Remove a stored credential so onboard re-prompts",
},Also applies to: 72-77, 141-146, 177-200, 321-326
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/command-registry.ts` around lines 26 - 40, The CommandDef.usage field
is being used both as the canonical command ID and as the help-facing syntax,
which causes help() to show non-runnable placeholders; add an optional helpUsage
(or help?) property to CommandDef and update help() to render cmd.helpUsage ??
cmd.usage while leaving canonicalUsageList() and any logic that expects the
canonical ID using the original usage value; locate CommandDef,
canonicalUsageList(), and help() in command-registry.ts and change types and
rendering accordingly so docs/ID generation still use usage but help output
prefers helpUsage when present.
Comment on lines
+353
to
+358
| { | ||
| usage: "nemoclaw uninstall", | ||
| description: "Run uninstall.sh (local only; no remote fallback)", | ||
| group: "Cleanup", | ||
| scope: "global", | ||
| }, |
Contributor
There was a problem hiding this comment.
Fix the uninstall description before it becomes user-facing help.
The registry says nemoclaw uninstall is “local only; no remote fallback”, but src/nemoclaw.ts still passes a remoteScriptUrl into runUninstallCommand(). With help now driven from this registry, the CLI will advertise the opposite of its actual behavior.
Suggested fix
{
usage: "nemoclaw uninstall",
- description: "Run uninstall.sh (local only; no remote fallback)",
+ description: "Run uninstall.sh (local first; remote fallback if missing)",
group: "Cleanup",
scope: "global",
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| { | |
| usage: "nemoclaw uninstall", | |
| description: "Run uninstall.sh (local only; no remote fallback)", | |
| group: "Cleanup", | |
| scope: "global", | |
| }, | |
| { | |
| usage: "nemoclaw uninstall", | |
| description: "Run uninstall.sh (local first; remote fallback if missing)", | |
| group: "Cleanup", | |
| scope: "global", | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/command-registry.ts` around lines 353 - 358, The registry entry for
the "nemoclaw uninstall" command is wrong: either remove the code that passes a
remoteScriptUrl into runUninstallCommand or update the registry description to
match the actual behavior; specifically, because src/nemoclaw.ts still passes
remoteScriptUrl into runUninstallCommand, change the command registry object's
description (the entry with usage "nemoclaw uninstall") to note that it will
attempt a remote fallback (or otherwise accurately describe remoteScriptUrl
behavior) so user-facing help matches the implementation.
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
cv
reviewed
Apr 24, 2026
cv
left a comment
Collaborator
There was a problem hiding this comment.
Pulled this locally and merged current main in e3309437 to clear the conflicts in src/nemoclaw.ts, test/image-cleanup.test.ts, and the docs-parity normalizer used by check-docs.sh.
A few review notes after reading the diff end-to-end and reconciling it with current main:
- The strongest part of this change is the elimination of three independent command inventories. Deriving dispatch/help/docs parity from one registry is exactly the right direction, and
--dump-commandsis much less fragile than scraping ANSI help output. - The main thing I’d still watch is that
CommandDef.usageis currently serving two jobs at once: the canonical docs key and the rendered help syntax. Because of that, the generated help now drops some operand/detail text that used to be visible to users (onboard --from <Dockerfile>,skill install <path>,channels add <channel>,credentials reset <KEY>,debug --output FILE, etc.). The docs parity check still passes because headings are normalized, but the interactive help UX regresses a bit. I think this probably wants either a separate canonical identifier vs. display string, or a more structured representation for args/options. - While merging
main, I preserved the newersnapshot restore ... --to <dst>guidance and the long-formopenshell inference set --model/--providerguidance. Those landed after this branch forked and are easy to lose if the registry data stops carrying the same level of detail as help text. - I also had to harden the docs normalizer to peel multiple trailing bracket groups. Current docs now have forms like
snapshot restore [selector] [--to <dst>], which exposed that the previous normalization logic still assumed a single optional tail. A small regression test around canonicalization would make this path a lot less brittle. - Coverage-wise, the registry invariants are well tested, but there’s still a gap at the final rendered-help layer. A normalized snapshot/golden test for
nemoclaw --helpwould catch exactly the class of regressions above.
Local verification on the merged branch:
npm run build:clinpx vitest run src/lib/command-registry.test.ts test/image-cleanup.test.tsCHECK_DOC_LINKS_REMOTE=0 bash test/e2e/e2e-cloud-experimental/check-docs.sh
(Full remote link checking still trips on the existing https://platform.openai.com/api-keys docs URL, which looks unrelated to this PR.)
cv
approved these changes
Apr 24, 2026
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
jyaunches
added a commit
to jyaunches/NemoClaw
that referenced
this pull request
Apr 28, 2026
…DIA#2644) Replace the 931-line cloud-experimental-e2e monolith with 4 independent, focused E2E tests: - cloud-onboard-e2e: public installer + Landlock + security checks - cloud-inference-e2e: live chat via inference.local + skill filesystem - skill-agent-e2e: skill injection + agent verification (with retry + fuzzy match) - docs-validation-e2e: CLI/docs parity via check-docs.sh Key improvements: - Flaky Phase 5d (skill agent) no longer blocks Landlock/security signal - Phase 5e (TUI smoke) dropped from nightly — timing-dependent expect - Phase 5f (docs validation) re-enabled as own job (root cause fixed by NVIDIA#2446) - Each test installs independently — no cascade failures - skill-agent-e2e adds retry logic + fuzzy token matching for LLM non-determinism Also removes dead scripts from e2e-cloud-experimental/skip/ (01-onboard-completion, 04-nemoclaw-openshell-status-parity, 05-network-policy — all covered by dedicated jobs). Fixes NVIDIA#2644 Signed-off-by: Jessica Yaunches <jyaunches@nvidia.com>
jyaunches
added a commit
that referenced
this pull request
Apr 29, 2026
…) (#2647) ## Summary Replace the 931-line `cloud-experimental-e2e` monolith with 4 independent, focused E2E tests. Flaky phases no longer block signal from stable phases. Fixes #2644 ## New Jobs | Job | What it tests | Timeout | Former phases | |-----|--------------|---------|---------------| | `cloud-onboard-e2e` | Public installer, Landlock enforcement, API key leak detection, inference.local HTTPS | 45 min | 0, 1, 3, 5 checks, 6 | | `cloud-inference-e2e` | Live chat via inference.local, skill filesystem validation | 30 min | 5b, 5c | | `skill-agent-e2e` | Skill injection + agent verification (with retry + fuzzy match) | 30 min | 5d | | `docs-validation-e2e` | CLI/docs parity (nemoclaw --help vs commands.md) | 15 min | 5f | ## Key Improvements - **Flaky Phase 5d** (skill agent) no longer blocks Landlock/security signal - **Phase 5e** (TUI smoke) dropped from nightly — timing-dependent expect with 7+ tunable sleeps is fundamentally unsuited for CI - **Phase 5f** (docs validation) re-enabled as its own job — root cause fixed by #2446 (typed command registry) - Each test installs independently — no cascade failures - `skill-agent-e2e` adds **retry logic** (3 attempts, 15s between) and **fuzzy token matching** (case-insensitive, strips quotes/backticks) to handle LLM non-determinism - Remote link probing disabled for docs validation (`CHECK_DOC_LINKS_REMOTE=0`) to avoid 429 rate-limit flakes ## Deleted - `test/e2e/test-e2e-cloud-experimental.sh` (931-line monolith) - `test/e2e/e2e-cloud-experimental/skip/` (3 dead scripts — coverage already exists in dedicated jobs: `network-policy-e2e`, `sandbox-operations-e2e`) ## Kept - All helpers under `test/e2e/e2e-cloud-experimental/` (checks/, features/, cleanup.sh, check-docs.sh) — referenced by the new scripts ## Type of Change - Code change (refactor) ## Verification - YAML validated locally - All 29 nightly jobs have selective dispatch guards (validated with script) - All 4 new jobs in `notify-on-failure` and `report-to-pr` needs lists - `workflow_dispatch.inputs.jobs` description updated with new job names <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Replaced one monolithic nightly E2E job with four targeted nightly jobs (onboard, inference, skill-agent, docs validation); updated notification/reporting workflow dependencies and workflow inputs. * Shortened job timeouts and adjusted failure-artifact uploads for clearer, faster diagnostics. * Updated review tooling guidance to map paths to the new E2E jobs. * **Tests** * Added standalone E2E runners for cloud onboarding, cloud inference, skill-agent verification, and docs validation. * Removed legacy experimental E2E orchestration and several obsolete test scripts. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Jessica Yaunches <jyaunches@nvidia.com> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
DemianHeyGen
pushed a commit
to DemianHeyGen/NemoClaw
that referenced
this pull request
Apr 30, 2026
) (NVIDIA#2446) ## Summary Create a typed command registry (`src/lib/command-registry.ts`) as the single source of truth for all 46 NemoClaw CLI commands. Derive help output, dispatch arrays, and docs-parity checking from it, replacing three independent hand-maintained sources. Re-enable the `cloud-experimental-e2e` nightly job now that the check-docs parity check passes reliably. ## Related Issue Fixes NVIDIA#2388 ## Changes - **New `src/lib/command-registry.ts`**: Defines `CommandDef` interface, `CommandGroup` union type, and `COMMANDS` array with all 46 CLI commands (23 global + 23 sandbox). Exports helper functions: `globalCommands`, `sandboxCommands`, `visibleCommands`, `commandsByGroup`, `canonicalUsageList`, `globalCommandTokens`, `sandboxActionTokens`. - **New `src/lib/command-registry.test.ts`**: 21 unit tests covering all registry invariants — counts, deduplication, ordering, token extraction, group membership. - **Rewritten `help()` in `src/nemoclaw.ts`**: Now iterates `commandsByGroup()` from registry instead of hand-written console.log lines. Reconfiguration section rewritten with bullet points to prevent phantom `nemoclaw`-prefixed parser matches. - **Derived dispatch arrays**: `GLOBAL_COMMANDS` now calls `globalCommandTokens()` and `sandboxActions` now calls `sandboxActionTokens()` — no more hand-maintained sets/arrays. - **Added `--dump-commands` internal flag**: Prints canonical command list for check-docs.sh parity checks, replacing the fragile perl help-parsing pipeline. - **Simplified `check-docs.sh`**: Phase 1 now uses `--dump-commands` instead of perl regex extraction. Phase 2 strips `<arg>` and `[optional]` placeholders from doc headings for clean comparison. - **Added missing doc headings**: `nemoclaw onboard --from`, `nemoclaw setup`, `nemoclaw start`, `nemoclaw stop` in `docs/reference/commands.md`. - **Re-enabled `cloud-experimental-e2e`**: Removed DISABLED comment, `CLOUD_EXPERIMENTAL_E2E_ENABLED` variable gate, and `SKIP_CHECK_DOCS` flag. - **Updated `test/image-cleanup.test.ts`**: Adapted to verify `gc` via registry source instead of regex-matching the old `GLOBAL_COMMANDS` Set literal. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] 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 - [x] 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: Claude Code (pi agent) --- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added `nemoclaw onboard --from <Dockerfile>` usage guide and documented deprecated aliases (`nemoclaw start`, `nemoclaw stop`, `nemoclaw setup`) with warnings. * **New Features** * Added a developer-facing `--dump-commands` option to emit the canonical command list. * **Improvements** * Overhauled help/command organization for clearer grouping and more accurate, stable command listings. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
DemianHeyGen
pushed a commit
to DemianHeyGen/NemoClaw
that referenced
this pull request
Apr 30, 2026
…DIA#2644) (NVIDIA#2647) ## Summary Replace the 931-line `cloud-experimental-e2e` monolith with 4 independent, focused E2E tests. Flaky phases no longer block signal from stable phases. Fixes NVIDIA#2644 ## New Jobs | Job | What it tests | Timeout | Former phases | |-----|--------------|---------|---------------| | `cloud-onboard-e2e` | Public installer, Landlock enforcement, API key leak detection, inference.local HTTPS | 45 min | 0, 1, 3, 5 checks, 6 | | `cloud-inference-e2e` | Live chat via inference.local, skill filesystem validation | 30 min | 5b, 5c | | `skill-agent-e2e` | Skill injection + agent verification (with retry + fuzzy match) | 30 min | 5d | | `docs-validation-e2e` | CLI/docs parity (nemoclaw --help vs commands.md) | 15 min | 5f | ## Key Improvements - **Flaky Phase 5d** (skill agent) no longer blocks Landlock/security signal - **Phase 5e** (TUI smoke) dropped from nightly — timing-dependent expect with 7+ tunable sleeps is fundamentally unsuited for CI - **Phase 5f** (docs validation) re-enabled as its own job — root cause fixed by NVIDIA#2446 (typed command registry) - Each test installs independently — no cascade failures - `skill-agent-e2e` adds **retry logic** (3 attempts, 15s between) and **fuzzy token matching** (case-insensitive, strips quotes/backticks) to handle LLM non-determinism - Remote link probing disabled for docs validation (`CHECK_DOC_LINKS_REMOTE=0`) to avoid 429 rate-limit flakes ## Deleted - `test/e2e/test-e2e-cloud-experimental.sh` (931-line monolith) - `test/e2e/e2e-cloud-experimental/skip/` (3 dead scripts — coverage already exists in dedicated jobs: `network-policy-e2e`, `sandbox-operations-e2e`) ## Kept - All helpers under `test/e2e/e2e-cloud-experimental/` (checks/, features/, cleanup.sh, check-docs.sh) — referenced by the new scripts ## Type of Change - Code change (refactor) ## Verification - YAML validated locally - All 29 nightly jobs have selective dispatch guards (validated with script) - All 4 new jobs in `notify-on-failure` and `report-to-pr` needs lists - `workflow_dispatch.inputs.jobs` description updated with new job names <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Replaced one monolithic nightly E2E job with four targeted nightly jobs (onboard, inference, skill-agent, docs validation); updated notification/reporting workflow dependencies and workflow inputs. * Shortened job timeouts and adjusted failure-artifact uploads for clearer, faster diagnostics. * Updated review tooling guidance to map paths to the new E2E jobs. * **Tests** * Added standalone E2E runners for cloud onboarding, cloud inference, skill-agent verification, and docs validation. * Removed legacy experimental E2E orchestration and several obsolete test scripts. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Jessica Yaunches <jyaunches@nvidia.com> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
ksapru
pushed a commit
to ksapru/NemoClaw
that referenced
this pull request
May 12, 2026
) (NVIDIA#2446) ## Summary Create a typed command registry (`src/lib/command-registry.ts`) as the single source of truth for all 46 NemoClaw CLI commands. Derive help output, dispatch arrays, and docs-parity checking from it, replacing three independent hand-maintained sources. Re-enable the `cloud-experimental-e2e` nightly job now that the check-docs parity check passes reliably. ## Related Issue Fixes NVIDIA#2388 ## Changes - **New `src/lib/command-registry.ts`**: Defines `CommandDef` interface, `CommandGroup` union type, and `COMMANDS` array with all 46 CLI commands (23 global + 23 sandbox). Exports helper functions: `globalCommands`, `sandboxCommands`, `visibleCommands`, `commandsByGroup`, `canonicalUsageList`, `globalCommandTokens`, `sandboxActionTokens`. - **New `src/lib/command-registry.test.ts`**: 21 unit tests covering all registry invariants — counts, deduplication, ordering, token extraction, group membership. - **Rewritten `help()` in `src/nemoclaw.ts`**: Now iterates `commandsByGroup()` from registry instead of hand-written console.log lines. Reconfiguration section rewritten with bullet points to prevent phantom `nemoclaw`-prefixed parser matches. - **Derived dispatch arrays**: `GLOBAL_COMMANDS` now calls `globalCommandTokens()` and `sandboxActions` now calls `sandboxActionTokens()` — no more hand-maintained sets/arrays. - **Added `--dump-commands` internal flag**: Prints canonical command list for check-docs.sh parity checks, replacing the fragile perl help-parsing pipeline. - **Simplified `check-docs.sh`**: Phase 1 now uses `--dump-commands` instead of perl regex extraction. Phase 2 strips `<arg>` and `[optional]` placeholders from doc headings for clean comparison. - **Added missing doc headings**: `nemoclaw onboard --from`, `nemoclaw setup`, `nemoclaw start`, `nemoclaw stop` in `docs/reference/commands.md`. - **Re-enabled `cloud-experimental-e2e`**: Removed DISABLED comment, `CLOUD_EXPERIMENTAL_E2E_ENABLED` variable gate, and `SKIP_CHECK_DOCS` flag. - **Updated `test/image-cleanup.test.ts`**: Adapted to verify `gc` via registry source instead of regex-matching the old `GLOBAL_COMMANDS` Set literal. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] 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 - [x] 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: Claude Code (pi agent) --- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added `nemoclaw onboard --from <Dockerfile>` usage guide and documented deprecated aliases (`nemoclaw start`, `nemoclaw stop`, `nemoclaw setup`) with warnings. * **New Features** * Added a developer-facing `--dump-commands` option to emit the canonical command list. * **Improvements** * Overhauled help/command organization for clearer grouping and more accurate, stable command listings. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Create a typed command registry (
src/lib/command-registry.ts) as the single source of truth for all 46 NemoClaw CLI commands. Derive help output, dispatch arrays, and docs-parity checking from it, replacing three independent hand-maintained sources. Re-enable thecloud-experimental-e2enightly job now that the check-docs parity check passes reliably.Related Issue
Fixes #2388
Changes
src/lib/command-registry.ts: DefinesCommandDefinterface,CommandGroupunion type, andCOMMANDSarray with all 46 CLI commands (23 global + 23 sandbox). Exports helper functions:globalCommands,sandboxCommands,visibleCommands,commandsByGroup,canonicalUsageList,globalCommandTokens,sandboxActionTokens.src/lib/command-registry.test.ts: 21 unit tests covering all registry invariants — counts, deduplication, ordering, token extraction, group membership.help()insrc/nemoclaw.ts: Now iteratescommandsByGroup()from registry instead of hand-written console.log lines. Reconfiguration section rewritten with bullet points to prevent phantomnemoclaw-prefixed parser matches.GLOBAL_COMMANDSnow callsglobalCommandTokens()andsandboxActionsnow callssandboxActionTokens()— no more hand-maintained sets/arrays.--dump-commandsinternal flag: Prints canonical command list for check-docs.sh parity checks, replacing the fragile perl help-parsing pipeline.check-docs.sh: Phase 1 now uses--dump-commandsinstead of perl regex extraction. Phase 2 strips<arg>and[optional]placeholders from doc headings for clean comparison.nemoclaw onboard --from,nemoclaw setup,nemoclaw start,nemoclaw stopindocs/reference/commands.md.cloud-experimental-e2e: Removed DISABLED comment,CLOUD_EXPERIMENTAL_E2E_ENABLEDvariable gate, andSKIP_CHECK_DOCSflag.test/image-cleanup.test.ts: Adapted to verifygcvia registry source instead of regex-matching the oldGLOBAL_COMMANDSSet literal.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)AI Disclosure
Signed-off-by: Julie Yaunches jyaunches@nvidia.com
Summary by CodeRabbit
Documentation
nemoclaw onboard --from <Dockerfile>usage guide and documented deprecated aliases (nemoclaw start,nemoclaw stop,nemoclaw setup) with warnings.New Features
--dump-commandsoption to emit the canonical command list.Improvements