fix(inference): create missing compatible provider - #7746
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
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:
📝 WalkthroughWalkthroughThe inference-set flow now supports direct provider bindings, recognizes generic OpenShell lookup misses, retries route configuration after registering absent providers, and updates rollback handling. Tests cover endpoint validation and credential isolation, while documentation describes compatible-endpoint setup and retry behavior. ChangesCompatible provider inference flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant runInferenceSet
participant finalizeInferenceSetRoute
participant OpenShell
runInferenceSet->>finalizeInferenceSetRoute: resolve endpoint and provider bindings
finalizeInferenceSetRoute-->>runInferenceSet: return directProviderBinding or httpsPinProviderBinding
runInferenceSet->>OpenShell: set inference route
OpenShell-->>runInferenceSet: provider not found
runInferenceSet->>OpenShell: create provider and retry inference set
OpenShell-->>runInferenceSet: route selection succeeds
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 2c31c1a in the TypeScript / code-coverage/cliThe overall coverage in commit 2c31c1a in the Show a code coverage summary of the most impacted files.
Updated |
|
🌿 Preview your docs: https://nvidia-preview-pr-7746.docs.buildwithfern.com/nemoclaw |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/actions/inference-set-route-containment.ts (1)
427-450: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDirect
process.envread bypasses the adapters layer.
credentialValueis now read straight fromprocess.env[httpsPinCredentialEnv]inside this actions-layer module, rather than through an injected adapter (contrast withoptions.ensureHttpsPinRuntimeAdapter, which is properly dependency-injected). This couples the direct-binding path to the global process environment, making it harder to fake in tests and mixing a host-boundary read into orchestration code.Consider threading credential resolution through an injected function (e.g., alongside
ensureHttpsPinRuntimeAdapter) so actions compose rather than touchprocess.envdirectly.Based on path instructions: "Host-boundary calls (e.g., OpenShell interactions, process/network/file boundaries) should be isolated behind
src/lib/adapters/**so actions/tests can inject fakes... avoid creating new cross-layer dependencies (actions should compose, not call host boundaries directly)."🤖 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-route-containment.ts` around lines 427 - 450, Replace the direct process.env access in the finalize flow with an injected credential-resolution function on options, alongside ensureHttpsPinRuntimeAdapter. Use that adapter to resolve the credential for httpsPinCredentialEnv while preserving the existing empty-value fallback and provider binding behavior; keep host-environment access isolated under the adapters layer.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 876-923: Update the flow around setInferenceRoute and
prepareInferenceSetProviderBinding to converge direct provider bindings before
the first inference:set call, including updating an existing compatible provider
when its endpoint changes. Preserve the existing provider-state validation and
commit behavior, while retaining retry handling for provider-not-found failures.
Add a regression test covering an existing provider whose endpoint is changed
via --endpoint-url.
---
Nitpick comments:
In `@src/lib/actions/inference-set-route-containment.ts`:
- Around line 427-450: Replace the direct process.env access in the finalize
flow with an injected credential-resolution function on options, alongside
ensureHttpsPinRuntimeAdapter. Use that adapter to resolve the credential for
httpsPinCredentialEnv while preserving the existing empty-value fallback and
provider binding behavior; keep host-environment access isolated under the
adapters layer.
🪄 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: be2972af-2dd9-4c45-8dd1-7c161445667b
📒 Files selected for processing (6)
docs/inference/switch-providers.mdxsrc/lib/actions/inference-set-compatible-provider.test.tssrc/lib/actions/inference-set-provider.test.tssrc/lib/actions/inference-set-provider.tssrc/lib/actions/inference-set-route-containment.tssrc/lib/actions/inference-set.ts
PR Review Advisor — Blocking findings reportedAdvisor assessment: Blockers require maintainer review Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
Blockers
|
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary This follow-up prevents the behavior reported by PR Review Advisor on #7746: OpenShell verified an existing direct provider's old endpoint before NemoClaw replaced it with the requested endpoint. NemoClaw now reuses only the exact direct binding recorded for the sandbox and refuses endpoint replacement that cannot be rolled back. ## Related Issue Follow-up to #7746 (`PRA-1`); related to #7725. ## Changes - Compare an existing direct compatible-provider binding with the sandbox registry before route selection. - Reuse the provider without mutation when its recorded endpoint and credential environment match the request. - Refuse replacement when the requested direct binding differs because OpenShell redacts the previous endpoint required for rollback. - Verify newly created direct providers during route selection and remove them when verification fails. - Add regressions for verified creation, failed-selection cleanup, existing-binding refusal, and exact recorded-binding reuse. - Document the direct-provider restriction while preserving the DNS-backed HTTPS Pin Runtime update and rollback guidance. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [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: Codex Desktop exact-head review at `e8a9f8b65` passed all nine categories with no findings. The change fails before an unverifiable direct-provider mutation; endpoint validation, invocation-local credential handling, mutation locking, and HTTPS Pin Runtime rollback remain intact. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/inference/switch-providers.mdx`; verified direct-provider refusal and exact-binding reuse, absent-provider verification and cleanup, and the unchanged DNS-backed HTTPS Pin Runtime update and rollback guidance. The focused inference tests passed 76/76, and `npm run docs` passed with 0 errors and 2 pre-existing warnings. - Agent: Codex Desktop <!-- docs-review-head-sha: e8a9f8b --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable - Station profile/scenario: Not applicable - Result: Not applicable - Supporting evidence: Not applicable ## Verification - [x] PR description includes a `Signed-off-by:` line 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 — `npx vitest run --project cli src/lib/actions/inference-set-compatible-provider.test.ts src/lib/actions/inference-set-degraded-state.test.ts src/lib/actions/inference-set-provider-alias.test.ts src/lib/actions/inference-set-provider.test.ts src/lib/actions/inference-set-gateway-route-containment.test.ts`: 76 passed - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — not applicable to this isolated inference correction - [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) — passed with 0 errors and 2 pre-existing warnings - [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) --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved compatible-provider switching by allowing model-only changes when the recorded endpoint remains valid. * Added safeguards requiring re-onboarding when changing an existing provider’s endpoint. * Enhanced recovery guidance for failed endpoint updates, including restoration attempts and onboarding requirements. * **Bug Fixes** * Prevented unnecessary provider updates when endpoint details already match. * Improved cleanup and rollback behavior when route verification fails. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical dated changelog entry for NemoClaw v0.0.97 before the release plan captures `origin/main`. The entry groups the user-visible and maintainer-facing changes since v0.0.96 while preserving the Deferred dual-Station status, experimental runtime-identity boundary, and pending physical IGX validation. ## Changes - Add `docs/changelog/2026-07-28.mdx` with the parser-safe MDX SPDX comment and exact `## v0.0.97` heading. - Summarize the 43 merged PRs in the release range, omitting internal-only changes from the public entry and linking each grouped change to its most specific published documentation. - Keep the experimental Okta reference explicitly opt-in and outside normal onboarding, keep the two-Station path Deferred, and state that physical IGX Orin validation remains pending. ### Source summary - [#7440](#7440), [#7443](#7443), and [#7445](#7445) -> `docs/changelog/2026-07-28.mdx`: Document read-only host readiness reports and fail-closed platform qualification. - [#7030](#7030) -> `docs/changelog/2026-07-28.mdx`: Document the Deferred trusted two-Station vLLM evaluation. - [#7265](#7265) -> `docs/changelog/2026-07-28.mdx`: Document the bounded experimental direct-runner Okta runtime-identity reference. - [#7711](#7711) and [#7648](#7648) -> `docs/changelog/2026-07-28.mdx`: Document compatible-endpoint reasoning effort and retired NVIDIA Build model paths. - [#7746](#7746), [#7763](#7763), and [#7681](#7681) -> `docs/changelog/2026-07-28.mdx`: Document safe compatible-provider creation, replacement refusal, and narrow OpenShell bridge URL handling. - [#7641](#7641), [#7690](#7690), [#7631](#7631), and [#7710](#7710) -> `docs/changelog/2026-07-28.mdx`: Document paused-container recovery, recreation journaling, pre-mutation uninstall checks, and source-checkout OpenShell selection. - [#7624](#7624) and [#7762](#7762) -> `docs/changelog/2026-07-28.mdx`: Document Jetson release diagnostics and bounded render-device group propagation. - [#7639](#7639), [#7760](#7760), [#7721](#7721), and [#7761](#7761) -> `docs/changelog/2026-07-28.mdx`: Document Telegram, MCP media-type, Hermes image-mode, and locked-restart fixes. - [#7653](#7653) and [#7680](#7680) -> `docs/changelog/2026-07-28.mdx`: Document Deep Agents policy tasks and the bounded Claude Code OAuth path. - [#7679](#7679) -> `docs/changelog/2026-07-28.mdx`: Document the checksum-bound libssh2 and Python HTMLParser backports. - [#7655](#7655), [#7651](#7651), [#7664](#7664), [#7666](#7666), [#7670](#7670), [#7719](#7719), and [#7741](#7741) -> `docs/changelog/2026-07-28.mdx`: Document exact candidate E2E evidence, Launchable selection, diagnostic consolidation, and trusted WSL validation. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated changelog contract, MDX header, heading uniqueness, and release-entry structure. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: The committed `docs/changelog/2026-07-28.mdx` blob exactly matches the reviewed file. Completeness, factual accuracy, link shape, parser-safe MDX header, one-sentence-per-line style, `.docs-skip` compliance, and bounded product claims passed. - Agent: Codex Desktop documentation writer subagent <!-- docs-review-head-sha: da6aa27 --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; this PR changes only the dated changelog. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## Verification - [x] PR description includes a `Signed-off-by:` line 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 — `npx vitest run test/changelog-docs.test.ts` passed 6/6. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — not applicable to this doc-only release entry. - [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) — completed with 0 errors and 2 pre-existing Fern warnings. - [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) — native changelog entries use the required parser-safe MDX SPDX comment and intentionally have no frontmatter. --- Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added improved host readiness reporting and Jetson onboarding guidance. * Added controls for reasoning effort with compatible endpoints and enhanced managed MCP discovery. * Improved Deep Agents task publication and preset support. * **Bug Fixes** * Hardened provider switching, sandbox recovery, uninstall behavior, and Telegram connectivity. * Improved container image integrity checks, media-type handling, and checksum validation. * Enhanced vLLM evaluation behavior and release diagnostics. * **Documentation** * Added the NemoClaw v0.0.97 changelog. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Fix
nemoclaw inference setwhen a direct compatible provider is absent or its endpoint changes. NemoClaw now inspects the provider binding before route selection, creates and verifies an absent provider before the first route attempt, and commits a verified update for an existing provider after route selection so failures can restore the prior selection.Related Issue
Fixes #7725
Changes
provider getmissing-entity response as an absent provider only inside the exact-name provider lookup.The compatibility path is required by #7725 for the
inference setconsumer running against OpenShell 0.0.85. A global missing-provider parser would be unsafe because route-update output may identify a different provider; exact-name lookup and product-level route tests protect the scoped behavior.Type of Change
Quality Gates
2c31c1a99passed all nine categories with no findings; endpoint SSRF validation, exact-name provider parsing, host-only credential handling, provider ownership verification, rollback behavior, and mutation locking remain intact, and the final test-timeout-only commit changes no production pathDocumentation Writer Review
docs-updateddocs/inference/switch-providers.mdx; verified pre-selection provider creation and verification, correlated one-time retry, created-provider rollback, existing-provider update ordering, DNS-backed HTTPS ordering, generated agent variants, and writing rules at exact head2c31c1a99. The docs build passed with 0 errors and 2 pre-existing warnings; the automatic main merge preserved the reviewed behavior, the empty E2E-retrigger commit is tree-identical to that reviewed head, the post-merge inference suite passed 177/177, and the timeout-only follow-up changed no user-facing contract, its file passed 33/33, and the affected test passed three consecutive focused runs.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm run test:changed: 169 passed; post-merge inference suite: 177 passed; workflow-boundary file: 33 passed; exact affected test: 3 consecutive focused passes;npm run typecheck:cli, repository checks, source-shape checks, and test-condition scan passednpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — not applicable to this scoped inference fix and timeout-budget correctionnpm run docsbuilds without warnings (doc changes only) — passed with 0 errors and 2 pre-existing warningsSigned-off-by: Prekshi Vyas prekshiv@nvidia.com
Summary by CodeRabbit