Skip to content

ci: label open PRs with who owes the next move - #598

Merged
Sapd merged 2 commits into
masterfrom
ci/review-state-labels
Oct 20, 2026
Merged

Sapd merged 2 commits into
masterfrom
ci/review-state-labels

Conversation

@Sapd

@Sapd Sapd commented Oct 20, 2026

Copy link
Copy Markdown
Owner

Keeps awaiting-maintainer / awaiting-author on every open PR, plus needs-rebase when GitHub reports the branch as conflicting. is:open label:awaiting-maintainer is then the review queue.

Who moves the labels. Only the maintainer and the PR author. Anyone else commenting - people reporting test results on someone else's PR, which happens a lot here - is ignored, so a drive-by comment doesn't pull a PR back into the queue. A push always means awaiting-maintainer: there is new code to look at, whoever pushed it. Drafts and closed PRs carry none of the three labels.

Why both events and a schedule. The events keep it current; the nightly run recomputes every open PR from scratch, so a missed webhook, a hand-edited label or a PR older than this workflow all heal by themselves. It also back-fills the current open PRs on the first run.

No checkout, so pull_request_target has nothing to execute from the fork.

Dry-run of the same logic against the repo as it stands:

#590  -> awaiting-author                   (last move 2026-10-14, @Dasio)
#587  draft -> no labels
#584  -> awaiting-author                   (last move 2026-09-22, @marctew)
#580  -> awaiting-author, needs-rebase     (last move 2026-09-27, @gdevenyi)
#570  -> awaiting-author                   (last move 2026-10-03, @gdevenyi)
#569  -> awaiting-maintainer, needs-rebase (last move 2026-09-27, @Kirth)

The three labels already exist. workflow_dispatch takes a dry_run input that only logs what it would change - worth one run with it on after merging, since workflow_dispatch and schedule only work from the default branch.

Adds repo-review-state.yml, which keeps awaiting-maintainer / awaiting-author
on every open PR, plus needs-rebase when GitHub reports the branch as
conflicting. The open PR list then says who is blocked on whom, and
"label:awaiting-maintainer" is the review queue.

Only the maintainer and the PR author move the labels. People reporting test
results on someone else's PR are ignored, otherwise a drive-by comment would
pull a PR back into the queue.

Events keep it current; the daily run recomputes every open PR from scratch, so
a missed webhook, a hand-edited label, or a PR older than this workflow all
correct themselves. Drafts and closed PRs carry none of the three labels.

workflow_dispatch takes a dry_run input that only logs what it would change.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-20T13:28:26.855560Z fc3b624 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Oct 20, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc3b624951

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +27 to +28
pull_request_review:
types: [submitted]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reconcile fork reviews through a write-capable event

For PRs from forks under GitHub's default token policy, this pull_request_review run has its requested write scopes downgraded to read-only, as described in GitHub's workflow-permissions documentation. A maintainer submitting a review therefore reaches issues.addLabels or removeLabel without permission and fails, leaving the previous turn label in place until another write-capable event or the nightly schedule runs.

Useful? React with 👍 / 👎.

Comment on lines +97 to +100
const classify = (login, association) => {
if (isMaintainerSide(association)) return AWAITING_AUTHOR;
if (login === author) return AWAITING_MAINTAINER;
return null; // third party: does not move the labels

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prioritize the PR author before maintainer association

When a repository owner, member, or collaborator authors a PR, their comments satisfy both conditions, but the maintainer-side check wins and marks the PR awaiting-author. This inverts the intended state after an internal contributor responds on their own PR; check login === author before the association-based classification.

Useful? React with 👍 / 👎.

Comment on lines +25 to +30
issue_comment:
types: [created]
pull_request_review:
types: [submitted]
pull_request_target:
types: [opened, reopened, synchronize, ready_for_review, converted_to_draft, closed]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Trigger reconciliation for inline review replies

When the author or maintainer replies directly in an existing diff thread, GitHub emits pull_request_review_comment, not issue_comment or a newly submitted review; GitHub documents it as the event for comments on a PR diff. Although whoseTurn reads these comments, none of the configured triggers runs for the reply, so the turn label remains stale until the nightly reconciliation.

Useful? React with 👍 / 👎.

Comment on lines +94 to +95
const lastCommit = commits[commits.length - 1];
note(lastCommit?.commit?.committer?.date, AWAITING_MAINTAINER);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Record push activity instead of the commit's embedded date

When a synchronize event moves the PR head to an existing or deliberately backdated commit, commit.committer.date can predate the latest maintainer response because it is commit metadata rather than the time the commit was pushed. Sorting by this value then leaves awaiting-author even though the workflow's stated rule says every push must switch to awaiting-maintainer; the synchronize occurrence needs to be represented using actual push/event activity.

Useful? React with 👍 / 👎.

@github-actions github-actions Bot added the awaiting-maintainer The author replied or pushed - our move label Oct 20, 2026
…t the push event

Three fixes from review:

- A maintainer's own PR matched the association check before the author check,
  so their comments read as "we answered" and labelled it awaiting-author.
  Author wins now; it is still their change to push.
- Replies inside an existing diff thread raise pull_request_review_comment,
  which was not a trigger, so whoseTurn() read them but nothing ran until the
  nightly pass.
- A push is taken from the event rather than from the new head commit's
  embedded date, which can predate the last reply after a rebase or a
  cherry-pick and leave the PR on awaiting-author.
@Sapd

Sapd commented Oct 20, 2026

Copy link
Copy Markdown
Owner Author

Took three of the four, in 6d281af.

Author before association - right, and it was wrong on this very PR: before the fix #598 computed as awaiting-author, now it computes as awaiting-maintainer. Author wins now.

pull_request_review_comment - right, that trigger was missing. whoseTurn() read diff-thread replies but nothing ran on them, so they waited for the nightly pass. Added.

Push time vs committer date - right in principle: a rebase or cherry-pick can carry a date older than the last reply. A push is now taken from the event itself rather than derived from the head commit, so synchronize/opened/reopened/ready_for_review go straight to awaiting-maintainer.

Fork token on pull_request_review - not taking this one. The read-only downgrade applies to pull_request from a fork; issue_comment, pull_request_review and pull_request_review_comment all fire in the base repository's context and get the declared write scopes. If that were wrong, issue_comment would fail the same way and that is the main path here, so the finding is also internally inconsistent. Easy to check after merge: a review on a fork PR either relabels or it doesn't, and the nightly reconcile repairs it either way - which is what it's for.

Dry run against the current open list, unchanged apart from #598:

#598  -> awaiting-maintainer
#590  -> awaiting-author
#587  draft -> no labels
#584  -> awaiting-author
#580  -> awaiting-author, needs-rebase
#570  -> awaiting-author
#569  -> awaiting-maintainer, needs-rebase

@Sapd
Sapd merged commit efec64c into master Oct 20, 2026
5 checks passed
@github-actions github-actions Bot removed the awaiting-maintainer The author replied or pushed - our move label Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant