Detailed changes
@@ -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,
@@ -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))
}
}
@@ -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(
@@ -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)]
@@ -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!(