fix(e2e): redispatch complete plans after fixes - #7867
Closed
jyaunches wants to merge 2 commits into
Closed
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR updates PR E2E gate authorization and dispatch semantics, removes internal control-plane credential authorization, restricts ChangesPR E2E gate authorization and dispatch
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PRGate
participant GitHubChecks
participant E2EWorkflow
PRGate->>GitHubChecks: validate PR CI and coordination state
PRGate->>E2EWorkflow: dispatch selected internal or approved fork plan
E2EWorkflow-->>GitHubChecks: report E2E check status
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 039f7dd in the TypeScript / code-coverage/cliThe overall coverage in commit 039f7dd in the Show a code coverage summary of the most impacted files.
Updated |
Contributor
PR Review Advisor — Blocking findings reportedAdvisor assessment: Blockers require maintainer review 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: Blockers
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/pr-e2e-gate-retry-history.test.ts (1)
234-251: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the dispatch payload, not just the dispatch count.
toHaveLength(1)passes even if the controller dispatched for the wrong checkout SHA, base SHA, or job set — exactly the identity guarantees this gate is supposed to enforce.test/pr-e2e-gate-remediation.test.ts(Lines 300-309) already checks the body; mirroring that here keeps the "dispatches internal E2E" claim meaningful.♻️ Suggested strengthening
await expect(startPrGate(startCommand(workDir))).resolves.toBeUndefined(); - expect(requests.filter((request) => request.url.endsWith("/dispatches"))).toHaveLength(1); + const dispatches = requests.filter((request) => request.url.endsWith("/dispatches")); + expect(dispatches).toHaveLength(1); + expect(dispatches[0]?.body).toMatchObject({ + inputs: { checkout_sha: HEAD_SHA, base_sha: BASE_SHA }, + });As per path instructions, prefer observable outcomes through the public boundary over mock-call assertions that can pass without exercising the claim.
🤖 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-retry-history.test.ts` around lines 234 - 251, Strengthen the dispatch assertion in the startPrGate test around startPrGate and the /dispatches request by inspecting the captured request body, not only requests count. Mirror the payload validation used in test/pr-e2e-gate-remediation.test.ts to verify the dispatched checkout SHA, base SHA, and expected job set through the public request boundary.Source: Path instructions
🤖 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.
Nitpick comments:
In `@test/pr-e2e-gate-retry-history.test.ts`:
- Around line 234-251: Strengthen the dispatch assertion in the startPrGate test
around startPrGate and the /dispatches request by inspecting the captured
request body, not only requests count. Mirror the payload validation used in
test/pr-e2e-gate-remediation.test.ts to verify the dispatched checkout SHA, base
SHA, and expected job set through the public request boundary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 212cfa86-1ca9-4a4a-b453-9b15e9853173
📒 Files selected for processing (15)
.github/workflows/e2e.yamltest/e2e/README.mdtest/pr-e2e-gate-command.test.tstest/pr-e2e-gate-dispatch-recovery.test.tstest/pr-e2e-gate-fork-approval.test.tstest/pr-e2e-gate-lifecycle.test.tstest/pr-e2e-gate-remediation.test.tstest/pr-e2e-gate-retry-history.test.tstest/pr-e2e-gate-typed-target.test.tstest/pr-e2e-required.test.tstest/pr-risk-plan.test.tstools/advisors/risk-plan.mtstools/e2e/live-vitest-invocation.mtstools/e2e/pr-e2e-gate.mtstools/e2e/trusted-hermes-swap-workflow-boundary.mts
💤 Files with no reviewable changes (2)
- tools/advisors/risk-plan.mts
- test/pr-risk-plan.test.ts
23 tasks
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
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
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
After a same-repository PR fixes a failed E2E result, the next eligible ordinary CI completion now
rebuilds and immediately dispatches the complete selected E2E plan for the new PR commit. Fork PRs
retain their existing maintainer approval boundary.
Changes
every internal selected plan through the existing automatic dispatch path.
.github/workflows/pr-e2e-gate.yamlbehavior unchanged. Its existing completedworkflow_runtrigger observes each newCI / Pull Requestcompletion and passes every eligiblerun's PR commit to the controller, so the missing behavior was the controller decision after the
trigger rather than workflow event routing.
approve-e2efor fork PRs only, including maintainer-role, PR/base SHA, deterministic-plan,live-state, child-run, and evidence validation.
commit, processes its successful CI completion, and asserts one dispatch containing
cloud-inference,cloud-onboard, andsecurity-posture. The test does not accept old-commitevidence or rerun only the formerly failed test.
exact head,exact-head, andexact revisionterminology with plain PRvocabulary while retaining SHA fields required for checkout and evidence integrity.
and maintainer skill-policy tests are unchanged.
Type of Change
Quality Gates
Documentation Writer Review
docs-updatedfork-only
approve-e2e, zero skill or maintainer-skill-policy changes, and zero scopedexact head,exact-head, orexact revisionmatches. No findings or additional documentationedits were needed.
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 unavailablenpm run docs, andgit diff --checkpassed.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: Julie Yaunches jyaunches@nvidia.com