fix(e2e): reuse CLI artifact for failed-job retries - #8478
Conversation
|
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:
📝 WalkthroughWalkthroughThe E2E retry flow reruns failed jobs and dependents, reuses the successful producer artifact, and validates producer and consumer workflow attempts during restoration. Tests, workflow pins, and documentation cover the updated behavior. ChangesE2E retry behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant E2EMainRetry
participant FailedJobRerun
participant RestoreAction
participant ArtifactProvenance
E2EMainRetry->>FailedJobRerun: Request failed-job and dependent rerun
FailedJobRerun->>RestoreAction: Restore CLI artifact
RestoreAction->>ArtifactProvenance: Validate run IDs, attempts, and payload digest
ArtifactProvenance-->>RestoreAction: Return validated producer artifact
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 1ca71dc in the TypeScript / code-coverage/cliThe overall coverage in commit 1ca71dc 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
4 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
1 additional E2E selection 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. Since last review: 0 prior items resolved · 0 still apply · 0 new items found 3 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 against this exact revision. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
![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/e2e/support/cli-artifact-workflow-boundary.test.ts`:
- Around line 410-413: Update the test around runIdentityValidation to exercise
the complete artifact-reuse boundary, including payload verification, manifest
attempt matching, and file restoration through the public workflow. Ensure the
scenario validates the immutable producer artifact during the later failed-job
rerun; otherwise rename the test to describe identity validation only.
🪄 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: cd31d970-62c5-4c6c-822f-04327ce33473
📒 Files selected for processing (6)
.github/actions/restore-e2e-cli-artifact/action.yamltest/e2e/README.mdtest/e2e/docs/README.mdtest/e2e/support/cli-artifact-workflow-boundary.test.tstest/e2e/support/main-run-retry.test.tstools/e2e/cli-artifact-workflow-boundary.mts
cv
changed the title
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Addressed the artifact-reuse review request in The test now carries the validated producer attempt into manifest verification and file restoration during a later failed-job rerun. It also proves that a producer-attempt value that disagrees with the artifact manifest fails before activation. CodeRabbit has marked the thread resolved. Validation completed on the current branch revision:
Nine-category security review: PASS, no findings.
The earlier amd64 and arm64 image-job failures were infrastructure failures: both runners received |
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/e2e/support/cli-artifact-workflow-boundary.test.ts`:
- Line 423: Replace the raw identity.stderr assertion message in the identity
status check with fixed text or a bounded, sanitized diagnostic, and apply the
same change to the corresponding fixture.output assertion around the other
referenced location. Ensure failure messages never expose child process output,
credentials, or tokens while retaining concise debugging context.
🪄 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: 222252f8-376b-4383-99ae-76f6ba5a763d
📒 Files selected for processing (1)
test/e2e/support/cli-artifact-workflow-boundary.test.ts
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Security-review addendum for the current branch revision:
Required GitHub Actions checks are still queued during the service outage, and human approval is still missing, so the PR remains blocked from merge. |
|
Broad-gate validation update for the current branch revision:
The broad-gate checkbox remains intentionally open until trusted CI completes. This PR also still requires independent approval; there is no focused PR-related failure to fix at this point. |
apurvvkumaria
left a comment
apurvvkumaria
left a comment
There was a problem hiding this comment.
Approve — the current branch revision preserves failed-job reruns while reusing only an earlier successful CLI artifact from the same workflow run and reviewed workflow revision. Producer/consumer attempt ordering, workflow and candidate identity, immutable artifact identity, archive and payload digests, manifest matching, safe extraction, and restored-file checks remain fail closed. Both actionable review findings are resolved, the focused E2E-support suite passes 49/49, fixed diagnostics cannot expose child output, the documentation and nine-category security reviews are current, and all contributor attribution, DCO, and verification evidence is preserved. Trusted hosted gates remain required before merge.
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Current branch revision review receipt for b401641:
Fresh CI, automated review, current independent approval, and all repository gates remain required before merge. No waiver or bypass is requested. |
3 participants
Summary
Failed-job reruns now reuse the successful producer's immutable CLI artifact from an earlier attempt of the same workflow run. This avoids repeating successful E2E jobs while preserving candidate, workflow, run, artifact, and payload provenance.
Changes
E2E / Main Retryon GitHub's failed-job rerun endpoint, which reruns failed jobs and their dependent jobs.Type of Change
Quality Gates
Documentation Writer Review
docs-updatedtest/e2e/README.mdandtest/e2e/docs/README.mdagainst the retry controller, restore action, identity-to-restore fixture, manifest-attempt mismatch coverage, and focused tests. The internal E2E guidance accurately covers failed-job reruns, producer-artifact reuse and retention, same-run identity and attempt ordering, manifest verification, and fail-closed restoration. The content-free assertion diagnostics are test-only. No user-facing Fern page is affected.DGX Station Hardware Evidence
scripts/prepare-dgx-station-host.shis unchanged.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/main-run-retry.test.ts test/e2e/support/cli-artifact-workflow-boundary.test.tspassed 49 tests. Repository project-membership, test-title, source-shape, pre-commit, commit-message, and pre-push checks 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: San Dang sdang@nvidia.com
Signed-off-by: Carlos Villela cvillela@nvidia.com
Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
Bug Fixes
Documentation