ci(review): review a PR retargeted onto master or marked ready - #7085
Conversation
|
Claude finished @otavio's task in 2m 29s —— View job Code Review CompleteReviewed 1 file. 2 inline issues posted — Standards: 0 violations, 1 judgement call. Spec: 0. Correctness: 1. StandardsThe repo rules are clean — the deleted comment had gone stale and the reason it carried (why CorrectnessLine 25 — the gate is a job-level Verified clean: the SpecThe Spec axis did not run — the PR references PRs #7084 and #7079 but no issue, so there is no spec to review against. To request another review round, comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
A stacked PR never got an automatic review. #7084 was opened against the branch of #7079 at 22:01:33, #7079 merged two seconds later, and GitHub retargeted #7084 onto master at 22:01:37. The opened event carried the old base, so branches: [master] filtered it out and no run was ever queued; the retarget arrives as edited, which nothing listened for; and the force-push that followed is synchronize, which stays excluded so the author drives re-review. The PR fell through every trigger and only got a review when its author asked for one by hand. edited covers the retarget, gated on changes.base so editing a title or a description does not spend a review. ready_for_review closes the sibling hole, where a PR opened as a draft is filtered by the draft check at open and has nothing to re-trigger it once it is ready. The same guard has to sit on cancel-in-progress, not only on the job. A run joins the concurrency group and cancels the one in flight before any job-level if: is evaluated, so gating the job alone would let a title edit cancel a running review and then skip its own, leaving no review at all — and it would defeat this change's own case, where a retarget starts a review that the author's next title edit kills. The other triggers cannot produce an edited action, so their runs keep cancelling as they do today. The comment saying the review runs only on open went with the change that made it false.
5708f78 to
4e4f911
Compare
|
/review |
Code Review CompleteThe automated review ran but did not post an updated summary — this usually means no new issues were found since the previous review. If you've pushed changes and want a fresh pass, comment |
1 participant
What
Adds
ready_for_reviewandeditedto the review workflow'spull_request_targettrigger, soa PR retargeted onto
masteror marked ready gets the automatic review it currently misses.Why
#7084 never got an automatic review. It was opened at 22:01:33 against the branch of #7079,
#7079 merged two seconds later, and GitHub retargeted it onto
masterat 22:01:37(
automatic_base_change_succeeded). Three filters in sequence let it through:openedevent carried the old base, sobranches: [master]excluded it — no run wasqueued, not skipped;
edited, which the workflow did not listen for;synchronize, deliberately excluded so the author drivesre-review.
It only got reviewed because its author commented
/review. Any PR stacked on another sees thesame thing the moment the parent merges.
Changes
editedgated onchanges.base: a retarget re-triggers the review, while editing a titleor a description does not spend one. The
branches: [master]filter is evaluated against thenew base, so only PRs that land on
masterqualify.ready_for_review: closes the sibling hole. A PR opened as a draft is filtered by the!draftcheck atopened, and nothing re-triggers it once it is ready.cancel-in-progresscarries the same guard:concurrencyis workflow-level and evaluatedwhen a run is created, before any job
if:. Gating only the job would let a title or body editcreate a run that cancels an in-flight review and then skip its own — no review at all, and the
retarget case this PR exists for would die to the author's next title edit. The other triggers
cannot produce an
editedaction, so their runs keep cancelling as they do today.synchronizestays out, unchanged.Paired with shellhub-io/cloud#2555, which makes the identical change to the same workflow there.
Testing
actionlintis clean. The trigger cannot be exercised before merge — the workflow that runs isthe one on the base branch, so
pull_request_targetchanges only take effect once they are onmaster. The check after merging is a PR stacked on another: merge the parent and confirm areview starts without anyone commenting
/review.