rustbot assigned two reviewers

[RalfJung]

In rust-lang/rust#135423 I wrote "r? compiler" in a review summary comment and now the PR has two reviewers assigned...

[rustbot]

Error: Parsing assign command in comment failed: ...' compiler' | error: specify user to assign to at >| '" in a [re'...

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[apiraino]

Adding some context after looking at the triagebot logs. I see two events at two close timestamps:

• 2025-01-14T08:19:12.073Z (PullRequestReviewEvent, action: Edited -> assignment to chenyukang)
• 2025-01-14T08:19:12.311Z (PullRequestReviewEvent, action: Submitted -> assignment to sparrowlii)

and then:

• 2025-01-14T08:21:39.988219Z Assignment to Nadrieril

I don't have an explaination for that 🤷 but I tend to exclude an issue on the triagebot. I can only speculate your browser accidentally sending two events at close distance (for any reason between you clicking the web UI and the network) but the sequence of events is weird (first the edit, then the submission) 🤔

[RalfJung]

and then:

I wrote r? compiler again to resolve the reviewer tie, so that is expected.

Seems like me submitting the review somehow triggered two events (maybe that's a GH bug or it's just how GH works some times -- I only clicked "submit review" and didn't edit anything), and furthermore there's a race condition in triagebot where if two events occur at the same time, we can end up with two reviewers.

[Nadrieril]

Same thing happened on rust-lang/rust#135102 a few days ago

[apiraino]

there's a race condition in triagebot where if two events occur at the same time, we can end up with two reviewers.

Just to be clear, multiple reviewers are supported by the GH API so the triagebot reflects that (it's just that we don't use multiple reviewers, as far as I know).

Why sometimes we receive two events (1 create, 1 edit ?!) when someone requests an assignment is the interesting question 🤔

[RalfJung]

Just to be clear, multiple reviewers are supported by the GH API so the triagebot reflects that (it's just that we don't use multiple reviewers, as far as I know).

Yeah I know, but triagebot is supposed to maintain the general property of there being only one reviewer for our PRs.

[apiraino]

Another case (from Zulip)

https://togithub.com/rust-lang/rust/pull/137549#event-16442291855

[ehuss]

For rust-lang/rust#137549 (comment), the logs show that GitHub sent two events in quick succession:

2025-02-25T01:05:15.525531Z -- pull_request_review edited, changes=Some(Changes { title: None, body: None }) (body=None makes no sense)
2025-02-25T01:05:15.743609Z -- pull_request_review submitted (happened after "edited"!?)

I'm curious what the raw JSON of the first event looked like, since it doesn't make sense to send an "edited" event with no changes.

Perhaps triagebot should ignore "edited" events where changes has a None body? The docs are not really clear why that might be unset, but my guess is that is intended for when comments are edited to an empty string, or deleted? Either way, I think triagebot should always ignore those.

Looking through the logs, I see this weird "edited" with no changes followed by a submitted happened 10 times in the past 24 hours.