Skip to content

fix(inference): redirect bare inference set to openshell#4566

Merged
cv merged 4 commits into
mainfrom
fix-4544-inference-set-openshell-redirect
May 30, 2026
Merged

fix(inference): redirect bare inference set to openshell#4566
cv merged 4 commits into
mainfrom
fix-4544-inference-set-openshell-redirect

Conversation

@laitingsheng
Copy link
Copy Markdown
Contributor

@laitingsheng laitingsheng commented May 30, 2026

Summary

Bare nemoclaw inference set now redirects users to openshell inference set instead of emitting an oclif "Missing required flag" error, matching the existing UX contract used by term, policy set, and gateway stop. Passing both --provider and --model still drives the existing NemoClaw sandbox-sync orchestration.

Related Issue

Fixes #4544.

Changes

  • src/commands/inference/set.ts: drop required: true from --provider/--model. When either is absent, short-circuit with the standard OpenShell-redirect message (exit 1) and mention that supplying both flags additionally runs NemoClaw's sandbox-sync step.
  • test/cli.test.ts: rewrite the previous "oclif validation failures without stack traces" assertion to verify the new redirect surface; preserve the no-stack-trace / no-oclif-internals guard.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

  • Enhanced Command Guidance

    • The inference set command now fails gracefully when required parameters are missing, showing a clear multi-line message directing users to run the equivalent OpenShell command and how to sync sandbox config.
  • Improved Validation

    • Provider and model inputs are validated to reject empty values (trimming applied).
  • Tests

    • CLI tests updated to expect the new redirect message and exit behavior instead of prior flag-validation errors.

Review Change Stack

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 30, 2026

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: 7c12ce4c-3387-4e9c-bd21-2292fc92cc06

📥 Commits

Reviewing files that changed from the base of the PR and between 196d02e and eee3478.

📒 Files selected for processing (1)
  • test/cli.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/cli.test.ts

📝 Walkthrough

Walkthrough

The inference:set command now detects missing provider or model flags early and redirects users to the OpenShell equivalent command with a clear message, replacing prior oclif validation errors.

Changes

OpenShell inference set redirect

Layer / File(s) Summary
Flag parsing infrastructure
src/commands/inference/set.ts
New nonEmptyFlag(description) helper creates string flags with async parsing that trims input and rejects empty values. Imports updated to include CLI_NAME. provider and model flags now use nonEmptyFlag().
Redirect on missing flags
src/commands/inference/set.ts
Early guard in run() checks for missing provider or model, triggers printOpenShellRedirect(), and exits without calling runInferenceSet(). printOpenShellRedirect() emits a multi-line message indicating the operation belongs to OpenShell and shows the openshell inference set syntax with flag placeholders.
Test verification of redirect
test/cli.test.ts
Tests replaced the prior oclif validation assertions with expectations that inference set exits code 1, reports "Unknown nemoclaw command", includes "This operation belongs to OpenShell." messaging, suggests the openshell inference set -g nemoclaw --model <model> --provider <provider> command, and no longer contains "Missing required flag". Also verifies the Hermes CLI path.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#3447: Implements shared dispatcher and parse-error handling to route OpenShell-shaped commands and avoid raw oclif validation traces; related to this PR's OpenShell redirect behavior.

Suggested labels

NemoClaw CLI, OpenShell

Suggested reviewers

  • ericksoa
  • cv

Poem

🐰
I trim the flags that tumble and hide,
Reject the blanks, keep inputs bona fide.
If provider or model go astray,
I point to OpenShell and show the way.
Hop along — run the right command today.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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 accurately describes the main change: redirecting bare inference set invocations to openshell when flags are missing.
Linked Issues check ✅ Passed The PR implementation matches issue #4544's objective: making nemoclaw inference set redirect to openshell with proper messaging when required flags are absent.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the openshell redirect behavior; no unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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-4544-inference-set-openshell-redirect

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 30, 2026

E2E Advisor Recommendation

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

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

Auto-dispatched E2E: openclaw-inference-switch-e2e, hermes-inference-switch-e2e via nightly-e2e.yaml at eee347889608c1f1ddfa1246d43ca18bf2d212a3nightly run

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • openclaw-inference-switch-e2e (high): Directly exercises nemoclaw inference set --provider ... --model ... --sandbox ... against a running OpenClaw sandbox and verifies the OpenShell route, registry/session state, openclaw.json sync, and live inference still work after the flag parsing change.
  • hermes-inference-switch-e2e (high): The command uses branded CLI output and is also covered by the Hermes CLI path. This E2E verifies nemohermes inference set still switches a running Hermes sandbox and syncs config after making provider/model optional at parse time.

Optional E2E

  • inference-routing-e2e (medium): Useful adjacent confidence for inference route/provider behavior and credential/error classification, but the PR does not change provider storage, credential isolation, or routing internals directly.

New E2E recommendations

  • CLI OpenShell redirect negative path (medium): Existing inference-switch E2Es cover successful provider/model switching, but not the installed CLI negative path where inference set, inference set --provider ..., or inference set --model ... should redirect to openshell inference set without oclif stack traces. Current coverage is unit-level in test/cli.test.ts.
    • Suggested test: Add a lightweight E2E or scenario assertion that runs installed nemoclaw inference set and nemohermes inference set with missing provider/model and verifies the OpenShell redirect message and exit code.

Dispatch hint

  • Workflow: nightly-e2e.yaml
  • jobs input: openclaw-inference-switch-e2e,hermes-inference-switch-e2e

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 30, 2026

E2E Scenario Advisor Recommendation

Required scenario E2E: None
Optional scenario E2E: None

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

  • None. No scenario E2E required: the PR changes CLI inference:set missing-flag/redirect behavior and a non-scenario unit test. The changed files are outside test/e2e-scenario/ and the scenario workflows, and the scenario suite catalog does not contain a validation step for the missing provider/model redirect path.

Optional scenario E2E

  • None.

Relevant changed files

  • None.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 30, 2026

PR Review Advisor

Findings: 0 needs attention, 0 worth checking, 0 nice ideas
Since last review: 0 prior items resolved, 0 still apply, 0 new items found

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@github-actions
Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 26685485045
Target ref: 99149f85198c5cc4c8a9226993942f515c24b085
Workflow ref: main
Requested jobs: openclaw-inference-switch-e2e,hermes-inference-switch-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job Result
hermes-inference-switch-e2e ✅ success
openclaw-inference-switch-e2e ✅ success

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@github-actions
Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 26685971792
Target ref: 196d02e5171317997aaea26ae47486d49882d258
Workflow ref: main
Requested jobs: openclaw-inference-switch-e2e,hermes-inference-switch-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job Result
hermes-inference-switch-e2e ✅ success
openclaw-inference-switch-e2e ✅ success

@laitingsheng laitingsheng added the v0.0.56 Release target label May 30, 2026
@cv cv merged commit 7da4960 into main May 30, 2026
31 checks passed
@cv cv deleted the fix-4544-inference-set-openshell-redirect branch May 30, 2026 18:29
@github-actions
Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 26691529480
Target ref: eee347889608c1f1ddfa1246d43ca18bf2d212a3
Workflow ref: main
Requested jobs: openclaw-inference-switch-e2e,hermes-inference-switch-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job Result
hermes-inference-switch-e2e ✅ success
openclaw-inference-switch-e2e ✅ success

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix v0.0.56 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[All Platforms][CLI&UX] 'nemoclaw inference set' does not redirect to openshell command (inconsistent with term/policy/gateway)

2 participants