feat(runtime): persist engine lifecycle recovery - #8058
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 9209d31 in the Show a code coverage summary of the most impacted files.
TypeScript / code-coverage/cliThe overall coverage in commit 9209d31 in the Show a code coverage summary of the most impacted files.
Updated |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence Model lanes
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 warning · 0 suggestionsWarningsWarnings do not block.
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
apurvvkumaria
left a comment
There was a problem hiding this comment.
Comment — reviewed exact head baa495a. Persisted engine lifecycle recovery is safe to merge and I found no blocking defect. Non-blocking fast follow: add a recovery contract for the case where an external mutation succeeds but the callback or receipt publication fails. The mutation-authorized retry can otherwise repeat the operation unless each provider reconciles or resumes idempotently; a narrow follow-up test would make that requirement explicit.
<!-- markdownlint-disable MD041 --> ## Summary This PR adds the dormant Podman GPU-qualification and inference-command translation boundary for the incremental runtime stack in #7744. It does not register Podman for production selection or advertise user-visible Podman support. ## Changes - Add an operation-scoped `host-local-inference` engine contract so runtime providers can request GPU-qualified execution without a Podman switch in central orchestration. Provider contract and source-shape tests protect this boundary. - Record the exact Podman authority and qualified CDI device inventory during preflight. Qualification now preserves the raw inventory through validation and fails closed on both exact and normalization-equivalent duplicate device identities before any deduplication can hide them. - Translate Docker-compatible inference GPU arguments into Podman CDI device arguments. Unit tests cover `all`, indexed GPUs, GPU UUIDs, MIG devices, container-name filters, and fail-closed rejection of unsupported NVIDIA runtime, duplicate, unqualified, or raw-device options. - Keep the new path dormant. Production provider selection, agent behavior, Docker behavior, and Kubernetes behavior do not change in this slice. ## Advisor disposition - The duplicate-CDI fail-closed finding is fixed at exact head `e74e262c2`; the denial test supplies the same canonical CDI identity from both endpoint inventory and the separately qualified inventory and proves qualification rejects it. - The dormant-translator suggestion is intentionally not applied in this slice. Epic #7744 defines this review unit as the inert GPU/translation contract; stacked PR #8061 supplies its named, operation-scoped host-local inference provider consumer. Production activation remains owned by the later qualification slice after every agent, GPU/local inference path, recovery path, and protected E2E pass. Removing the contract here or adding a central Podman switch would defeat that incremental, pluggable stack design. ## 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 - [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 slice adds an internal, dormant provider contract and does not activate or document a supported surface. - [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: Independent exact-head review verified fail-closed CDI identity, endpoint authority, provider-neutral operation scope, and dormant production selection. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Advisor Disposition - Earlier PRA-1, which found duplicate CDI identities lost before normalization, is resolved by d4622df. Endpoint-reported and additional identities now remain in one raw array until normalizePodmanCdiInventory rejects normalized duplicates; the focused test covers both prefixed and shorthand collisions. - Earlier PRA-2, now published as the PRA-1 dormant-consumer suggestion, is explicitly retained for the maintainer-approved incremental stack. Wiring production here would violate the dormant review thesis for this slice and bypass the receipt authority that lands in #8060. Stacked #8061 is the named production-bound provider consumer and adds provider-level contract tests for qualified CDI devices. Current central selection still excludes Podman, and the source-shape test rejects Podman switches in central orchestration. ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: Independent review of the exact 13-file slice at e74e262 against base baa495a, WRITING.md, the controlled word list, and docs/CONTRIBUTING.md found no documentation update required. CURRENT_RUNTIME_PROVIDER_BUNDLES still contains only Docker and Kubernetes, the Podman provider still reports hostLocalInference false, and user-visible Podman support remains inactive. The duplicate-CDI repair collects endpoint and additional device identities before normalization and rejects normalized duplicates. Stable patch ID 681a1e01ee7d6328fef26c359ef7b75b88301c9e and binary diff SHA-256 4fc440a2a439e604553eb24aba928dbd507ec5558cfa8d31383795c49eb4098d bind the receipt. CLI build and typecheck, 97 focused CLI tests, 2 source-shape tests, repository checks, and git diff --check passed. - Agent: Codex Desktop <!-- docs-review-head-sha: e74e262 --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; this slice remains dormant and protected runtime qualification is owned by the activation slice. - 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 validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: At exact head e74e262 on base baa495a, six focused CLI files passed 97 tests; the runtime-provider source-shape file passed 2 tests; npm run build:cli, npm run typecheck:cli, npm run checks:repository, and git diff --check passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Exact-head CI and protected E2E are running. - [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](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: Aaron Erickson <aerickson@nvidia.com> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary This PR adds a provider-neutral durable receipt contract for host-local Ollama, NVIDIA NIM, and vLLM inference. The contract remains dormant and does not activate Podman or any user-visible inference path. ## Changes - Define provider-neutral endpoint and runtime-authority receipt variants for host-process Ollama and managed-container NIM/vLLM services. - Bind every receipt to persisted engine authority, provider identity, exact endpoint/network authority, and immutable runtime proof. - Require digest-pinned OCI images, normalized NVIDIA CDI device inventory, and a secret-free managed-container specification digest. - Add canonical parse and serialization rules that reject authority drift, mutable image references, malformed devices, invalid endpoints, unsupported fields, and noncanonical persisted data. - Demonstrate through an MXC-style fixture that another provider can produce valid receipts without Podman-specific orchestration or central switches. The lifecycle command interface and provider implementation intentionally remain outside this receipt-only slice. They are introduced with their first consumer in stacked PR #8061. ## 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 - [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 slice defines a dormant internal persistence contract and activates no supported runtime or inference surface. - [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: Independent exact-head review verified strict authority normalization, canonical parsing, immutable image/device proof, and the dormant boundary on `4b75eb4af615060a535c9354f17096109fa2fa93`. - [ ] 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: `no-docs-needed` - Evidence: Exact-head review found no user-facing behavior, command, configuration, or supported-runtime activation to document in this receipt-only slice. - Agent: Codex Desktop <!-- docs-review-head-sha: 4b75eb4 --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## 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 validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npm test -- --run src/lib/onboard/runtime-provider/host-local-inference.test.ts test/runtime-provider-source-shape.test.ts` passed 9/9 tests on `4b75eb4af615060a535c9354f17096109fa2fa93`. - [x] Applicable broad gate passed — `npm run build:cli`, `npm run typecheck:cli`, and `npm run checks:repository` passed on the exact head. - [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](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ## Exact Slice Identity - Head: `4b75eb4af615060a535c9354f17096109fa2fa93` - Base: `e74e262c28f514bc9e782b157c2c2549a710ad8c` - Stable patch ID: `1def8b6763adaaf86faec8bc90f60184fbf9dbd2` - Binary diff SHA256: `e03b55f97e2f5f713252106de7f04320353345dd08c9d4464f1664c7e18ea5e3` - Scope: 3 files, 357 additions, 0 deletions ## Advisor Disposition - The exact-head Nemotron lane completed with zero blockers, warnings, or suggestions. - The exact-head Terra lane failed its response-schema check and published no substantive finding; this is an advisor execution failure, not a code finding. - The earlier valid architecture concern about an unconsumed lifecycle interface is resolved on this head: the interface and command DTOs were removed from #8060 and moved to #8061, where the Podman implementation consumes them. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
apurvvkumaria
left a comment
There was a problem hiding this comment.
Comment — reviewed exact head 9209d31 against its stacked base. The provider-neutral lifecycle and new host-local inference authority remain dormant; receipts use closed schemas and digest-pinned images, engine authority is operation-scoped, and requested NVIDIA devices must match the qualified CDI inventory. Focused validation passes 53 of 53 tests. No blocking correctness, security, compatibility, or regression defect found. Non-blocking fast follow: before activating this boundary, normalize host device paths before applying the raw /dev/nvidia rejection, or allowlist the exact non-CDI device forms emitted by supported launchers, so dot-segment aliases cannot bypass the intended CDI-only rule. A narrow follow-up PR covering that translator edge is sufficient; it is not reachable from the current trusted launcher subset and does not need to delay this PR. The branch currently conflicts with its stacked base, so any conflict-resolution revision needs re-review.
cv
left a comment
There was a problem hiding this comment.
Reviewed commit 9209d3180a in bottom-up stack order. This PR depends on #8056, which is not approval-ready. This lifecycle layer conflicts with its current base and has failing image, audit, CI, and E2E evidence. Review remains blocked until the parent is reconstructed or repaired, this branch is restacked on the accepted parent commit, and current CI, security, and documentation evidence passes for the resulting commit. No approval is granted for the current branch.
Summary
Adds a dormant, provider-neutral lifecycle ledger that binds snapshot, clone, rebuild, backup, restore, and recovery mutations to the exact persisted engine authority from the preceding slice. It records restart-safe phases, one exclusive process-owned mutation lease, target-position-bound commands, and completion receipts without selecting a provider, activating Podman, or interpreting provider-owned acceleration state.
Related Issue
Part of #7744.
Changes
prepared,mutation-authorized, andcompletedphases before publishing an exact retirement tombstone that prevents transaction-ID reuse.src/lib/onboard/managed-bootstrap/README.md.The lifecycle ledger is required by #7744 so later provider slices can recover mutation authority without process-global engine state. A direct orchestration switch would bypass the pluggable provider boundary.
persisted-engine-lifecycle.test.tsprotects the contract.Type of Change
Quality Gates
Documentation Writer Review
docs-updatedsrc/lib/onboard/managed-bootstrap/README.mdagainstWRITING.mdanddocs/CONTRIBUTING.md. The text accurately documents the provider-neutral ledger, restart lease, exact runtime-target authority, retirement tombstone, and dormant activation boundary. Exact-head CLI build, typecheck, 29/29 focused lifecycle tests, 2/2 source-boundary tests, repository checks, Biome, markdownlint, andgit diff --checkpassed.DGX 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 unavailablenpm run build:cliandnpm run typecheck:clipassed;npx vitest run --project cli src/lib/onboard/runtime-provider/persisted-engine-authority.test.ts src/lib/onboard/runtime-provider/persisted-engine-lifecycle.test.tspassed 29/29;npx vitest run --project integration test/runtime-provider-source-shape.test.tspassed 2/2;npm run checks:repository, the test-size and test-conditional scans, Biome,git diff --check, exact-slice commitlint, and the pre-push hooks passed at exact head52ecd0b76.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Aaron Erickson aerickson@nvidia.com