an agent gets one todo when its pr starts conflicting with the target branch

swarm-notify-router/src/event.rs

@@ -13,6 +13,8 @@ use std::collections::BTreeSet;
 use serde::{Deserialize, Serialize};
 use serde_json::Value as Json;
 
+use swarm_wire::forge_events::MERGEABILITY_EVENT;
+
 use crate::flow::{Fields, Value};
 
 /// The parts of an `issues` or `pull_request` family delivery the router
@@ -165,6 +167,41 @@ pub struct Snapshot {
     /// and tells nobody anything.
     #[serde(default)]
     pub head: Option<String>,
+    /// Whether the last mergeability event for this PR found it conflicting
+    /// with its target branch, or `None` before the first.
+    #[serde(default)]
+    pub conflicting: Option<bool>,
+}
+
+/// Whether a mergeability event moves the PR into or out of conflict: a
+/// `conflicting` finding for a PR not already conflicting, or a `mergeable`
+/// one for a PR that was.
+fn mergeability_changed(event_type: &str, action: &str, snapshot: &Snapshot) -> bool {
+    if event_type != MERGEABILITY_EVENT {
+        return false;
+    }
+    match action {
+        "conflicting" => snapshot.conflicting != Some(true),
+        "mergeable" => snapshot.conflicting == Some(true),
+        _ => false,
+    }
+}
+
+/// The PR's conflict state after `event_type`, or `snapshot`'s when the event
+/// says nothing about it. A closed PR forgets it: the `closed` flow clears the
+/// conflict todo, so a reopened PR that still conflicts is told again.
+pub fn conflicting_after(event_type: &str, action: &str, snapshot: &Snapshot) -> Option<bool> {
+    if event_type == "pull_request" && action == "closed" {
+        return None;
+    }
+    if event_type != MERGEABILITY_EVENT {
+        return snapshot.conflicting;
+    }
+    match action {
+        "conflicting" => Some(true),
+        "mergeable" => Some(false),
+        _ => snapshot.conflicting,
+    }
 }
 
 /// Whether `event_type` is an assign delivery, whose assignee list is the
@@ -199,6 +236,14 @@ pub fn fields(event_type: &str, delivery: &Delivery, snapshot: &Snapshot) -> Fie
         ("author", text(&delivery.author)),
         ("draft", text(bool_text(delivery.draft))),
         ("undrafted", text(bool_text(delivery.undrafted))),
