test(ci): require lifecycle publisher paths - #8772
Conversation
|
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)
📝 WalkthroughWalkthroughThe base-image publication E2E test now includes managed-image activation tests and helper files in its expected reviewed path families. ChangesBase-image validation
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit db0240b in the TypeScript / code-coverage/cliThe overall coverage in commit db0240b 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
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: None This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
<!-- markdownlint-disable MD041 --> ## Summary Installer host preflight adopted canonical readiness admission in #8738 but hardcoded storage remediation off. Managed Docker overlayfs/containerd hosts therefore exited before onboarding could build the documented `fuse-overlayfs` replacement image. This change derives the waiver from canonical gateway lifecycle authority, keeps external, invalid, and non-remediable states blocked, and reports bounded admission IDs when preflight fails. The first manual PR E2E run then exposed a separate parser mismatch after installer preflight: the generic GPU host reported valid NVIDIA driver `595.84`, while the detector required exactly three numeric components. The bounded follow-up accepts two- or three-component NVIDIA versions without changing the minimum, resolver, schema, or supported surface. ## Changes - Allow the installer storage exception only when the canonical gateway-management declaration is valid and absent or explicitly `nemoclaw-managed`; normal onboarding revalidates that authority before lifecycle effects. - Print deduplicated finding and capability IDs that satisfy the readiness schema's stable-ID grammar and length limit. - Cover absent and explicit managed authority, external supervision, invalid authority, non-remediable storage, onboarding continuation, and unsafe diagnostic input in installer integration tests. - Accept bounded two- or three-component NVIDIA driver versions while retaining numeric comparison, identical multi-GPU inventory, and fail-closed malformed-input behavior. ## 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 restores the documented managed-only storage remediation path and accepts a valid NVIDIA driver form above the existing documented `580.65.06` minimum. It adds no command, configuration, schema, minimum, or support claim. - [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: an independent Codex Desktop review of exact head `4d78d7355` on base `acda7aa22` passed all nine security categories with no findings. It verified canonical authority parsing and revalidation, managed-only lifecycle effects, fail-closed external/invalid/non-remediable states, sanitized admission IDs, and strict bounded driver parsing. - [ ] 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: `docs/get-started/prerequisites.mdx`, `docs/reference/system-readiness.mdx`, and `docs/deployment/gateway-lifecycle-authority.mdx` already document managed storage remediation and external lifecycle authority. `docs/inference/set-up-llama-cpp.mdx` already requires NVIDIA driver version `580.65.06` or later; accepting valid two- or three-component versions restores that contract. The generic Linux preset remains explicit-only and experimental. - Agent: Codex Desktop <!-- docs-review-head-sha: 4d78d73 --> <!-- docs-review-agents-blob-sha: fca0e02 --> ## 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: - `npm exec -- vitest run --project installer-integration test/install-preflight.test.ts --maxWorkers=4` — exact reviewed patch passed 98/99; the unrelated five-second release-tag fixture timed out, then passed alone 1/1. The six changed readiness cases passed, and the byte-identical pre-merge candidate passed 99/99 under the same worker limit. - `npm exec -- vitest run --project cli src/lib/inference/nim-driver-version.test.ts src/lib/inference/llama-cpp/managed-selection.test.ts src/lib/inference/serving/resolver.test.ts` — 49/49 passed. - `npm exec -- vitest run --project e2e-support test/e2e/support/base-image-publication.test.ts` — 39/39 passed after merging upstream repairs #8771 and #8772. - `npm run test:changed` — passed; the clean checkout selected no CLI, plugin, or E2E-support source tests and the integrated growth contract passed 4/4. - `npm run validate:pr` — passed all pre-commit, commit-message, and pre-push checks at exact head. - The requested `npm exec -- vitest run --project integration test/install-preflight.test.ts` reports no matching tests because project membership assigns this file to `installer-integration`; the owning lane is recorded above. - Ordinary manual PR E2E [run 31444706504](https://github.com/NVIDIA/NemoClaw/actions/runs/31444706504) passed six of the seven installer-path jobs at the prior head. `llama-cpp-generic-gpu` passed installer preflight and exposed the two-component driver parser mismatch fixed here. Exact-head E2E is pending. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: not applicable to these bounded installer-admission and driver-parser changes; targeted installer and CLI suites plus required CI are the applicable evidence. - [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: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Installer preflight now limits storage remediation to supported, explicitly managed gateway configurations. - Host-admission failures provide validated, deduplicated finding and capability identifiers alongside existing diagnostics. - Installer onboarding behavior and failure reporting are now more consistent across gateway lifecycle states. - NVIDIA driver detection more reliably handles repeated version output and rejects malformed or contaminated results. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
Require both literal managed lifecycle files in the base-image publisher path contract.
This preserves the main-only lifecycle trigger added by merged PR #8771.
Changes
test/e2e/live/managed-image-activation-e2e.test.tstriggers the main publisher.test/e2e/live/managed-image-activation-e2e-helpers.tstriggers the main publisher.Type of Change
Quality Gates
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 unavailablenpx vitest run --project e2e-support test/e2e/support/base-image-publication.test.tspassed 39 tests.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: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit