fix(e2e): preserve managed-image producer attempts - #10451
Closed
prekshivyas wants to merge 1 commit into
Closed
Conversation
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall line coverage in commit 8df1bd2 in the TypeScript / code-coverage/cliThe overall line coverage in commit 8df1bd2 in the Show a line coverage summary of the most impacted files.
Updated |
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 (4)
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review. 📝 WalkthroughWalkthroughThe change adds producer workflow run metadata to managed-image cohort records. Validation now checks run IDs, positive attempts, attempt ordering, and SLSA builder identities. End-to-end fixtures and documentation cover earlier, mismatched, and newer producer attempts. ChangesProvenance validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change preserves producer-attempt identity and adds validation and regression coverage; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant ManagedImagesWorkflow
participant CohortManifest
participant jqValidation
participant SLSAProvenance
ManagedImagesWorkflow->>CohortManifest: stage candidate run metadata
ManagedImagesWorkflow->>jqValidation: provide repository, run ID, and attempt
jqValidation->>SLSAProvenance: validate builder identity for producer attempt
SLSAProvenance-->>jqValidation: accept or reject provenance
jqValidation-->>ManagedImagesWorkflow: validation result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Collaborator
Author
|
Superseded by #10450. The consolidated PR now includes the per-platform producer run contract, exact SLSA builder-attempt binding, bounded mixed-attempt validation, documentation, and regression tests. |
prekshivyas
added a commit
that referenced
this pull request
Sep 8, 2026
<!-- markdownlint-disable MD041 --> ## Outcome Managed-image cohort validation now accepts immutable cohort and platform evidence retained from earlier attempts of the selected workflow run. It still rejects evidence from another run, a future attempt, or a SLSA builder that does not match the platform producer. ## Reason GitHub failed-job reruns can retain successful producer jobs from an earlier attempt. Requiring every platform producer and immutable cohort alias to equal the selected workflow attempt rejected that valid state. A rerun of every job still executes the publication-identity job and mints a fresh cohort alias. ### Related issues - Refs #10113. - Completes the remaining producer-provenance scope from #10451. - Relates to #10736, which now owns the stable PR artifact and failed-job rerun selection contract on `main`. - Relates to #10452. Its Hermes lifecycle correction is already present on `main`, so this PR no longer carries a Hermes lifecycle diff. ## Changes - Preserve each platform producer's `{id, attempt}` in the final cohort contract. - Accept a cohort or producer attempt only when it belongs to the selected run and is no newer than the selected publication attempt. - Bind every SLSA builder ID to the exact platform producer attempt in both workflow-side and E2E-side validation. - Emit the validated immutable cohort identity in the downstream receipt. - Execute the publication-identity workflow step to prove rerun-all mints `ghrun-<run>-<attempt>`. - Execute the cohort validator entry point and verify its complete `GITHUB_OUTPUT` handoff. - Keep identity coverage in the workflow owner and split its promotion boundary into a focused test file. - Validate normal and mixed-attempt promoted cohorts through the E2E consumer. - Reject a foreign producer run before publication and compare pointer order with the last cohort staging call. - Model registry bytes by exact immutable reference and mutable alias, and reject a pointer command that reports success while leaving stale alias bytes. The current consumer is the managed-image receipt boundary used by downstream E2E jobs. The cohort validator tests protect retained, future, mixed-provenance, incomplete-agent, architecture, revision, and immutable-base cases. ## Verification - `npm run validate:pr` — passed against `09a52be945ed699fe186e4ff4baac7cdd7a5cc8b`. - `npx vitest run --project e2e-support test/e2e/support/managed-image-cohort-contract.test.ts` — 17 tests passed. - `npx vitest run --project integration test/inference/managed/managed-image-publication-workflow.test.ts test/inference/managed/managed-image-publication-promotion.test.ts` — 39 tests passed. - `npx vitest run --project e2e-support test/e2e/support/pr-managed-image-publication.test.ts` — 25 tests passed for the overlapping `main` rerun contract. - `npx vitest run --project integration test/automation/pull-requests/growth-guardrails.test.ts` — 45 tests passed. - Pre-commit and pre-push hooks — passed, including YAML, repository, growth, secret-scan, and CLI TypeScript gates. - GitHub commit verification — `1d3c879bf9120251b79f750ae473a65316f2ffeb` is Verified; all 30 PR commits are Verified. - No secrets, API keys, or credentials are present in the diff. ## Review notes - Security review: PASS. The change narrows the managed-image trust boundary with exact repository, run, attempt, digest, base-reference, and SLSA builder bindings. It adds no credential flow, privilege, network destination, dependency, or mutable external input. - CodeRabbit's identity-execution, final-stage ordering, and foreign-run rejection findings are addressed in the workflow and promotion tests. - `cjagwani` confirmed that the immutable cohort-alias blocker is resolved and requested a current-base refresh for `E2E / PR Gate`. The refresh is present. Current `main` removed that controller in #8625 and its merge-gate tests classify the former check as advisory, so GitHub cannot schedule that retired check. - The [PR Review Advisor run for `94062e5`](https://github.com/NVIDIA/NemoClaw/actions/runs/34016671873) completed all nine specialists. Commit `1d3c879` closes its sole finding by making the Docker substitute reference-aware and proving that the production byte comparison rejects a stale pointer alias. The other eight specialists found no issue. - Documentation review: `no-docs-needed`. The final diff changes an internal CI/E2E provenance contract and tests, with no user-facing command, configuration, or supported workflow change. --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Managed image publications now support multi-platform cohort staging and promotion with run and attempt metadata. - Publication contracts include provenance details, including builder identities and producing workflow attempts. - Workflow reruns can retrieve and validate contracts across attempts using stable run identifiers. - **Bug Fixes** - Improved validation rejects stale, mismatched, cross-run, or newer publication candidates. - Promotion now preserves and restores image references reliably, including digest and cohort aliases. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
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
1 participant
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
Preserve each managed-image cohort entry's producer run attempt instead of replacing it with the selected publication attempt. This allows a retried cohort publication to reuse successful platform artifacts from earlier attempts while still binding every SLSA provenance builder ID to the exact producing workflow attempt.
Changes
runidentity into the final cohort platform entry.Type of Change
Quality Gates
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 unavailablenpx vitest run --project e2e-support test/e2e/support/managed-image-cohort-contract.test.ts(12 passed)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: Prekshi Vyas prekshiv@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests