compliance: Allow Zippy version bumps (#54342)

Finn Evers created

This adds checks for the Zed Zippy version bumps. These are rather
strict, but since we should also be strict with the bumps themselves, I
consider this the better approach.

Note this intentionally only includes the version bumps here for now.

Release Notes:

- N/A

Change summary

tooling/compliance/src/checks.rs | 425 ++++++++++++++++++++++++++++++---
tooling/compliance/src/git.rs    |  80 ++++++
tooling/compliance/src/github.rs | 105 +++++--
tooling/compliance/src/report.rs |   4 
4 files changed, 537 insertions(+), 77 deletions(-)

Detailed changes

tooling/compliance/src/checks.rs 🔗

@@ -3,22 +3,24 @@ use std::{fmt, ops::Not as _, rc::Rc};
 use itertools::Itertools as _;
 
 use crate::{
-    git::{CommitDetails, CommitList},
+    git::{CommitDetails, CommitList, ZED_ZIPPY_LOGIN},
     github::{
         CommitAuthor, GithubApiClient, GithubLogin, PullRequestComment, PullRequestData,
-        PullRequestReview, Repository, ReviewState, ZED_ZIPPY_AUTHOR,
+        PullRequestReview, Repository, ReviewState,
     },
     report::Report,
 };
 
 const ZED_ZIPPY_COMMENT_APPROVAL_PATTERN: &str = "@zed-zippy approve";
 const ZED_ZIPPY_GROUP_APPROVAL: &str = "@zed-industries/approved";
+const EXPECTED_VERSION_BUMP_LOC: u64 = 2;
 
 #[derive(Debug)]
 pub enum ReviewSuccess {
     ApprovingComment(Vec<PullRequestComment>),
     CoAuthored(Vec<CommitAuthor>),
     PullRequestReviewed(Vec<PullRequestReview>),
+    ZedZippyCommit(GithubLogin),
 }
 
 impl ReviewSuccess {
@@ -34,6 +36,7 @@ impl ReviewSuccess {
                 .iter()
                 .map(|comment| format!("@{}", comment.user.login))
                 .collect_vec(),
+            Self::ZedZippyCommit(login) => vec![login.to_string()],
         };
 
         let reviewers = reviewers.into_iter().unique().collect_vec();
@@ -56,6 +59,9 @@ impl fmt::Display for ReviewSuccess {
             Self::ApprovingComment(_) => {
                 formatter.write_str("Approved by an organization approval comment")
             }
+            Self::ZedZippyCommit(_) => {
+                formatter.write_str("Fully untampered automated version bump commit")
+            }
         }
     }
 }
@@ -65,6 +71,7 @@ pub enum ReviewFailure {
     // todo: We could still query the GitHub API here to search for one
     NoPullRequestFound,
     Unreviewed,
+    UnexpectedZippyAction(VersionBumpFailure),
     Other(anyhow::Error),
 }
 
@@ -74,11 +81,50 @@ impl fmt::Display for ReviewFailure {
             Self::NoPullRequestFound => formatter.write_str("No pull request found"),
             Self::Unreviewed => formatter
                 .write_str("No qualifying organization approval found for the pull request"),
+            Self::UnexpectedZippyAction(failure) => {
+                write!(formatter, "Validating Zed Zippy change failed: {failure}")
+            }
             Self::Other(error) => write!(formatter, "Failed to inspect review state: {error}"),
         }
     }
 }
 
