compliance: Catch all Zippy version bumps on stable (#54525)

I checked the temporary fix against the 0.233.2 bump - however, some
other guy fixed (and thus changed) the commit authors from 0.233.3. Or,
to put it differently:

<img width="356" height="250" alt="image"
src="https://github.com/user-attachments/assets/d053c1cc-efc3-462e-a0a5-45b999eec9f6"
/>

Anyway, this fixes the temporary fix and also cherry-picks the singular
checking support to stable so that should something else come up, we can
look at that.


Release Notes:

- N/A

.github/workflows/compliance_check.yml

@@ -36,7 +36,7 @@ jobs:
     - id: run-compliance-check
       name: release::add_compliance_steps::run_compliance_check
       run: |
-        cargo xtask compliance "$LATEST_TAG" --branch main --report-path "compliance-report-${GITHUB_REF_NAME}.md"
+        cargo xtask compliance version "$LATEST_TAG" --branch main --report-path "compliance-report-${GITHUB_REF_NAME}.md"
       env:
         GITHUB_APP_ID: ${{ secrets.ZED_ZIPPY_APP_ID }}
         GITHUB_APP_KEY: ${{ secrets.ZED_ZIPPY_APP_PRIVATE_KEY }}

.github/workflows/release.yml

@@ -309,7 +309,7 @@ jobs:
     - id: run-compliance-check
       name: release::add_compliance_steps::run_compliance_check
       run: |
-        cargo xtask compliance "$GITHUB_REF_NAME" --report-path "compliance-report-${GITHUB_REF_NAME}.md"
+        cargo xtask compliance version "$GITHUB_REF_NAME" --report-path "compliance-report-${GITHUB_REF_NAME}.md"
       env:
         GITHUB_APP_ID: ${{ secrets.ZED_ZIPPY_APP_ID }}
         GITHUB_APP_KEY: ${{ secrets.ZED_ZIPPY_APP_PRIVATE_KEY }}
@@ -678,7 +678,7 @@ jobs:
     - id: run-compliance-check
       name: release::add_compliance_steps::run_compliance_check
       run: |
-        cargo xtask compliance "$GITHUB_REF_NAME" --report-path "compliance-report-${GITHUB_REF_NAME}.md"
+        cargo xtask compliance version "$GITHUB_REF_NAME" --report-path "compliance-report-${GITHUB_REF_NAME}.md"
       env:
         GITHUB_APP_ID: ${{ secrets.ZED_ZIPPY_APP_ID }}
         GITHUB_APP_KEY: ${{ secrets.ZED_ZIPPY_APP_PRIVATE_KEY }}

tooling/compliance/src/checks.rs

@@ -1,11 +1,11 @@
-use std::{fmt, ops::Not as _};
+use std::{fmt, ops::Not as _, rc::Rc};
 
 use itertools::Itertools as _;
 
 use crate::{
     git::{CommitDetails, CommitList},
     github::{
-        CommitAuthor, GithubClient, GithubLogin, PullRequestComment, PullRequestData,
+        CommitAuthor, GithubApiClient, GithubLogin, PullRequestComment, PullRequestData,
         PullRequestReview, Repository, ReviewState, ZED_ZIPPY_AUTHOR,
     },
     report::Report,
@@ -87,26 +87,37 @@ impl<E: Into<anyhow::Error>> From<E> for ReviewFailure {
     }
 }
 
-pub struct Reporter<'a> {
+pub struct Reporter {
     commits: CommitList,
-    github_client: &'a GithubClient,
+    github_client: Rc<dyn GithubApiClient>,
 }
 
-impl<'a> Reporter<'a> {
+impl Reporter {
-    pub fn new(commits: CommitList, github_client: &'a GithubClient) -> Self {
+    pub fn new(commits: CommitList, github_client: Rc<dyn GithubApiClient>) -> Self {
         Self {
             commits,
             github_client,
         }
     }
 
+    pub async fn result_for_commit(
+        commit: CommitDetails,
+        github_client: Rc<dyn GithubApiClient>,
+    ) -> ReviewResult {
+        Self::new(Default::default(), github_client)
+            .check_commit(&commit)
+            .await
+    }
+
     /// Method that checks every commit for compliance
     pub async fn check_commit(
         &self,
         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")
+            let author_name = commit.author().name();
+            if (author_name.contains("Zed Zippy") || author_name.contains("zed-zippy[bot]"))
+                && commit.title().starts_with("Bump to")
             {
                 return Ok(ReviewSuccess::CoAuthored(vec![ZED_ZIPPY_AUTHOR.clone()]));
             }
@@ -297,8 +308,8 @@ mod tests {
 
     use crate::git::{CommitDetails, CommitList, CommitSha};
     use crate::github::{
-        AuthorsForCommits, GithubApiClient, GithubClient, GithubLogin, GithubUser,
+        AuthorsForCommits, GithubApiClient, GithubLogin, GithubUser, PullRequestComment,
-        PullRequestComment, PullRequestData, PullRequestReview, Repository, ReviewState,
+        PullRequestData, PullRequestReview, Repository, ReviewState,
     };
 
     use super::{Reporter, ReviewFailure, ReviewSuccess};
@@ -470,8 +481,8 @@ mod tests {
                 commit_authors_json: self.commit_authors_json,
                 org_members: self.org_members,
             };
-            let client = GithubClient::new(Rc::new(mock));
+            let client = Rc::new(mock);
-            let reporter = Reporter::new(CommitList::default(), &client);
+            let reporter = Reporter::new(CommitList::default(), client);
             reporter.check_commit(&self.commit).await
         }
     }

tooling/compliance/src/git.rs

@@ -138,6 +138,18 @@ impl CommitDetails {
     }
 }
 
+impl FromStr for CommitDetails {
+    type Err = anyhow::Error;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        CommitList::from_str(s).and_then(|list| {
+            list.into_iter()
+                .next()
+                .ok_or_else(|| anyhow!("No commit found"))
+        })
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Committer {
     name: String,
@@ -322,6 +334,30 @@ impl FromStr for VersionTagList {
     }
 }
 
+pub struct InfoForCommit {
+    sha: String,
+}
+
+impl InfoForCommit {
+    pub fn new(sha: impl ToString) -> Self {
+        Self {
+            sha: sha.to_string(),
+        }
+    }
+}
+
+impl Subcommand for InfoForCommit {
+    type ParsedOutput = CommitDetails;
+
+    fn args(&self) -> impl IntoIterator<Item = String> {
+        [
+            "log".to_string(),
+            format!("--pretty=format:{}", CommitDetails::FORMAT_STRING),
+            format!("{sha}~1..{sha}", sha = self.sha),
+        ]
+    }
+}
+
 pub struct CommitsFromVersionToVersion {
     version_tag: VersionTag,
     branch: String,

tooling/compliance/src/github.rs

@@ -1,4 +1,4 @@
-use std::{borrow::Cow, collections::HashMap, fmt, ops::Not, rc::Rc, sync::LazyLock};
+use std::{borrow::Cow, collections::HashMap, fmt, ops::Not, sync::LazyLock};
 
 use anyhow::Result;
 use derive_more::Deref;
@@ -218,23 +218,6 @@ pub trait GithubApiClient {
     ) -> Result<()>;
 }
 
-#[derive(Deref)]
-pub struct GithubClient {
-    api: Rc<dyn GithubApiClient>,
-}
-
-impl GithubClient {
-    pub fn new(api: Rc<dyn GithubApiClient>) -> Self {
-        Self { api }
-    }
-
-    #[cfg(feature = "octo-client")]
-    pub async fn for_app_in_repo(app_id: u64, app_private_key: &str, org: &str) -> Result<Self> {
-        let client = OctocrabClient::new(app_id, app_private_key, org).await?;
-        Ok(Self::new(Rc::new(client)))
-    }
-}
-
 pub mod graph_ql {
     use anyhow::{Context as _, Result};
     use itertools::Itertools as _;

tooling/xtask/src/tasks/compliance.rs

@@ -1,20 +1,37 @@
-use std::path::PathBuf;
+use std::{path::PathBuf, rc::Rc};
 
 use anyhow::{Context, Result};
-use clap::Parser;
+use clap::{Parser, Subcommand};
 
 use compliance::{
     checks::Reporter,
-    git::{CommitsFromVersionToVersion, GetVersionTags, GitCommand, VersionTag},
+    git::{CommitsFromVersionToVersion, GetVersionTags, GitCommand, InfoForCommit, VersionTag},
-    github::{GithubClient, Repository},
+    github::{GithubApiClient as _, OctocrabClient, Repository},
     report::ReportReviewSummary,
 };
 
 #[derive(Parser)]
-pub struct ComplianceArgs {
+pub(crate) struct ComplianceArgs {
+    #[clap(subcommand)]
+    mode: ComplianceMode,
+}
+
+#[derive(Subcommand)]
+pub(crate) enum ComplianceMode {
+    // Check compliance for all commits between two version tags
+    Version(VersionArgs),
+    // Check compliance for a single commit
+    Single {
+        // The full commit SHA to check
+        commit_sha: String,
+    },
+}
+
+#[derive(Parser)]
+pub(crate) struct VersionArgs {
     #[arg(value_parser = VersionTag::parse)]
     // The version to be on the lookout for
-    pub(crate) version_tag: VersionTag,
+    version_tag: VersionTag,
     #[arg(long)]
     // The markdown file to write the compliance report to
     report_path: PathBuf,
@@ -23,7 +40,7 @@ pub struct ComplianceArgs {
     branch: Option<String>,
 }
 
-impl ComplianceArgs {
+impl VersionArgs {
     pub(crate) fn version_tag(&self) -> &VersionTag {
         &self.version_tag
     }
@@ -39,6 +56,35 @@ async fn check_compliance_impl(args: ComplianceArgs) -> Result<()> {
     let app_id = std::env::var("GITHUB_APP_ID").context("Missing GITHUB_APP_ID")?;
     let key = std::env::var("GITHUB_APP_KEY").context("Missing GITHUB_APP_KEY")?;
 
+    let client = Rc::new(
+        OctocrabClient::new(
+            app_id.parse().context("Failed to parse app ID as int")?,
+            key.as_ref(),
+            Repository::ZED.owner(),
+        )
+        .await?,
+    );
+
+    println!("Initialized GitHub client for app ID {app_id}");
+
+    let args = match args.mode {
+        ComplianceMode::Version(version) => version,
+        ComplianceMode::Single { commit_sha } => {
+            let commit = GitCommand::run(InfoForCommit::new(&commit_sha))?;
+
+            return match Reporter::result_for_commit(commit, client).await {
+                Ok(review_success) => {
+                    println!("Check for commit {commit_sha} succeeded. Result: {review_success}",);
+                    Ok(())
+                }
+
+                Err(review_failure) => Err(anyhow::anyhow!(
+                    "Check for commit {commit_sha} failed. Result: {review_failure}"
+                )),
+            };
+        }
+    };
+
     let tag = args.version_tag();
 
     let previous_version = GitCommand::run(GetVersionTags)?
@@ -69,16 +115,9 @@ async fn check_compliance_impl(args: ComplianceArgs) -> Result<()> {
 
     println!("Checking commit range {range}, {} total", commits.len());
 
-    let client = GithubClient::for_app_in_repo(
+    let report = Reporter::new(commits, client.clone())
-        app_id.parse().context("Failed to parse app ID as int")?,
+        .generate_report()
-        key.as_ref(),
+        .await?;
-        Repository::ZED.owner(),
-    )
-    .await?;
-
-    println!("Initialized GitHub client for app ID {app_id}");
-
-    let report = Reporter::new(commits, &client).generate_report().await?;
 
     println!(
         "Generated report for version {}",

tooling/xtask/src/tasks/workflows/release.rs

@@ -185,7 +185,7 @@ pub(crate) fn add_compliance_steps(
     fn run_compliance_check(context: &ComplianceContext) -> (Step<Run>, StepOutput) {
         let job = named::bash(
             formatdoc! {r#"
-                cargo xtask compliance {target} --report-path "{COMPLIANCE_REPORT_PATH}"
+                cargo xtask compliance version {target} --report-path "{COMPLIANCE_REPORT_PATH}"
                 "#,
                 target = if context.tag_source().is_some() { r#""$LATEST_TAG" --branch main"# } else { r#""$GITHUB_REF_NAME""# },
             }