+        (
+            "mergeability_changed",
+            text(bool_text(mergeability_changed(
+                event_type,
+                &delivery.action,
+                snapshot,
+            ))),
+        ),
         (
             "assignees",
             Value::List(delivery.assignees.iter().cloned().collect()),
@@ -428,6 +473,60 @@ mod tests {
         assert_eq!(f.get("undrafted"), Some(&Value::Text("true".to_owned())));
     }
 
+    /// The body swarm-controller publishes for a mergeability finding reads as
+    /// a PR, and `mergeability_changed` is true only for a change.
+    #[test]
+    fn a_mergeability_event_reads_as_a_pull_request_and_flags_only_a_change() {
+        let body = |action: &str| {
+            serde_json::to_vec(&serde_json::json!({
+                "action": action,
+                "number": 9,
+                "pull_request": {
+                    "number": 9,
+                    "title": "t",
+                    "html_url": "https://forge/pulls/9",
+                    "user": { "login": "atlas" },
+                    "assignees": [{ "login": "iris" }],
+                    "head": { "sha": "abc" },
+                    "base": { "ref": "main" },
+                    "draft": false,
+                },
+                "repository": { "id": 42, "full_name": "o/r" },
+            }))
+            .expect("encodes")
+        };
+        let d = Delivery::from_event(MERGEABILITY_EVENT, &body("conflicting")).expect("parses");
+        assert!(d.is_pull);
+        assert_eq!(
+            (d.author.as_str(), d.number, d.action.as_str()),
+            ("atlas", 9, "conflicting")
+        );
+        assert_eq!(d.assignees, BTreeSet::from(["iris".to_owned()]));
+
+        let changed = |action: &str, before: Option<bool>, event: &str| {
+            let d = Delivery::from_event(event, &body(action)).expect("parses");
+            let snapshot = Snapshot {
+                conflicting: before,
+                ..Snapshot::default()
+            };
+            fields(event, &d, &snapshot)
+                .get("mergeability_changed")
+                .cloned()
+        };
+        let yes = Some(Value::Text("true".to_owned()));
+        let no = Some(Value::Text("false".to_owned()));
+        assert_eq!(changed("conflicting", None, MERGEABILITY_EVENT), yes);
+        assert_eq!(changed("conflicting", Some(false), MERGEABILITY_EVENT), yes);
+        assert_eq!(changed("conflicting", Some(true), MERGEABILITY_EVENT), no);
+        assert_eq!(changed("mergeable", Some(true), MERGEABILITY_EVENT), yes);
+        assert_eq!(changed("mergeable", Some(false), MERGEABILITY_EVENT), no);
+        assert_eq!(changed("mergeable", None, MERGEABILITY_EVENT), no);
+        assert_eq!(
+            changed("conflicting", None, "pull_request"),
+            no,
+            "another event type never changes it"
+        );
+    }
     #[test]
     fn the_snapshot_turns_the_after_list_into_added_and_removed() {
         let d = Delivery::parse(&issue_assign("unassigned", &["iris"])).expect("parses");

swarm-notify-router/src/route.rs

@@ -5,7 +5,7 @@ use std::collections::BTreeSet;
 
 use swarm_wire::router_todo::Op;
 
-use crate::event::{Delivery, Snapshot, fields, is_assign};
+use crate::event::{Delivery, Snapshot, conflicting_after, fields, is_assign};
 use swarm_wire::notify_flows::Flow;
 
 use crate::flow::{Action, ReviewRequest, evaluate, requests};
@@ -67,6 +67,7 @@ pub fn route(
     if !is_run && delivery.is_pull && !delivery.head.is_empty() {
         next.head = Some(delivery.head.clone());
     }
+    next.conflicting = conflicting_after(event_type, &delivery.action, snapshot);
     if is_assign(event_type) {
         // A skipped assignee is left out of the snapshot, so the next assign
         // event counts it as added again: an agent assigned before it is in
@@ -204,6 +205,110 @@ mod tests {
         d
     }
 
+    /// A PR starts conflicting: its author gets one todo, a second finding of
+    /// the same changes nothing, and the PR merging again clears it. Closing
+    /// the PR clears it too and forgets the state, so a reopened PR that
+    /// still conflicts is told again.
+    #[test]
+    fn a_pull_request_that_starts_conflicting_tells_the_author_once() {
+        let flows = builtins();
+        let is_agent = |a: &str| a == "atlas";
+        let found = |action: &str, snapshot: &Snapshot| {
+            route(
+                "pull_request_mergeability",
+                &pull_by("atlas", action),
+                snapshot,
+                &flows,
+                is_agent,
+            )
+        };
+
+        let first = found("conflicting", &Snapshot::default());
+        assert_eq!(agents(&first), [("atlas", Op::Upsert)]);
+        assert_eq!(first.actions[0].op.key, "forge:o/r#9:conflict");
+        assert_eq!(first.actions[0].flow, "conflict");
+        assert!(
+            first.actions[0]
+                .op
+                .summary
+                .starts_with("PR o/r#9 no longer merges into its target branch")
+        );
+        assert_eq!(first.next.conflicting, Some(true));
+        assert_eq!(first.next.holders, BTreeSet::from(["atlas".to_owned()]));
+
+        let again = found("conflicting", &first.next);
+        assert!(again.actions.is_empty(), "no todo for a repeat: {again:?}");
+        assert_eq!(again.next.conflicting, Some(true));
+
+        let merges = found("mergeable", &again.next);
+        assert_eq!(agents(&merges), [("atlas", Op::Clear)]);
+        assert_eq!(merges.actions[0].op.key, "forge:o/r#9:conflict");
+        assert!(!merges.actions[0].op.prefix);
+        assert_eq!(merges.next.conflicting, Some(false));
+
+        let later = found("conflicting", &merges.next);
+        assert_eq!(
+            agents(&later),
+            [("atlas", Op::Upsert)],
+            "a PR that conflicts again after merging is told again"
+        );
+
+        let closed = route(
+            "pull_request",
+            &pull_by("atlas", "closed"),
+            &later.next,
+            &flows,
+            is_agent,
+        );
+        assert!(
+            closed
+                .actions
+                .iter()
+                .any(|a| a.op.op == Op::Clear && a.op.prefix && a.op.key == "forge:o/r#9:"),
+            "the close clears every todo for the PR, the conflict one included"
+        );
+        assert_eq!(closed.next.conflicting, None);
+        assert_eq!(
+            agents(&found("conflicting", &closed.next)),
+            [("atlas", Op::Upsert)]
+        );
+    }
+
+    /// A PR that merges and never conflicted has nothing to clear, and the
+    /// mergeability events leave the other state alone.
+    #[test]
+    fn a_mergeable_finding_on_a_pull_request_that_never_conflicted_does_nothing() {
+        let flows = builtins();
+        for before in [None, Some(false)] {
+            let snapshot = Snapshot {
+                conflicting: before,
+                ..Snapshot::default()
+            };
+            let r = route(
+                "pull_request_mergeability",
+                &pull_by("atlas", "mergeable"),
+                &snapshot,
+                &flows,
+                |_| true,
+            );
+            assert!(r.actions.is_empty(), "{before:?}: {:?}", r.actions);
+            assert_eq!(r.next.conflicting, Some(false));
+        }
+        // Control: the same finding on a conflicting PR does clear.
+        let conflicting = Snapshot {
+            conflicting: Some(true),
+            ..Snapshot::default()
+        };
+        let r = route(
+            "pull_request_mergeability",
+            &pull_by("atlas", "mergeable"),
+            &conflicting,
+            &flows,
+            |_| true,
+        );
+        assert_eq!(agents(&r), [("atlas", Op::Clear)]);
+    }
+
     /// The push that clears the CI todo also cancels the runs before it, and
     /// one of them can report in after the clear. It began at the old head, so
     /// it must not bring the todo back.

swarm-notify-router/src/stored.rs

@@ -150,7 +150,7 @@ mod tests {
     use super::*;
 
     /// The built-in flows, in the order the router runs them.
-    const BUILTIN_IDS: [&str; 7] = [
+    const BUILTIN_IDS: [&str; 9] = [
         "assigned",
         "unassigned",
         "closed",
@@ -158,6 +158,8 @@ mod tests {
         "ci-failed",
         "ci-cancelled",
         "ci-cleared",
+        "conflict",
+        "conflict-cleared",
     ];
 
     fn ids(f: &Flows) -> Vec<&str> {
@@ -220,6 +222,8 @@ mod tests {
                 "ci-failed",
                 "ci-cancelled",
                 "ci-cleared",
+                "conflict",
+                "conflict-cleared",
                 "triage"
             ]
         );
@@ -232,7 +236,7 @@ mod tests {
         let before = ids(&f).join(",");
         assert_eq!(
             before,
-            "assigned,unassigned,head-classifier,ci-failed,ci-cancelled,ci-cleared,triage"
+            "assigned,unassigned,head-classifier,ci-failed,ci-cancelled,ci-cleared,conflict,conflict-cleared,triage"
         );
         for bad in [
             junk(2, b"not json"),