fix(e2e): remove internal approval stop - #7857
Conversation
jyaunches
added
the
area: security
|
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 changes PR E2E coordination so internal revisions dispatch selected plans automatically, while maintainer approval applies only to fork revisions. It updates recovery behavior, risk-plan helpers, tests, and documentation to reflect the revised gate state machine. ChangesPR E2E dispatch and approval flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PRGate
participant GitHub
participant E2EWorkflow
PRGate->>GitHub: Read PR identity and selected targets
PRGate->>E2EWorkflow: Dispatch internal PR plan
PRGate->>GitHub: Create fork approval state
GitHub->>PRGate: Return approved fork revision
PRGate->>E2EWorkflow: Dispatch reviewed fork commit
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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/maintainer-skills-policy.test.ts (1)
344-359: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winCover the remaining dispatch guardrails in the contract test.
The assertions cover authorization and several security phrases, but not the required input review, duplicate-run avoidance, SHA/plan-change handling, or reporting of the dispatched plan, SHAs, and workflow URL. Add assertions for those clauses so future policy edits cannot silently remove them.
The PR objective explicitly calls for retained security checks; as per path instructions, this test should preserve behavioral confidence at the policy boundary.
🤖 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/maintainer-skills-policy.test.ts` around lines 344 - 359, Add assertions in the maintainer policy contract test near the existing followUp and mergeGate checks to cover required input review, duplicate-run avoidance, SHA and plan-change handling, and reporting the dispatched plan, SHAs, and workflow URL. Assert the exact policy clauses or stable identifying phrases from the relevant generated text so these dispatch guardrails remain enforced.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/maintainer-skills-policy.test.ts`:
- Around line 344-359: Add assertions in the maintainer policy contract test
near the existing followUp and mergeGate checks to cover required input review,
duplicate-run avoidance, SHA and plan-change handling, and reporting the
dispatched plan, SHAs, and workflow URL. Assert the exact policy clauses or
stable identifying phrases from the relevant generated text so these dispatch
guardrails remain enforced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b732fd19-944f-470b-9cd9-e82d6016936f
📒 Files selected for processing (3)
.agents/skills/_shared/pr-follow-up.md.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.mdtest/maintainer-skills-policy.test.ts
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 36ca8c5 in the TypeScript / code-coverage/cliThe overall coverage in commit 36ca8c5 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — InformationalAdvisor assessment: Informational / low 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. |
jyaunches
changed the title
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
1 participant
Summary
Internal PRs now automatically dispatch their deterministic credential-bearing E2E plan after
eligible PR CI passes. Fork PRs retain maintainer approval for the reviewed PR and base SHAs.
Changes
Maintainer approval required to run E2E.approve-e2efor same-repository PRs and remove the obsolete internal authorizationclassifier and tests.
approve-e2egate and its maintainer, SHA, plan, coordination,live-state, and evidence validation.
the complete selected plan. The test proves that the fixed commit gets a fresh plan and one
dispatch for
cloud-inference,cloud-onboard, andsecurity-posture; it neither reruns onlythe previously failed job nor accepts evidence from the prior commit.
.github/workflows/pr-e2e-gate.yamlbehavior unchanged. Its existingworkflow_runcompletedtrigger observes every newCI / Pull Requestcompletion, and the coordinatoralready passes each eligible run's PR commit into
startPrGate. The missing behavior was in thecontroller's same-repository decision after that trigger, not in workflow event routing.
exact headandexact revisionterminology with plain PR vocabulary.Type of Change
Quality Gates
Documentation Writer Review
docs-updated36ca8c506, confirmed zeroskill-file changes, automatic complete-plan dispatch after a fixed PR commit passes CI,
fork-only
approve-e2ewith mechanical SHA bindings, and zero scopedexact headorexact revisionterminology matches. No findings remain.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