fix(pr-review): do not restart completed reviews on delayed submitted events (#647)

Recognize the review a submitted delivery reports as completion even when it predates the latest review_requested event, let a newer verdict on the same head supersede it, and ignore dismissed reviews. Re-applied on current main; bundle 1.9.1. Closes #646.

Author: all-hands-bot

automations/bundle-index.js

@@ -1,6 +1,6 @@
 {
   "id": "github-pr-reviewer",
-  "version": "1.9.0",
+  "version": "1.9.1",
   "name": "GitHub code review",
   "category": "Code review",
   "description": "Review requested pull-request heads, resume an outstanding reviewer request on a schedule once its checks are green, classify all open non-draft pull requests that nobody requested and review eligible green heads, drain the backlog oldest-first under a small per-scan new-conversation bound, record exact-head decisions, stop early on out-of-scope changes, and optionally request a code-aware human maintainer after approval or a maintainer-decision scope stop.",
@@ -135,7 +135,7 @@
       }
     },
     "bundle": {
-      "version": "1.9.0",
+      "version": "1.9.1",
       "entrypoint": "python3 worker.py",
       "timeout": 300,
       "files": {

automations/catalog/github-pr-reviewer/manifest.json

@@ -283,30 +283,55 @@ def _prompt(self, pr, trigger, label=None, delivery_key=None):
             f"second result.{self_review_note}"
         )
 
-    def _finish_completed_review(self, pr, trigger, label=None):
+    def _finish_completed_review(self, pr, trigger, label=None, submitted_review=None):
         """Complete an exact-head review, including an optional human handoff.
 
         `trigger` is the event the current work is keyed on. It is None for an
         unrequested scheduled-scan PR, which has no trigger event: the head SHA
         is the whole key, so any submitted review by this account on the current
         head is the completion of that work. That is what lets an unrequested
         review the scan started be reconciled and handed off on a later scan
-        instead of being restarted.
+        instead of being restarted. `submitted_review` is the review a
+        `submitted` delivery reports. It can predate the latest
+        `review_requested` event, so it is found by id rather than by the
+        trigger window; this account's later reviews of the head still count, so
+        a newer verdict supersedes it. A dismissed review completes nothing.
         """
         head_sha = pr["head"]["sha"]
         if trigger is not None:
             triggered_at = trigger.get("created_at") or ""
         else:
             triggered_at = ""
         reviews = self.gh_pages(f"/pulls/{pr['number']}/reviews")
-        completed = [
+        mine = [
             review
             for review in reviews
             if review.get("commit_id") == head_sha
             and ((review.get("user") or {}).get("login") or "").lower()
             == self.github_login.lower()
-            and (review.get("submitted_at") or "") > triggered_at
+            and (review.get("state") or "").upper() != "DISMISSED"
         ]
+        if submitted_review and submitted_review.get("id") is not None:
+            reported = next(
+                (review for review in mine if review.get("id") == submitted_review["id"]),
+                None,
+            )
+            reported_at = (reported or {}).get("submitted_at") or ""
+            completed = (
+                []
+                if reported is None
+                else [
+                    review
+                    for review in mine
+                    if (review.get("submitted_at") or "") >= reported_at
+                ]
+            )
+        else:
+            completed = [
+                review
+                for review in mine
+                if (review.get("submitted_at") or "") > triggered_at
+            ]
         if not completed:
             return False
         approved = None
@@ -858,6 +883,11 @@ def run(self):
         for candidate in prs:
             try:
                 pr = self.gh("GET", f"/pulls/{candidate['number']}")
+                submitted_review = (
+                    payload.get("review")
+                    if event_mode and payload.get("action") == "submitted"
+                    else None
+                )
                 has_label = label in {
                     item["name"] for item in pr.get("labels", [])
                 }
@@ -894,7 +924,11 @@ def run(self):
                     delivery_key = self._unrequested_head(pr)
                     unrequested_candidate = True
                 if trigger is None and delivery_key is None:
-                    continue
+                    # A submitted review is its own completion signal: its
+                    # request can predate the issue-event window, and that
+                    # review must still be reconciled.
+                    if submitted_review is None:
+                        continue
                 # A head this account already reviewed, with no clarifying
                 # comment since, is done: another conversation would publish a
                 # second review of identical code. This must come before the
@@ -918,7 +952,7 @@ def run(self):
                         ),
                         flush=True,
                     )
