Repository navigation
fix(onboard): record managed vLLM serving-profile provenance - #12764
Merged
Merged
Conversation
Preserve catalog-derived serving-profile provenance after a managed vLLM profile is selected. Fresh onboard now carries the selected profile through session, registry, and status. Fixes #12289 Agent-Id: agent-c5819b3a-f2af-4bee-a591-193fc379dcf4
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe requested serving-profile model now includes catalog provenance. Managed vLLM onboarding records that provenance in the session, registry entry, and sandbox status report. Reused running vLLM servers do not receive managed-profile provenance. ChangesServing-profile provenance
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant Setup as createSetupNim
participant Model as servingProfileModel
participant Session as Onboard session
participant Registry as Sandbox registry
participant Status as Sandbox status report
Setup->>Model: Resolve requested profile and provenance
Setup->>Session: Record provenance for managed vLLM install
Session->>Registry: Supply provenance for registry entry
Registry->>Status: Provide provenance for status report
Suggested reviewers: Merge Risk: ⚪ Minimal · up to No confirmed implementation issue blocks merging; the unresolved human-status documentation detail should be checked separately. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 5 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall line coverage in commit 032b321 in the Show a line coverage summary of the most impacted files.
TypeScript / code-coverage/cliThe overall line coverage in commit 032b321 in the Show a line coverage summary of the most impacted files.
Updated |
rsliter
marked this pull request as ready for review
October 7, 2026 17:15
Collaborator
Author
|
/ok to test 1b4c9c2 |
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-12764.docs.buildwithfern.com/nemoclaw |
Collaborator
Author
|
/ok to test 032b321 |
This was referenced Oct 7, 2026
Contributor
|
PR Review Advisor finished for commit Request review only when Require no Advisor blockers is green. |
deepujain
approved these changes
Oct 8, 2026
deepujain
left a comment
deepujain
left a comment
Collaborator
There was a problem hiding this comment.
LGTM.
Catalog provenance reaches durable sandbox state only after this invocation installs and validates managed vLLM. The earlier Advisor warning about reused-server status is fixed in the documentation; no inline comments follow.
Advisor feedback: 1 of 1 addressed.
Recommended E2E: inference-routing, network-policy, onboard-repair, and onboard-resume have not run on this commit.
Tested: the session-to-registry regression reproduces on the base and passes here; endpoint reuse, vLLM resume, direct caller and catalog tests, CLI/plugin builds, and docs pass. Three local status/resume cases, PR validation, and CLI typechecking were environment-blocked. Same-commit CI and the trusted checker pass.
rsliter
added a commit
that referenced
this pull request
Oct 8, 2026
## Outcome CLI shards and the Advisor runtime now install pinned Pi search tools from one Ubuntu-only APT transaction. CLI metadata refresh has up to three 120-second attempts, repeated only after an outer timeout; CLI install has a 180-second limit. Advisor retains a single five-minute limit per command. Failures report the operation, attempt, exit status, and at most 8 KiB from the last 60 diagnostic lines. The CLI package and executable version checks remain in place. ## Reason Five CLI shards in the October 7 runs for #12763 and #12764 timed out in the quiet pinned-tool step before tests. Those logs do not show whether update or install stalled. Main scopes update to `ubuntu.sources` but leaves install able to use ambient package lists. The closed, unmerged #11319 helper addressed source and list isolation for the September repository failure; it did not bound or identify a stalled APT command. ### Related issues Fixes #11320. ## Changes - Place the installer at `.github/actions/ci-install-pinned-ubuntu-packages.sh` as a shared CI helper. The CLI shard and Advisor runtime call it; the trusted CLI shard sparse checkout includes it. - Use only `/etc/apt/sources.list.d/ubuntu.sources` and a fresh package-list directory for both update and install. Reject a missing source and unpinned package specifications before APT. - Preserve #12713's CLI transport retries and 120-second update and 180-second install limits. Retry the idempotent metadata update only after an outer timeout, up to three attempts; fail closed on other statuses. Restore the runner temporary directory mode on exit. - Run `sudo timeout` so it directly supervises `apt-get`, including forced termination. Execute both workflow callers with fake APT commands to cover source isolation, update and install failures, both timeout paths, and fail-closed input checks. - Cover both workflow callers on the successful path, restore the runner temporary-directory mode in the fixture, and label exit status 137 as a possible force-killed timeout. ## Verification - `npx vitest run test/automation/pull-requests/pr-workflow-contract.test.ts test/automation/pull-requests/growth-guardrails.test.ts --coverage.enabled false` passed: 74 tests on the latest repair, including recovered, exhausted, and non-timeout CLI update paths and forced-kill supervision. - `npm run checks:repository` passed: 18 checks. Commit and pre-push hooks passed on the latest repair, including ShellCheck, source-shape, growth, secret scan, commitlint, publication validation, and CLI TypeScript checking. - The repository project-membership check passed: 2706 candidates across 7 projects. - `bash -n .github/actions/ci-cli-coverage-shard/install-pinned-ubuntu-packages.sh` and Oxfmt checks passed. - A local sparse checkout using the CLI shard action and shared-helper paths included the executable helper. - GitHub-hosted CLI shard execution remains required to prove affected shards complete their tests. PR CI uses the base branch's trusted action until this fix merges. - The diff contains no secrets, API keys, or credentials. ## Review notes This changes sensitive `.github/**` paths. The local candidate received self-review and the focused checks above; no independent pre-publication review exists. The September #11319 PR is closed and unmerged. This PR is the separate October repair and does not change #12763 or #12764. --- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Standardized pinned system package installation across automated checks, with bounded timeouts and retries for transient update failures. * Updated pull request checks to include the shared package installer when validating the revised CI action. * **Tests** * Expanded automated coverage for package installation failures, timeouts, and input validation, helping detect build environment issues earlier. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
wscurran
added
area: providers
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
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.
Outcome
Fresh onboarding with an explicit managed vLLM profile preserves its catalog provenance in the session, sandbox registry, and status report. Reusing an existing vLLM endpoint does not claim that NemoClaw installed its recipe or image.
Original author: @harjothkhara (Harjoth Khara). The original signed commit
1d9f58d3713ac2ea52e958ad59c46ea0e18b33ddis preserved unchanged. Rebecca Sliter supplied the managed-install guard and maintainer follow-up.Reason
Provider selection previously erased valid profile provenance before sandbox registration. This same-repository replacement also enables the required reviewed OpenShell SDK CI path, which rejects fork PRs.
Related issues
Fixes #12289. Replaces #12346 with the contributor's history intact.
Changes
Verification
Candidate:
032b321a4ee88141198fe41eedb767d4c5d49907.npx vitest run --project cli src/lib/onboard/setup-nim-flow-serving-profile.test.ts src/lib/onboard/machine/handlers/provider-inference-managed-llama-resume.test.ts src/lib/inference/serving/requested-profile-model.test.ts src/lib/onboard/setup-nim-flow-vllm-resume.test.ts --no-file-parallelism: 4 files and 26 tests passed. The existing-endpoint regression failed before the guard.npm run docs: passed with zero errors; OpenClaw, Hermes, and Deep Agents generated command references inspected. Fern reported the existing optional authenticated redirect check and light-theme contrast warnings.Review notes
The five changed code and test files are sensitive inference or onboarding paths. The additional command-reference clarification received independent local documentation review. The maintainer review on the original PR covers
NVIDIA/NemoClawcommit1b4c9c2c2c09f70488884370236bd3f826e4300f, records the provenance repair and regression evidence, and thanks the original contributor. The current candidate awaits its required CI, GPU qualification, and automated reviews.Signed-off-by: harjoth harjoth.khara@gmail.com
Signed-off-by: Rebecca Sliter 571084+rsliter@users.noreply.github.com
Summary by CodeRabbit