Skip to content

fix(inference): add sandbox-scoped inference get/set commands (#5977) - #5989

Merged
cv merged 8 commits into
mainfrom
fix/5977-sandbox-inference-commands
Jul 3, 2026
Merged

fix(inference): add sandbox-scoped inference get/set commands (#5977)#5989
cv merged 8 commits into
mainfrom
fix/5977-sandbox-inference-commands

Conversation

@yimoj

@yimoj yimoj commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

The sandbox-first grammar nemohermes <sb> inference get / ... inference set ... failed with Unknown action: inference because inference get/set existed only as global commands. This adds sandbox-scoped equivalents that route the sandbox name through the existing global inference code paths, making the reporter's exact workflow succeed.

Related Issue

Fixes #5977

Changes

  • Add src/commands/sandbox/inference/get.ts (sandbox:inference:get) and src/commands/sandbox/inference/set.ts (sandbox:inference:set). Each takes the sandbox name in sandbox-first position and delegates to the existing runInferenceGet / runInferenceSet actions — set threads the positional name into the same path as inference set --sandbox <name>; get reads the same gateway-wide route so the grammar stays symmetric.
  • Public dispatch now derives <sb> inference get/set routes from the new command ids (no routing changes needed), and inference becomes a recognized sandbox action token. Bare <sb> inference / --help defer to oclif exactly as the existing config action does.
  • Add hidden public-display metadata for the two leaf commands so they satisfy the display-coverage contract without duplicating the visible global inference get/set entries in root help.
  • Document the sandbox-first grammar for both aliases in the command reference (commands.mdx source + generated commands-nemohermes.mdx).

Type of Change

  • Code change with doc updates

Quality Gates

  • Tests added or updated for changed behavior — new public-argv-translation cases for sandbox-scoped inference get/set + oclif-parent parity; updated command-registry counts (sandbox actions 29→30, hidden 12→14, sandbox commands 49→51).
  • Docs updated for user-facing behavior changes — command reference for nemoclaw/nemohermes.
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) — inference/sandbox dispatch.
  • Sensitive-path review completed or maintainer-approved waiver recorded — justification: new commands are thin delegators to the already-reviewed runInferenceGet/runInferenceSet actions; no new inference/credential logic; routing mirrors the existing config precedent.

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Targeted tests pass for changed behavior — vitest --project package-contract for public-argv-translation.test.ts and command-registry.test.ts (46 passing); typecheck:cli clean.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only) — docs:strict reports 0 errors; agent-variant sync check passes (generated commands-nemohermes.mdx matches).

Real worktree-CLI transcript (E2E — reporter workflow)

Run from this PR's worktree via the actual launcher (node ./bin/nemoclaw.js, NEMOCLAW_CLI_NAME=nemohermes). The reporter's commands now route to the new sandbox-scoped inference commands instead of NemoClaw's Unknown action: inference.

$ NEMOCLAW_CLI_NAME=nemohermes node ./bin/nemoclaw.js issue5977-sb inference get
  ...Starting OpenShell gateway... ✓ Docker-driver gateway is healthy
  Sandbox 'issue5977-sb' does not exist.
  Registered sandboxes: ...
[exit 1]                # recognized action → sandbox:inference:get → probes gateway → standard name check (NOT "Unknown action: inference")

$ NEMOCLAW_CLI_NAME=nemohermes node ./bin/nemoclaw.js issue5977-sb inference set \
    --provider nvidia-prod --model nvidia/nemotron-3-super-120b-a12b
  Sandbox 'issue5977-sb' does not exist.
  Registered sandboxes: ...
[exit 1]                # recognized action → sandbox:inference:set → threads positional name like `--sandbox`

$ NEMOCLAW_CLI_NAME=nemohermes node ./bin/nemoclaw.js test-sb bogus-action-5977
  Unknown action: bogus-action-5977
  Valid actions: ... inference ...
[exit 1]                # `inference` is now a recognized sandbox action token

Completing an actual provider switch requires a live onboarded Hermes sandbox + GPU/OpenShell, unavailable on this CI-class host; the delegation into the already-reviewed runInferenceGet/runInferenceSet actions is covered by the package-contract translation tests.


Signed-off-by: Yimo Jiang yimoj@nvidia.com

Summary by CodeRabbit

  • New Features
    • Added sandbox-scoped inference commands to view and update the active inference route (get and set), with get supporting --json and set requiring --provider and --model.
  • Documentation
    • Expanded command reference docs to support “sandbox-first” inference grammar for both NemoHermes and NemoClaw, including new examples.
  • Bug Fixes
    • Improved CLI routing/argument translation so inference subcommands dispatch correctly and provide the expected “valid actions” behavior.
  • Tests
    • Updated and added contract/adapter/dispatch tests to cover the new sandbox inference forms and error handling.

`inference get`/`set` only existed as global commands, so the sandbox-first
grammar `nemohermes <sb> inference get` (and `... inference set ...`) failed
with `Unknown action: inference`. The only working path was the global
`nemohermes inference set --provider ... --model ... --sandbox <sb>`, which is
asymmetric with every other per-sandbox action.

Add `sandbox:inference:get` and `sandbox:inference:set` oclif commands that
delegate to the existing `runInferenceGet`/`runInferenceSet` actions with the
sandbox name supplied in sandbox-first position. The set command threads the
positional sandbox name into the same code path as `inference set --sandbox`;
the get command reads the same gateway-wide route and accepts the name so the
grammar stays symmetric. Public dispatch now routes `<sb> inference get/set`
to these command ids (and `<sb> inference` / `--help` to topic help, matching
the existing `config` precedent), and `inference` becomes a valid sandbox
action token.

Add hidden public display metadata so the new leaf commands satisfy the
display-coverage contract without duplicating the visible global entries in
root help. Update the docs reference grammar for both aliases.

Tests: extend the public-argv-translation contract with sandbox-scoped
inference get/set translation and topic-help routing, and update the
command-registry counts (sandbox actions 29->30, hidden 12->14, sandbox
commands 49->51).

Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: cc897e1a-4177-4d68-912c-030811fbefc8

📥 Commits

Reviewing files that changed from the base of the PR and between cb21fc0 and 1ade1c5.

📒 Files selected for processing (3)
  • docs/reference/commands-nemohermes.mdx
  • docs/reference/commands.mdx
  • src/lib/cli/public-display-defaults.ts
💤 Files with no reviewable changes (1)
  • src/lib/cli/public-display-defaults.ts
✅ Files skipped from review due to trivial changes (2)
  • docs/reference/commands-nemohermes.mdx
  • docs/reference/commands.mdx

📝 Walkthrough

Walkthrough

Adds sandbox-first inference get and inference set command forms, registers them in public display metadata, updates CLI contract tests, and documents the new grammar in both command references.

Changes

Sandbox inference get/set commands

Layer / File(s) Summary
Shared CLI contracts and command wiring
src/lib/cli/flag-helpers.ts, src/commands/inference/set.ts, src/commands/sandbox/inference/set.ts
Adds a shared nonEmptyFlag helper, switches the global inference set command to use it, and introduces sandbox:inference:set with sandbox-first arguments, required provider/model flags, optional passthrough flags, and redirect/error handling.
Sandbox inference get command
src/commands/sandbox/inference/get.ts
Adds sandbox:inference:get with JSON flag support, sandbox-name argument parsing, shared route lookup, and typed error handling.
Public display registration
src/lib/cli/public-display-defaults.ts, test/package-contract/cli/command-registry.test.ts
Adds hidden public display entries for sandbox:inference:get and sandbox:inference:set, and updates command registry expectations for counts, hidden usages, and action tokens.
CLI routing and adapter tests
test/package-contract/cli/public-argv-translation.test.ts, src/commands/sandbox/inference/oclif-command-adapters.test.ts, test/cli/dispatch-basics.test.ts
Adds argv translation and dispatch coverage for sandbox inference routing, plus adapter tests for success paths, JSON output, validation failures, and typed exit-code handling.
Reference documentation updates
docs/reference/commands-nemohermes.mdx, docs/reference/commands.mdx
Adds sandbox-first inference get and set examples and equivalence notes in both command reference documents.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#3424: Introduces the sandbox-scoped sandbox:inference:get and sandbox:inference:set command modules that this PR builds on and tests against.
  • NVIDIA/NemoClaw#4847: Updates shared inference-set behavior that the new sandbox-first set command delegates to.

Suggested labels: feature

