fix(ci): reopen completed PR E2E checks - #7054
Merged
Merged
Conversation
Clear the prior terminal conclusion before restoring exact-diff authorization. Fixes #7052 Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
added
area: ci
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
|
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 PR gate controller adds versioned retry markers, validates immutable exact-diff history, reopens eligible infrastructure failures, and preserves terminal outcomes. Required-gate logic, tests, and documentation cover coordination selection, evidence failures, and fork approval recovery. ChangesPR E2E gate retry flow
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant PRCI
participant PRGateController
participant GitHubChecks
participant RequiredGate
PRCI->>PRGateController: Start with CI conclusion and exact-diff history
PRGateController->>GitHubChecks: Validate or create exact-diff check
PRGateController->>GitHubChecks: Complete with retry marker when eligible
PRCI->>PRGateController: Run again for unchanged head and base
PRGateController->>GitHubChecks: Create fresh in_progress check
RequiredGate->>GitHubChecks: Select current coordination check
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage remains at 80%, unchanged from the Updated |
Contributor
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
21 tasks
Record versioned retry reasons only for conclusively retryable infrastructure failures. Keep product, evidence-integrity, reconciliation, controller, and unknown outcomes terminal. Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@tools/e2e/pr-e2e-gate.mts`:
- Around line 2519-2533: Update the outer fallback completion attempt in the
surrounding evidence-download error flow to pass retryableFailureReason:
"evidence-download" as well. Preserve this reason when the first
completeFailureAfterControllerError call fails, while keeping the existing
childRunUrl and error-completion behavior unchanged.
🪄 Autofix (Beta)
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: c8226670-8d7f-4941-8d81-0f6033f51e3a
📒 Files selected for processing (6)
.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.mdtest/e2e/README.mdtest/pr-e2e-gate-fork-skip.test.tstest/pr-e2e-gate-lifecycle.test.tstest/pr-e2e-gate.test.tstools/e2e/pr-e2e-gate.mts
Treat completed checks as immutable. Create a new exact-diff check only after a trusted, marker-backed infrastructure retry. Fail closed on suspicious duplicate history and validate each check mutation. Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Collaborator
Author
|
@coderabbitai review |
Carry the evidence-download reason through the outer completion fallback. Add a regression for a failed first completion update. Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/pr-e2e-gate-lifecycle.test.ts (1)
78-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider deduplicating
prGateMutationResponseacross sibling test files.This helper is byte-for-byte duplicated in
test/pr-e2e-gate-fork-skip.test.tsandtest/pr-e2e-gate.test.ts. Extracting it into the sharedtest/support/github-fetch-router.ts(or a new small support module) would avoid drift across the three files as the mock contract evolves.♻️ Suggested extraction
-function prGateMutationResponse(request: RecordedGitHubRequest, id = 17): Response { - return githubResponse( - exactPrGateCheck({ - id, - ...(request.body as Record<string, unknown> | undefined), - }), - ); -} +import { prGateMutationResponse } from "./support/pr-gate-mocks";🤖 Prompt for 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. In `@test/pr-e2e-gate-lifecycle.test.ts` around lines 78 - 85, Extract the duplicated prGateMutationResponse helper into shared test support, preferably test/support/github-fetch-router.ts, and remove the local copies from pr-e2e-gate-lifecycle.test.ts, pr-e2e-gate-fork-skip.test.ts, and pr-e2e-gate.test.ts. Update all three tests to import and reuse the shared helper while preserving its request-body handling and default id behavior.
🤖 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 `@tools/e2e/pr-e2e-required.mts`:
- Around line 226-229: Update matchingChecks() to paginate through all check-run
responses before filtering and validating coordination history, rather than
relying on the first per_page=100 page. Preserve the existing
GITHUB_ACTIONS_APP_ID filter and currentCoordinationCheck() selection only after
the complete matching history has been collected.
---
Nitpick comments:
In `@test/pr-e2e-gate-lifecycle.test.ts`:
- Around line 78-85: Extract the duplicated prGateMutationResponse helper into
shared test support, preferably test/support/github-fetch-router.ts, and remove
the local copies from pr-e2e-gate-lifecycle.test.ts,
pr-e2e-gate-fork-skip.test.ts, and pr-e2e-gate.test.ts. Update all three tests
to import and reuse the shared helper while preserving its request-body handling
and default id behavior.
🪄 Autofix (Beta)
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: 65ef4b12-6cc7-4702-a4c4-aa7924742f36
📒 Files selected for processing (9)
.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.mdtest/e2e/README.mdtest/pr-e2e-gate-fork-skip.test.tstest/pr-e2e-gate-lifecycle.test.tstest/pr-e2e-gate-retry-history.test.tstest/pr-e2e-gate.test.tstest/pr-e2e-required.test.tstools/e2e/pr-e2e-gate.mtstools/e2e/pr-e2e-required.mts
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/e2e/pr-e2e-gate.mts
apurvvkumaria
marked this pull request as ready for review
July 17, 2026 00:10
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
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 @.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md:
- Line 36: Update the prerequisite-ci recovery wording in MERGE-GATE.md to state
the sequence explicitly: first rerun eligible CI for the unchanged open head and
base when prerequisite-ci is recorded, then allow the controller to retry only
after that new CI run succeeds. Preserve the existing sequencing and behavior
for child-cancelled and evidence-download reasons.
🪄 Autofix (Beta)
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: b4c6b674-41be-4d9b-8e27-b597c178144a
📒 Files selected for processing (4)
.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.mdtest/e2e/README.mdtest/pr-e2e-required.test.tstools/e2e/pr-e2e-required.mts
🚧 Files skipped from review as they are similar to previous changes (2)
- tools/e2e/pr-e2e-required.mts
- test/e2e/README.md
Co-authored-by: Carlos Villela <cvillela@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
enabled auto-merge (squash)
July 17, 2026 01:19
Collaborator
|
Final exact-revision handoff for independent human review:
@jyaunches @ericksoa @cjagwani — please make the independent review and merge decision when satisfied. I have not approved or merged this PR. |
Collaborator
Author
|
Ready for human review at exact head
Human approval is the only remaining gate; no merge was performed. |
21 tasks
cv
approved these changes
Jul 17, 2026
21 tasks
21 tasks
apurvvkumaria
pushed a commit
that referenced
this pull request
Jul 17, 2026
<!-- markdownlint-disable MD041 --> ## Summary Restore the trusted PR E2E controller after #7054 by omitting the invalid `conclusion: null` field when updating an active GitHub check. GitHub currently rejects that request with HTTP 422 before control-plane authorization or E2E dispatch can occur. ## Changes - Omit `conclusion` from in-progress check-run PATCH requests while retaining strict validation that GitHub returns `conclusion === null` for the persisted active check. - Assert the outbound field is absent across initial authorization, restored authorization, and authorized dispatch transitions. - Preserve exact PR/head/base identity, trusted-app validation, maintainer authorization, immutable history, and final stale-revision checks unchanged. The escaped defect was caused by treating GitHub's response representation as a valid request representation. Existing mocks spread the request into a successful response, `toMatchObject` allowed the extra request key, and one test explicitly required the invalid null field. Live controller run https://github.com/NVIDIA/NemoClaw/actions/runs/29549179465 reproduced the API rejection after all ordinary CI passed. This prerequisite cannot satisfy its own required `E2E / PR Gate Coordination` check: trusted workflow-run evaluation checks out the current `main` controller, which contains the defect being repaired. After ordinary CI and automated review pass, an independent maintainer must review this exact commit and explicitly accept that one check as a manual merge exception. Subsequent release PRs will then refresh on the repaired base and produce normal exact-head E2E evidence. ## 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 only corrects an internal GitHub check-run request body; observable maintainer guidance, CLI/configuration, product behavior, and release notes do not change. Independent docs-writer review found no update necessary. - [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 nine-category security review of exact commit `a76af0b9c2d15486c0f5b6f869a802e3bb9ed631` returned PASS/GO with no findings; outbound null removal does not alter strict inbound identity, authorization, TOCTOU, or fail-closed validation. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: Pending independent maintainer review of the expected `E2E / PR Gate Coordination` self-bootstrap failure described above. Do not merge before ordinary CI and automated review are otherwise green. ## 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 check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/pr-e2e-gate*.test.ts test/pr-e2e-required.test.ts` passed 149/149 tests across 9 files; the focused regression failed before the fix with received `conclusion: null` and passed afterward. Source-shape, test-size, Biome, CLI build, and CLI typecheck also passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not run; the focused controller suite and normal hooks cover this three-file request-shape fix. - [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: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated E2E check-run progress updates to omit an unnecessary conclusion value while checks are in progress. * Improved handling of retryable authorization states after incompatible changes. * **Tests** * Expanded coverage to verify check-run updates use the correct in-progress payload. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Carlos Villela <cvillela@nvidia.com>
21 tasks
23 tasks
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
3 participants
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
Allow an unchanged exact PR diff to recover from conclusively retryable E2E infrastructure failures without mutating completed checks or reopening product, evidence-integrity, reconciliation, controller, or unknown terminal failures. The controller records a versioned machine-readable retry reason and, after a later trusted ordinary-CI run succeeds, creates a fresh validated check for the same exact-diff identity.
Related Issue
Fixes #7052
Changes
prerequisite-ci,child-cancelled, andevidence-download.evidence-downloadretry reason when a failed completion update falls through to the outer completion fallback.Type of Change
Quality Gates
7bd36fdabb5f255cf3237295ed273db6d41874f0adds the CodeRabbit-requested fallback-marker preservation with an adversarial 503 regression. An independent documentation review also corrected stale-base, fresh-check, retry-history, and native-versus-coordination guidance. Keep this draft until automated reviewers are clean.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run test/pr-e2e-gate*.test.ts test/pr-e2e-required.test.ts(148/148 passed);npm run build:cli;npm run typecheck:cli;npm run test:titles:check; andnpx prek run --filesacross all changed files passed, including source-shape and test-size budgets.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: The focused controller suite and all changed-file hooks passed; exact-head GitHub CI for7bd36fdabb5f255cf3237295ed273db6d41874f0is running.npm run docsbuilds without warnings (doc changes only) — not applicable to the maintainer merge-gate reference and E2E runbook; both passed changed-file markdown checks.Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit