From 4d6311749a24d31dfacdd0462e9c0910adfc4b8f Mon Sep 17 00:00:00 2001 From: Finn Evers Date: Wed, 22 Apr 2026 01:17:21 +0200 Subject: [PATCH] compliance: Allow Zippy version bumps (#54342) 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 --- 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(-) diff --git a/tooling/compliance/src/checks.rs b/tooling/compliance/src/checks.rs index 8db325d75b0c86c50f9935cdef3e13b2a1a27c47..6d5116b4a8c5264c4ec09688932c5d8cd3ac978d 100644 --- a/tooling/compliance/src/checks.rs +++ b/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), CoAuthored(Vec), PullRequestReviewed(Vec), + 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; impl> From for ReviewFailure { @@ -115,11 +161,11 @@ impl Reporter { commit: &CommitDetails, ) -> Result { 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 { + 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, comments: Vec, - commit_authors_json: serde_json::Value, + commit_metadata_json: serde_json::Value, org_members: Vec, } @@ -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 { - serde_json::from_value(self.commit_authors_json.clone()).map_err(Into::into) + ) -> anyhow::Result { + 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, comments: Vec, - commit_authors_json: serde_json::Value, + commit_metadata_json: serde_json::Value, org_members: Vec, 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 { 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(_)))); + } } diff --git a/tooling/compliance/src/git.rs b/tooling/compliance/src/git.rs index eb26f3540618317a38a7bbea2f9734b4b88da8f9..08adcb4b1376b0c40366cbaa374beb28d35c180b 100644 --- a/tooling/compliance/src/git.rs +++ b/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; @@ -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 = 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; diff --git a/tooling/compliance/src/github.rs b/tooling/compliance/src/github.rs index 42c32592d088581d5f7e6db069e731637f7f86e0..d6ebcc227d758c9de000c25d5d7a9e51b2c7ad2f 100644 --- a/tooling/compliance/src/github.rs +++ b/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, } -pub(crate) static ZED_ZIPPY_AUTHOR: LazyLock = 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, +} + +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, + #[serde(default)] + signature: Option, + #[serde(default)] + additions: u64, + #[serde(default)] + deletions: u64, } -impl CommitAuthors { +impl CommitMetadata { pub fn co_authors(&self) -> Option> { - 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); +pub struct CommitMetadataBySha(HashMap); -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(deserializer: D) -> std::result::Result where D: serde::Deserializer<'de>, { - let raw = HashMap::::deserialize(deserializer)?; + let raw = HashMap::::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>; - async fn get_commit_authors( + async fn get_commit_metadata( &self, repo: &Repository<'_>, commit_shas: &[&CommitSha], - ) -> Result; + ) -> Result; 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 { @@ -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, D::Error> @@ -268,7 +301,7 @@ pub mod graph_ql { Nodes::::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, @@ -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 { - let query = graph_ql::build_co_authors_query( + ) -> Result { + 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::(&query) + self.graphql::(&query) .await .map(|response| response.repository) } diff --git a/tooling/compliance/src/report.rs b/tooling/compliance/src/report.rs index 1f99a2f061da8d6a52ae459e91170b6470b1c36a..50a02c49459d9f653b63b7703d030a8908cb82f3 100644 --- a/tooling/compliance/src/report.rs +++ b/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(),