Suggested reviewers: cv, ericksoa

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the main change: adding sandbox-scoped inference get/set commands.
Linked Issues check ✅ Passed The PR implements sandbox-scoped inference get/set, updates sandbox routing, and aligns docs with the issue's expected behavior.
Out of Scope Changes check ✅ Passed The docs, routing, tests, and helper changes all support the sandbox-scoped inference fix.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/5977-sandbox-inference-commands

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the fix/5977-sandbox-inf... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main fix/5977-sandbox-inf... 1ade1c5 +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the fix/5977-sandbox-inf... branch is 69%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main fix/5977-sandbox-inf... 1ade1c5 +/-
src/lib/actions...dbox/rebuild.ts 82%
src/lib/actions...all/run-plan.ts 80%
src/lib/state/o...oard-session.ts 79%
src/lib/shields/index.ts 75%
src/lib/state/sandbox.ts 73%
src/lib/onboard/preflight.ts 69%
src/lib/onboard...er-gpu-patch.ts 59%
src/lib/actions...licy-channel.ts 58%
src/lib/policy/index.ts 56%
src/lib/onboard.ts 20%

Updated July 02, 2026 05:24 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: openclaw-inference-switch, hermes-inference-switch
Optional E2E: inference-routing

Dispatch hint: openclaw-inference-switch,hermes-inference-switch,inference-routing

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • openclaw-inference-switch (high): Validates the real OpenClaw user flow most directly affected by inference set command-boundary changes: onboard, switch the OpenShell route through the NemoClaw CLI, sync the running OpenClaw config/hash/registry state, and complete an agent turn.
  • hermes-inference-switch (high): Validates the Hermes/nemohermes inference switching path, including route/config synchronization and live runtime preservation, which is adjacent to the new sandbox-first inference set/get grammar documented for nemohermes.

Optional E2E

  • inference-routing (medium): Useful adjacent coverage for inference-route onboarding failure classification, credential/endpoint safety, cleanup, and stack-trace/secret exposure boundaries. It does not specifically exercise the new sandbox-first get/set grammar, so it is lower priority than the switch jobs.

New E2E recommendations

  • sandbox-scoped inference CLI grammar (high): Existing live inference-switch jobs exercise nemoclaw inference set and OpenShell inference get, but not the new public nemoclaw <name> inference get / nemoclaw <name> inference set path through sandbox-first dispatch and oclif adapters.
    • Suggested test: Add or extend a live inference-switch smoke to call nemoclaw <sandbox-name> inference get --json and nemoclaw <sandbox-name> inference set --provider ... --model ..., then assert the OpenShell route and in-sandbox config/registry state match.

Dispatch hint

  • Workflow: .github/workflows/e2e.yaml
  • jobs input: openclaw-inference-switch,hermes-inference-switch,inference-routing

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: hermes-inference-switch-vitest, openclaw-inference-switch-vitest
Optional Vitest E2E scenarios: None

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=hermes-inference-switch-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=openclaw-inference-switch-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required Vitest E2E scenarios

  • hermes-inference-switch-vitest: The PR adds sandbox-scoped inference get/set command adapters and documents nemohermes sandbox-first inference grammar. The Hermes inference switch live Vitest job is the focused free-standing coverage for the Hermes inference set path, route/config sync, registry/session update, and post-switch runtime health.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=hermes-inference-switch-vitest
  • openclaw-inference-switch-vitest: The new sandbox-scoped inference set adapter delegates to the same live inference switch machinery for named OpenClaw sandboxes. This job is the focused free-standing Vitest coverage for OpenClaw inference route/config sync, registry/session state, inference.local, and an agent turn after switching.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=openclaw-inference-switch-vitest

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • src/commands/sandbox/inference/get.ts
  • src/commands/sandbox/inference/set.ts
  • src/lib/cli/public-display-defaults.ts

@yimoj

yimoj commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator Author

Real worktree-CLI verification (E2E)

Run from the PR worktree via the actual launcher (node ./bin/nemoclaw.js, NEMOCLAW_CLI_NAME=nemohermes). The reporter's commands now route to the new sandbox-scoped inference commands instead of the NemoClaw-owned Unknown action: inference error.

1. nemohermes <sb> inference get is now a recognized action (previously printed Unknown action: inference). It routes into sandbox:inference:get, probes the live gateway, then applies the standard sandbox-existence check:

$ NEMOCLAW_CLI_NAME=nemohermes node ./bin/nemoclaw.js issue5977-sb inference get
  ...Starting OpenShell gateway... ✓ Docker-driver gateway is healthy
  Sandbox 'issue5977-sb' does not exist.
  Registered sandboxes: ...
[exit 1]

2. nemohermes <sb> inference set --provider ... --model ... routes into sandbox:inference:set and threads the positional sandbox name through the same path as inference set --sandbox <name>:

$ NEMOCLAW_CLI_NAME=nemohermes node ./bin/nemoclaw.js issue5977-sb inference set \
    --provider nvidia-prod --model nvidia/nemotron-3-super-120b-a12b
  Sandbox 'issue5977-sb' does not exist.
  Registered sandboxes: ...
[exit 1]

3. inference is now listed among the valid sandbox actions (registered sandbox + a genuinely-unknown action for contrast):

$ NEMOCLAW_CLI_NAME=nemohermes node ./bin/nemoclaw.js test-sb bogus-action-5977
  Unknown action: bogus-action-5977
  Valid actions: ... inference ...
[exit 1]

Both inference get/set reach their commands and apply the same sandbox-name validation as every other per-sandbox action; the Unknown action: inference symptom is gone. (Completing a full provider switch requires a live onboarded Hermes sandbox + GPU/OpenShell, unavailable on this CI-class host — the delegation into the already-reviewed runInferenceGet/runInferenceSet actions is covered by the package-contract translation tests.)

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-2: Local provider verification bypass lacks source-of-truth resolution and e2e coverage; then add or justify PRA-T1.
Open items: 1 required · 3 warnings · 2 suggestions · 5 test follow-ups
Since last review: 0 prior items resolved · 4 still apply · 0 new items found

Action checklist

  • PRA-2 Fix: Local provider verification bypass lacks source-of-truth resolution and e2e coverage in src/lib/actions/inference-set.ts:650
  • PRA-1 Resolve or justify: Source-of-truth review needed: src/lib/actions/inference-set.ts:650-675 (pre-existing local provider bypass)
  • PRA-3 Resolve or justify: Source-of-truth review needed for localized patch at inference-set.ts:650-675 in src/lib/actions/inference-set.ts:650
  • PRA-4 Resolve or justify: Local provider verification bypass lacks e2e integration test coverage in src/lib/actions/inference-set.ts:650
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Local provider verification bypass lacks e2e integration test coverage
  • PRA-T5 Add or justify test follow-up: src/lib/actions/inference-set.ts:650-675 (pre-existing local provider bypass)
  • PRA-5 In-scope improvement: Monolith growth in public-display-defaults.ts (+17 lines for 2 new hidden commands) in src/lib/cli/public-display-defaults.ts:318
  • PRA-6 In-scope improvement: Empty string check for provider/model flags duplicated between global and sandbox-first commands in src/commands/sandbox/inference/set.ts:75

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Required architecture src/lib/actions/inference-set.ts:650 Identify the invalid state, source boundary (OpenShell gateway verify), source-fix constraint (requires OpenShell change to support container-hostname resolution from host), regression test (unit tests exist at inference-set.test.ts:1224-1342), and removal condition before merging. This is a pre-existing issue not introduced by this PR but must be resolved or explicitly justified by a maintainer.
PRA-3 Resolve/justify architecture src/lib/actions/inference-set.ts:650 Complete the source-of-truth analysis: document the invalid state, source boundary, why the source cannot be fixed in this PR, what regression test proves the source cannot regress, and when the workaround can be removed. This is a pre-existing localized patch not modified by this PR.
PRA-4 Resolve/justify tests src/lib/actions/inference-set.ts:650 Add an e2e test in test/e2e-scenario/live/ for local inference switch (ollama-local or vllm-local), or document in the PR why unit-test coverage with mocks is sufficient for this bypass logic. This is a pre-existing gap not introduced by this PR.
PRA-5 Improvement scope src/lib/cli/public-display-defaults.ts:318 No action required for this PR — growth is proportional to new commands. Consider extraction pattern in future refactor when file exceeds ~800 lines or when multiple teams need independent display sections.
PRA-6 Improvement correctness src/commands/sandbox/inference/set.ts:75 Consider extracting the missing-flags redirect logic to a shared helper if more sandbox-first mirrors are added. Not required for this PR.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-2 Required — Local provider verification bypass lacks source-of-truth resolution and e2e coverage

  • Location: src/lib/actions/inference-set.ts:650
  • Category: architecture
  • Problem: The workaround at inference-set.ts:650-675 forces --no-verify for ollama-local/vllm-local because host-side OpenShell verify cannot resolve container hostname host.openshell.internal. No clear removal condition tied to OpenShell fix; REMOVAL CONDITION comment in local-inference-topology.ts:197 depends on external change. If ensureLocalProviderReachable returns false positive, unreachable local provider could be set silently.
  • Impact: Silent misconfiguration risk: user switches to ollama-local/vllm-local believing route is valid; sandbox runtime inference fails later with no early detection. No integration test exercises Docker networking / host-gateway resolution / container health check chain.
  • Required action: Identify the invalid state, source boundary (OpenShell gateway verify), source-fix constraint (requires OpenShell change to support container-hostname resolution from host), regression test (unit tests exist at inference-set.test.ts:1224-1342), and removal condition before merging. This is a pre-existing issue not introduced by this PR but must be resolved or explicitly justified by a maintainer.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Inspect inference-set.ts:650-675 for bypass logic and inference-set.test.ts:1224-1342 for unit test coverage. Search test/e2e-scenario/live/ for 'ollama-local' or 'vllm-local' — no matches found.
  • Missing regression test: Integration test exercising full local-provider switch flow including sandbox runtime inference reachability, or documented rationale for unit-test-only coverage with explicit acceptance of the risk.
  • Done when: The required change is committed and verification passes: Inspect inference-set.ts:650-675 for bypass logic and inference-set.test.ts:1224-1342 for unit test coverage. Search test/e2e-scenario/live/ for 'ollama-local' or 'vllm-local' — no matches found.
  • Evidence: Workaround implemented at inference-set.ts:650-675 with detailed logging. Unit tests cover bypass logic (validateLocalProvider ok → forces --no-verify; validateLocalProvider fails but reachable → warns + forces --no-verify; unreachable → aborts). No e2e test in test/e2e-scenario/live/ covers local provider switch. Documented REMOVAL CONDITION in local-inference-topology.ts:197 depends on external OpenShell change.
