fix: reuse gateway NVIDIA credential for non-interactive recreate - #5454
Merged
cv merged 4 commits intoJun 17, 2026
Conversation
Non-interactive `onboard --recreate-sandbox` recovers the NVIDIA Endpoints
('build') provider from the existing sandbox's gateway state. The gateway
already holds the validated credential and nothing is written to disk, so the
host process has no local API key. The flow correctly continued past the
credential check (provider exists in gateway) but then ran the endpoint
validation probe unauthenticated, failing at [3/8] with 'endpoint validation
failed'. Skip the re-validation probe when reusing a gateway-only credential,
so recreate proceeds through all stages.
Fixes #5441
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a ChangesConditional smoke testing feature and build credential reuse
Sequence DiagramsequenceDiagram
participant User as User/CI
participant OnboardFlow as onboard.ts<br/>(setupNim)
participant ResolveCredential as resolveNonInteractive<br/>BuildCredential
participant ResolveAPI as resolveBuild<br/>PreferredInferenceApi
participant ValidateEP as validateOpenAiLike<br/>Selection (probe)
participant SetupInf as setupInference
participant VerifySmoke as verifyOnboard<br/>InferenceSmoke
User->>OnboardFlow: Call setupNim (non-interactive "build")
OnboardFlow->>ResolveCredential: Check local key & gateway existence<br/>(recoveredFromSandbox=true)
ResolveCredential->>OnboardFlow: Return reuseGatewayCredentialWithoutLocalKey=true
OnboardFlow->>ResolveAPI: Resolve preferred API with reuse flag
alt reuseGatewayCredentialWithoutLocalKey is true
ResolveAPI->>OnboardFlow: Return "openai-completions"<br/>(skip probe loop)
else reuseGatewayCredentialWithoutLocalKey is false
ResolveAPI->>ValidateEP: Loop probe() until validation succeeds
ValidateEP->>ResolveAPI: Return validated API
end
OnboardFlow->>OnboardFlow: Set skipHostInferenceSmoke=true<br/>(when credential reused)
OnboardFlow->>SetupInf: Call setupInference<br/>with skipHostInferenceSmoke option
alt skipHostInferenceSmoke === true
SetupInf->>SetupInf: Skip smoke verification
else skipHostInferenceSmoke is false
SetupInf->>VerifySmoke: Validate endpoint
VerifySmoke->>SetupInf: Endpoint probe result
end
SetupInf->>User: Return (success or error)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
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 |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/onboard.ts (1)
3638-3669:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCI blocker:
src/lib/onboard.tsgrowth guardrail is failing and will block merge.The new logic is correct for
#5441, but CI currently fails because this file grew past the allowed net budget. Please move this new branch/flag handling into ansrc/lib/onboard/*helper and keepsrc/lib/onboard.tsnet-neutral.As per coding guidelines,
src/lib/onboard.tsis core onboarding logic, and this repo enforces growth guardrails against net growth in this top-level entrypoint.Also applies to: 3910-3916
🤖 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/onboard.ts` around lines 3638 - 3669, The file src/lib/onboard.ts has exceeded its growth budget due to the new NVIDIA API key validation and reuseGatewayCredentialWithoutLocalKey flag handling logic. Extract the branch containing the isNonInteractive() check along with the calls to validateNvidiaApiKeyValue, providerExistsInGateway, and logMissingNvidiaApiKeyHelp, plus the setting of the reuseGatewayCredentialWithoutLocalKey flag, into a new helper function in the src/lib/onboard/ directory. Replace the extracted logic in src/lib/onboard.ts (at lines 3638-3669 and the additional location at lines 3910-3916) with calls to this new helper function, ensuring the helper returns or sets the reuseGatewayCredentialWithoutLocalKey value as needed. This will keep src/lib/onboard.ts net-neutral while preserving the correct functionality for issue `#5441`.Sources: Coding guidelines, Pipeline failures
🧹 Nitpick comments (1)
src/lib/onboard.ts (1)
3346-3943: Run the recommended onboarding E2E slice for this change before merge.Given this is a stage [3/8] provider-selection regression in core onboarding, run at least:
cloud-e2e,sandbox-operations-e2e,rebuild-openclaw-e2e,channels-stop-start-e2e,messaging-compatible-endpoint-e2e,bedrock-runtime-compatible-anthropic-e2e.As per coding guidelines, these are the explicitly recommended suites for onboarding and provider-path regressions.
🤖 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/onboard.ts` around lines 3346 - 3943, Before merging this pull request, you must run the recommended E2E test suites to verify no regressions were introduced in the setupNim function and provider-selection flow. Execute the following E2E test slices: cloud-e2e, sandbox-operations-e2e, rebuild-openclaw-e2e, channels-stop-start-e2e, messaging-compatible-endpoint-e2e, and bedrock-runtime-compatible-anthropic-e2e. These suites are required per coding guidelines for onboarding and provider-path regressions since this change affects stage 3/8 of core provider selection in the setupNim function.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@src/lib/onboard.ts`:
- Around line 3638-3669: The file src/lib/onboard.ts has exceeded its growth
budget due to the new NVIDIA API key validation and
reuseGatewayCredentialWithoutLocalKey flag handling logic. Extract the branch
containing the isNonInteractive() check along with the calls to
validateNvidiaApiKeyValue, providerExistsInGateway, and
logMissingNvidiaApiKeyHelp, plus the setting of the
reuseGatewayCredentialWithoutLocalKey flag, into a new helper function in the
src/lib/onboard/ directory. Replace the extracted logic in src/lib/onboard.ts
(at lines 3638-3669 and the additional location at lines 3910-3916) with calls
to this new helper function, ensuring the helper returns or sets the
reuseGatewayCredentialWithoutLocalKey value as needed. This will keep
src/lib/onboard.ts net-neutral while preserving the correct functionality for
issue `#5441`.
---
Nitpick comments:
In `@src/lib/onboard.ts`:
- Around line 3346-3943: Before merging this pull request, you must run the
recommended E2E test suites to verify no regressions were introduced in the
setupNim function and provider-selection flow. Execute the following E2E test
slices: cloud-e2e, sandbox-operations-e2e, rebuild-openclaw-e2e,
channels-stop-start-e2e, messaging-compatible-endpoint-e2e, and
bedrock-runtime-compatible-anthropic-e2e. These suites are required per coding
guidelines for onboarding and provider-path regressions since this change
affects stage 3/8 of core provider selection in the setupNim function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d29e93b0-da63-4fdf-b3dc-74e5f26604d5
📒 Files selected for processing (2)
src/lib/onboard.tstest/onboard-build-recreate-credential-reuse.test.ts
Contributor
PR Review AdvisorFindings: 0 needs attention, 5 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Consider writing more tests for
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
The #5441 fix added net lines to src/lib/onboard.ts, tripping the codebase growth guardrail (the top-level onboard entrypoint must be net-neutral or smaller; growth is allowed under src/lib/onboard/). Move the non-interactive NVIDIA Endpoints ('build') credential gate and the endpoint-validation/skip decision into src/lib/onboard/build-credential-reuse.ts (resolveNonInteractiveBuildCredential + resolveBuildPreferredInferenceApi). Behavior is unchanged: when the gateway already holds a validated credential and no local key is staged, reuse it and skip endpoint re-validation. The onboard.ts call sites shrink, leaving the file net-smaller than main. Capture the flow-narrowed model into a const so the deferred probe closure keeps the string narrowing the original synchronous call relied on. Refs #5441 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cv
changed the title
Collaborator
|
Addressed the merge conflict and review feedback:
|
Collaborator
|
Correction to previous comment: the net-negative file is src/lib/onboard.ts (+39/-40 in the PR file diff). |
cv
approved these changes
Jun 17, 2026
wscurran
added
NV QA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #5441: non-interactive
nemoclaw onboard --recreate-sandboxaborting at [3/8] with "NVIDIA Endpoints endpoint validation failed". When the NVIDIA Endpoints ("build") provider is recovered from an existing sandbox's saved gateway state, the OpenShell gateway already holds the validated credential and nothing is written to disk, so the host process has no local API key. The credential check insetupNimalready allowed the flow to continue (provider exists in gateway), but the subsequent endpoint validation probe calledgetCredential('NVIDIA_INFERENCE_API_KEY'), got null, and probed the endpoint unauthenticated — which fails and exits at [3/8]. The fix reuses the gateway's already-validated credential and skips the re-validation probe in that branch.Related Issue
Fixes #5441
Changes
src/lib/onboard.ts: when the non-interactive build flow has no local key but the gateway holds the credential, reuse it and skip endpoint re-validation; otherwise probe as before. DefaultspreferredInferenceApitoopenai-completions(consistent with the build provider's normal probe result).src/lib/onboard/build-credential-reuse.ts(new): extractedresolveNonInteractiveBuildCredential(credential gate → reuse flag) andresolveBuildPreferredInferenceApi(skip-or-probe decision) sosrc/lib/onboard.tsstays net-neutral/smaller per the codebase growth guardrail. The growth guardrail allows growth undersrc/lib/onboard/**;onboard.tsis net −5 lines vsmain.logMissingNvidiaApiKeyHelpimport fromonboard.ts(the helper imports its own copy).test/onboard-build-recreate-credential-reuse.test.ts(new): regression guard that drives the exportedsetupNim()in a child process with a fake openshell reporting an nvidia-prod gateway route and no local key; assertssetupNimresolves (exit 0), logs the skip message, selectsnvidia-prod, and never probes/chat/completions.Type of Change
Verification
npx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Verification details reported by Claude Code:
npm run build:cli— clean.npm run typecheck:cli— no new errors insrc/lib/onboard.tsor the new helper.npx vitest run test/onboard-build-recreate-credential-reuse.test.ts test/onboard-inference-smoke.test.ts test/onboard-resume-provider-recovery.test.ts— 25 tests pass.npx biome check --writeon the changed files — no fixes needed.src/lib/onboard.tsis net +32/−37 (−5) vsmain, satisfying thecodebase-growth-guardrailscheck that previously failed.Signed-off-by: Jason Ma jama@nvidia.com
Summary by CodeRabbit
Bug Fixes
New Features
skipHostInferenceSmokein onboarding provider/inference flow to optionally bypass host inference smoke checks.Tests