Skip to content

fix(e2e): separate PR verdict from controller status#6819

Closed
cjagwani wants to merge 2 commits into
mainfrom
codex/fix-pr-gate-status-reporting
Closed

fix(e2e): separate PR verdict from controller status#6819
cjagwani wants to merge 2 commits into
mainfrom
codex/fix-pr-gate-status-reporting

Conversation

@cjagwani

@cjagwani cjagwani commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

GitHub attaches the trusted workflow_run controller to the default-branch workflow SHA. Previously, expected negative PR outcomes updated the exact-diff E2E / PR Gate check to failure and then threw, so the controller job on main also went red. This change lets the controller succeed after it reliably publishes a failed PR-scoped verdict, while malformed state, provenance mismatches, and API or check-publication failures still fail the controller.

Related Issue

Part of #6145

Changes

  • Treat failed prerequisite CI and failed or incomplete E2E evidence as completed PR-gate verdicts instead of controller exceptions.
  • Fail obsolete exact-diff checks without dispatching when a PR closes or its head or base changes, including replacing an obsolete completed success.
  • Validate workflow conclusions and exact-diff check states before using GitHub data, preserving fail-closed behavior for malformed or untrusted controller state.
  • Cover failed CI, failed and unfinished children, stale revisions, closed PRs, malformed states, and controller-error boundaries in the gate tests and maintainer E2E documentation.

Type of Change

  • 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

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification:
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Focused security review covered exact-diff identity and state validation, stale completed-success replacement, child-run provenance, and fail-closed API/publication errors; no findings remain.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npx vitest run --project integration test/pr-e2e-gate.test.ts test/pr-e2e-gate-lifecycle.test.ts test/pr-e2e-gate-exceptions.test.ts test/pr-e2e-gate-workflow.test.ts (96 passed)
  • Applicable broad gate passed — not applicable; the change is isolated to the PR E2E controller and all four controller suites passed.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Charan Jagwani cjagwani@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved E2E gate handling when pull requests are closed, retargeted, or updated during validation.
    • Prevented stale CI/E2E results from mutating newer revisions, and ensured obsolete gates are closed as failure with clearer details.
    • Added stricter validation for workflow status/conclusion combinations and exact head/base matches.
  • Tests
    • Expanded lifecycle tests for superseded, malformed, and invalid workflow evidence scenarios, updating expectations for error/patch behavior.
  • Documentation
    • Clarified PR gate initialization and the expected pass/fail contract for pending or unsuccessful validation outcomes.

@cjagwani cjagwani self-assigned this Jul 14, 2026
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 3fc45668-a039-4c5e-bb55-fb03c8ceacdc

📥 Commits

Reviewing files that changed from the base of the PR and between 7181bab and ff1e757.

📒 Files selected for processing (1)
  • test/pr-e2e-gate-lifecycle.test.ts

📝 Walkthrough

Walkthrough

The E2E PR gate validates workflow conclusions, PR snapshots, and exact-diff check states. Closed or retargeted PRs are treated as obsolete, affected checks are failed, stale E2E dispatches are avoided, and lifecycle tests and documentation cover the updated behavior.

Changes

Exact-diff E2E gate

Layer / File(s) Summary
Workflow and PR validation contracts
tools/e2e/pr-e2e-gate.mts
Adds strict workflow-result and exact-check-state validation, constrains PR state values, and separates PR snapshot validation.
Start-gate obsolescence handling
tools/e2e/pr-e2e-gate.mts
Validates CI and existing checks, detects closed or changed PR revisions, and records obsolete CI results without dispatching E2E.
Finish-gate obsolescence handling
tools/e2e/pr-e2e-gate.mts
Validates E2E results and PR snapshots, closes obsolete checks as failures, and completes current checks from evidence.
Lifecycle tests and documentation
test/pr-e2e-gate*.test.ts, test/e2e/README.md
Expands coverage for superseded events, malformed states, PR closure, prerequisite failures, workflow outcomes, and finalization behavior; documents the exact-diff contract.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant WorkflowRun
  participant PRGateController
  participant GitHubChecks
  participant E2EWorkflow
  WorkflowRun->>PRGateController: Start with CI result and revision
  PRGateController->>GitHubChecks: Validate PR snapshot and exact-diff check
  PRGateController->>E2EWorkflow: Dispatch E2E for current revision
  E2EWorkflow->>PRGateController: Return workflow status and conclusion
  PRGateController->>GitHubChecks: Complete exact-diff check
Loading

Suggested labels: area: e2e, bug-fix, area: docs

Suggested reviewers: cv

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main change: separating PR-scoped verdicts from controller failure status in E2E gating.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-pr-gate-status-reporting

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the branch.

TypeScript / code-coverage/cli

The overall coverage in the branch remains at 79%, unchanged from the branch.

Show a code coverage summary of the most impacted files.
File b634f01 ff1e757 +/-
src/lib/name-validation.ts 100% 94% -6%
src/lib/runner.ts 75% 72% -3%
src/lib/securit...ntial-filter.ts 99% 98% -1%
src/lib/security/redact.ts 95% 99% +4%
src/lib/actions...ge-preflight.ts 74% 89% +15%

Updated July 14, 2026 01:24 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / high confidence
Primary next action: No advisor follow-up required beyond maintainer review.
Findings: 0 blockers · 0 warnings · 0 optional suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (non-blocking second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

Nemotron is a non-blocking second opinion. Its prose, findings, and E2E guidance do not change the primary assessment above and remain in workflow artifacts only.

E2E guidance

Advisory only: coverage and selector recommendations are non-authoritative. E2E / PR Gate independently computes and dispatches trusted jobs without consuming this output.

Recommended coverage: cloud-onboard, credential-sanitization, security-posture
Recommended selectors: cloud-onboard, credential-sanitization, security-posture

  • cloud-onboard — Selected from the trusted checked-in E2E coverage inventory.

  • credential-sanitization — Selected from the trusted checked-in E2E coverage inventory.

  • security-posture — Selected from the trusted checked-in E2E coverage inventory.

  • cloud-onboard — Selected as a trusted checked-in E2E job.

  • credential-sanitization — Selected as a trusted checked-in E2E job.

  • security-posture — Selected as a trusted checked-in E2E job.

Workflow run details

This is an automated, non-authoritative review. Findings are inputs to maintainer adjudication. Warnings and optional suggestions do not require a response or follow-up. A human maintainer makes the final merge decision.

@cjagwani cjagwani closed this Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant