ci(e2e): identify controller runs by revision - #7862
Conversation
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cjagwani
added
area: ci
📝 WalkthroughWalkthroughThe E2E gate workflow now uses the prerequisite workflow display title for ChangesE2E gate updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit be9f66e in the TypeScript / code-coverage/cliThe overall coverage in commit be9f66e in the Show a code coverage summary of the most impacted files.
Updated |
![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/pr-e2e-gate-workflow.test.ts`:
- Around line 285-291: Strengthen the run-name assertions around
workflow["run-name"] by testing complete observable names for representative
workflow_run and workflow_dispatch inputs, including the expected coordinate,
approval, and identity values. Ensure the tests distinguish trigger branches and
validate the &&/|| precedence through actual evaluated run-name behavior rather
than separate source-fragment toContain checks.
🪄 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: c532e0fa-0726-470b-a6d1-8e5a7daf21cd
📒 Files selected for processing (3)
.github/workflows/pr-e2e-gate.yamltest/e2e/README.mdtest/pr-e2e-gate-workflow.test.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
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. |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/e2e/README.md (2)
677-706: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRestore the manual approval run-name contract in the operator documentation.
The workflow contract includes
PR,head, andbaseidentity forapprove-e2eruns, but this change removes the README’s operator-facing description of that naming. Retain a concise statement in the fork approval section so operators can discover and use those identity-bearing names during diagnosis.🤖 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/e2e/README.md` around lines 677 - 706, Restore a concise operator-facing statement in the fork approval section describing the approve-e2e manual run-name contract, including the PR, head, and base identity fields. Keep the documentation aligned with the existing workflow behavior and place it near the manual approval guidance.
117-121: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winQualify “PR-gate dispatches” as internal/trusted dispatches.
This wording broadly includes fork approval dispatches, but Line 149 says fork-owned runs cannot reach the fallback. Say “trusted internal PR-gate dispatches” or explicitly exclude fork-owned approval dispatches here to avoid an operationally contradictory scope.
🤖 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/e2e/README.md` around lines 117 - 121, Update the “PR-gate dispatches” wording in the README to explicitly mean trusted internal dispatches, excluding fork-owned approval dispatches. Preserve the surrounding description of fallback behavior and runner routing.
🤖 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.
Outside diff comments:
In `@test/e2e/README.md`:
- Around line 677-706: Restore a concise operator-facing statement in the fork
approval section describing the approve-e2e manual run-name contract, including
the PR, head, and base identity fields. Keep the documentation aligned with the
existing workflow behavior and place it near the manual approval guidance.
- Around line 117-121: Update the “PR-gate dispatches” wording in the README to
explicitly mean trusted internal dispatches, excluding fork-owned approval
dispatches. Preserve the surrounding description of fallback behavior and runner
routing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7fb4f26a-6e00-4f57-be8a-2860a7c452af
📒 Files selected for processing (1)
test/e2e/README.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/e2e/README.md (1)
171-171: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign the larger-runner scope wording with the eligible set.
This sentence broadly says the fallback covers Hermes inference-switch executions, while the eligible list limits that mode to Anthropic-compatible inference switching. Clarify the sentence to avoid implying that non-Anthropic modes are routed to the larger runner.
Proposed wording
- the Hermes inference switch and shields, + the Anthropic-compatible Hermes inference-switch mode and shields,Also applies to: 191-192
🤖 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/e2e/README.md` at line 171, Update the larger-runner scope wording in the Hermes execution description to state that inference-switch executions are included only when using Anthropic-compatible inference switching. Keep the eligible-set wording for the Hermes stable MCP shard, common-egress, and channel unchanged, and apply the same clarification to the repeated wording around the later referenced section.
🤖 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.
Outside diff comments:
In `@test/e2e/README.md`:
- Line 171: Update the larger-runner scope wording in the Hermes execution
description to state that inference-switch executions are included only when
using Anthropic-compatible inference switching. Keep the eligible-set wording
for the Hermes stable MCP shard, common-egress, and channel unchanged, and apply
the same clarification to the repeated wording around the later referenced
section.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c457f8b8-118d-4b4a-af5d-cdf5f5ca07cf
📒 Files selected for processing (1)
test/e2e/README.md
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Describe the PR, head, and base identity in automatic and manual controller run names. Refs: #7862 Signed-off-by: Carlos Villela <cvillela@nvidia.com>
apurvvkumaria
left a comment
apurvvkumaria
left a comment
There was a problem hiding this comment.
Approve — reviewed exact head be9f66e. This change only improves controller run identity for workflow_run and manual approval events; it does not alter dispatch, authorization, or gate state. The workflow expression is covered by an exact contract assertion, operator docs match the behavior, and exact-head CI plus E2E gate pass. No blocking correctness, security, compatibility, or regression defect found.
3 participants
Summary
Make every trusted PR E2E controller run identify its exact PR revision in the Actions list. Coordination runs now carry the authenticated source CI display title, while manual approval runs show the supplied PR, head SHA, and base SHA; authorization behavior is unchanged.
Related Issue
Fixes #7860
Changes
workflow_runcontrollers from the exact-revision CI display title, including when fork payloads omit pull-request metadata.approve-e2econtrollers with the supplied PR, head, and base identity.Type of Change
Quality Gates
be9f66ecda1dffeb837a25c8799352aa7f7688dfagainst based5b64a72a5fcb8299e8cec1c2f22746b5c0a7f32(treea28243171958653e0fc9a039024f3b0e46aa8b6c; stable patchb0332efd486aec1e231052d80584402f90079cc5). All nine security categories PASS; run-name-only changes do not alter triggers, predicates, permissions, authentication, dispatch, evidence, or secret handling.Documentation Writer Review
docs-updatedtest/e2e/README.mddocuments automatic controller run names carrying the prerequisite-CI PR, head, and base display identity, and manualapprove-e2erun names showing the supplied PR, head, and base identity. Reviewed all changed workflow text, operator guidance, tests, comments, and commit text against the repository Writing Guide, documentation contract, and controlled terminology. Validation evidence reviewed: focused workflow contract tests passed 13/13;npm run docspassed with 0 errors and 2 existing warnings; exact-headnpm run validate:prpassed.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 exec vitest -- run --project integration test/pr-e2e-gate-workflow.test.ts(13/13)npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not applicable to a workflow naming and contract-test change.npm run docsbuilds without warnings (doc changes only)Additional exact-head validation: focused workflow contract tests passed 13/13;
npm run validate:prpassed;npm run docscompleted with 0 errors and 2 existing warnings.Signed-off-by: Charan Jagwani cjagwani@nvidia.com
Summary by CodeRabbit
Improvements
Documentation
Tests