Review findings by urgency: 1 required fix, 3 items to resolve/justify, 2 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: src/lib/actions/inference-set.ts:650-675 (pre-existing local provider bypass)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Unit tests at inference-set.test.ts:1224-1342 mock validateLocalProvider and ensureLocalProviderReachable covering three branches
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Workaround implemented at inference-set.ts:650-675 with detailed logging. Unit tests cover bypass logic. No e2e test in test/e2e-scenario/live/ covers local provider switch. REMOVAL CONDITION comment in local-inference-topology.ts:197.

PRA-3 Resolve/justify — Source-of-truth review needed for localized patch at inference-set.ts:650-675

  • Location: src/lib/actions/inference-set.ts:650
  • Category: architecture
  • Problem: The workaround preserves an invalid state (host-side verify guaranteed false negative) without a clear source-fix path or removal condition in this PR.
  • Impact: Workaround may hide or preserve invalid state; future OpenShell changes could render the bypass unnecessary or incorrect without detection.
  • Recommended action: Complete the source-of-truth analysis: document the invalid state, source boundary, why the source cannot be fixed in this PR, what regression test proves the source cannot regress, and when the workaround can be removed. This is a pre-existing localized patch not modified by this PR.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch at inference-set.ts:650-675 for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Unit tests in inference-set.test.ts:1224-1342 mock validateLocalProvider and ensureLocalProviderReachable covering three branches.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch at inference-set.ts:650-675 for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Workaround implemented at inference-set.ts:650-675. Unit tests cover bypass logic. No e2e test in test/e2e-scenario/live/ covers local provider switch. REMOVAL CONDITION comment in local-inference-topology.ts:197.

PRA-4 Resolve/justify — Local provider verification bypass lacks e2e integration test coverage

  • Location: src/lib/actions/inference-set.ts:650
  • Category: tests
  • Problem: Unit tests mock validateLocalProvider and ensureLocalProviderReachable but no live e2e test exercises the full ollama-local/vllm-local switch flow including sandbox runtime inference reachability.
  • Impact: Regression in the host→sandbox reachability chain (Docker networking, host-gateway resolution, container health check) would not be detected until a user attempts a local provider switch at runtime.
  • Recommended action: Add an e2e test in test/e2e-scenario/live/ for local inference switch (ollama-local or vllm-local), or document in the PR why unit-test coverage with mocks is sufficient for this bypass logic. This is a pre-existing gap not introduced by this PR.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search test/e2e-scenario/live/ for 'ollama-local' or 'vllm-local' — no matches found. Check inference-set.test.ts lines 1224-1342 for unit test coverage of the bypass.
  • Missing regression test: Integration test exercising full local-provider switch flow including sandbox runtime inference reachability, or documented rationale for unit-test-only coverage.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search test/e2e-scenario/live/ for 'ollama-local' or 'vllm-local' — no matches found. Check inference-set.test.ts lines 1224-1342 for unit test coverage of the bypass.
  • Evidence: inference-set.ts lines 650-675 implement the bypass. inference-set.test.ts has unit tests for this path (lines 1224-1342) mocking validateLocalProvider and ensureLocalProviderReachable. No e2e test in test/e2e-scenario/live/ covers local provider switch.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-5 Improvement — Monolith growth in public-display-defaults.ts (+17 lines for 2 new hidden commands)

  • Location: src/lib/cli/public-display-defaults.ts:318
  • Category: scope
  • Problem: Current file at ~592 lines; growth is proportional to new commands but extraction pattern should be considered when file exceeds ~800 lines or multiple teams need independent display sections.
  • Impact: File continues to grow as central display registry. Not a blocker for this PR.
  • Suggested action: No action required for this PR — growth is proportional to new commands. Consider extraction pattern in future refactor when file exceeds ~800 lines or when multiple teams need independent display sections.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: File is at 592 lines with 2 new hidden entries added. No immediate action needed.
  • Missing regression test: N/A - structural concern.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: public-display-defaults.ts grew from 575 to 592 lines (+17) adding two hidden command entries for sandbox:inference:get and sandbox:inference:set.

PRA-6 Improvement — Empty string check for provider/model flags duplicated between global and sandbox-first commands

  • Location: src/commands/sandbox/inference/set.ts:75
  • Category: correctness
  • Problem: Both commands check `!flags.provider || !flags.model` before delegating. Could be centralized in a shared helper or base class, but current duplication is minimal and explicit.
  • Impact: Minor code duplication. Both commands correctly redirect to OpenShell when flags missing.
  • Suggested action: Consider extracting the missing-flags redirect logic to a shared helper if more sandbox-first mirrors are added. Not required for this PR.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Compare src/commands/inference/set.ts:58-60 with src/commands/sandbox/inference/set.ts:73-75.
  • Missing regression test: N/A - code quality improvement.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Both command files have identical guard clause checking for missing provider/model flags and printing OpenShell redirect.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Cross-alias parity test: verify nemoclaw <sb> inference get and nemohermes <sb> inference get produce identical behavior (both delegate to same action). Runtime/sandbox/infrastructure paths need behavioral runtime validation: docs/reference/commands-nemohermes.mdx, docs/reference/commands.mdx, src/commands/inference/set.ts, src/commands/sandbox/inference/get.ts, src/commands/sandbox/inference/set.ts, src/lib/cli/flag-helpers.ts, src/lib/cli/public-display-defaults.ts. Dispatch tests spawn real CLI processes providing behavioral coverage for the dispatch fix.
  • PRA-T2 Runtime validation — Negative test: inference get with non-existent sandbox but healthy gateway (dispatch test uses mock openshell that exits 1). Runtime/sandbox/infrastructure paths need behavioral runtime validation: docs/reference/commands-nemohermes.mdx, docs/reference/commands.mdx, src/commands/inference/set.ts, src/commands/sandbox/inference/get.ts, src/commands/sandbox/inference/set.ts, src/lib/cli/flag-helpers.ts, src/lib/cli/public-display-defaults.ts. Dispatch tests spawn real CLI processes providing behavioral coverage for the dispatch fix.
  • PRA-T3 Runtime validation — Negative test: inference set with shields up (covered in action layer unit tests, not command adapter tests). Runtime/sandbox/infrastructure paths need behavioral runtime validation: docs/reference/commands-nemohermes.mdx, docs/reference/commands.mdx, src/commands/inference/set.ts, src/commands/sandbox/inference/get.ts, src/commands/sandbox/inference/set.ts, src/lib/cli/flag-helpers.ts, src/lib/cli/public-display-defaults.ts. Dispatch tests spawn real CLI processes providing behavioral coverage for the dispatch fix.
  • PRA-T4 Local provider verification bypass lacks e2e integration test coverage — Add an e2e test in test/e2e-scenario/live/ for local inference switch (ollama-local or vllm-local), or document in the PR why unit-test coverage with mocks is sufficient for this bypass logic. This is a pre-existing gap not introduced by this PR.
  • PRA-T5 src/lib/actions/inference-set.ts:650-675 (pre-existing local provider bypass) — Unit tests at inference-set.test.ts:1224-1342 mock validateLocalProvider and ensureLocalProviderReachable covering three branches. Workaround implemented at inference-set.ts:650-675 with detailed logging. Unit tests cover bypass logic. No e2e test in test/e2e-scenario/live/ covers local provider switch. REMOVAL CONDITION comment in local-inference-topology.ts:197.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: src/lib/actions/inference-set.ts:650-675 (pre-existing local provider bypass)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Unit tests at inference-set.test.ts:1224-1342 mock validateLocalProvider and ensureLocalProviderReachable covering three branches
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Workaround implemented at inference-set.ts:650-675 with detailed logging. Unit tests cover bypass logic. No e2e test in test/e2e-scenario/live/ covers local provider switch. REMOVAL CONDITION comment in local-inference-topology.ts:197.

PRA-2 Required — Local provider verification bypass lacks source-of-truth resolution and e2e coverage

  • Location: src/lib/actions/inference-set.ts:650
  • Category: architecture
  • Problem: The workaround at inference-set.ts:650-675 forces --no-verify for ollama-local/vllm-local because host-side OpenShell verify cannot resolve container hostname host.openshell.internal. No clear removal condition tied to OpenShell fix; REMOVAL CONDITION comment in local-inference-topology.ts:197 depends on external change. If ensureLocalProviderReachable returns false positive, unreachable local provider could be set silently.
  • Impact: Silent misconfiguration risk: user switches to ollama-local/vllm-local believing route is valid; sandbox runtime inference fails later with no early detection. No integration test exercises Docker networking / host-gateway resolution / container health check chain.
  • Required action: Identify the invalid state, source boundary (OpenShell gateway verify), source-fix constraint (requires OpenShell change to support container-hostname resolution from host), regression test (unit tests exist at inference-set.test.ts:1224-1342), and removal condition before merging. This is a pre-existing issue not introduced by this PR but must be resolved or explicitly justified by a maintainer.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Inspect inference-set.ts:650-675 for bypass logic and inference-set.test.ts:1224-1342 for unit test coverage. Search test/e2e-scenario/live/ for 'ollama-local' or 'vllm-local' — no matches found.
  • Missing regression test: Integration test exercising full local-provider switch flow including sandbox runtime inference reachability, or documented rationale for unit-test-only coverage with explicit acceptance of the risk.
  • Done when: The required change is committed and verification passes: Inspect inference-set.ts:650-675 for bypass logic and inference-set.test.ts:1224-1342 for unit test coverage. Search test/e2e-scenario/live/ for 'ollama-local' or 'vllm-local' — no matches found.
  • Evidence: Workaround implemented at inference-set.ts:650-675 with detailed logging. Unit tests cover bypass logic (validateLocalProvider ok → forces --no-verify; validateLocalProvider fails but reachable → warns + forces --no-verify; unreachable → aborts). No e2e test in test/e2e-scenario/live/ covers local provider switch. Documented REMOVAL CONDITION in local-inference-topology.ts:197 depends on external OpenShell change.

PRA-3 Resolve/justify — Source-of-truth review needed for localized patch at inference-set.ts:650-675

  • Location: src/lib/actions/inference-set.ts:650
  • Category: architecture
  • Problem: The workaround preserves an invalid state (host-side verify guaranteed false negative) without a clear source-fix path or removal condition in this PR.
  • Impact: Workaround may hide or preserve invalid state; future OpenShell changes could render the bypass unnecessary or incorrect without detection.
  • Recommended action: Complete the source-of-truth analysis: document the invalid state, source boundary, why the source cannot be fixed in this PR, what regression test proves the source cannot regress, and when the workaround can be removed. This is a pre-existing localized patch not modified by this PR.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch at inference-set.ts:650-675 for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Unit tests in inference-set.test.ts:1224-1342 mock validateLocalProvider and ensureLocalProviderReachable covering three branches.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch at inference-set.ts:650-675 for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Workaround implemented at inference-set.ts:650-675. Unit tests cover bypass logic. No e2e test in test/e2e-scenario/live/ covers local provider switch. REMOVAL CONDITION comment in local-inference-topology.ts:197.

PRA-4 Resolve/justify — Local provider verification bypass lacks e2e integration test coverage

  • Location: src/lib/actions/inference-set.ts:650
  • Category: tests
  • Problem: Unit tests mock validateLocalProvider and ensureLocalProviderReachable but no live e2e test exercises the full ollama-local/vllm-local switch flow including sandbox runtime inference reachability.
  • Impact: Regression in the host→sandbox reachability chain (Docker networking, host-gateway resolution, container health check) would not be detected until a user attempts a local provider switch at runtime.
  • Recommended action: Add an e2e test in test/e2e-scenario/live/ for local inference switch (ollama-local or vllm-local), or document in the PR why unit-test coverage with mocks is sufficient for this bypass logic. This is a pre-existing gap not introduced by this PR.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search test/e2e-scenario/live/ for 'ollama-local' or 'vllm-local' — no matches found. Check inference-set.test.ts lines 1224-1342 for unit test coverage of the bypass.
  • Missing regression test: Integration test exercising full local-provider switch flow including sandbox runtime inference reachability, or documented rationale for unit-test-only coverage.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search test/e2e-scenario/live/ for 'ollama-local' or 'vllm-local' — no matches found. Check inference-set.test.ts lines 1224-1342 for unit test coverage of the bypass.
  • Evidence: inference-set.ts lines 650-675 implement the bypass. inference-set.test.ts has unit tests for this path (lines 1224-1342) mocking validateLocalProvider and ensureLocalProviderReachable. No e2e test in test/e2e-scenario/live/ covers local provider switch.

PRA-5 Improvement — Monolith growth in public-display-defaults.ts (+17 lines for 2 new hidden commands)

  • Location: src/lib/cli/public-display-defaults.ts:318
  • Category: scope
  • Problem: Current file at ~592 lines; growth is proportional to new commands but extraction pattern should be considered when file exceeds ~800 lines or multiple teams need independent display sections.
  • Impact: File continues to grow as central display registry. Not a blocker for this PR.
  • Suggested action: No action required for this PR — growth is proportional to new commands. Consider extraction pattern in future refactor when file exceeds ~800 lines or when multiple teams need independent display sections.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: File is at 592 lines with 2 new hidden entries added. No immediate action needed.
  • Missing regression test: N/A - structural concern.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: public-display-defaults.ts grew from 575 to 592 lines (+17) adding two hidden command entries for sandbox:inference:get and sandbox:inference:set.

PRA-6 Improvement — Empty string check for provider/model flags duplicated between global and sandbox-first commands

  • Location: src/commands/sandbox/inference/set.ts:75
  • Category: correctness
  • Problem: Both commands check `!flags.provider || !flags.model` before delegating. Could be centralized in a shared helper or base class, but current duplication is minimal and explicit.
  • Impact: Minor code duplication. Both commands correctly redirect to OpenShell when flags missing.
  • Suggested action: Consider extracting the missing-flags redirect logic to a shared helper if more sandbox-first mirrors are added. Not required for this PR.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Compare src/commands/inference/set.ts:58-60 with src/commands/sandbox/inference/set.ts:73-75.
  • Missing regression test: N/A - code quality improvement.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Both command files have identical guard clause checking for missing provider/model flags and printing OpenShell redirect.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: No advisor follow-up required beyond maintainer review.
Open items: 0 required · 0 warnings · 0 suggestions · 0 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
test/package-contract/cli/command-registry.test.ts (1)

59-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Prefer presence assertions over rebasing registry totals.

These 51 / 14 / 30 expectations will churn on any unrelated command addition, even when the sandbox-first inference contract still works. Keep asserting the new inference usages and the "inference" token, but avoid tying this test to the full registry size. As per path instructions, "Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions."

Also applies to: 89-102, 224-237

🤖 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 `@test/package-contract/cli/command-registry.test.ts` around lines 59 - 66, The
command registry tests are overfitted to exact totals, so replace the brittle
length checks in command-registry.test.ts with presence-focused assertions
around the public inference contract. Keep the assertions that verify the new
inference usages and the "inference" token via sandboxCommands and the related
registry checks, but stop asserting the full registry sizes in the command
registry and related test blocks. Use the existing test helpers and symbols like
sandboxCommands and the inference-related command entries to validate observable
behavior without depending on unrelated command count changes.

Source: Path instructions

🤖 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/reference/commands-nemohermes.mdx`:
- Around line 1528-1529: The added prose in the affected MDX reference entries
is wrapped across multiple source lines, violating the one-sentence-per-line
docs rule. Reflow the new text in the nemohermes command reference so each
sentence in the added paragraphs stays on a single source line, including the
sentence near nemohermes <name> inference get and the other added prose around
the referenced section.

In `@docs/reference/commands.mdx`:
- Around line 1877-1878: The added prose in the reference docs wraps across
multiple source lines, violating the one-sentence-per-line MDX formatting rule.
Reflow the new sentences in the affected `commands.mdx` sections so each
sentence appears on a single source line, keeping the text unchanged while
preserving the existing `$$nemoclaw <name> inference get` and related prose.

In `@src/commands/sandbox/inference/set.ts`:
- Around line 57-62: The missing-flags path in SandboxInferenceSetCommand.run
still uses the “Unknown ${CLI_NAME} command: inference set” lead line, which
makes the matched sandbox command look unsupported. Update the message shown
when --provider or --model is absent so it clearly indicates missing required
flags instead of unknown command, while keeping the existing OpenShell redirect
from printOpenShellRedirect. Use the run method and the printOpenShellRedirect
helper to locate the branch and adjust the text there.

---

Nitpick comments:
In `@test/package-contract/cli/command-registry.test.ts`:
- Around line 59-66: The command registry tests are overfitted to exact totals,
so replace the brittle length checks in command-registry.test.ts with
presence-focused assertions around the public inference contract. Keep the
assertions that verify the new inference usages and the "inference" token via
sandboxCommands and the related registry checks, but stop asserting the full
registry sizes in the command registry and related test blocks. Use the existing
test helpers and symbols like sandboxCommands and the inference-related command
entries to validate observable behavior without depending on unrelated command
count changes.
🪄 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: d387e0e6-6c51-4ea8-b5e1-a1b356dd9752

📥 Commits

Reviewing files that changed from the base of the PR and between c6113be and 1c77e7b.

📒 Files selected for processing (7)
  • docs/reference/commands-nemohermes.mdx
  • docs/reference/commands.mdx
  • src/commands/sandbox/inference/get.ts
  • src/commands/sandbox/inference/set.ts
  • src/lib/cli/public-display-defaults.ts
  • test/package-contract/cli/command-registry.test.ts
  • test/package-contract/cli/public-argv-translation.test.ts

Comment thread docs/reference/commands-nemohermes.mdx
Comment thread docs/reference/commands.mdx
Comment thread src/commands/sandbox/inference/set.ts
CodeRabbit feedback:
- `sandbox:inference:set` printed `Unknown <cli> command: inference set` when
  `--provider`/`--model` were missing, even though the command had already
  matched. Replace the lead line with an explicit missing-flags message while
  keeping the OpenShell route hint.
- Split the sandbox-first `inference set` doc note into one sentence per source
  line to satisfy the Markdown/MDX one-sentence-per-line rule (regenerated the
  nemohermes variant from commands.mdx).

Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@yimoj

yimoj commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @coderabbitai — addressed in 62971d8:

  • set.ts "Unknown command" lead (functional): Fixed. When --provider/--model are missing, the matched sandbox:inference:set now prints <cli> <name> inference set requires --provider and --model. and keeps the OpenShell route hint, instead of implying the command is unknown.
  • One-sentence-per-line (docs): The added sentences were already one-per-line, but the inference set note joined two clauses with a semicolon — split into two sentences on separate source lines (and regenerated commands-nemohermes.mdx from commands.mdx).
  • Exact registry totals in command-registry.test.ts (nitpick): Keeping the exact 51/14/30 assertions intentionally. This file is a registry-composition contract — every sibling assertion pins exact counts (e.g. globalCommandTokens() "exactly 26", sandboxActionTokens() exact set). A presence-only assertion here would diverge from the file's established convention and weaken the contract that catches accidental command additions/removals.

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@yimoj: Thanks for the detailed follow-up — that all makes sense.

The sandbox:inference:set wording fix sounds right, and I agree the docs formatting update resolves the sentence-style concern. The explanation for keeping the exact 51 / 14 / 30 assertions in test/package-contract/cli/command-registry.test.ts is also reasonable given that file’s role as a registry-composition contract.

🐇👍


✏️ Learnings added
Learnt from: yimoj
URL: https://github.com/NVIDIA/NemoClaw/pull/5989

Timestamp: 2026-06-29T11:18:39.614Z
Learning: In `test/package-contract/cli/command-registry.test.ts`, exact registry totals such as `51`, `14`, and `30` are intentionally asserted because the file serves as a registry-composition contract; sibling assertions in the same test file also pin exact counts, so presence-only assertions would weaken the intended contract.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Address PR Review Advisor PRA-1/PRA-T1..T5: the package-contract tests proved
public argv translation and registry discovery but stopped before executing the
new `SandboxInferenceSetCommand` / `SandboxInferenceGetCommand` wrappers.

Add co-located adapter tests (mirroring the global inference adapter tests)
that execute the commands with mocked actions and assert the parser-to-action
contract:
- positional sandbox name + all inference flags (--no-verify, --endpoint-url,
  --credential-env, --inference-api) are forwarded to runInferenceSet
- missing --provider/--model prints the missing-flags redirect and never calls
  the action
- an empty --provider is rejected by the non-empty parser before the action runs
- `get --json` calls runInferenceGet with quiet:true and emits the JSON payload
- typed InferenceSetError/InferenceGetError map to the process exit code without
  throwing an oclif ExitError

Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@yimoj

yimoj commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator Author

@nemoclaw-pr-review-advisor — PRA-1 and PRA-T1..PRA-T5 resolved in 1f416e1.

Added src/commands/sandbox/inference/oclif-command-adapters.test.ts, mirroring the global inference adapter tests, executing the new wrappers with mocked actions:

  • PRA-T1forwards the positional sandbox name and custom-provider flags to runInferenceSet (argv alpha --provider compatible-endpoint --model ... --no-verify --endpoint-url ... --credential-env ... --inference-api openai-completions).
  • PRA-T2maps the sandbox inference get --json output into oclif JSON handling (asserts runInferenceGet({ quiet: true }) and the emitted JSON payload).
  • PRA-T3rejects an empty --provider before runInferenceSet is called (non-empty parser guard).
  • PRA-T4records typed inference action failures without throwing oclif ExitError (InferenceGetError→exit 3, InferenceSetError→exit 4).
  • Plus prints the missing-flags redirect without calling runInferenceSet, covering the corrected redirect from 62971d8.

All 5 pass in the cli vitest project.

@wscurran wscurran added area: cli Command line interface, flags, terminal UX, or output area: inference Inference routing, serving, model selection, or outputs bug-fix PR fixes a bug or regression labels Jun 29, 2026
@wscurran

Copy link
Copy Markdown
Contributor

Respond to PR Review Advisor findings on the sandbox-scoped inference
commands:

- Add command-layer tests asserting the get "route not configured" path
  and typed set validation failures (unsupported provider, unsafe model
  id) surface their message and exit code (PRA-1, PRA-2, PRA-8, T1-T8).
- Document the global/sandbox command pairing and that deep validation is
  intentionally centralized in the shared runInferenceSet/runInferenceGet
  action layer (PRA-3, PRA-5).
- Extract the duplicated nonEmptyFlag helper into src/lib/cli/flag-helpers
  shared by the global and sandbox-first inference set commands (PRA-7).

Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@yimoj

yimoj commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator Author

Response to PR Review Advisor (Nemotron Ultra)

Addressed in b4f8f5ce0:

  • PRA-1, PRA-2, PRA-8, PRA-T1–T8 (tests) — Added command-layer tests in oclif-command-adapters.test.ts: sandbox:inference:get surfacing the OpenShell inference route is not configured. failure (exit 1), and sandbox:inference:set surfacing typed validation failures (unsupported provider, unsafe model id) with their messages and exit codes. Note the command layer is a thin adapter that delegates to the shared action; the deep validation logic itself (assertSupportedProvider, isSafeModelId, endpoint/credential/API normalization) lives in and is exhaustively unit-tested at the action layer (test/lib/actions/inference-set.test.ts). The new tests cover the command→action error-propagation boundary.
  • PRA-3 (architecture) — Added code comments in inference/set.ts, sandbox/inference/set.ts, and sandbox/inference/get.ts documenting the global ↔ sandbox-first command pairing and shared runInferenceSet/runInferenceGet delegation.
  • PRA-5 (validation boundary) — Added a comment in sandbox/inference/set.ts explaining that flags enforce only the non-empty contract and deep validation is intentionally centralized in the shared action layer for one validation surface across both grammars.
  • PRA-7 (dedup) — Extracted the duplicated nonEmptyFlag helper into src/lib/cli/flag-helpers.ts, now imported by both the global and sandbox-first inference set commands.

Justified / already-resolved:

  • PRA-4 (local-provider bypass integration coverage) — Integration coverage already exists: test/e2e-scenario/live/openclaw-inference-switch.test.ts performs a real nemoclaw inference set route switch, then checks inference.local reachability and a real OpenClaw agent turn across the live host/sandbox boundary (including the --no-verify transient-failure path). The ensureLocalProviderReachable bypass lives unchanged in the action layer (inference-set.ts, unit-tested at inference-set.test.ts:1224-1342); this PR only adds a delegating command adapter and does not alter the bypass. A dedicated host-check-false-positive simulation would require a GPU host with a real ollama/vllm endpoint (no CI runner), so it is out of scope for this command-adapter PR.
  • PRA-6 (docs cross-reference) — Already present: docs/reference/commands.mdx cross-references the sandbox-first grammar from the global inference get (line ~1877) and inference set (line ~1908) sections.

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: openclaw-inference-switch
Optional E2E targets: hermes-inference-switch

Dispatch required E2E targets:

  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=openclaw-inference-switch

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required E2E targets

  • openclaw-inference-switch: Inference command boundary changes affect the live nemoclaw inference set path and its shared non-empty flag parsing. The wired OpenClaw inference-switch job is the smallest live E2E dispatch that exercises route switching, config sync, registry/session state, and inference.local after the CLI change.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=openclaw-inference-switch

Optional E2E targets

  • hermes-inference-switch: Optional adjacent coverage for the same inference command surface on Hermes, validating Hermes-specific config sync and runtime preservation after the shared CLI inference command changes.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=hermes-inference-switch

Relevant changed files

  • src/commands/inference/set.ts
  • src/commands/sandbox/inference/get.ts
  • src/commands/sandbox/inference/set.ts
  • src/lib/cli/flag-helpers.ts
  • src/lib/cli/public-display-defaults.ts

…fs (#5977)

Address the refreshed PR Review Advisor pass on the sandbox-scoped
inference commands:

- Add command-layer negative tests covering the remaining validation
  categories the advisor enumerated (PRA-2): malformed endpoint-url
  (ftp://, embedded credentials, missing scheme), credential-env
  mismatch, disallowed inference-api, and custom metadata flags with a
  non-compatible provider — each asserts the typed InferenceSetError
  surfaces with exit code 2 and message at the command boundary.
- Note the sandbox-first grammar in the global inference get/set command
  descriptions for both nemoclaw and nemohermes docs (PRA-5).

Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@yimoj

yimoj commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator Author

Response to refreshed advisor pass (head 3c62843c3)

Addressed in 3c62843c3:

  • PRA-2 / PRA-T1–T6 (command-layer negative tests) — Added command-layer tests for the four remaining validation categories the advisor enumerated: malformed --endpoint-url (ftp://, embedded credentials, missing scheme), --credential-env mismatch, disallowed --inference-api, and custom metadata flags with a non-compatible provider. Each asserts the typed InferenceSetError surfaces with exit code 2 and the expected message at the command boundary. oclif-command-adapters.test.ts now has 10 tests.
  • PRA-5 (docs cross-reference) — Added an explicit sandbox-first note to the global inference get/inference set command descriptions in both commands.mdx and commands-nemohermes.mdx.

Justified — out of this PR's scope (file not in the diff):

  • PRA-1, PRA-3, PRA-8 (local-provider verify bypass, inference-set.ts:415-445) — This code is not modified by this PR (git diff main...HEAD does not touch src/lib/actions/inference-set.ts); the PR adds a thin sandbox-first command adapter that delegates to the pre-existing shared runInferenceSet action. The bypass is already unit-tested at inference-set.test.ts:1224-1342 (host-validation-passes → --no-verify; fails-but-reachable → warn + --no-verify; unreachable → abort). A dedicated ollama-local/vllm-local live e2e requires a GPU host running a real local inference server, for which there is no CI runner; per the advisor's own accepted resolution ("document why unit-test coverage with mocks is sufficient"), we accept unit-test-only coverage for this bypass branch in this PR. The closest live coverage that does exist, test/e2e-scenario/live/openclaw-inference-switch.test.ts, exercises a real nemoclaw inference set route switch + inference.local reachability + agent turn.

Justified — already covered / no action needed:

  • base advisor PRA-T1, PRA-T2 (runtime dispatch) — Already covered: test/package-contract/cli/public-argv-translation.test.ts:214 proves nemoclaw <name> inference get/set routes to sandbox:inference:get/sandbox:inference:set with the positional sandbox name forwarded, and :244 proves the bare/help form routes to the oclif parent and never hits the NemoClaw Unknown action: inference path that this issue ([Linux][CLI&UX] nemohermes <sb> inference get/set returns "Unknown action: inference" — per-sandbox subcommand missing on the nemohermes alias #5977) reported.
  • base advisor PRA-T3 (global form preserved) — The PR leaves the global inference set --sandbox <name> route untouched and adds the equivalent sandbox-first form; public-argv-translation.test.ts covers both. The global inference get is intentionally gateway-wide (no --sandbox), documented as such.
  • base advisor PRA-T4 (v0.0.68 re-verification) — This is external report provenance from the issue, not an implementation requirement; the review was static.
  • PRA-4 (type-enforce command pairing) — The advisor itself states this is "a future improvement, not a current-PR requirement"; the pairing comments added are sufficient for this PR.
  • PRA-6, PRA-7 — Advisor: "No action needed — validation is solid."

Both advisors report 0 required findings (Nemotron Ultra: merge_after_fixes; base: merge_as_is).

…5977)

Add the symmetric empty --model parse-level rejection test the advisor
flagged (PRA-4): both --provider and --model use the shared nonEmptyFlag
helper, so assert --model whitespace is rejected before runInferenceSet
is called, mirroring the existing --provider case.

Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@yimoj

yimoj commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator Author

Source-of-truth review — local provider verify bypass (PRA-2 required; PRA-1, PRA-3)

The advisor requires a source-of-truth review of the local-provider verify bypass at src/lib/actions/inference-set.ts:578-605. This code is not introduced or modified by this PRgit diff main...HEAD does not touch src/lib/actions/inference-set.ts; this PR adds the sandbox-first command adapter (sandbox:inference:set/get) that delegates to the pre-existing runInferenceSet/runInferenceGet actions. Structured review as requested:

  • Invalid state: OpenShell's host-side openshell inference set verification probes host.openshell.internal — the sandbox-facing container hostname — which does not resolve from the host network namespace. For ollama-local/vllm-local this makes the default verify a guaranteed false negative on an otherwise-valid route.
  • Source boundary: OpenShell's gateway inference set verify step. It is owned by OpenShell (the openshell binary), not by NemoClaw, and cannot be corrected from this repository.
  • Source-fix constraint: A real fix requires an OpenShell change so its host-side verify resolves/routes the container hostname (or natively skips verify for local providers). Until then, NemoClaw cannot make OpenShell's verify succeed for a local route.
  • Localized behavior (not a blind skip): Rather than unconditionally passing --no-verify, NemoClaw substitutes its own host-stack probe before touching the route (inference-set.ts:584-605): validateLocalProvider(provider).ok → force --no-verify; else ensureLocalProviderReachable(provider) true → warn + force --no-verify; else throw InferenceSetError(exit 1) — the route is never set. So an unreachable host stack hard-fails before the gateway route changes; it does not silently set an unreachable route.
  • Regression test: test/lib/actions/inference-set.test.ts:1224-1342 covers all three branches (validated→--no-verify; reachable-with-warning→--no-verify; unreachable→abort exit 1) by mocking validateLocalProvider/ensureLocalProviderReachable. PRA-3 (e2e): a live ollama-local/vllm-local switch test requires a GPU host running a real local inference server, for which there is no CI runner; we accept unit-test-only coverage for this pre-existing bypass branch (the advisor's stated alternative resolution). The closest existing live coverage, test/e2e-scenario/live/openclaw-inference-switch.test.ts, exercises a real nemoclaw inference set route switch + inference.local reachability + agent turn.
  • Removal condition: Delete the bypass (stop forcing --no-verify for local providers and drop the validateLocalProvider/ensureLocalProviderReachable pre-check) once OpenShell's host-side inference verify can resolve host.openshell.internal or natively supports local-provider verification.

Also addressed in 53500a275:

  • PRA-4 / PRA-T2 — Added the symmetric empty---model parse-level rejection test (mirrors the existing empty---provider test); both flags use the shared nonEmptyFlag helper. oclif-command-adapters.test.ts now has 11 tests.

This required item concerns pre-existing, out-of-PR-scope code; resolution here is the explicit justification above. A maintainer makes the final merge decision.

…rence action (#5977)

Add two real-CLI public-dispatch tests proving the #5977 fix end to end:
a missing sandbox name with the new `inference` action routes through
sandbox-name validation ("Sandbox '<name>' does not exist") instead of the
old "Unknown action: inference" dead end, and an unknown action on an
existing sandbox lists `inference` among the reporter-facing Valid actions.

Signed-off-by: Yimo Jiang <yimoj@nvidia.com>

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@yimoj

yimoj commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@yimoj yimoj added the v0.0.73 label Jun 30, 2026
…ference-commands

# Conflicts:
#	test/package-contract/cli/command-registry.test.ts
@yimoj

yimoj commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cv cv added v0.0.74 and removed v0.0.73 labels Jul 2, 2026

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the sandbox-scoped inference command adapters, shared flag validation, dispatch/display integration, and focused tests. Both required OpenClaw inference-switch variants passed on exact head; CI, CodeRabbit, GPT PR Advisor, DCO, and commit verification are clean. Approved for v0.0.74, sequenced after the OpenShell dependency foundation.

@cv
cv merged commit 0ffbbc1 into main Jul 3, 2026
118 of 119 checks passed
@cv
cv deleted the fix/5977-sandbox-inference-commands branch July 3, 2026 04:00
@ericksoa ericksoa mentioned this pull request Jul 4, 2026
21 tasks
ericksoa added a commit that referenced this pull request Jul 4, 2026
<!-- markdownlint-disable MD041 -->
## Summary
This PR prepares the user-facing documentation for v0.0.74 before the
release plan is frozen.
It expands the release notes across the 56-commit train and closes
durable documentation gaps found during the pre-tag commit scan.

## Changes
- Expand the `v0.0.74` release notes to cover OpenShell 0.0.72, managed
MCP, progressive tool disclosure, LangChain Deep Agents Code,
onboarding, local inference, messaging, recovery, and contributor
workflows.
- Correct the `destroy` contract for retained per-name volumes,
gateway-unreachable `--force` cleanup, managed MCP ownership, and
same-name recovery.
- Document separate remediation for an unreachable container DNS
resolver versus one that answers with `NXDOMAIN` or `REFUSED`.
- Document the Windows on Arm N1X automatic Ollama safeguard and its
remaining large-model limitations.
- State that messaging conflicts abort rebuild before backup or
deletion, leaving the original sandbox intact.
- Link the agent-runnable value benchmark from the contributor task
index.
- Synchronize generated agent command variants.
- Validate with `npm run docs:sync-agent-variants` and `npm run docs`;
Fern completed with 0 errors and 2 existing warnings.
- Source summary:
- [#6020](#6020) and
[#5876](#5876) ->
`docs/about/release-notes.mdx`: Consolidate the OpenShell 0.0.72 policy
boundary and managed MCP lifecycle.
- [#6251](#6251) and
[#5989](#5989) ->
`docs/about/release-notes.mdx`: Summarize progressive tool disclosure
and sandbox-first inference controls.
- [#6232](#6232),
[#6082](#6082),
[#6219](#6219),
[#6214](#6214),
[#6215](#6215),
[#6230](#6230), and
[#6260](#6260) ->
`docs/about/release-notes.mdx`: Summarize the experimental LangChain
Deep Agents Code status, secret, version, rebuild, snapshot, and MCP
boundaries.
- [#6166](#6166),
[#6254](#6254),
[#6265](#6265),
[#6164](#6164), and
[#6017](#6017) ->
`docs/about/release-notes.mdx`: Summarize BuildKit prebuild, validated
image reuse, bounded readiness, and preflight improvements.
- [#6150](#6150) ->
`docs/about/release-notes.mdx` and `docs/reference/troubleshooting.mdx`:
Separate unreachable-resolver remediation from reachable-but-rejected
DNS responses.
- [#6234](#6234) ->
`docs/about/release-notes.mdx`,
`docs/inference/use-local-inference.mdx`, and
`docs/get-started/windows-preparation.mdx`: Document N1X automatic 9B
selection and the remaining explicit-large-model boundary.
- [#6129](#6129),
[#5987](#5987),
[#5955](#5955), and
[#6220](#6220) ->
`docs/about/release-notes.mdx`,
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Document messaging policy
persistence, status, and the pre-destructive conflict check.
- [#5963](#5963),
[#6050](#6050),
[#6094](#6094),
[#6238](#6238),
[#5988](#5988),
[#6235](#6235),
[#6181](#6181), and
[#5986](#5986) ->
`docs/about/release-notes.mdx`, `docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Summarize day-two recovery and
clarify retained-volume and local-only destroy semantics.
- [#6200](#6200),
[#6248](#6248),
[#6168](#6168),
[#6270](#6270), and
[#5649](#5649) ->
`docs/about/release-notes.mdx` and `CONTRIBUTING.md`: Summarize
contributor setup and verification improvements and expose the advisory
value benchmark.

## 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)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: documentation-only release
preparation; generated-variant synchronization and the Fern docs build
validate the changed pages and routes.
- [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
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: tests
are not applicable to this documentation-only change; `npm run docs`
validates the source and generated routes.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run 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)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
* Expanded setup guidance for Windows on Arm devices with safer default
local model selection.
* Clarified local inference and sandbox messaging behavior, including
conflict checks before rebuilds and safer recovery steps.
* Updated destroy/rebuild/reference docs with more detailed warnings,
failure handling, and volume-retention guidance.
* Improved troubleshooting instructions for Docker DNS issues with
clearer paths for unreachable vs. blocked resolvers.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
…#5977) (NVIDIA#5989)

## Summary
The sandbox-first grammar `nemohermes <sb> inference get` / `...
inference set ...` failed with `Unknown action: inference` because
`inference get`/`set` existed only as global commands. This adds
sandbox-scoped equivalents that route the sandbox name through the
existing global inference code paths, making the reporter's exact
workflow succeed.

## Related Issue
Fixes NVIDIA#5977

## Changes
- Add `src/commands/sandbox/inference/get.ts` (`sandbox:inference:get`)
and `src/commands/sandbox/inference/set.ts` (`sandbox:inference:set`).
Each takes the sandbox name in sandbox-first position and delegates to
the existing `runInferenceGet` / `runInferenceSet` actions — `set`
threads the positional name into the same path as `inference set
--sandbox <name>`; `get` reads the same gateway-wide route so the
grammar stays symmetric.
- Public dispatch now derives `<sb> inference get/set` routes from the
new command ids (no routing changes needed), and `inference` becomes a
recognized sandbox action token. Bare `<sb> inference` / `--help` defer
to oclif exactly as the existing `config` action does.
- Add hidden public-display metadata for the two leaf commands so they
satisfy the display-coverage contract without duplicating the visible
global `inference get/set` entries in root help.
- Document the sandbox-first grammar for both aliases in the command
reference (`commands.mdx` source + generated `commands-nemohermes.mdx`).

## Type of Change
- [x] Code change with doc updates

## Quality Gates
- [x] Tests added or updated for changed behavior — new
public-argv-translation cases for sandbox-scoped inference get/set +
oclif-parent parity; updated command-registry counts (sandbox actions
29→30, hidden 12→14, sandbox commands 49→51).
- [x] Docs updated for user-facing behavior changes — command reference
for `nemoclaw`/`nemohermes`.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging) —
inference/sandbox dispatch.
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — justification: new commands are thin delegators to the
already-reviewed `runInferenceGet`/`runInferenceSet` actions; no new
inference/credential logic; routing mirrors the existing `config`
precedent.

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Targeted tests pass for changed behavior — `vitest --project
package-contract` for `public-argv-translation.test.ts` and
`command-registry.test.ts` (46 passing); `typecheck:cli` clean.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [x] `npm run docs` builds without warnings (doc changes only) —
`docs:strict` reports 0 errors; agent-variant sync check passes
(generated `commands-nemohermes.mdx` matches).

### Real worktree-CLI transcript (E2E — reporter workflow)
Run from this PR's worktree via the actual launcher (`node
./bin/nemoclaw.js`, `NEMOCLAW_CLI_NAME=nemohermes`). The reporter's
commands now route to the new sandbox-scoped inference commands instead
of NemoClaw's `Unknown action: inference`.

```console
$ NEMOCLAW_CLI_NAME=nemohermes node ./bin/nemoclaw.js issue5977-sb inference get
  ...Starting OpenShell gateway... ✓ Docker-driver gateway is healthy
  Sandbox 'issue5977-sb' does not exist.
  Registered sandboxes: ...
[exit 1]                # recognized action → sandbox:inference:get → probes gateway → standard name check (NOT "Unknown action: inference")

$ NEMOCLAW_CLI_NAME=nemohermes node ./bin/nemoclaw.js issue5977-sb inference set \
    --provider nvidia-prod --model nvidia/nemotron-3-super-120b-a12b
  Sandbox 'issue5977-sb' does not exist.
  Registered sandboxes: ...
[exit 1]                # recognized action → sandbox:inference:set → threads positional name like `--sandbox`

$ NEMOCLAW_CLI_NAME=nemohermes node ./bin/nemoclaw.js test-sb bogus-action-5977
  Unknown action: bogus-action-5977
  Valid actions: ... inference ...
[exit 1]                # `inference` is now a recognized sandbox action token
```

Completing an actual provider switch requires a live onboarded Hermes
sandbox + GPU/OpenShell, unavailable on this CI-class host; the
delegation into the already-reviewed `runInferenceGet`/`runInferenceSet`
actions is covered by the package-contract translation tests.

---
Signed-off-by: Yimo Jiang <yimoj@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added sandbox-scoped inference commands to view and update the active
inference route (`get` and `set`), with `get` supporting `--json` and
`set` requiring `--provider` and `--model`.
* **Documentation**
* Expanded command reference docs to support “sandbox-first” inference
grammar for both NemoHermes and NemoClaw, including new examples.
* **Bug Fixes**
* Improved CLI routing/argument translation so inference subcommands
dispatch correctly and provide the expected “valid actions” behavior.
* **Tests**
* Updated and added contract/adapter/dispatch tests to cover the new
sandbox inference forms and error handling.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
This PR prepares the user-facing documentation for v0.0.74 before the
release plan is frozen.
It expands the release notes across the 56-commit train and closes
durable documentation gaps found during the pre-tag commit scan.

## Changes
- Expand the `v0.0.74` release notes to cover OpenShell 0.0.72, managed
MCP, progressive tool disclosure, LangChain Deep Agents Code,
onboarding, local inference, messaging, recovery, and contributor
workflows.
- Correct the `destroy` contract for retained per-name volumes,
gateway-unreachable `--force` cleanup, managed MCP ownership, and
same-name recovery.
- Document separate remediation for an unreachable container DNS
resolver versus one that answers with `NXDOMAIN` or `REFUSED`.
- Document the Windows on Arm N1X automatic Ollama safeguard and its
remaining large-model limitations.
- State that messaging conflicts abort rebuild before backup or
deletion, leaving the original sandbox intact.
- Link the agent-runnable value benchmark from the contributor task
index.
- Synchronize generated agent command variants.
- Validate with `npm run docs:sync-agent-variants` and `npm run docs`;
Fern completed with 0 errors and 2 existing warnings.
- Source summary:
- [NVIDIA#6020](NVIDIA#6020) and
[NVIDIA#5876](NVIDIA#5876) ->
`docs/about/release-notes.mdx`: Consolidate the OpenShell 0.0.72 policy
boundary and managed MCP lifecycle.
- [NVIDIA#6251](NVIDIA#6251) and
[NVIDIA#5989](NVIDIA#5989) ->
`docs/about/release-notes.mdx`: Summarize progressive tool disclosure
and sandbox-first inference controls.
- [NVIDIA#6232](NVIDIA#6232),
[NVIDIA#6082](NVIDIA#6082),
[NVIDIA#6219](NVIDIA#6219),
[NVIDIA#6214](NVIDIA#6214),
[NVIDIA#6215](NVIDIA#6215),
[NVIDIA#6230](NVIDIA#6230), and
[NVIDIA#6260](NVIDIA#6260) ->
`docs/about/release-notes.mdx`: Summarize the experimental LangChain
Deep Agents Code status, secret, version, rebuild, snapshot, and MCP
boundaries.
- [NVIDIA#6166](NVIDIA#6166),
[NVIDIA#6254](NVIDIA#6254),
[NVIDIA#6265](NVIDIA#6265),
[NVIDIA#6164](NVIDIA#6164), and
[NVIDIA#6017](NVIDIA#6017) ->
`docs/about/release-notes.mdx`: Summarize BuildKit prebuild, validated
image reuse, bounded readiness, and preflight improvements.
- [NVIDIA#6150](NVIDIA#6150) ->
`docs/about/release-notes.mdx` and `docs/reference/troubleshooting.mdx`:
Separate unreachable-resolver remediation from reachable-but-rejected
DNS responses.
- [NVIDIA#6234](NVIDIA#6234) ->
`docs/about/release-notes.mdx`,
`docs/inference/use-local-inference.mdx`, and
`docs/get-started/windows-preparation.mdx`: Document N1X automatic 9B
selection and the remaining explicit-large-model boundary.
- [NVIDIA#6129](NVIDIA#6129),
[NVIDIA#5987](NVIDIA#5987),
[NVIDIA#5955](NVIDIA#5955), and
[NVIDIA#6220](NVIDIA#6220) ->
`docs/about/release-notes.mdx`,
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Document messaging policy
persistence, status, and the pre-destructive conflict check.
- [NVIDIA#5963](NVIDIA#5963),
[NVIDIA#6050](NVIDIA#6050),
[NVIDIA#6094](NVIDIA#6094),
[NVIDIA#6238](NVIDIA#6238),
[NVIDIA#5988](NVIDIA#5988),
[NVIDIA#6235](NVIDIA#6235),
[NVIDIA#6181](NVIDIA#6181), and
[NVIDIA#5986](NVIDIA#5986) ->
`docs/about/release-notes.mdx`, `docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Summarize day-two recovery and
clarify retained-volume and local-only destroy semantics.
- [NVIDIA#6200](NVIDIA#6200),
[NVIDIA#6248](NVIDIA#6248),
[NVIDIA#6168](NVIDIA#6168),
[NVIDIA#6270](NVIDIA#6270), and
[NVIDIA#5649](NVIDIA#5649) ->
`docs/about/release-notes.mdx` and `CONTRIBUTING.md`: Summarize
contributor setup and verification improvements and expose the advisory
value benchmark.

## 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)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: documentation-only release
preparation; generated-variant synchronization and the Fern docs build
validate the changed pages and routes.
- [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
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: tests
are not applicable to this documentation-only change; `npm run docs`
validates the source and generated routes.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run 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)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
* Expanded setup guidance for Windows on Arm devices with safer default
local model selection.
* Clarified local inference and sandbox messaging behavior, including
conflict checks before rebuilds and safer recovery steps.
* Updated destroy/rebuild/reference docs with more detailed warnings,
failure handling, and volume-retention guidance.
* Improved troubleshooting instructions for Docker DNS issues with
clearer paths for unreachable vs. blocked resolvers.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli Command line interface, flags, terminal UX, or output area: inference Inference routing, serving, model selection, or outputs bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Linux][CLI&UX] nemohermes <sb> inference get/set returns "Unknown action: inference" — per-sandbox subcommand missing on the nemohermes alias

3 participants