fix(inference): show registered providers and onboard tip on provider-not-found - #6023
Conversation
…-not-found (#5924) When `openshell inference set` fails with a provider-not-found error, the error message now lists registered providers and suggests running `nemoclaw onboard` to register a new one. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com> Signed-off-by: kagura-agent <kagura.agent.ai@gmail.com>
Address CodeRabbit review: - Don't report 'No providers registered' when listSandboxes() throws - Add regression test for listSandboxes() throwing Signed-off-by: kagura-agent <kagura.agent.ai@gmail.com>
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates OpenShell failure handling to capture command output, detect provider-not-found errors, and format messages with registered providers and onboarding guidance. Tests and docs cover the new message shapes, redaction, fallback behavior, and user guidance. ChangesOpenShell provider-not-found error enrichment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
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. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
PR Review Advisor (Nemotron Ultra) — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Vitest E2E Target Results — ❌ Some jobs failedRun: 28425855354
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/actions/inference-set-error.ts (1)
21-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove this pure failure-parsing helper out of
actions/.This module is pure provider-detection and message-formatting logic, so it makes
src/lib/actions/own domain behavior instead of orchestration. Prefer a domain helper and keeprunInferenceSetas the caller. As per path instructions, "keep pure provider-detection/message-formatting logic in domain-level helpers (if applicable)" and "src/lib/{actions,domain,adapters,state}/**: actions orchestrate, domain modules make pure decisions."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/actions/inference-set-error.ts` around lines 21 - 53, Move the pure helper logic out of the actions layer: relocate openshellReportsProviderNotFound and buildOpenshellInferenceSetFailureMessage from inference-set-error into a domain-level helper module, then have runInferenceSet call that helper instead of owning the parsing/formatting itself. Keep the functions pure and preserve their current behavior, using the same exported symbols or updated equivalents so callers can still detect provider-not-found and build the failure message. Update any imports/usages in the actions flow to reference the new domain helper and leave src/lib/actions focused on orchestration only.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 `@src/lib/actions/inference-set.ts`:
- Around line 629-637: The provider lookup in the provider-not-found fallback is
using sandbox entries via the `providerNotFound` branch in `inference-set.ts`,
but `use-command-deps` only guarantees `name` on sandboxes, so this path can
miss valid providers and fall back to `No providers registered`. Update the
fallback to read provider names from the authoritative registry source that owns
provider data, or adjust the dependency contract exposed by `use-command-deps`
so `deps.listSandboxes()` reliably includes provider information; keep the
enriched error path in `providerNotFound` and the registration lookup logic
aligned.
---
Nitpick comments:
In `@src/lib/actions/inference-set-error.ts`:
- Around line 21-53: Move the pure helper logic out of the actions layer:
relocate openshellReportsProviderNotFound and
buildOpenshellInferenceSetFailureMessage from inference-set-error into a
domain-level helper module, then have runInferenceSet call that helper instead
of owning the parsing/formatting itself. Keep the functions pure and preserve
their current behavior, using the same exported symbols or updated equivalents
so callers can still detect provider-not-found and build the failure message.
Update any imports/usages in the actions flow to reference the new domain helper
and leave src/lib/actions focused on orchestration only.
🪄 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: ec3612ea-9436-436d-bf54-28ae0a724cca
📒 Files selected for processing (4)
src/lib/actions/inference-set-error.test.tssrc/lib/actions/inference-set-error.tssrc/lib/actions/inference-set.test.tssrc/lib/actions/inference-set.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/actions/inference-set.test.ts
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
🌿 Preview your docs: https://nvidia-preview-pr-6023.docs.buildwithfern.com/nemoclaw |
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28425898821
|
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28426243228
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28426909065
|
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28427403988
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28428062734
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28429009674
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28429719656
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28430843790
|
cv
left a comment
There was a problem hiding this comment.
Exact-head automated review resolution for 94fbfbf472196c02d6a80618ceebca25e8e05d6f:
- Nemotron run 28430837636 reports
merge_after_fixes, but its sole required item,PRA-2, explicitly says “No action needed — resolved in this PR.” Its own verification confirms the former 1,349-line test monolith is deleted and the routing tests are split into focused files under 300 lines. The repository file-size gate and the full commit hook pass; the focused suite is 51/51. inference-set.tsis 755 lines, below its 758-line base. Provider failure reporting and provider-list diagnostics are isolated in dedicated modules.- The provider-list catch-all is intentional: secondary diagnostics must not mask the primary route failure or emit thrown details that may contain secrets. Thrown, timeout, ENOBUFS, and nonzero paths are directly tested to return the same observable static warning, with no raw query error logged.
- Redaction is intentionally ordered as full token masking first, then URL-aware userinfo/query cleanup. Regression tests cover env assignments, bearer tokens, URL credentials, and sensitive query values.
- The 64 KiB capture limit is an intentional bounded-output/DoS control shared with the classification scan. ENOBUFS behavior is tested for bounded, redacted output and no state mutation.
- Exact-head standard CI is green. Exact-head E2E run 28430843790 passed inference routing and hosted/Anthropic variants for both OpenClaw and Hermes. GPT-5.5 is exact-head
merge_as_is; CodeRabbit has no unresolved findings. - Sequencing remains relevant for overlapping open PRs: #5969 touches
command-support.ts; #5857 and #5874 touchinference-set.ts; #5645 touches the provider-switching docs. The current PR is mergeable with no conflict, but whichever lands later must rebase and revalidate semantic compatibility.
This is a COMMENTED review recording evidence and sequencing risk, not an approval or advisor override. A human approval is still required.
prekshivyas
left a comment
There was a problem hiding this comment.
Provider-not-found error path is correct — bounded scan, proper redaction, clean fallback. CodeRabbit finding already addressed by author. LGTM.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Prek Shiv <prekshiv@nvidia.com>
Preserve captured provider diagnostics and the latest shields mutation lock. Retain current endpoint validation behavior. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
## Summary - Add the `v0.0.72` release-note section with links to the deeper docs pages for installer recovery, command diagnostics, inference, policy, and sandbox repair changes. - Document the custom preset `allowed_ips` guard for user-authored policy files. ## Related Issue None. ## Source summary - #6132 -> `docs/about/release-notes.mdx`: Summarizes installer and upgrade recovery before generic onboarding, with links to quickstart and lifecycle docs. - #6087 -> `docs/network-policy/customize-network-policy.mdx`: Documents that user-authored custom presets reject `allowed_ips` for ordinary endpoints; also summarized in release notes. - #5975 -> `docs/about/release-notes.mdx`: Summarizes safer curl-based inference probes that keep API keys out of process arguments. - #6044 -> `docs/about/release-notes.mdx`: Summarizes compact `channels status` configuration reporting. - #6096 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw EC2 metadata discovery disablement and links to security guidance. - #5980 and #5991 -> `docs/about/release-notes.mdx`: Summarizes `exec` multiline argument rejection and recovery guidance. - #6023 -> `docs/about/release-notes.mdx`: Summarizes registered-provider diagnostics for `inference set` failures. - #6074 -> `docs/about/release-notes.mdx`: Summarizes the refreshed NVIDIA Endpoints featured-model selection behavior. - #5969 -> `docs/about/release-notes.mdx`: Summarizes `credentials add` provider credential registration. - #6060 -> `docs/about/release-notes.mdx`: Summarizes mutable OpenClaw config permission restoration after `exec`. - #6134 -> `docs/about/release-notes.mdx`: Summarizes restored Tavily access for managed Python workflows. - #6089 -> `docs/about/release-notes.mdx`: Summarizes Hermes runtime version-scheme comparison during upgrade checks. - #6131 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw gateway watchdog recovery behavior. - #5976 and #5990 -> `docs/about/release-notes.mdx`: Summarizes prompt stdin EOF cancellation behavior during onboarding. - #5540 -> `docs/about/release-notes.mdx`: Summarizes clarified host-level and per-sandbox status command scope. - #5978 and #6018 -> `docs/about/release-notes.mdx`: Summarizes policy-denial log breadcrumbs in connect shells. ## Testing - `npm run docs:sync-agent-variants` - `npm run docs` - Commit hooks passed during `git commit`, including commitlint and gitleaks. - Pre-push hook passed during `git push`, including TypeScript CLI and package/tag version sync. ## Checklist - [x] Documentation updated. - [x] `npm run docs` completed with 0 errors and 1 existing Fern warning. - [x] No source code or generated build artifacts committed. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.72 covering improved installer recovery, clearer CLI diagnostics, safer inference setup and provider switching, better credential handling, stronger policy boundaries, and more robust runtime repair behavior. * Updated network policy guidance to clarify when `allowed_ips` can be used, including a specific exception for the sandbox-to-host bridge endpoint. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…-not-found (NVIDIA#6023) <!-- markdownlint-disable MD041 --> ## Summary <!-- 1-3 sentences: what this PR does and why. --> When `nemoclaw inference set` fails because the requested provider is not registered, preserve a bounded, fully redacted failure diagnostic and add the registered provider list plus an onboarding recovery tip. This same-repository replacement preserves NVIDIA#5983's verified contributor history so the mandatory PR Review Advisor can run; fork-origin advisor jobs are skipped by policy. ## Related Issue <!-- Fixes #NNN or Closes #NNN. Remove this section if none. --> Closes NVIDIA#5924 Supersedes NVIDIA#5983 ## Changes <!-- Bullet list of key changes. --> - Capture `openshell inference set` output and enrich only a quoted provider-not-found diagnostic naming the provider actually requested. - Fully redact, whitespace-compact, and cap generic and enhanced OpenShell failure details at 500 characters so diagnostics remain useful without exposing credentials or unbounded subprocess output. - Query the sandbox registry for known providers without allowing registry lookup failures to hide the original route-update error; emit only a static safe warning when that lookup fails. - Isolate parsing and message formatting in `inference-set-error.ts`, with focused security/correctness tests separate from the already-large action test. - Cover matching, mismatched, malformed, stderr, and stdout diagnostics; registered and empty registries; registry failures; full secret-shape redaction; enhanced-path output bounds; and generic failures. - Compatibility boundary: OpenShell 0.0.71 exposes this condition only as subprocess text. Unknown or drifted formats stay generic; replace this parser when OpenShell provides a structured provider-not-found error carrying the missing provider as a field. - Preserve NVIDIA#5983's exact four GitHub-Verified commits without rewriting or cherry-picking, then add two signed maintainer hardening/refactor commits on top. ## Type of Change - [x] 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) ## Quality Gates <!-- Check all that apply. For any "covered by existing tests", "not applicable", or waiver entry, add a brief justification on the same line or in the Changes section. --> - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: this enriches an existing failure path without changing commands, configuration, or successful behavior. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: full redaction, exact-provider correlation, bounded output, malformed-input behavior, safe registry-warning text, and focused regressions were manually reviewed. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each item you ran and confirmed. Leave unchecked items you skipped. Doc-only changes do not require npm test unless you ran it. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [x] Targeted tests pass for changed behavior — 38/38 focused tests - [x] `npm run typecheck:cli` passes - [ ] Full `npm test` passes (broad runtime changes only) - [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) - [ ] Doc pages follow the style guide (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Exact-head E2E: https://github.com/NVIDIA/NemoClaw/actions/runs/28425898821 --- <!-- 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: kagura-agent <kagura.agent.ai@gmail.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Enhanced “provider not found” failure diagnostics when switching inference providers, including a registered-provider list (or “No providers registered”) and an onboarding tip to run `nemoclaw onboard`. * Improved visibility into command output while redacting sensitive details. * **Bug Fixes** * More reliable detection of provider-missing failures and clearer, actionable error reporting; non-provider-related failures remain generic. * When the requested provider can’t be found, existing route/sandbox state remains unchanged. * **Documentation** * Added prerequisites and troubleshooting guidance for cross-provider switching, including what happens when OpenShell can’t locate the requested provider. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: kagura-agent <kagura.agent.ai@gmail.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Signed-off-by: Prek Shiv <prekshiv@nvidia.com> Co-authored-by: kagura-agent <kagura.agent.ai@gmail.com> Co-authored-by: Claude Opus 4 <noreply@anthropic.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
## Summary - Add the `v0.0.72` release-note section with links to the deeper docs pages for installer recovery, command diagnostics, inference, policy, and sandbox repair changes. - Document the custom preset `allowed_ips` guard for user-authored policy files. ## Related Issue None. ## Source summary - NVIDIA#6132 -> `docs/about/release-notes.mdx`: Summarizes installer and upgrade recovery before generic onboarding, with links to quickstart and lifecycle docs. - NVIDIA#6087 -> `docs/network-policy/customize-network-policy.mdx`: Documents that user-authored custom presets reject `allowed_ips` for ordinary endpoints; also summarized in release notes. - NVIDIA#5975 -> `docs/about/release-notes.mdx`: Summarizes safer curl-based inference probes that keep API keys out of process arguments. - NVIDIA#6044 -> `docs/about/release-notes.mdx`: Summarizes compact `channels status` configuration reporting. - NVIDIA#6096 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw EC2 metadata discovery disablement and links to security guidance. - NVIDIA#5980 and NVIDIA#5991 -> `docs/about/release-notes.mdx`: Summarizes `exec` multiline argument rejection and recovery guidance. - NVIDIA#6023 -> `docs/about/release-notes.mdx`: Summarizes registered-provider diagnostics for `inference set` failures. - NVIDIA#6074 -> `docs/about/release-notes.mdx`: Summarizes the refreshed NVIDIA Endpoints featured-model selection behavior. - NVIDIA#5969 -> `docs/about/release-notes.mdx`: Summarizes `credentials add` provider credential registration. - NVIDIA#6060 -> `docs/about/release-notes.mdx`: Summarizes mutable OpenClaw config permission restoration after `exec`. - NVIDIA#6134 -> `docs/about/release-notes.mdx`: Summarizes restored Tavily access for managed Python workflows. - NVIDIA#6089 -> `docs/about/release-notes.mdx`: Summarizes Hermes runtime version-scheme comparison during upgrade checks. - NVIDIA#6131 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw gateway watchdog recovery behavior. - NVIDIA#5976 and NVIDIA#5990 -> `docs/about/release-notes.mdx`: Summarizes prompt stdin EOF cancellation behavior during onboarding. - NVIDIA#5540 -> `docs/about/release-notes.mdx`: Summarizes clarified host-level and per-sandbox status command scope. - NVIDIA#5978 and NVIDIA#6018 -> `docs/about/release-notes.mdx`: Summarizes policy-denial log breadcrumbs in connect shells. ## Testing - `npm run docs:sync-agent-variants` - `npm run docs` - Commit hooks passed during `git commit`, including commitlint and gitleaks. - Pre-push hook passed during `git push`, including TypeScript CLI and package/tag version sync. ## Checklist - [x] Documentation updated. - [x] `npm run docs` completed with 0 errors and 1 existing Fern warning. - [x] No source code or generated build artifacts committed. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.72 covering improved installer recovery, clearer CLI diagnostics, safer inference setup and provider switching, better credential handling, stronger policy boundaries, and more robust runtime repair behavior. * Updated network policy guidance to clarify when `allowed_ips` can be used, including a specific exception for the sandbox-to-host bridge endpoint. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
When
nemoclaw inference setfails because the requested provider is not registered, preserve a bounded, fully redacted failure diagnostic and add the registered provider list plus an onboarding recovery tip. This same-repository replacement preserves #5983's verified contributor history so the mandatory PR Review Advisor can run; fork-origin advisor jobs are skipped by policy.Related Issue
Closes #5924
Supersedes #5983
Changes
openshell inference setoutput and enrich only a quoted provider-not-found diagnostic naming the provider actually requested.inference-set-error.ts, with focused security/correctness tests separate from the already-large action test.Type of Change
Quality Gates
Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesnpm run typecheck:clipassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Exact-head E2E: https://github.com/NVIDIA/NemoClaw/actions/runs/28425898821
Signed-off-by: kagura-agent kagura.agent.ai@gmail.com
Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
nemoclaw onboard.