Repository navigation
Conversation
- docs/inference: custom-llm-provider, switch-to-brev-nemotron-120b, switch-to-nemotron-super-120b - skill/nat: SKILL.md and reference docs - nemoclaw-sandbox-policy.yaml Rebased onto NVIDIA/NemoClaw main (at d9aced4). Prior edits to files that upstream refactored or deleted were dropped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds `nemoclaw completion [bash|zsh|fish]` which outputs a shell completion script that can be sourced for tab completion of: - Global commands (onboard, list, status, credentials, inference, ...) - Registered sandbox names (fetched dynamically from `nemoclaw list`) - Sandbox subcommands (logs, exec, policy-add, channels, snapshot, ...) - Per-command flags (--json, --yes, --dry-run, --sandbox, ...) Auto-detects the current shell via $SHELL when no argument is given. Installation: # Bash (add to ~/.bashrc): source <(nemoclaw completion bash) # Zsh (add to ~/.zshrc): source <(nemoclaw completion zsh) # Fish (one-time install): nemoclaw completion fish > ~/.config/fish/completions/nemoclaw.fish Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds documentation for configuring custom LLM providers and switching to Brev-hosted Nemotron 3 Super 120B endpoints, a new sandbox network policy YAML, NAT skill reference documentation, and a new oclif ChangesInference Provider and Nemotron Switch Documentation
NAT Skill Reference Documentation
Shell Completion Command
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
nemoclaw-sandbox-policy.yaml (1)
1-103: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winWire up or remove this root policy file — the repo points at
nemoclaw-blueprint/policies/openclaw-sandbox.yamlin docs, tests, and sandbox setup, whilenemoclaw-sandbox-policy.yamlhas no references. If this is meant to be active, hook it into the load path; otherwise delete it to avoid a stale duplicate.🤖 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 `@nemoclaw-sandbox-policy.yaml` around lines 1 - 103, The root sandbox policy file is currently orphaned and duplicated against the referenced policy path used by docs, tests, and sandbox setup. Update the policy loading/packaging path to point to nemoclaw-sandbox-policy.yaml if it is intended to be the active source, using the existing sandbox setup and policy loader symbols to wire it in; otherwise remove this file so there is only one authoritative policy definition.skill/nat/references/examples.md (1)
15-73: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winClean up the markdownlint failures in this section.
The tree fence needs an explicit language, and the headings below need blank lines around them, so this file will keep failing the markdownlint rules until it is normalized.
🤖 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 `@skill/nat/references/examples.md` around lines 15 - 73, The markdown in examples.md needs normalization to satisfy lint rules. Update the fenced tree block to use an explicit language identifier on the opening fence, and add the required blank lines around the section headings in the Categories and Using Examples as Starting Points sections. Keep the existing content and structure intact while making the formatting consistent.Source: Linters/SAST tools
🧹 Nitpick comments (3)
src/commands/completion.ts (1)
238-248: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFish sandbox-subcommand list drifts from
SANDBOX_SUBCOMMANDS.
fishScript()hardcodes its ownsandbox_subcommandsset instead of reusingSANDBOX_SUBCOMMANDS, and it's already missing many entries present in the shared list (e.g.policy-explain,config:get/config:set,hosts-*,share:*,tunnel:*,sessions:reset,snapshot:*,channels:*variants collapsed to bare group names). Any future addition toSANDBOX_SUBCOMMANDSsilently won't propagate to fish completion.♻️ Suggested fix
-set -l sandbox_subcommands status logs exec agent connect destroy rebuild \ - recover download doctor dashboard-url policy-add policy-remove policy-list \ - channels snapshot shields skill inference +set -l sandbox_subcommands ${subcommands}(using a
subcommandsvariable built the same way as inbashScript/zshScript, e.g.SANDBOX_SUBCOMMANDS.join(" "))🤖 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/completion.ts` around lines 238 - 248, The Fish completion logic in fishScript() is maintaining its own sandbox_subcommands list, which has drifted from SANDBOX_SUBCOMMANDS. Update the fish completion generation to build the subcommand list from SANDBOX_SUBCOMMANDS the same way bashScript() and zshScript() do, so additions like policy-explain, config:get/set, hosts-*, share:*, tunnel:*, sessions:reset, and snapshot:* stay in sync automatically. Keep the existing __nemoclaw_sandboxes helper and complete invocations, but replace the hardcoded list with the shared source used by the other shells.nemoclaw-sandbox-policy.yaml (1)
25-34: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winInconsistent enforcement strictness across network policy entries.
The
openai,npm_registry, andgithubsections use a coarseaccess: fullshorthand with noprotocol/enforcement/tls/rules, whileclawhub,openclaw_api, andopenclaw_docsuse fine-grainedprotocol: rest,enforcement: enforce,tls: terminate, and explicit method/pathrules.access: fullonapi.openai.comand the plaintextinference.local:80endpoint effectively bypasses the finer-grained enforcement used elsewhere for comparable outbound API traffic. Consider using the sameprotocol/enforcement/tls/rulesschema for these entries unlessaccess: fullis intentionally reserved for a distinct, lower-risk trust tier.Also applies to: 80-102
🤖 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 `@nemoclaw-sandbox-policy.yaml` around lines 25 - 34, The network policy entries for openai (and the related coarse-grained sections) should use the same strict schema as clawhub/openclaw_api/openclaw_docs instead of access: full. Update the openai endpoints to define protocol, enforcement, tls, and explicit rules via the network_policies.openai block so api.openai.com is governed consistently; also remove or properly classify the plaintext inference.local:80 endpoint if it is not meant to bypass TLS and method/path controls. Use the existing endpoint definitions for clawhub and openclaw_api as the pattern to align the policy structure.skill/nat/references/function-groups.md (1)
36-57: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid blocking boto3 calls in the async example.
put_objectandget_objectare synchronous; wrapping them withasyncio.to_threador using an async S3 client would keep the sample aligned with the async builder API it shows.🤖 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 `@skill/nat/references/function-groups.md` around lines 36 - 57, The async example in build_object_store is using blocking boto3 S3 calls inside save_fn and load_fn, which conflicts with the async API shown by register_function_group and FunctionGroup. Update those functions to run put_object and get_object off the event loop, for example by wrapping the existing s3_client calls with asyncio.to_thread, or switch to an async S3 client while keeping the save/load function names and behavior unchanged.
🤖 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.
Inline comments:
In `@docs/inference/custom-llm-provider.md`:
- Around line 23-24: Remove the em dashes from the prose in
custom-llm-provider.md and rewrite those sentences using plain punctuation or
shorter clauses that match the docs style guide. Update the affected copy in the
overview text and the sections around the references in the guide so the wording
stays natural without using em dashes, keeping the same meaning throughout.
In `@docs/inference/switch-to-brev-nemotron-120b.md`:
- Around line 136-139: Fix the markdownlint fence warnings in this doc by
updating the unnamed table fence near the model list to include a language label
(use text), and add a blank line before the fenced block that follows “Check
with:” so the closing/opening fence separation is valid. Update the fenced
sections in the affected markdown content, including the blocks around the model
table and the command example, to keep the formatting consistent.
- Line 24: Remove the em dash styling from the prose and headings in
switch-to-brev-nemotron-120b.md: update the “No API key required” sentence so it
is not both bolded and separated by an em dash, and change each step heading in
the guide to the same colon-based format used by
switch-to-nemotron-super-120b.md. Keep the wording aligned with the existing
structure, but replace the “Step N — …” pattern with “Step N: …” in the affected
headings and remove unnecessary bold where it appears in that sentence.
In `@docs/inference/switch-to-nemotron-super-120b.md`:
- Around line 418-421: The markdown fenced expected-output snippets in this
document are missing language identifiers, which triggers markdownlint. Update
each affected fenced block to use a text language tag in the relevant sections,
including the snippet near the current diff and the other two referenced
snippets, so the blocks are consistently marked as plain text.
- Around line 319-338: The “Before” network-policy snapshot in
switch-to-nemotron-super-120b.md is inconsistent with the actual default sandbox
policy. Update the example under the migration guide to use the NVIDIA hosts
from the real pre-change policy, keeping the snapshot aligned with the existing
policy structure shown in the document’s network-policy YAML.
In `@skill/nat/references/custom-tools.md`:
- Around line 11-19: The fenced directory tree example is missing an explicit
language tag, which triggers markdown linting. Update the code block in the
references document to use the proper `text` fence label, keeping the example
content the same. Use the existing tree snippet near the custom tools example as
the target and ensure the fenced block is consistently tagged so markdownlint
passes.
- Around line 32-52: The example for my_custom_tool_function is missing the
imports for Builder and FunctionInfo, so the snippet is not runnable as shown.
Update the imports at the top of the custom-tools example to include the symbols
used by my_custom_tool_function and FunctionInfo.from_fn, alongside the existing
FunctionBaseConfig, EmbedderRef, register_function, and LLMFrameworkEnum
imports.
In `@skill/nat/references/function-groups.md`:
- Around line 32-57: The example in build_object_store is missing required
imports, so it will not run as written. Update the snippet to import boto3 and
the Builder type used in the build_object_store function signature, alongside
the existing register_function_group and FunctionGroup imports, so all
referenced symbols are defined in the example.
In `@skill/nat/references/install-from-source.md`:
- Around line 4-12: The install-from-source flow in the references doc is
missing a prerequisites note before the clone and uv setup commands. Add a brief
prerequisites section near the existing install steps that explicitly states
`git-lfs`, `uv`, and Python 3.13 must already be installed and available on the
PATH before running the `git clone`, `git lfs`, and `uv venv`/`uv sync`
commands.
In `@skill/nat/SKILL.md`:
- Around line 81-84: The agent list in SKILL.md contains a typo in the
`rewwo_agent` entry. Update the identifier to `rewoo_agent` in the agent
examples so the documented name matches the actual agent type and users do not
copy a nonexistent symbol.
In `@src/commands/completion.ts`:
- Around line 274-285: The CompletionCommand shell selection is effectively
hardcoded to bash because the Args.string default prevents detectShell() from
ever being used. Remove the default from the static args definition in
CompletionCommand and keep the args.shell ?? detectShell() fallback so omitted
input can auto-detect the shell; ensure the shell option handling still
validates bash, zsh, and fish.
- Around line 99-101: The completion generators are parsing the human-readable
output from the list command, which can include multi-line row tokens and footer
text instead of only sandbox names. Update the completion logic in completion.ts
to use nemoclaw list --json and extract names from .sandboxes[].name, and apply
the same change across the bash, zsh, and fish completion paths so sandbox
completion only returns valid names.
---
Outside diff comments:
In `@nemoclaw-sandbox-policy.yaml`:
- Around line 1-103: The root sandbox policy file is currently orphaned and
duplicated against the referenced policy path used by docs, tests, and sandbox
setup. Update the policy loading/packaging path to point to
nemoclaw-sandbox-policy.yaml if it is intended to be the active source, using
the existing sandbox setup and policy loader symbols to wire it in; otherwise
remove this file so there is only one authoritative policy definition.
In `@skill/nat/references/examples.md`:
- Around line 15-73: The markdown in examples.md needs normalization to satisfy
lint rules. Update the fenced tree block to use an explicit language identifier
on the opening fence, and add the required blank lines around the section
headings in the Categories and Using Examples as Starting Points sections. Keep
the existing content and structure intact while making the formatting
consistent.
---
Nitpick comments:
In `@nemoclaw-sandbox-policy.yaml`:
- Around line 25-34: The network policy entries for openai (and the related
coarse-grained sections) should use the same strict schema as
clawhub/openclaw_api/openclaw_docs instead of access: full. Update the openai
endpoints to define protocol, enforcement, tls, and explicit rules via the
network_policies.openai block so api.openai.com is governed consistently; also
remove or properly classify the plaintext inference.local:80 endpoint if it is
not meant to bypass TLS and method/path controls. Use the existing endpoint
definitions for clawhub and openclaw_api as the pattern to align the policy
structure.
In `@skill/nat/references/function-groups.md`:
- Around line 36-57: The async example in build_object_store is using blocking
boto3 S3 calls inside save_fn and load_fn, which conflicts with the async API
shown by register_function_group and FunctionGroup. Update those functions to
run put_object and get_object off the event loop, for example by wrapping the
existing s3_client calls with asyncio.to_thread, or switch to an async S3 client
while keeping the save/load function names and behavior unchanged.
In `@src/commands/completion.ts`:
- Around line 238-248: The Fish completion logic in fishScript() is maintaining
its own sandbox_subcommands list, which has drifted from SANDBOX_SUBCOMMANDS.
Update the fish completion generation to build the subcommand list from
SANDBOX_SUBCOMMANDS the same way bashScript() and zshScript() do, so additions
like policy-explain, config:get/set, hosts-*, share:*, tunnel:*, sessions:reset,
and snapshot:* stay in sync automatically. Keep the existing
__nemoclaw_sandboxes helper and complete invocations, but replace the hardcoded
list with the shared source used by the other shells.
🪄 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: 4fb0871f-a8d2-4f93-936c-f90ad3b529d6
📒 Files selected for processing (11)
docs/inference/custom-llm-provider.mddocs/inference/switch-to-brev-nemotron-120b.mddocs/inference/switch-to-nemotron-super-120b.mdnemoclaw-sandbox-policy.yamlskill/nat/SKILL.mdskill/nat/references/a2a-server.mdskill/nat/references/custom-tools.mdskill/nat/references/examples.mdskill/nat/references/function-groups.mdskill/nat/references/install-from-source.mdsrc/commands/completion.ts
| NemoClaw routes inference through any **OpenAI-compatible** `/v1/chat/completions` endpoint. | ||
| This guide shows how to point NemoClaw at a third-party LLM provider — no code changes required. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove em dashes per docs style guide.
Multiple prose lines use em dashes (—) at lines 24, 44, 107-110, and 316. As per path instructions, docs/**/*.{md,mdx} should "Avoid filler, hype, rhetorical questions, emoji, em dashes, and unnecessary bold text."
✏️ Example fixes
-This guide shows how to point NemoClaw at a third-party LLM provider — no code changes required.
+This guide shows how to point NemoClaw at a third-party LLM provider. No code changes are required.
-The simplest path — the wizard walks you through everything:
+The simplest path: the wizard walks you through everything.
-- `azure/openai/gpt-5.4` — OpenAI GPT-5.4 via Azure
+- `azure/openai/gpt-5.4`: OpenAI GPT-5.4 via Azure
-- NemoClaw accepts any key format — there is no prefix requirement.
+- NemoClaw accepts any key format. There is no prefix requirement.Also applies to: 44-44, 106-110, 313-316
🤖 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 `@docs/inference/custom-llm-provider.md` around lines 23 - 24, Remove the em
dashes from the prose in custom-llm-provider.md and rewrite those sentences
using plain punctuation or shorter clauses that match the docs style guide.
Update the affected copy in the overview text and the sections around the
references in the guide so the wording stays natural without using em dashes,
keeping the same meaning throughout.
Source: Path instructions
| # Fresh Setup: NemoClaw with Brev-Hosted Nemotron 3 Super 120B | ||
|
|
||
| Prerequisites: Docker running, openshell CLI installed, a Brev-hosted Nemotron 3 Super 120B | ||
| endpoint accessible. **No API key required** — the Brev endpoint has open access. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove em dashes from headings and prose; drop unnecessary bold.
Line 24 uses both bold and an em dash (**No API key required** — ...), and every step heading (lines 39, 57, 127, 145, 153) uses — instead of a colon. The sibling guide (switch-to-nemotron-super-120b.md) uses ## Step N: ... headings — following that pattern here would also fix the style-guide violation and improve consistency between the two guides.
✏️ Suggested heading fixes
-Prerequisites: Docker running, openshell CLI installed, a Brev-hosted Nemotron 3 Super 120B
-endpoint accessible. **No API key required** — the Brev endpoint has open access.
+Prerequisites: Docker running, openshell CLI installed, a Brev-hosted Nemotron 3 Super 120B
+endpoint accessible. No API key is required: the Brev endpoint has open access.
...
-## Step 2 — Fix the Dockerfile model primary (line 70)
+## Step 2: Fix the Dockerfile model primary (line 70)
-## Step 3 — Fix `scripts/nemoclaw-start.sh`
+## Step 3: Fix `scripts/nemoclaw-start.sh`
-## Step 4 — Build and verify
+## Step 4: Build and verify
-## Step 5 — Run the onboard wizard
+## Step 5: Run the onboard wizard
-## Step 6 — Verify end-to-end
+## Step 6: Verify end-to-endAlso applies to: 39-39, 57-57, 127-127, 145-145, 153-153
🤖 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 `@docs/inference/switch-to-brev-nemotron-120b.md` at line 24, Remove the em
dash styling from the prose and headings in switch-to-brev-nemotron-120b.md:
update the “No API key required” sentence so it is not both bolded and separated
by an em dash, and change each step heading in the guide to the same colon-based
format used by switch-to-nemotron-super-120b.md. Keep the wording aligned with
the existing structure, but replace the “Step N — …” pattern with “Step N: …” in
the affected headings and remove unnecessary bold where it appears in that
sentence.
Source: Path instructions
| ``` | ||
| Model Input Ctx Local Auth Tags | ||
| openai/nvidia/nemotron-3-super-120b-a12b text 128k no yes default,configured | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix markdownlint fence issues.
Add a language to the fenced block at line 136 (e.g. text) and add a blank line before the fence that follows "Check with:" at line 197.
🔧 Fixes
Expected output:
-```
+```text
Model Input Ctx Local Auth Tags
openai/nvidia/nemotron-3-super-120b-a12b text 128k no yes default,configured...
Check with:
+
openclaw models list</details>
Also applies to: 195-198
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 136-136: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @docs/inference/switch-to-brev-nemotron-120b.md around lines 136 - 139, Fix
the markdownlint fence warnings in this doc by updating the unnamed table fence
near the model list to include a language label (use text), and add a blank line
before the fenced block that follows “Check with:” so the closing/opening fence
separation is valid. Update the fenced sections in the affected markdown
content, including the blocks around the model table and the command example, to
keep the formatting consistent.
</details>
<!-- fingerprinting:phantom:poseidon:beignet -->
<!-- cr-indicator-types:potential_issue -->
<!-- cr-comment:v1:f0269c97b6124be6b60e203c -->
_Source: Linters/SAST tools_
<!-- This is an auto-generated comment by CodeRabbit -->
| **Before:** | ||
|
|
||
| ```yaml | ||
| nvidia: | ||
| name: nvidia | ||
| endpoints: | ||
| - host: api.openai.com | ||
| port: 443 | ||
| protocol: rest | ||
| enforcement: enforce | ||
| tls: terminate | ||
| rules: | ||
| - allow: { method: "*", path: "/**" } | ||
| binaries: | ||
| - { path: /usr/local/bin/claude } | ||
| - { path: /usr/local/bin/openclaw } | ||
| - { path: /usr/local/bin/node } | ||
| - { path: /usr/bin/curl } | ||
| - { path: "*" } | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== switch-to-nemotron-super-120b.md around cited lines ==\n'
nl -ba docs/inference/switch-to-nemotron-super-120b.md | sed -n '280,360p'
printf '\n== custom-llm-provider.md matches for network_policies.nvidia/endpoints ==\n'
rg -n -A6 -B6 'network_policies|nvidia|integrate\.api\.nvidia\.com|inference-api\.nvidia\.com|api\.openai\.com' docs/inference/custom-llm-provider.mdRepository: NVIDIA/NemoClaw
Length of output: 252
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== switch-to-nemotron-super-120b.md around cited lines ==\n'
sed -n '300,350p' docs/inference/switch-to-nemotron-super-120b.md
printf '\n== custom-llm-provider.md matches and context ==\n'
rg -n -A8 -B8 'network_policies|integrate\.api\.nvidia\.com|inference-api\.nvidia\.com|api\.openai\.com|nvidia:' docs/inference/custom-llm-provider.mdRepository: NVIDIA/NemoClaw
Length of output: 4624
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== locate openclaw-sandbox.yaml ==\n'
fd -a 'openclaw-sandbox.yaml' . || true
printf '\n== search for network_policies.nvidia.endpoints definitions ==\n'
rg -n -A8 -B8 'network_policies:|nvidia:|integrate\.api\.nvidia\.com|inference-api\.nvidia\.com|api\.openai\.com' . \
-g '!*node_modules*' -g '!*dist*' -g '!*build*' -g '!*.lock' -g '!*.min.*'Repository: NVIDIA/NemoClaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== openclaw-sandbox.yaml locations =="
fd -a 'openclaw-sandbox.yaml' . || true
echo
echo "== candidate policy file content (first match) =="
file="$(fd -a 'openclaw-sandbox.yaml' . | head -n 1)"
if [ -n "${file:-}" ]; then
rg -n -A12 -B4 'nvidia:|integrate\.api\.nvidia\.com|inference-api\.nvidia\.com|api\.openai\.com' "$file"
fiRepository: NVIDIA/NemoClaw
Length of output: 960
Update the Before network-policy snapshot to match the actual default. docs/inference/switch-to-nemotron-super-120b.md:319-338 shows api.openai.com, but the sandbox policy/docs use NVIDIA hosts (integrate.api.nvidia.com and inference-api.nvidia.com). Keep the migration example aligned with the real pre-change policy.
🤖 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 `@docs/inference/switch-to-nemotron-super-120b.md` around lines 319 - 338, The
“Before” network-policy snapshot in switch-to-nemotron-super-120b.md is
inconsistent with the actual default sandbox policy. Update the example under
the migration guide to use the NVIDIA hosts from the real pre-change policy,
keeping the snapshot aligned with the existing policy structure shown in the
document’s network-policy YAML.
| ``` | ||
| Provider: brev-nemotron | ||
| Model: nvidia/nemotron-3-super-120b-a12b | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to fenced blocks.
markdownlint flags three fenced blocks without a language (expected-output snippets). Add text to each.
Also applies to: 435-438, 483-487
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 418-418: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/inference/switch-to-nemotron-super-120b.md` around lines 418 - 421, The
markdown fenced expected-output snippets in this document are missing language
identifiers, which triggers markdownlint. Update each affected fenced block to
use a text language tag in the relevant sections, including the snippet near the
current diff and the other two referenced snippets, so the blocks are
consistently marked as plain text.
Source: Linters/SAST tools
| ```python | ||
| from nemo_agent_toolkit.functions import register_function_group | ||
| from nemo_agent_toolkit.data_models.function import FunctionGroup | ||
|
|
||
| @register_function_group(config_type=ObjectStoreConfig) | ||
| async def build_object_store(config: ObjectStoreConfig, builder: Builder): | ||
| s3_client = boto3.client('s3', | ||
| endpoint_url=config.endpoint, | ||
| aws_access_key_id=config.access_key, | ||
| aws_secret_access_key=config.secret_key | ||
| ) | ||
|
|
||
| group = FunctionGroup(config=config, instance_name="storage") | ||
|
|
||
| async def save_fn(filename: str, content: bytes) -> str: | ||
| s3_client.put_object(Bucket=config.bucket, Key=filename, Body=content) | ||
| return f"Saved {filename}" | ||
|
|
||
| async def load_fn(filename: str) -> bytes: | ||
| response = s3_client.get_object(Bucket=config.bucket, Key=filename) | ||
| return response['Body'].read() | ||
|
|
||
| group.add_function(name="save", fn=save_fn, description="Save file to storage") | ||
| group.add_function(name="load", fn=load_fn, description="Load file from storage") | ||
|
|
||
| yield group |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target section with line numbers.
sed -n '1,120p' skill/nat/references/function-groups.md | cat -n
printf '\n---\n'
# Look for nearby usage patterns of Builder and boto3 in the repo docs/examples.
rg -n "Builder|boto3|register_function_group|FunctionGroup" skill/nat/references skill -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'Repository: NVIDIA/NemoClaw
Length of output: 6358
Add the missing imports.
boto3 and Builder are used in this snippet but not imported, so the example won’t run as written.
🤖 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 `@skill/nat/references/function-groups.md` around lines 32 - 57, The example in
build_object_store is missing required imports, so it will not run as written.
Update the snippet to import boto3 and the Builder type used in the
build_object_store function signature, alongside the existing
register_function_group and FunctionGroup imports, so all referenced symbols are
defined in the example.
| git clone -b main https://github.com/NVIDIA/NeMo-Agent-Toolkit.git nemo-agent-toolkit | ||
| cd nemo-agent-toolkit | ||
| git submodule update --init --recursive | ||
| git lfs install | ||
| git lfs fetch | ||
| git lfs pull | ||
| uv venv --python 3.13 --seed .venv | ||
| source .venv/bin/activate | ||
| uv sync --all-groups --extra most |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add a prerequisites note before the install commands.
This flow assumes git-lfs, uv, and Python 3.13 are already available, but the page never says so. Without that, the install flow will fail on a fresh machine.
🤖 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 `@skill/nat/references/install-from-source.md` around lines 4 - 12, The
install-from-source flow in the references doc is missing a prerequisites note
before the clone and uv setup commands. Add a brief prerequisites section near
the existing install steps that explicitly states `git-lfs`, `uv`, and Python
3.13 must already be installed and available on the PATH before running the `git
clone`, `git lfs`, and `uv venv`/`uv sync` commands.
| - `react_agent` — Reasoning and acting | ||
| - `reasoning_agent` — Advanced reasoning | ||
| - `rewwo_agent` — Reasoning Without Observation | ||
| - `responses_api_agent` — OpenAI Responses API |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the rewwo_agent typo.
This looks like a misspelling of rewoo_agent, so readers will copy a non-existent agent type.
Suggested fix
- - `rewwo_agent` — Reasoning Without Observation
+ - `rewoo_agent` — Reasoning Without Observation📝 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.
| - `react_agent` — Reasoning and acting | |
| - `reasoning_agent` — Advanced reasoning | |
| - `rewwo_agent` — Reasoning Without Observation | |
| - `responses_api_agent` — OpenAI Responses API | |
| - `react_agent` — Reasoning and acting | |
| - `reasoning_agent` — Advanced reasoning | |
| - `rewoo_agent` — Reasoning Without Observation | |
| - `responses_api_agent` — OpenAI Responses API |
🤖 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 `@skill/nat/SKILL.md` around lines 81 - 84, The agent list in SKILL.md contains
a typo in the `rewwo_agent` entry. Update the identifier to `rewoo_agent` in the
agent examples so the documented name matches the actual agent type and users do
not copy a nonexistent symbol.
| # Fetch sandbox names (cached for the lifetime of this completion call) | ||
| local sandboxes | ||
| sandboxes=$(nemoclaw list 2>/dev/null | awk 'NR>1 {print $1}') || sandboxes="" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- completion.ts outline ---'
ast-grep outline src/commands/completion.ts --view expanded || true
echo
echo '--- relevant line ranges ---'
nl -ba src/commands/completion.ts | sed -n '1,320p'
echo
echo '--- search for list output docs/usages ---'
rg -n "nemoclaw list|Sandboxes:|--json" src README.md docs .github -g '!**/node_modules/**' || trueRepository: NVIDIA/NemoClaw
Length of output: 845
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- completion.ts relevant slices ---'
sed -n '90,110p;160,180p;235,250p' src/commands/completion.ts
echo
echo '--- locate list command and output formatting ---'
fd -a 'list.ts' src || true
rg -n "class .*List|summary = .*list|--json|Sandboxes:|console\.log|table|awk 'NR>1" src -g '!**/node_modules/**' || trueRepository: NVIDIA/NemoClaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/commands/completion.ts slices ---'
sed -n '90,110p;165,180p;235,250p' src/commands/completion.ts
echo
echo '--- src/lib/inventory/index.ts around text rendering ---'
sed -n '280,320p;440,475p' src/lib/inventory/index.ts
echo
echo '--- locate list command ---'
fd -a 'list.ts' src/commands src/lib || trueRepository: NVIDIA/NemoClaw
Length of output: 5821
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/commands/list.ts outline ---'
ast-grep outline src/commands/list.ts --view expanded || true
echo
echo '--- src/commands/list.ts ---'
sed -n '1,260p' src/commands/list.ts
echo
echo '--- src/lib/openshell-sandbox-list.ts outline ---'
ast-grep outline src/lib/openshell-sandbox-list.ts --view expanded || true
echo
echo '--- src/lib/openshell-sandbox-list.ts ---'
sed -n '1,260p' src/lib/openshell-sandbox-list.tsRepository: NVIDIA/NemoClaw
Length of output: 5382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- renderSandboxInventoryText implementation ---'
ast-grep outline src/lib/inventory/index.ts --view expanded || true
sed -n '320,420p' src/lib/inventory/index.ts
echo
echo '--- completion.ts JSON / helper usage ---'
rg -n --context 2 "json|renderSandbox|list --json|nemoclaw list" src/commands/completion.ts src/lib -g '!**/node_modules/**'Repository: NVIDIA/NemoClaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- renderSandboxInventoryText symbol location ---'
rg -n "renderSandboxInventoryText" src/lib/inventory/index.ts src/lib/inventory/index.test.ts
echo
echo '--- renderer implementation slice ---'
sed -n '250,360p' src/lib/inventory/index.ts
echo
echo '--- inventory text tests around Sandboxes ---'
rg -n -A8 -B4 "Sandboxes:" src/lib/inventory/index.test.tsRepository: NVIDIA/NemoClaw
Length of output: 19459
Sandbox completion should read names from JSON, not nemoclaw list text
nemoclaw list renders multi-line rows, so awk 'NR>1 {print $1}' will pull in tokens like agent:, dashboard:, and the default-sandbox footer, not just sandbox names. Switch the bash/zsh/fish generators to nemoclaw list --json and extract .sandboxes[].name instead.
🤖 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/completion.ts` around lines 99 - 101, The completion generators
are parsing the human-readable output from the list command, which can include
multi-line row tokens and footer text instead of only sandbox names. Update the
completion logic in completion.ts to use nemoclaw list --json and extract names
from .sandboxes[].name, and apply the same change across the bash, zsh, and fish
completion paths so sandbox completion only returns valid names.
| static args = { | ||
| shell: Args.string({ | ||
| description: "Target shell: bash, zsh, or fish", | ||
| options: ["bash", "zsh", "fish"], | ||
| required: false, | ||
| default: "bash", | ||
| }), | ||
| }; | ||
|
|
||
| public async run(): Promise<void> { | ||
| const { args } = await this.parse(CompletionCommand); | ||
| const shell = args.shell ?? detectShell(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
oclif Args.string default value behavior when option not passed
💡 Result:
In oclif, when using Args.string (or other argument types), the default value behavior is triggered if the argument is not provided by the user [1]. If a default value is defined, oclif applies it during the parsing process if the argument value is undefined [1]. Specifically: 1. If no value is provided, oclif checks if a default is defined for the argument [1]. 2. If a default value exists, it is used [1]. The default can be a static value or a function that returns a value (in which case the function is executed to determine the default) [1]. 3. If no default is provided and the argument is not required, the resulting value in the parsed arguments object will be undefined [1]. This behavior is handled within the oclif core parser, which ensures that if an argument remains undefined after parsing the user input, the default value is injected into the arguments object [1]. This aligns with standard oclif practices where optional arguments can be given fallback values to ensure the command has the necessary data to execute [1][2].
Citations:
- 1: https://github.com/oclif/core/blob/main/src/parser/parse.ts
- 2: https://github.com/oclif/core/blob/b4c209f4616bf0d9135fbe4696b19e5016559589/guides/V2_MIGRATION.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant command implementation and nearby shell-detection logic.
sed -n '240,320p' src/commands/completion.ts
echo '---'
# Find all references to detectShell in the command and related files.
rg -n "detectShell|Args\.string\(|default: \"bash\"" src/commands src/lib -S
echo '---'
# Read the command help/docs if present.
rg -n "completion|auto-detect|SHELL|bash, zsh, or fish" README.md src -SRepository: NVIDIA/NemoClaw
Length of output: 50372
Drop the bash default if $SHELL auto-detection is intended.
Args.string populates omitted args with "bash", so args.shell ?? detectShell() never reaches detectShell(). That leaves completion generation pinned to bash unless the user passes another shell explicitly.
🤖 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/completion.ts` around lines 274 - 285, The CompletionCommand
shell selection is effectively hardcoded to bash because the Args.string default
prevents detectShell() from ever being used. Remove the default from the static
args definition in CompletionCommand and keep the args.shell ?? detectShell()
fallback so omitted input can auto-detect the shell; ensure the shell option
handling still validates bash, zsh, and fish.
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I think we should not merge this implementation as-is.
NemoClaw already uses oclif for command discovery/parsing/help, and oclif has an autocomplete plugin for generated shell completion. The only NemoClaw-specific gap is our public sandbox-first grammar (nemoclaw <sandbox-name> <action>) and dynamic sandbox-name completion. This PR instead adds a second, hand-maintained completion registry, which will drift from the actual CLI.
Concrete blockers:
- The command/subcommand lists are hard-coded instead of being derived from oclif metadata and the existing public routing helpers (
src/lib/cli/command-registry.ts,src/lib/cli/public-route-metadata.ts). That duplicates a source of truth we have intentionally centralized. - Several completions are already stale or inconsistent with the public CLI grammar. For example, the script advertises colon-style public commands such as
inference:get, while the public help/route usesinference get; it also misses existing commands such asresources,use,uninstall,upload,gateway restart, and the newersessionssubcommands. - Sandbox-name discovery parses human-readable
nemoclaw listoutput withawk 'NR>1 {print $1}'. Current list output includes headings/detail lines, so this produces invalid candidates such asSandboxes:,agent:, anddashboard:. If dynamic sandbox completion is needed, it should use a JSON/no-side-effect source. - The PR mixes unrelated additions (inference docs, NAT skill files, sandbox policy) with shell completion. Those should be split out or removed from this PR.
- This is user-facing CLI behavior and needs tests that prove the generated completions stay in sync with the command registry and do not regress sandbox-name parsing.
Recommended direction: either wire in oclif's autocomplete support for the native command/flag surface and add only a thin NemoClaw-specific adapter for sandbox-first grammar, or keep a nemoclaw completion command but generate its command/action/flag data from the existing oclif metadata and public route registry. Please avoid introducing a manually maintained completion registry.
<!-- markdownlint-disable MD041 --> ## Summary Adds `nemoclaw completion [bash|zsh|fish]` for global commands, the public sandbox-first grammar, flags, shell choices, and registered sandbox names. The completion tree is generated from oclif's discovered command metadata and NemoClaw's public-route helpers, so it stays aligned with the CLI instead of maintaining a second command registry. Replaces #6267, which was closed with unrelated history. ## Changes - Generate global and sandbox-first command trees from `this.config.commands`, `globalRouteTokenVariants`, and `sandboxRouteTokens`. - Derive long/short flags and fixed argument choices from oclif metadata for Bash, Zsh, and Fish. - Read sandbox names through a hidden, local-registry-only completion path instead of the full `list --json` workflow. - Cache sandbox names for the current shell session so Fish and repeated tab completion do not rerun registry reads. - Bind generated scripts to the active CLI alias (`nemoclaw`, `nemohermes`, or `nemo-deepagents`). - Register and document the new public command. ## 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) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every new maintainer commit is signed - [x] Commit and push hooks pass - [x] Targeted tests pass: 37 CLI and package-contract tests - [ ] Full `npm test` passes (broad runtime changes only) - [x] Repository checks, source-shape guard, and test-size guard pass - [x] Bash and Zsh generated-script syntax checks pass - [x] Bash command, flag, and sandbox-first completion behavior passes - [x] Docs CLI parity passes (77/77 commands plus flag parity) - [x] Docs variant, route, and Fern checks pass (0 errors; 2 existing warnings) - [x] No secrets, API keys, or credentials committed ## Contributor credit The original implementation and every maintainer-authored follow-up commit retain `sauravdev` as a co-author. --- Signed-off-by: sauravdev <saurava@nvidia.com> --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com>
…IA#6271) <!-- markdownlint-disable MD041 --> ## Summary Adds `nemoclaw completion [bash|zsh|fish]` for global commands, the public sandbox-first grammar, flags, shell choices, and registered sandbox names. The completion tree is generated from oclif's discovered command metadata and NemoClaw's public-route helpers, so it stays aligned with the CLI instead of maintaining a second command registry. Replaces NVIDIA#6267, which was closed with unrelated history. ## Changes - Generate global and sandbox-first command trees from `this.config.commands`, `globalRouteTokenVariants`, and `sandboxRouteTokens`. - Derive long/short flags and fixed argument choices from oclif metadata for Bash, Zsh, and Fish. - Read sandbox names through a hidden, local-registry-only completion path instead of the full `list --json` workflow. - Cache sandbox names for the current shell session so Fish and repeated tab completion do not rerun registry reads. - Bind generated scripts to the active CLI alias (`nemoclaw`, `nemohermes`, or `nemo-deepagents`). - Register and document the new public command. ## 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) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every new maintainer commit is signed - [x] Commit and push hooks pass - [x] Targeted tests pass: 37 CLI and package-contract tests - [ ] Full `npm test` passes (broad runtime changes only) - [x] Repository checks, source-shape guard, and test-size guard pass - [x] Bash and Zsh generated-script syntax checks pass - [x] Bash command, flag, and sandbox-first completion behavior passes - [x] Docs CLI parity passes (77/77 commands plus flag parity) - [x] Docs variant, route, and Fern checks pass (0 errors; 2 existing warnings) - [x] No secrets, API keys, or credentials committed ## Contributor credit The original implementation and every maintainer-authored follow-up commit retain `sauravdev` as a co-author. --- Signed-off-by: sauravdev <saurava@nvidia.com> --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com>
Summary
Adds a
nemoclaw completion [bash|zsh|fish]command that outputs a shell completion script, enabling tab completion for global commands, registered sandbox names (fetched dynamically vianemoclaw list), sandbox subcommands, and per-command flags. The shell is auto-detected from$SHELLwhen no argument is given.Changes
src/commands/completion.tsgenerating bash, zsh, and fish completion scripts.complete -F _nemoclaw._nemoclawcompdef with_alternative/_argumentsfor commands, sandbox names, and flags.__nemoclaw_sandboxesfunction.--helpexamples (e.g.source <(nemoclaw completion bash)).Type of Change
Quality Gates
bash -nandzsh -n.--helpsummary and examples; happy to add a docs page in a follow-up if maintainers prefer.Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Verification detail:
npm run build:clipasses;nemoclaw completion bash | bash -nandnemoclaw completion zsh | zsh -nboth pass syntax checks; all three shells emit expected scripts.Signed-off-by: sauravdev saurava@nvidia.com
Summary by CodeRabbit
New Features
Documentation