fix(onboard): explain unavailable providers - #8901
Conversation
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cjagwani
added
the
area: onboarding
|
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:
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesProvider selection guidance
Managed image QA provenance
Readiness and deployment verification
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 340b830 in the TypeScript / code-coverage/cliThe overall coverage in commit 340b830 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
3 additional E2E selections from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite for the commit under review. Recommended E2E: Manual-only E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
|
CI follow-up on exact head |
prekshivyas
left a comment
prekshivyas
left a comment
There was a problem hiding this comment.
Reviewed the unavailable-provider remediation flow. The vLLM guidance is correctly gated on the resolved install-vllm menu option, call sites pass the menu keys through, and the focused coverage exercises both managed and unsupported hosts. No blocking findings.
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/managed-image-publication-workflow.test.ts`:
- Line 656: Add regression assertions near the staging QA job validation to
verify its environment does not include STAGING_PRODUCER_SHA or
STAGING_QA_RECORDED_INDEX_DIGEST, while preserving the existing
STAGING_QA_SOURCE_SHA assertion; do not add a runtime absent-variable test.
🪄 Autofix
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: 476080e3-4433-4a53-94be-2eb35fdd97cf
📒 Files selected for processing (2)
.github/workflows/managed-images.yamltest/managed-image-publication-workflow.test.ts
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/verify-deployment.test.ts (1)
580-603: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the final 30-second retry.
This test becomes healthy at
elapsedMs === 60_000. It would still pass if the new30_000entry were removed fromDEFAULT_RETRY_DELAYS_MS. Add a deterministic case that succeeds only at90_000and assertsresult.healthyand elapsed time. Keep the assertion at the publicverifyDeployment()boundary instead of asserting internal retry calls.As per path instructions, tests must provide behavioral confidence through observable outcomes rather than implementation lock-in.
🤖 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/verify-deployment.test.ts` around lines 580 - 603, Extend the retry behavior tests around verifyDeployment to add a deterministic scenario where the deployment becomes healthy only at elapsedMs === 90_000, then assert result.healthy is true and elapsedMs equals 90_000. Keep the setup and assertions at the public verifyDeployment boundary, without asserting internal retry calls.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.
Nitpick comments:
In `@src/lib/verify-deployment.test.ts`:
- Around line 580-603: Extend the retry behavior tests around verifyDeployment
to add a deterministic scenario where the deployment becomes healthy only at
elapsedMs === 90_000, then assert result.healthy is true and elapsedMs equals
90_000. Keep the setup and assertions at the public verifyDeployment boundary,
without asserting internal retry calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6eee46f3-b203-4ce7-842c-723b5f615948
📒 Files selected for processing (3)
src/lib/onboard.tssrc/lib/verify-deployment.test.tssrc/lib/verify-deployment.ts
💤 Files with no reviewable changes (1)
- src/lib/onboard.ts
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
|
Exact-head CI disposition ( |
|
Babysit update for exact head This matches the shared OpenClaw cold-start runner signature already reproduced outside this branch, with no branch-specific failure evidence. I am not issuing another failed-only retry because that mode does not recreate producer artifacts; attempt 4 demonstrated that it yields an invalid missing-contract run. The approved exact head remains unchanged. |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
Follow-up review fix published at exact head Validation: owning managed-image workflow suite 19/19 passed; Biome passed; normal pre-commit and pre-push hooks passed, including repository checks, secret scan, CLI typecheck, and version sync. Documentation writer review: |
|
Final babysit classification for exact head This is now the third coherent reproduction across two unrelated exact heads (#8901 at |
4 participants
Summary
Unavailable explicit providers now explain how to recover instead of ending at a generic dead end. For
NEMOCLAW_PROVIDER=vllm, onboarding distinguishes attachment to an already-running server from managed installation and recommendsinstall-vllmonly when that option is available on the host.Related Issue
Fixes #8799
Changes
vllmrequires an already-running local server and route eligible hosts to the existinginstall-vllmpath.Type of Change
Quality Gates
vllmattachment from the managedinstall-vllmpath; the CI-only staging-fixture correction and internal readiness timing adjustment do not change a user workflow or configuration.Documentation Writer Review
no-docs-neededDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailable63c09434: 42 focused deployment-verification tests and 19 managed-image workflow tests passed; normal pre-push TypeScript hook passed.npm run checks:repositorypassed after merging currentmain.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Charan Jagwani cjagwani@nvidia.com
Summary by CodeRabbit