compliance: Check repository status for actor (#53343)

Also includes two other minor fixes for stdout info prints

Release Notes:

- N/A

tooling/compliance/src/checks.rs

@@ -146,7 +146,7 @@ impl<'a> Reporter<'a> {
         if commit.co_authors().is_some()
             && let Some(commit_authors) = self
                 .github_client
-                .get_commit_authors([commit.sha()])
+                .get_commit_authors(&[commit.sha()])
                 .await?
                 .get(commit.sha())
                 .and_then(|authors| authors.co_authors())
@@ -156,7 +156,7 @@ impl<'a> Reporter<'a> {
                 if let Some(github_login) = co_author.user()
                     && self
                         .github_client
-                        .check_org_membership(github_login)
+                        .actor_has_repository_write_permission(github_login)
                         .await?
                 {
                     org_co_authors.push(co_author.clone());
@@ -180,7 +180,7 @@ impl<'a> Reporter<'a> {
         if let Some(user) = pull_request.user
             && self
                 .github_client
-                .check_org_membership(&GithubLogin::new(user.login))
+                .actor_has_repository_write_permission(&GithubLogin::new(user.login))
                 .await?
                 .not()
         {
@@ -219,7 +219,9 @@ impl<'a> Reporter<'a> {
                         .is_some_and(|state| state == ReviewState::Approved)
                     && self
                         .github_client
-                        .check_org_membership(&GithubLogin::new(github_login.login.clone()))
+                        .actor_has_repository_write_permission(&GithubLogin::new(
+                            github_login.login.clone(),
+                        ))
                         .await?
                 {
                     org_approving_reviews.push(review);
@@ -258,7 +260,9 @@ impl<'a> Reporter<'a> {
                     })
                     && self
                         .github_client
-                        .check_org_membership(&GithubLogin::new(comment.user.login.clone()))
+                        .actor_has_repository_write_permission(&GithubLogin::new(
+                            comment.user.login.clone(),
+                        ))
                         .await?
                 {
                     org_approving_comments.push(comment);
@@ -356,6 +360,10 @@ mod tests {
                 .any(|member| member == login.as_str()))
         }
 
+        async fn check_repo_write_permission(&self, _login: &GithubLogin) -> anyhow::Result<bool> {
+            Ok(false)
+        }
+
         async fn ensure_pull_request_has_label(
             &self,
             _label: &str,

tooling/compliance/src/git.rs

@@ -221,7 +221,7 @@ impl CommitList {
         self.0
             .first()
             .zip(self.0.last())
-            .map(|(first, last)| format!("{}..{}", first.sha().0, last.sha().0))
+            .map(|(first, last)| format!("{}..{}", last.sha().0, first.sha().0))
     }
 }
 

tooling/compliance/src/github.rs

@@ -44,7 +44,7 @@ pub struct GithubLogin {
 }
 
 impl GithubLogin {
-    pub(crate) fn new(login: String) -> Self {
+    pub fn new(login: String) -> Self {
         Self { login }
     }
 }
@@ -114,9 +114,18 @@ pub trait GitHubApiClient {
     async fn get_pull_request_comments(&self, pr_number: u64) -> Result<Vec<PullRequestComment>>;
     async fn get_commit_authors(&self, commit_shas: &[&CommitSha]) -> Result<AuthorsForCommits>;
     async fn check_org_membership(&self, login: &GithubLogin) -> Result<bool>;
+    async fn check_repo_write_permission(&self, login: &GithubLogin) -> Result<bool>;
+    async fn actor_has_repository_write_permission(
+        &self,
+        login: &GithubLogin,
+    ) -> anyhow::Result<bool> {
+        Ok(self.check_org_membership(login).await?
+            || self.check_repo_write_permission(login).await?)
+    }
     async fn ensure_pull_request_has_label(&self, label: &str, pr_number: u64) -> Result<()>;
 }
 
+#[derive(Deref)]
 pub struct GitHubClient {
     api: Rc<dyn GitHubApiClient>,
 }
@@ -131,39 +140,6 @@ impl GitHubClient {
         let client = OctocrabClient::new(app_id, app_private_key).await?;
         Ok(Self::new(Rc::new(client)))
     }
-
-    pub async fn get_pull_request(&self, pr_number: u64) -> Result<PullRequestData> {
-        self.api.get_pull_request(pr_number).await
-    }
-
-    pub async fn get_pull_request_reviews(&self, pr_number: u64) -> Result<Vec<PullRequestReview>> {
-        self.api.get_pull_request_reviews(pr_number).await
-    }
-
-    pub async fn get_pull_request_comments(
-        &self,
-        pr_number: u64,
-    ) -> Result<Vec<PullRequestComment>> {
-        self.api.get_pull_request_comments(pr_number).await
-    }
-
-    pub async fn get_commit_authors<'a>(
-        &self,
-        commit_shas: impl IntoIterator<Item = &'a CommitSha>,
-    ) -> Result<AuthorsForCommits> {
-        let shas: Vec<&CommitSha> = commit_shas.into_iter().collect();
-        self.api.get_commit_authors(&shas).await
-    }
-
-    pub async fn check_org_membership(&self, login: &GithubLogin) -> Result<bool> {
-        self.api.check_org_membership(login).await
-    }
-
-    pub async fn add_label_to_pull_request(&self, label: &str, pr_number: u64) -> Result<()> {
-        self.api
-            .ensure_pull_request_has_label(label, pr_number)
-            .await
-    }
 }
 
 #[cfg(feature = "octo-client")]
@@ -395,6 +371,44 @@ mod octo_client {
                 .any(|member| member.login == login.as_str()))
         }
 
+        async fn check_repo_write_permission(&self, login: &GithubLogin) -> Result<bool> {
+            // TODO: octocrab fails to deserialize the permission response and
+            // does not adhere to the scheme laid out at
+            // https://docs.github.com/en/rest/collaborators/collaborators?apiVersion=2026-03-10#get-repository-permissions-for-a-user
+
+            #[derive(serde::Deserialize)]
+            #[serde(rename_all = "lowercase")]
+            enum RepoPermission {
+                Admin,
+                Write,
+                Read,
+                #[serde(other)]
+                Other,
+            }
+
+            #[derive(serde::Deserialize)]
+            struct RepositoryPermissions {
+                permission: RepoPermission,
+            }
+
+            self.client
+                .get::<RepositoryPermissions, _, _>(
+                    format!(
+                        "/repos/{ORG}/{REPO}/collaborators/{user}/permission",
+                        user = login.as_str()
+                    ),
+                    None::<&()>,
+                )
+                .await
+                .map(|response| {
+                    matches!(
+                        response.permission,
+                        RepoPermission::Write | RepoPermission::Admin
+                    )
+                })
+                .map_err(Into::into)
+        }
+
         async fn ensure_pull_request_has_label(&self, label: &str, pr_number: u64) -> Result<()> {
             if self
                 .get_filtered(

tooling/compliance/src/report.rs

@@ -115,6 +115,10 @@ impl ReportSummary {
     fn has_errors(&self) -> bool {
         self.errors > 0
     }
+
+    pub fn prs_with_errors(&self) -> usize {
+        self.pull_requests - self.reviewed
+    }
 }
 
 #[derive(Clone, Copy, Debug, Display, PartialEq, Eq, PartialOrd, Ord)]

tooling/xtask/src/tasks/compliance.rs

@@ -92,7 +92,7 @@ async fn check_compliance_impl(args: ComplianceArgs) -> Result<()> {
 
     println!(
         "Applying compliance labels to {} pull requests",
-        summary.pull_requests
+        summary.prs_with_errors()
     );
 
     for report in report.errors() {
@@ -100,16 +100,14 @@ async fn check_compliance_impl(args: ComplianceArgs) -> Result<()> {
             println!("Adding review label to PR {}...", pr_number);
 
             client
-                .add_label_to_pull_request(compliance::github::PR_REVIEW_LABEL, pr_number)
+                .ensure_pull_request_has_label(compliance::github::PR_REVIEW_LABEL, pr_number)
                 .await?;
         }
     }
 
-    let report_path = args.report_path.with_extension("md");
+    report.write_markdown(&args.report_path)?;
 
-    report.write_markdown(&report_path)?;
-
-    println!("Wrote compliance report to {}", report_path.display());
+    println!("Wrote compliance report to {}", args.report_path.display());
 
     match summary.review_summary() {
         ReportReviewSummary::MissingReviews => Err(anyhow::anyhow!(