-                    if trigger is not None:
+                    if trigger is not None or submitted_review is not None:
                         # Reconcile the completed review (clearing the label and
                         # running the maintainer handoff) where the review is
                         # this trigger's result. When it predates the trigger -
@@ -927,7 +961,7 @@ def run(self):
                         # otherwise the label stays and every later scan re-reads
                         # this head and re-logs with no effect.
                         if not self._finish_completed_review(
-                            pr, trigger, trigger_label
+                            pr, trigger, trigger_label, submitted_review
                         ) and trigger_label:
                             self.gh(
                                 "DELETE",
@@ -936,7 +970,7 @@ def run(self):
                             )
                     continue
                 if delivery_key is None and self._finish_completed_review(
-                    pr, trigger, trigger_label
+                    pr, trigger, trigger_label, submitted_review
                 ):
                     continue
                 if event_mode and payload.get("action") == "submitted":

skills/github-pr-reviewer/scripts/worker.py

@@ -68,7 +68,7 @@
               "timeout": 300,
               "template": {
                 "id": "github-pr-reviewer",
-                "version": "1.9.0",
+                "version": "1.9.1",
                 "config": {
                   "repos": [
                     "OpenHands/agent-server-gui"
@@ -108,7 +108,7 @@
             "timeout": 300,
             "template": {
               "id": "github-pr-reviewer",
-              "version": "1.9.0",
+              "version": "1.9.1",
               "config": {
                 "repos": [
                   "OpenHands/agent-server-gui"
@@ -213,7 +213,7 @@
               "timeout": 300,
               "template": {
                 "id": "github-pr-reviewer",
-                "version": "1.9.0",
+                "version": "1.9.1",
                 "config": {
                   "repos": [
                     "OpenHands/agent-server-gui",
@@ -254,7 +254,7 @@
             "timeout": 300,
             "template": {
               "id": "github-pr-reviewer",
-              "version": "1.9.0",
+              "version": "1.9.1",
               "config": {
                 "repos": [
                   "OpenHands/agent-server-gui",
@@ -296,7 +296,7 @@
             "preset_metadata": {
               "template": {
                 "id": "github-pr-reviewer",
-                "version": "1.9.0",
+                "version": "1.9.1",
                 "config": {
                   "repos": [
                     "OpenHands/agent-server-gui",
@@ -351,7 +351,7 @@
               "timeout": 300,
               "template": {
                 "id": "github-pr-reviewer",
-                "version": "1.9.0",
+                "version": "1.9.1",
                 "config": {
                   "repos": [
                     "OpenHands/agent-server-gui"
@@ -444,7 +444,7 @@
             "timeout": 300,
             "template": {
               "id": "github-pr-reviewer",
-              "version": "1.9.0",
+              "version": "1.9.1",
               "config": {
                 "repos": [
                   "OpenHands/agent-server-gui"
@@ -540,7 +540,7 @@
             "timeout": 300,
             "template": {
               "id": "github-pr-reviewer",
-              "version": "1.9.0",
+              "version": "1.9.1",
               "config": {
                 "repos": [
                   "OpenHands/agent-server-gui"
@@ -616,7 +616,7 @@
               "timeout": 300,
               "template": {
                 "id": "github-pr-reviewer",
-                "version": "1.9.0",
+                "version": "1.9.1",
                 "config": {
                   "repos": [
                     "OpenHands/automation"