Repository navigation
Conversation
… failing job The Maintainer Approval job failed on every review comment on an unapproved PR, emailing the commenter each time. Split it so the review-triggered workflow only signals re-evaluation (skipping comment reviews and never failing), and a workflow_run workflow running main's copy posts the required `OpenShell / Maintainer Approval` commit status. Commit status failures send no email, a push leaves the new head pending, and merge-group runs get a passing status. Closes NVIDIA#4559 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Jim Meyer <jimeyer@nvidia.com>
|
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. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+10
to
+13
| on: | ||
| workflow_run: | ||
| workflows: [Maintainer Approval] | ||
| types: [completed] |
Collaborator
Author
|
If ever you wanted to see a kludged together approach, this is it. 🤢 |
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
2 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
The maintainer approval gate fails, and emails the commenter, on every review comment on an unapproved PR. This moves the gate from the job's own result to a commit status, so a missing approval never fails a workflow run. As the issue explains, skipping comment reviews alone is unsafe because a skipped required check counts as passing.
Related Issue
Closes #4559
Changes
maintainer-approval.ymlnow only asks for the gate to be checked again. It skipscommentedreviews and never fails. Its job is renamedMaintainer Approval Trigger, so it is no longer the required check. The run name carries the PR number.maintainer-approval-status.yml(workflow_run) runsmain's copy withstatuses: write, fork PRs included. It evaluates approval with the unchangedcheck_maintainer_approval.pyand posts theOpenShell / Maintainer Approvalcommit status on the PR head.head_sha. Otherwise it posts nothing.CONTRIBUTING.mdnames the commit status as the context to add to ruleset13332227, and notes that the decision now runsmain's copy.Testing
mise run test:maintainer-approvaldid not run.check_maintainer_approval.pyis unchanged. I checked the run-name regex by hand.workflow_runworkflows only run from the default branch, so the status workflow can only be checked end to end after merge. Before adding the context to the ruleset, confirm on a live PR that:Checklist
🤖 Generated with Claude Code