+#[derive(Debug)]
+pub enum VersionBumpFailure {
+    NoMentionInTitle,
+    MissingCommitData,
+    AuthorMismatch,
+    UnexpectedCoAuthors,
+    NotSigned,
+    InvalidSignature,
+    UnexpectedLineChanges { additions: u64, deletions: u64 },
+}
+
+impl fmt::Display for VersionBumpFailure {
+    fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::NoMentionInTitle => formatter.write_str("No @-mention found in commit title"),
+            Self::MissingCommitData => formatter.write_str("No commit data found on GitHub"),
+            Self::AuthorMismatch => {
+                formatter.write_str("GitHub author does not match bot identity")
+            }
+            Self::UnexpectedCoAuthors => formatter.write_str("Commit has unexpected co-authors"),
+            Self::NotSigned => formatter.write_str("Commit is not signed"),
+            Self::InvalidSignature => formatter.write_str("Commit signature is invalid"),
+            Self::UnexpectedLineChanges {
+                additions,
+                deletions,
+            } => {
+                write!(
+                    formatter,
+                    "Unexpected line changes ({additions} additions, {deletions} deletions, \
+                     expected {EXPECTED_VERSION_BUMP_LOC} each)"
+                )
+            }
+        }
+    }
+}
+
 pub(crate) type ReviewResult = Result<ReviewSuccess, ReviewFailure>;
 
 impl<E: Into<anyhow::Error>> From<E> for ReviewFailure {
@@ -115,11 +161,11 @@ impl Reporter {
         commit: &CommitDetails,
     ) -> Result<ReviewSuccess, ReviewFailure> {
         let Some(pr_number) = commit.pr_number() else {
-            if commit.author().name().contains("Zed Zippy") && commit.title().starts_with("Bump to")
-            {
-                return Ok(ReviewSuccess::CoAuthored(vec![ZED_ZIPPY_AUTHOR.clone()]));
+            if commit.author().is_zed_zippy() {
+                return self.check_zippy_version_bump(commit).await;
+            } else {
+                return Err(ReviewFailure::NoPullRequestFound);
             }
-            return Err(ReviewFailure::NoPullRequestFound);
         };
 
         let pull_request = self
@@ -148,6 +194,72 @@ impl Reporter {
         Err(ReviewFailure::Unreviewed)
     }
 
+    async fn check_zippy_version_bump(
+        &self,
+        commit: &CommitDetails,
+    ) -> Result<ReviewSuccess, ReviewFailure> {
+        let responsible_actor =
+            commit
+                .version_bump_mention()
+                .ok_or(ReviewFailure::UnexpectedZippyAction(
+                    VersionBumpFailure::NoMentionInTitle,
+                ))?;
+
+        let commit_data = self
+            .github_client
+            .get_commit_metadata(&Repository::ZED, &[commit.sha()])
+            .await?;
+
+        let authors = commit_data
+            .get(commit.sha())
+            .ok_or(ReviewFailure::UnexpectedZippyAction(
+                VersionBumpFailure::MissingCommitData,
+            ))?;
+
+        if !authors
+            .primary_author()
+            .user()
+            .is_some_and(|login| login.as_str() == ZED_ZIPPY_LOGIN)
+        {
+            return Err(ReviewFailure::UnexpectedZippyAction(
+                VersionBumpFailure::AuthorMismatch,
+            ));
+        }
+
+        if authors.co_authors().is_some() {
+            return Err(ReviewFailure::UnexpectedZippyAction(
+                VersionBumpFailure::UnexpectedCoAuthors,
+            ));
+        }
+
+        let signature = authors
+            .signature()
+            .ok_or(ReviewFailure::UnexpectedZippyAction(
+                VersionBumpFailure::NotSigned,
+            ))?;
+
+        if !signature.is_valid() {
+            return Err(ReviewFailure::UnexpectedZippyAction(
+                VersionBumpFailure::InvalidSignature,
+            ));
+        }
+
+        if authors.additions() != EXPECTED_VERSION_BUMP_LOC
+            || authors.deletions() != EXPECTED_VERSION_BUMP_LOC
+        {
+            return Err(ReviewFailure::UnexpectedZippyAction(
+                VersionBumpFailure::UnexpectedLineChanges {
+                    additions: authors.additions(),
+                    deletions: authors.deletions(),
+                },
+            ));
+        }
+
+        Ok(ReviewSuccess::ZedZippyCommit(GithubLogin::new(
+            responsible_actor.to_owned(),
+        )))
+    }
+
     async fn check_commit_co_authors(
         &self,
         commit: &CommitDetails,
@@ -155,7 +267,7 @@ impl Reporter {
         if commit.co_authors().is_some()
             && let Some(commit_authors) = self
                 .github_client
-                .get_commit_authors(&Repository::ZED, &[commit.sha()])
+                .get_commit_metadata(&Repository::ZED, &[commit.sha()])
                 .await?
                 .get(commit.sha())
                 .and_then(|authors| authors.co_authors())
@@ -304,19 +416,19 @@ mod tests {
     use std::rc::Rc;
     use std::str::FromStr;
 
-    use crate::git::{CommitDetails, CommitList, CommitSha};
+    use crate::git::{CommitDetails, CommitList, CommitSha, ZED_ZIPPY_EMAIL, ZED_ZIPPY_LOGIN};
     use crate::github::{
-        AuthorsForCommits, GithubApiClient, GithubLogin, GithubUser, PullRequestComment,
+        CommitMetadataBySha, GithubApiClient, GithubLogin, GithubUser, PullRequestComment,
         PullRequestData, PullRequestReview, Repository, ReviewState,
     };
 
-    use super::{Reporter, ReviewFailure, ReviewSuccess};
+    use super::{Reporter, ReviewFailure, ReviewSuccess, VersionBumpFailure};
 
     struct MockGithubApi {
         pull_request: PullRequestData,
         reviews: Vec<PullRequestReview>,
         comments: Vec<PullRequestComment>,
-        commit_authors_json: serde_json::Value,
+        commit_metadata_json: serde_json::Value,
         org_members: Vec<String>,
     }
 
@@ -346,12 +458,12 @@ mod tests {
             Ok(self.comments.clone())
         }
 
-        async fn get_commit_authors(
+        async fn get_commit_metadata(
             &self,
             _repo: &Repository<'_>,
             _commit_shas: &[&CommitSha],
-        ) -> anyhow::Result<AuthorsForCommits> {
-            serde_json::from_value(self.commit_authors_json.clone()).map_err(Into::into)
+        ) -> anyhow::Result<CommitMetadataBySha> {
+            serde_json::from_value(self.commit_metadata_json.clone()).map_err(Into::into)
         }
 
         async fn check_repo_write_permission(
@@ -412,11 +524,43 @@ mod tests {
         }
     }
 
+    fn alice_author() -> serde_json::Value {
+        serde_json::json!({
+            "name": "Alice",
+            "email": "alice@test.com",
+            "user": { "login": "alice" }
+        })
+    }
+
+    fn bob_author() -> serde_json::Value {
+        serde_json::json!({
+            "name": "Bob",
+            "email": "bob@test.com",
+            "user": { "login": "bob" }
+        })
+    }
+
+    fn charlie_author() -> serde_json::Value {
+        serde_json::json!({
+            "name": "Charlie",
+            "email": "charlie@test.com",
+            "user": { "login": "charlie" }
+        })
+    }
+
+    fn zippy_author() -> serde_json::Value {
+        serde_json::json!({
+            "name": "Zed Zippy",
+            "email": ZED_ZIPPY_EMAIL,
+            "user": { "login": ZED_ZIPPY_LOGIN }
+        })
+    }
+
     struct TestScenario {
         pull_request: PullRequestData,
         reviews: Vec<PullRequestReview>,
         comments: Vec<PullRequestComment>,
-        commit_authors_json: serde_json::Value,
+        commit_metadata_json: serde_json::Value,
         org_members: Vec<String>,
         commit: CommitDetails,
     }
@@ -434,7 +578,7 @@ mod tests {
                 },
                 reviews: vec![],
                 comments: vec![],
-                commit_authors_json: serde_json::json!({}),
+                commit_metadata_json: serde_json::json!({}),
                 org_members: vec![],
                 commit: make_commit(
                     "abc12345abc12345",
@@ -461,8 +605,8 @@ mod tests {
             self
         }
 
-        fn with_commit_authors_json(mut self, json: serde_json::Value) -> Self {
-            self.commit_authors_json = json;
+        fn with_commit_metadata_json(mut self, json: serde_json::Value) -> Self {
+            self.commit_metadata_json = json;
             self
         }
 
@@ -471,12 +615,45 @@ mod tests {
             self
         }
 
+        fn zippy_version_bump() -> Self {
+            Self {
+                pull_request: PullRequestData {
+                    number: 0,
+                    user: None,
+                    merged_by: None,
+                    labels: None,
+                },
+                reviews: vec![],
+                comments: vec![],
+                commit_metadata_json: serde_json::json!({
+                    "abc12345abc12345": {
+                        "author": zippy_author(),
+                        "authors": { "nodes": [] },
+                        "signature": {
+                            "isValid": true,
+                            "signer": { "login": ZED_ZIPPY_LOGIN }
+                        },
+                        "additions": 2,
+                        "deletions": 2
+                    }
+                }),
+                org_members: vec![],
+                commit: make_commit(
+                    "abc12345abc12345",
+                    "Zed Zippy",
+                    ZED_ZIPPY_EMAIL,
+                    "Bump to 0.230.2 for @cole-miller",
+                    "",
+                ),
+            }
+        }
+
         async fn run_scenario(self) -> Result<ReviewSuccess, ReviewFailure> {
             let mock = MockGithubApi {
                 pull_request: self.pull_request,
                 reviews: self.reviews,
                 comments: self.comments,
-                commit_authors_json: self.commit_authors_json,
+                commit_metadata_json: self.commit_metadata_json,
                 org_members: self.org_members,
             };
             let client = Rc::new(mock);
@@ -594,18 +771,10 @@ mod tests {
     async fn comment_takes_precedence_over_co_author() {
         let result = TestScenario::single_commit()
             .with_comments(vec![comment("bob", "@zed-zippy approve")])
-            .with_commit_authors_json(serde_json::json!({
+            .with_commit_metadata_json(serde_json::json!({
                 "abc12345abc12345": {
-                    "author": {
-                        "name": "Alice",
-                        "email": "alice@test.com",
-                        "user": { "login": "alice" }
-                    },
-                    "authors": { "nodes": [{
-                        "name": "Charlie",
-                        "email": "charlie@test.com",
-                        "user": { "login": "charlie" }
-                    }] }
+                    "author": alice_author(),
+                    "authors": { "nodes": [charlie_author()] }
                 }
             }))
             .with_commit(make_commit(
@@ -624,18 +793,10 @@ mod tests {
     #[tokio::test]
     async fn co_author_org_member_succeeds() {
         let result = TestScenario::single_commit()
-            .with_commit_authors_json(serde_json::json!({
+            .with_commit_metadata_json(serde_json::json!({
                 "abc12345abc12345": {
-                    "author": {
-                        "name": "Alice",
-                        "email": "alice@test.com",
-                        "user": { "login": "alice" }
-                    },
-                    "authors": { "nodes": [{
-                        "name": "Bob",
-                        "email": "bob@test.com",
-                        "user": { "login": "bob" }
-                    }] }
+                    "author": alice_author(),
+                    "authors": { "nodes": [bob_author()] }
                 }
             }))
             .with_commit(make_commit(
@@ -715,4 +876,186 @@ mod tests {
             .await;
         assert!(matches!(result, Err(ReviewFailure::Unreviewed)));
     }
+
+    #[tokio::test]
+    async fn zippy_version_bump_with_valid_signature_succeeds() {
+        let result = TestScenario::zippy_version_bump().run_scenario().await;
+        assert!(matches!(result, Ok(ReviewSuccess::ZedZippyCommit(_))));
+        if let Ok(ReviewSuccess::ZedZippyCommit(login)) = &result {
+            assert_eq!(login.as_str(), "cole-miller");
+        }
+    }
+
+    #[tokio::test]
+    async fn zippy_version_bump_without_mention_fails() {
+        let result = TestScenario::zippy_version_bump()
+            .with_commit(make_commit(
+                "abc12345abc12345",
+                "Zed Zippy",
+                ZED_ZIPPY_EMAIL,
+                "Bump to 0.230.2",
+                "",
+            ))
+            .run_scenario()
+            .await;
+        assert!(matches!(
+            result,
+            Err(ReviewFailure::UnexpectedZippyAction(
+                VersionBumpFailure::NoMentionInTitle
+            ))
+        ));
+    }
+
+    #[tokio::test]
+    async fn zippy_version_bump_without_signature_fails() {
+        let result = TestScenario::zippy_version_bump()
+            .with_commit_metadata_json(serde_json::json!({
+                "abc12345abc12345": {
+                    "author": zippy_author(),
+                    "authors": { "nodes": [] },
+                    "additions": 2,
+                    "deletions": 2
+                }
+            }))
+            .run_scenario()
+            .await;
+        assert!(matches!(
+            result,
+            Err(ReviewFailure::UnexpectedZippyAction(
+                VersionBumpFailure::NotSigned
+            ))
+        ));
+    }
+
+    #[tokio::test]
+    async fn zippy_version_bump_with_invalid_signature_fails() {
+        let result = TestScenario::zippy_version_bump()
+            .with_commit_metadata_json(serde_json::json!({
+                "abc12345abc12345": {
+                    "author": zippy_author(),
+                    "authors": { "nodes": [] },
+                    "signature": {
+                        "isValid": false,
+                        "signer": { "login": ZED_ZIPPY_LOGIN }
+                    },
+                    "additions": 2,
+                    "deletions": 2
+                }
+            }))
+            .run_scenario()
+            .await;
+        assert!(matches!(
+            result,
+            Err(ReviewFailure::UnexpectedZippyAction(
+                VersionBumpFailure::InvalidSignature
+            ))
+        ));
+    }
+
+    #[tokio::test]
+    async fn zippy_version_bump_with_unequal_line_changes_fails() {
+        let result = TestScenario::zippy_version_bump()
+            .with_commit_metadata_json(serde_json::json!({
+                "abc12345abc12345": {
+                    "author": zippy_author(),
+                    "authors": { "nodes": [] },
+                    "signature": {
+                        "isValid": true,
+                        "signer": { "login": ZED_ZIPPY_LOGIN }
+                    },
+                    "additions": 5,
+                    "deletions": 2
+                }
+            }))
+            .run_scenario()
+            .await;
+        assert!(matches!(
+            result,
+            Err(ReviewFailure::UnexpectedZippyAction(
+                VersionBumpFailure::UnexpectedLineChanges { .. }
+            ))
+        ));
+    }
+
+    #[tokio::test]
+    async fn zippy_version_bump_with_wrong_github_author_fails() {
+        let result = TestScenario::zippy_version_bump()
+            .with_commit_metadata_json(serde_json::json!({
+                "abc12345abc12345": {
+                    "author": alice_author(),
+                    "authors": { "nodes": [] },
+                    "signature": {
+                        "isValid": true,
+                        "signer": { "login": "alice" }
+                    },
+                    "additions": 2,
+                    "deletions": 2
+                }
+            }))
+            .run_scenario()
+            .await;
+        assert!(matches!(
+            result,
+            Err(ReviewFailure::UnexpectedZippyAction(
+                VersionBumpFailure::AuthorMismatch
+            ))
+        ));
+    }
+
+    #[tokio::test]
+    async fn zippy_version_bump_with_co_authors_fails() {
+        let result = TestScenario::zippy_version_bump()
+            .with_commit_metadata_json(serde_json::json!({
+                "abc12345abc12345": {
+                    "author": zippy_author(),
+                    "authors": { "nodes": [alice_author()] },
+                    "signature": {
+                        "isValid": true,
+                        "signer": { "login": ZED_ZIPPY_LOGIN }
+                    },
+                    "additions": 2,
+                    "deletions": 2
+                }
+            }))
+            .run_scenario()
+            .await;
+        assert!(matches!(
+            result,
+            Err(ReviewFailure::UnexpectedZippyAction(
+                VersionBumpFailure::UnexpectedCoAuthors
+            ))
+        ));
+    }
+
+    #[tokio::test]
+    async fn non_zippy_commit_without_pr_is_no_pr_found() {
+        let result = TestScenario::single_commit()
+            .with_commit(make_commit(
+                "abc12345abc12345",
+                "Alice",
+                "alice@test.com",
+                "Some direct push",
+                "",
+            ))
+            .run_scenario()
+            .await;
+        assert!(matches!(result, Err(ReviewFailure::NoPullRequestFound)));
+    }
+
+    #[tokio::test]
+    async fn zippy_commit_with_pr_number_goes_through_normal_flow() {
+        let result = TestScenario::single_commit()
+            .with_commit(make_commit(
+                "abc12345abc12345",
+                "Zed Zippy",
+                ZED_ZIPPY_EMAIL,
+                "Some change (#1234)",
+                "",
+            ))
+            .with_reviews(vec![review("bob", ReviewState::Approved)])
+            .with_org_members(vec!["bob"])
+            .run_scenario()
+            .await;
+        assert!(matches!(result, Ok(ReviewSuccess::PullRequestReviewed(_))));
+    }
 }

tooling/compliance/src/git.rs 🔗

@@ -15,6 +15,9 @@ use regex::Regex;
 use semver::Version;
 use serde::Deserialize;
 
+pub(crate) const ZED_ZIPPY_LOGIN: &str = "zed-zippy[bot]";
+pub(crate) const ZED_ZIPPY_EMAIL: &str = "234243425+zed-zippy[bot]@users.noreply.github.com";
+
 pub trait Subcommand {
     type ParsedOutput: FromStr<Err = anyhow::Error>;
 
@@ -164,8 +167,8 @@ impl Committer {
         }
     }
 
-    pub fn name(&self) -> &str {
-        &self.name
+    pub(crate) fn is_zed_zippy(&self) -> bool {
+        self.email == ZED_ZIPPY_EMAIL
     }
 }
 
@@ -242,6 +245,17 @@ impl CommitDetails {
     pub(crate) fn sha(&self) -> &CommitSha {
         &self.sha
     }
+
+    pub(crate) fn version_bump_mention(&self) -> Option<&str> {
+        static VERSION_BUMP_REGEX: LazyLock<Regex> = LazyLock::new(|| {
+            Regex::new(r"^Bump to [0-9]+\.[0-9]+\.[0-9]+ for @([a-zA-Z0-9][a-zA-Z0-9-]*)$").unwrap()
+        });
+
+        VERSION_BUMP_REGEX
+            .captures(&self.title)
+            .and_then(|cap| cap.get(1))
+            .map(|m| m.as_str())
+    }
 }
 
 #[derive(Debug, Deref, Default, DerefMut)]
@@ -615,6 +629,68 @@ mod tests {
         assert_eq!(sha.short(), "abcdef12");
     }
 
+    #[test]
+    fn version_bump_mention_extracts_username() {
+        let line = format!(
+            "abc123{d}Zed Zippy{d}bot@test.com{d}Bump to 0.230.2 for @cole-miller",
+            d = CommitDetails::FIELD_DELIMITER
+        );
+        let commit = CommitDetails::parse(&line, "").unwrap();
+        assert_eq!(commit.version_bump_mention(), Some("cole-miller"));
+    }
+
+    #[test]
+    fn version_bump_mention_returns_none_without_mention() {
+        let line = format!(
+            "abc123{d}Alice{d}alice@test.com{d}Fix a bug",
+            d = CommitDetails::FIELD_DELIMITER
+        );
+        let commit = CommitDetails::parse(&line, "").unwrap();
+        assert!(commit.version_bump_mention().is_none());
+    }
+
+    #[test]
+    fn version_bump_mention_rejects_wrong_prefix() {
+        let line = format!(
+            "abc123{d}Zed Zippy{d}bot@test.com{d}Fix thing for @cole-miller",
+            d = CommitDetails::FIELD_DELIMITER
+        );
+        let commit = CommitDetails::parse(&line, "").unwrap();
+        assert!(commit.version_bump_mention().is_none());
+    }
+
+    #[test]
+    fn version_bump_mention_rejects_bare_mention() {
+        let line = format!(
+            "abc123{d}Zed Zippy{d}bot@test.com{d}@cole-miller bumped something",
+            d = CommitDetails::FIELD_DELIMITER
+        );
+        let commit = CommitDetails::parse(&line, "").unwrap();
+        assert!(commit.version_bump_mention().is_none());
+    }
+
+    #[test]
+    fn version_bump_mention_rejects_trailing_text() {
+        let line = format!(
+            "abc123{d}Zed Zippy{d}bot@test.com{d}Bump to 0.230.2 for @cole-miller extra",
+            d = CommitDetails::FIELD_DELIMITER
+        );
+        let commit = CommitDetails::parse(&line, "").unwrap();
+        assert!(commit.version_bump_mention().is_none());
+    }
+
+    #[test]
+    fn committer_is_zed_zippy() {
+        let committer = Committer::new("Zed Zippy", ZED_ZIPPY_EMAIL);
+        assert!(committer.is_zed_zippy());
+    }
+
+    #[test]
+    fn committer_is_not_zed_zippy() {
+        let committer = Committer::new("Alice", "alice@test.com");
+        assert!(!committer.is_zed_zippy());
+    }
+
     #[test]
     fn parse_commit_list_from_git_log_format() {
         let fd = CommitDetails::FIELD_DELIMITER;

tooling/compliance/src/github.rs 🔗

@@ -1,4 +1,4 @@
-use std::{borrow::Cow, collections::HashMap, fmt, ops::Not, sync::LazyLock};
+use std::{borrow::Cow, collections::HashMap, fmt};
 
 use anyhow::Result;
 use derive_more::Deref;
@@ -73,14 +73,6 @@ pub struct CommitAuthor {
     user: Option<GithubLogin>,
 }
 
-pub(crate) static ZED_ZIPPY_AUTHOR: LazyLock<CommitAuthor> = LazyLock::new(|| CommitAuthor {
-    name: "Zed Zippy".to_string(),
-    email: "234243425+zed-zippy[bot]@users.noreply.github.com".to_string(),
-    user: Some(GithubLogin {
-        login: "zed-zippy[bot]".to_string(),
-    }),
-});
-
 impl CommitAuthor {
     pub(crate) fn user(&self) -> Option<&GithubLogin> {
         self.user.as_ref()
@@ -105,42 +97,83 @@ impl fmt::Display for CommitAuthor {
     }
 }
 
+#[derive(Debug, Deserialize, Clone)]
+pub struct CommitSignature {
+    #[serde(rename = "isValid")]
+    is_valid: bool,
+    signer: Option<GithubLogin>,
+}
+
+impl CommitSignature {
+    pub fn is_valid(&self) -> bool {
+        self.is_valid
+    }
+
+    pub fn signer(&self) -> Option<&GithubLogin> {
+        self.signer.as_ref()
+    }
+}
+
 #[derive(Debug, Deserialize)]
-pub struct CommitAuthors {
+pub struct CommitMetadata {
     #[serde(rename = "author")]
     primary_author: CommitAuthor,
     #[serde(rename = "authors", deserialize_with = "graph_ql::deserialize_nodes")]
     co_authors: Vec<CommitAuthor>,
+    #[serde(default)]
+    signature: Option<CommitSignature>,
+    #[serde(default)]
+    additions: u64,
+    #[serde(default)]
+    deletions: u64,
 }
 
-impl CommitAuthors {
+impl CommitMetadata {
     pub fn co_authors(&self) -> Option<impl Iterator<Item = &CommitAuthor>> {
-        self.co_authors.is_empty().not().then(|| {
-            self.co_authors
-                .iter()
-                .filter(|co_author| *co_author != &self.primary_author)
-        })
+        let mut co_authors = self
+            .co_authors
+            .iter()
+            .filter(|co_author| *co_author != &self.primary_author)
+            .peekable();
+
+        co_authors.peek().is_some().then_some(co_authors)
+    }
+
+    pub fn primary_author(&self) -> &CommitAuthor {
+        &self.primary_author
+    }
+
+    pub fn signature(&self) -> Option<&CommitSignature> {
+        self.signature.as_ref()
+    }
+
+    pub fn additions(&self) -> u64 {
+        self.additions
+    }
+
+    pub fn deletions(&self) -> u64 {
+        self.deletions
     }
 }
 
 #[derive(Debug, Deref)]
-pub struct AuthorsForCommits(HashMap<CommitSha, CommitAuthors>);
+pub struct CommitMetadataBySha(HashMap<CommitSha, CommitMetadata>);
 
-impl AuthorsForCommits {
+impl CommitMetadataBySha {
     const SHA_PREFIX: &'static str = "commit";
 }
 
-impl<'de> serde::Deserialize<'de> for AuthorsForCommits {
+impl<'de> serde::Deserialize<'de> for CommitMetadataBySha {
     fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
     where
         D: serde::Deserializer<'de>,
     {
-        let raw = HashMap::<String, CommitAuthors>::deserialize(deserializer)?;
+        let raw = HashMap::<String, CommitMetadata>::deserialize(deserializer)?;
         let map = raw
             .into_iter()
             .map(|(key, value)| {
                 let sha = key
-                    .strip_prefix(AuthorsForCommits::SHA_PREFIX)
+                    .strip_prefix(CommitMetadataBySha::SHA_PREFIX)
                     .unwrap_or(&key);
                 (CommitSha::new(sha.to_owned()), value)
             })
@@ -200,11 +233,11 @@ pub trait GithubApiClient {
         repo: &Repository<'_>,
         pr_number: u64,
     ) -> Result<Vec<PullRequestComment>>;
-    async fn get_commit_authors(
+    async fn get_commit_metadata(
         &self,
         repo: &Repository<'_>,
         commit_shas: &[&CommitSha],
-    ) -> Result<AuthorsForCommits>;
+    ) -> Result<CommitMetadataBySha>;
     async fn check_repo_write_permission(
         &self,
         repo: &Repository<'_>,
@@ -225,7 +258,7 @@ pub mod graph_ql {
 
     use crate::git::CommitSha;
 
-    use super::AuthorsForCommits;
+    use super::CommitMetadataBySha;
 
     #[derive(Debug, Deserialize)]
     pub struct GraphQLResponse<T> {
@@ -252,8 +285,8 @@ pub mod graph_ql {
     }
 
     #[derive(Debug, Deserialize)]
-    pub struct CommitAuthorsResponse {
-        pub repository: AuthorsForCommits,
+    pub struct CommitMetadataResponse {
+        pub repository: CommitMetadataBySha,
     }
 
     pub fn deserialize_nodes<'de, T, D>(deserializer: D) -> std::result::Result<Vec<T>, D::Error>
@@ -268,7 +301,7 @@ pub mod graph_ql {
         Nodes::<T>::deserialize(deserializer).map(|wrapper| wrapper.nodes)
     }
 
-    pub fn build_co_authors_query<'a>(
+    pub fn build_commit_metadata_query<'a>(
         org: &str,
         repo: &str,
         shas: impl IntoIterator<Item = &'a CommitSha>,
@@ -287,6 +320,12 @@ pub mod graph_ql {
                         user { login }
                     }
                 }
+                signature {
+                    isValid
+                    signer { login }
+                }
+                additions
+                deletions
             }
         "#;
 
@@ -295,7 +334,7 @@ pub mod graph_ql {
             .map(|commit_sha| {
                 format!(
                     "{sha_prefix}{sha}: object(oid: \"{sha}\") {{ {FRAGMENT} }}",
-                    sha_prefix = AuthorsForCommits::SHA_PREFIX,
+                    sha_prefix = CommitMetadataBySha::SHA_PREFIX,
                     sha = **commit_sha,
                 )
             })
@@ -324,7 +363,7 @@ mod octo_client {
     };
 
     use super::{
-        AuthorsForCommits, GithubApiClient, GithubLogin, GithubUser, PullRequestComment,
+        CommitMetadataBySha, GithubApiClient, GithubLogin, GithubUser, PullRequestComment,
         PullRequestData, PullRequestReview, ReviewState,
     };
 
@@ -472,18 +511,18 @@ mod octo_client {
                 .collect())
         }
 
-        async fn get_commit_authors(
+        async fn get_commit_metadata(
             &self,
             repo: &Repository<'_>,
             commit_shas: &[&CommitSha],
-        ) -> Result<AuthorsForCommits> {
-            let query = graph_ql::build_co_authors_query(
+        ) -> Result<CommitMetadataBySha> {
+            let query = graph_ql::build_commit_metadata_query(
                 repo.owner.as_ref(),
                 repo.name.as_ref(),
                 commit_shas.iter().copied(),
             );
             let query = serde_json::json!({ "query": query });
-            self.graphql::<graph_ql::CommitAuthorsResponse>(&query)
+            self.graphql::<graph_ql::CommitMetadataResponse>(&query)
                 .await
                 .map(|response| response.repository)
         }

tooling/compliance/src/report.rs 🔗

@@ -93,7 +93,9 @@ impl ReportSummary {
                 .filter(|entry| {
                     matches!(
                         entry.reason,
-                        Err(ReviewFailure::NoPullRequestFound | ReviewFailure::Unreviewed)
+                        Err(ReviewFailure::NoPullRequestFound
+                            | ReviewFailure::Unreviewed
+                            | ReviewFailure::UnexpectedZippyAction(_))
                     )
                 })
                 .count(),