fix(ci): add ready_for_review to check-sprint trigger types#180
Conversation
Required companion change for Mininglamp-OSS/.github#51 which adds draft-PR skip to reusable-check-sprint. Without ready_for_review in caller types, marking a draft PR ready does not re-trigger the Sprint check, leaving a stale green skipped status that can bypass the gate.
Jerry-Xin
left a comment
There was a problem hiding this comment.
✅ APPROVED
One-line addition of ready_for_review to check-sprint.yml trigger types. Correct fix — Draft PRs converting to Ready previously skipped the sprint check because ready_for_review was not in the types list. No other workflow files are affected.
yujiawei
left a comment
There was a problem hiding this comment.
Code Review — PR #180 (octo-web)
Verdict: APPROVED
A minimal, well-reasoned one-line CI fix. No blocking issues found.
Summary of change
Adds ready_for_review to pull_request_target.types in .github/workflows/check-sprint.yml:
- types: [opened, synchronize, reopened, edited]
+ types: [opened, synchronize, reopened, edited, ready_for_review]Assessment
Correctness — ✅
The fix addresses a real merge-gate bypass. The synchronize event fires on pushed commits, not on a draft→ready transition. So once the companion change (.github#51) makes the reusable Sprint workflow skip drafts, a draft PR receives a cached green "skipped" status. Without ready_for_review in the caller's trigger types, marking that PR ready never re-runs the Sprint check, and the stale "skipped" status persists — letting an unvalidated PR merge. Adding ready_for_review ensures the check re-runs at the moment the PR becomes mergeable.
Security — ✅
This workflow uses pull_request_target (elevated privileges, access to PROJECT_TOKEN). The file's own header comment confirms no PR code is checked out or executed, and the job only forwards the PR number / repo name to the org reusable workflow. Adding the ready_for_review trigger type does not expand the attack surface — it triggers the same trusted, no-checkout job on an additional event.
Completeness — ✅
The change is complete on its own and correctly depends on the companion PR (Mininglamp-OSS/.github#51) as documented in the PR description. permissions: {} remains correctly scoped.
Nits / suggestions (non-blocking)
- Consider whether
converted_to_draftis also worth handling for symmetry, so a PR moved back to draft re-evaluates state. Not required here — re-converting to draft would simply re-skip, which is harmless — so this is purely optional.
Conclusion
The change is correct, minimal, and closes a genuine validation-bypass gap. Approving.
Summary
Add
ready_for_reviewtopull_request_target.typesincheck-sprint.yml.Required companion change for Mininglamp-OSS/.github#51, which adds draft-PR skip to the reusable Sprint check workflow.
Why
Without
ready_for_reviewin caller trigger types, marking a draft PR as ready does not re-trigger the Sprint check. The green "skipped" status cached on the draft SHA persists, allowing the now-ready PR to merge without Sprint validation.Change