request the review classifier when a pr is opened or reopened

docs/swarm/notify-router.md

@@ -79,7 +79,8 @@ levels deep, and 100 custom flows. A `todo` node sends to the agents an event
 field holds (`assignees_added`, `todo_holders`, …) or to one named agent.
 
 The router ships the three flows above, each a source → match on `action` →
-todo, and `head-classifier`: source `pull_request_sync` → `request_reviewer`
+todo, and `head-classifier`: source `pull_request` and `pull_request_sync` →
+match on `action` (`opened`, `reopened`, `synchronized`) → `request_reviewer`
 for `review-classifier` ([below](#requesting-a-review)). The code defines them
 (`swarm_wire::notify_flows::builtins`), so a build that changes one reaches
 every swarm. Each switches off on `/notifications`. A custom flow has the same type and
@@ -150,8 +151,9 @@ not retried: the next push asks again. A request repeated for the same head is
 harmless: a reviewer with a pending request is a no-op for the forge, and one
 who has reviewed gets `skipped`.
 
-`head-classifier` requests the review classifier's review on a push to any open
+`head-classifier` requests the review classifier's review when someone opens or
-PR. The classifier then runs as it does for any requested review
+reopens a PR and on every push to an open PR. The classifier then runs as it does
+for any requested review
 ([`review-classifier.md`](review-classifier.md#trigger-and-lifecycle)), through
 its own request queue, which replaces a waiting request for the same PR. A
 repo the classifier's account can't read, or a PR that account opened, answers

docs/swarm/review-classifier.md

@@ -86,9 +86,10 @@ as a reviewer on a PR starts a run:
 4. The controller posts the result on the PR as the service account's
    review, on the commit it read.
 
-A push starts no run by itself. The notification router's `head-classifier`
+Opening or pushing to a PR starts no run by itself. The notification router's
-flow requests the account's review on every push to an open PR, unless its
+`head-classifier` flow requests the account's review when someone opens or
-review of the new head already stands ([`notify-router.md`](notify-router.md#requesting-a-review)).
+reopens a PR and on every push to an open PR, unless its review of the new head
+already stands ([`notify-router.md`](notify-router.md#requesting-a-review)).
 
 When a post can't reach the secret store or the forge, because the
 connection fails or a try gets no answer within two minutes, the controller

swarm-controller/src/review_classifier/review.rs

@@ -2,7 +2,7 @@
 //! ([`LOGIN`]) runs the repo's rules against the PR and posts the result as
 //! that account's review, on the commit the run read.
 //!
-//! Only a review request starts a run. A push makes none itself: the
+//! Only a review request starts a run. Opening or pushing to a PR makes none itself: the
 //! notification router's `head-classifier` flow requests the review. A fresh
 //! request retires the account's earlier review: the forge clears the
 //! `official` flag on a reviewer's previous reviews when they are requested

swarm-notify-router/src/dry_run.rs

@@ -118,7 +118,7 @@ mod tests {
         assert_eq!(count(&r, "closed", "sink"), 1);
         // No pull request push: the classifier flow counts zero.
         assert_eq!(count(&r, "head-classifier", "source"), 0);
-        assert_eq!(count(&r, "head-classifier", "request"), 0);
+        assert_eq!(count(&r, "head-classifier", "request-opened"), 0);
     }
 
     /// A condition on a field that depends on the previous event of the

swarm-notify-router/src/flow.rs

@@ -435,6 +435,29 @@ mod tests {
         assert!(evaluate(&f, "pull_request_sync", &pull("assigned", "PR")).is_empty());
     }
 
+    /// The built-in classifier flow requests on a push, an open and a reopen,
+    /// and on no other action of a `pull_request` event.
+    #[test]
+    fn the_classifier_flow_requests_on_open_reopen_and_push_only() {
+        let f = swarm_wire::notify_flows::builtins()
+            .into_iter()
+            .find(|f| f.id == "head-classifier")
+            .expect("the built-in");
+        assert_eq!(f.validate(), Ok(()));
+        let pull = |action: &str| {
+            let mut fields = event(action, &[], &[], &[]);
+            fields.insert("kind", text("PR"));
+            fields
+        };
+        let got = |event_type: &str, action: &str| requests(&f, event_type, &pull(action)).len();
+        assert_eq!(got("pull_request", "opened"), 1);
+        assert_eq!(got("pull_request", "reopened"), 1);
+        assert_eq!(got("pull_request_sync", "synchronized"), 1);
+        assert_eq!(got("pull_request", "closed"), 0);
+        assert_eq!(got("pull_request", "edited"), 0);
+        assert_eq!(got("pull_request_assign", "opened"), 0);
+    }
+
     #[test]
     fn a_placeholder_expands_to_its_field_and_an_unknown_one_is_an_error() {
         let fields = event("assigned", &["a", "b"], &[], &[]);

swarm-notify-router/src/route.rs

@@ -212,7 +212,7 @@ mod tests {
             r.requests,
             [ReviewRequest {
                 flow: "head-classifier".to_owned(),
-                node: "request".to_owned(),
+                node: "request-synchronized".to_owned(),
                 repo: "o/r".to_owned(),
                 number: 9,
                 reviewer: "review-classifier".to_owned(),
@@ -222,10 +222,19 @@ mod tests {
         assert_eq!(r.next, before);
 
         // The source gates: another PR event requests nothing.
-        for other in ["pull_request", "pull_request_assign", "issues"] {
+        for other in ["pull_request_assign", "issues"] {
             let r = route(other, &pull, &before, &flows, |_| true);
             assert!(r.requests.is_empty(), "{other}");
         }
+        // Opening a PR is one `pull_request` event with action `opened`.
+        let opened = Delivery {
+            is_pull: true,
+            ..delivery("opened", &[])
+        };
+        let r = route("pull_request", &opened, &before, &flows, |_| false);
+        assert_eq!(r.requests.len(), 1);
+        assert_eq!(r.requests[0].node, "request-opened");
+        assert!(r.actions.is_empty() && r.skipped.is_empty());
         // Only a PR can be asked to review.
         let issue = delivery("synchronized", &[]);
         let r = route("pull_request_sync", &issue, &before, &flows, |_| true);

swarm-wire/src/notify_flows.rs

@@ -771,7 +771,8 @@ impl TodoSink {
 ///   then tells each agent that held one with a `closed` todo. The clear is the
 ///   first child of the match and runs first: its prefix also covers the
 ///   `closed` key.
-/// - `head-classifier`: a push to a PR requests the review classifier's review.
+/// - `head-classifier`: a PR opened, reopened or pushed to requests the review
+///   classifier's review.
 #[must_use]
 pub fn builtins() -> Vec<Flow> {
     let assignment_key = "forge:{repo}#{number}:assignment";
@@ -837,30 +838,48 @@ fn closed() -> Flow {
     flow
 }
 
-/// source(`pull_request_sync`) → request the review classifier's review.
+/// source(`pull_request`, `pull_request_sync`) → match(`action` = `opened`,
+/// `reopened` or `synchronized`) → request the review classifier's review.
 fn head_classifier() -> Flow {
+    let mut nodes = vec![Node {
+        id: "source".to_owned(),
+        kind: NodeKind::Source {
+            events: vec!["pull_request".to_owned(), "pull_request_sync".to_owned()],
+        },
+    }];
+    let mut edges = Vec::new();
+    for action in ["opened", "reopened", "synchronized"] {
+        let (matched, request) = (format!("match-{action}"), format!("request-{action}"));
+        nodes.push(Node {
+            id: matched.clone(),
+            kind: NodeKind::Match {
+                when: vec![Condition {
+                    field: "action".to_owned(),
+                    equals: action.to_owned(),
+                }],
+            },
+        });
+        nodes.push(Node {
+            id: request.clone(),
+            kind: NodeKind::RequestReviewer {
+                reviewer: CLASSIFIER_ACCOUNT.to_owned(),
+            },
+        });
+        edges.push(Edge {
+            from: "source".to_owned(),
+            to: matched.clone(),
+        });
+        edges.push(Edge {
+            from: matched,
+            to: request,
+        });
+    }
     Flow {
         id: "head-classifier".to_owned(),
         name: "head-classifier".to_owned(),
         enabled: true,
-        nodes: vec![
+        nodes,
-            Node {
+        edges,
-                id: "source".to_owned(),
-                kind: NodeKind::Source {
-                    events: vec!["pull_request_sync".to_owned()],
-                },
-            },
-            Node {
-                id: "request".to_owned(),
-                kind: NodeKind::RequestReviewer {
-                    reviewer: CLASSIFIER_ACCOUNT.to_owned(),
-                },
-            },
-        ],
-        edges: vec![Edge {
-            from: "source".to_owned(),
-            to: "request".to_owned(),
-        }],
     }
 }
 
@@ -1214,24 +1233,42 @@ mod tests {
         assert!(matches!(node.kind, NodeKind::RequestReviewer { .. }));
     }
 
-    /// The built-in that requests the classifier on every push.
+    /// The built-in that requests the classifier on an open, a reopen and a push.
     #[test]
-    fn head_classifier_requests_the_classifier_on_a_push() {
+    fn head_classifier_requests_the_classifier_on_open_reopen_and_push() {
         let f = builtin("head-classifier");
         assert_eq!(f.validate(), Ok(()));
         assert_eq!(
             f.nodes[0].kind,
             NodeKind::Source {
-                events: vec!["pull_request_sync".to_owned()]
+                events: vec!["pull_request".to_owned(), "pull_request_sync".to_owned()]
             }
         );
         assert_eq!(
-            f.nodes[1].kind,
+            f.children("source"),
-            NodeKind::RequestReviewer {
+            ["match-opened", "match-reopened", "match-synchronized"]
-                reviewer: CLASSIFIER_ACCOUNT.to_owned()
-            }
         );
-        assert_eq!(f.children("source"), ["request"]);
+        for action in ["opened", "reopened", "synchronized"] {
+            assert_eq!(
+                f.node(&format!("match-{action}")).map(|n| &n.kind),
+                Some(&NodeKind::Match {
+                    when: vec![Condition {
+                        field: "action".to_owned(),
+                        equals: action.to_owned()
+                    }]
+                })
+            );
+            assert_eq!(
+                f.node(&format!("request-{action}")).map(|n| &n.kind),
+                Some(&NodeKind::RequestReviewer {
+                    reviewer: CLASSIFIER_ACCOUNT.to_owned()
+                })
+            );
+            assert_eq!(
+                f.children(&format!("match-{action}")),
+                [format!("request-{action}")]
+            );
+        }
     }
 
     #[test]