fix(ci): keep Ready blocked while deterministic hygiene fails - #1329
Merged
Conversation
Stop the PR quality gate from marking review-ready when hygiene still fails (as on #1324), and re-run the gate when sponsorship or exception labels change.
Contributor
|
✅ Deterministic PR hygiene checks passed. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis change unifies deterministic hygiene and sponsored-surface checks. Both PR workflows retrieve changed files and labels, apply shared failure metadata, and enforce draft and readiness behavior. Tests cover label triggers, pagination, blocked restricted changes, and maintainer sponsorship. ChangesHygiene gate integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant enforce-pr-target
participant GitHubAPI
participant HygieneCollector
participant PullRequest
enforce-pr-target->>GitHubAPI: Read changed files and labels
GitHubAPI-->>enforce-pr-target: Return PR file list and labels
enforce-pr-target->>HygieneCollector: Evaluate files, labels, and author permission
HygieneCollector-->>enforce-pr-target: Return hygiene failures
enforce-pr-target->>PullRequest: Update comment, status, labels, and readiness
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/scripts/pr-hygiene.cjs:
- Around line 269-272: Update assessHygiene’s assessSponsoredSurface call to
pass a deduplicated union of each changed file’s filename and previous_filename,
preserving source paths for renames. Add a regression test covering a rename
from a restricted path, such as .github/workflows/release.yml, to a
non-restricted path and verify sponsorship is still required.
In @.github/workflows/pr-hygiene.yml:
- Around line 42-47: Update the checkout step in the workflow so
pull_request_target events check out github.event.pull_request.base.sha instead
of the repository default branch. Keep the existing require of pr-hygiene.cjs
and its collectDeterministicHygieneFailures usage unchanged, ensuring the script
is loaded from the trusted PR base revision.
- Around line 110-116: The authorHasPushPermission value passed to
collectDeterministicHygieneFailures must use repository permissions instead of
pr.author_association. Call
github.rest.repos.getCollaboratorPermissionLevel(pr.user.login), treat lookup
failures as no permission, and consider only write-capable permission levels
eligible for the sponsorship exemption; obtain explicit security review for this
.github/** change.
🪄 Autofix
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: ASSERTIVE
Plan: Pro Plus
Run ID: 240abeed-37ac-4b3d-ba49-e968614e4ca9
📒 Files selected for processing (7)
.github/scripts/enforce-pr-target.test.cjs.github/scripts/pr-hygiene.cjs.github/scripts/pr-hygiene.test.cjs.github/workflows/enforce-pr-target.yml.github/workflows/pr-hygiene.ymltests/ci-workflows.test.tstests/helpers/enforce-pr-target-harness.ts
Keep rename sources in sponsored-surface checks, load hygiene scripts from the PR base SHA, and exempt sponsorship only for write-capable repository permissions.
4 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
review-readycannot clear while hygiene still fails (seen on fix(codex): recover incomplete account deletion cleanup #1324).maintainer-sponsored, exception labels,intake: hygiene-blocked) so sponsorship can lift the draft without a new push.collectDeterministicHygieneFailuresbetweenPR hygieneandEnforce PR target branchso the two bots cannot disagree.Test plan
node --test .github/scripts/pr-hygiene.test.cjs .github/scripts/enforce-pr-target.test.cjsbun test tests/ci-workflows.test.tssrc/codex/auth-api.tsstays draft / noreview-readyuntilmaintainer-sponsoreddev, promote tomain(pull_request_targetloads from the default branch)Summary by CodeRabbit
New Features
Bug Fixes