Retry transient GitHub reads before failing a Pass

src/github.rs

@@ -3,6 +3,7 @@
 
 use std::fmt;
 use std::process::{Command, Output};
+use std::time::Duration;
 
 use anyhow::{Context, Result, bail};
 use chrono::{DateTime, Utc};
@@ -669,8 +670,11 @@ impl GitHub {
     /// here.
     pub fn checks_on(&self, issue: &IssueUrl, sha: &str) -> Result<Vec<Check>> {
         let commit = format!("repos/{}/commits/{sha}", issue.repo_slug());
-        let runs = self.gh_api_items(&format!("{commit}/check-runs?per_page=100"), "check_runs")?;
-        let statuses = self.gh_api_items(&format!("{commit}/status?per_page=100"), "statuses")?;
+        // CI retries the complete observation, not its individual requests.
+        let runs =
+            self.gh_api_items_once(&format!("{commit}/check-runs?per_page=100"), "check_runs")?;
+        let statuses =
+            self.gh_api_items_once(&format!("{commit}/status?per_page=100"), "statuses")?;
         let url = |value: &Value| {
             value
                 .as_str()
@@ -749,11 +753,18 @@ impl GitHub {
     /// Every item of the `field` list of the GitHub API's `path`, across all its
     /// pages.
     fn gh_api_items(&self, path: &str, field: &str) -> Result<Vec<Value>> {
+        self.retry_read(&format!("gh api {path}"), || {
+            self.gh_api_items_once(path, field)
+        })
+    }
+
+    fn gh_api_items_once(&self, path: &str, field: &str) -> Result<Vec<Value>> {
         let output = self.output(&["api", "--paginate", "--jq", &format!(".{field}[]"), path])?;
         if !output.status.success() {
-            bail!(check_read::ApiFailure {
-                path: path.to_string(),
+            bail!(check_read::ReadFailure {
+                command: format!("gh api {path}"),
                 stderr: String::from_utf8_lossy(&output.stderr).trim().to_string(),
+                private_status: None,
             });
         }
         serde_json::Deserializer::from_slice(&output.stdout)
@@ -762,16 +773,43 @@ impl GitHub {
             .with_context(|| format!("gh api {path} returned invalid JSON"))
     }
 
-    /// Run `gh <args>` and parse its stdout as JSON, failing with gh's stderr if
-    /// it exits non-zero.
+    /// Read-only JSON commands get three attempts, separated by one then two
+    /// seconds, for transient failures only. Mutations use `gh_stdout` or
+    /// `output_with_input` instead.
     fn gh_json(&self, args: &[&str]) -> Result<Value> {
         let command = format!("gh {}", args[..2].join(" "));
+        self.retry_read(&command, || self.gh_json_once(args, &command))
+    }
+
+    fn retry_read<T>(&self, command: &str, mut read: impl FnMut() -> Result<T>) -> Result<T> {
+        for attempt in 1..=3 {
+            let error = match read() {
+                Ok(value) => return Ok(value),
+                Err(error) => error,
+            };
+            if attempt == 3 || !check_read::is_transient(&error) {
+                return Err(error);
+            }
+            crate::progress::step(format!(
+                "{command} failed (attempt {attempt}/3): {error:#}; retrying in {attempt}s"
+            ));
+            let delay = Duration::from_secs(attempt);
+            match self.interruption {
+                Interruption::Ordinary => crate::poll::sleep(delay)?,
+                Interruption::Completion => std::thread::sleep(delay),
+            }
+        }
+        unreachable!("the third attempt returns success or the last cause")
+    }
+
+    fn gh_json_once(&self, args: &[&str], command: &str) -> Result<Value> {
         let output = self.output(args)?;
         if !output.status.success() {
-            bail!(
-                "{command} failed: {}",
-                String::from_utf8_lossy(&output.stderr).trim()
-            );
+            bail!(check_read::ReadFailure {
+                command: command.to_string(),
+                stderr: String::from_utf8_lossy(&output.stderr).trim().to_string(),
+                private_status: None,
+            });
         }
         serde_json::from_slice(&output.stdout)
             .with_context(|| format!("{command} returned invalid JSON"))

src/github/advisories.rs

@@ -5,7 +5,7 @@ use anyhow::{Context, Result, bail};
 use serde::Serialize;
 use serde_json::{Value, json};
 
-use super::GitHub;
+use super::{GitHub, check_read};
 use crate::issue::IssueUrl;
 use crate::labels::{Label, NEEDS_TRIAGE};
 use crate::security::reproduction::{FixSize, Reproduction, Severity};
@@ -447,15 +447,8 @@ impl GitHub {
             ),
             RecordData::Issue { number, .. } => (format!("repos/{repo}/issues/{number}"), "body"),
         };
-        let output = self.output_with_input(&["api", &path], None)?;
-        if !output.status.success() {
-            bail!(
-                "reading the private record before linking its fix failed ({})",
-                output.status
-            );
-        }
-        let current: Value =
-            serde_json::from_slice(&output.stdout).context("private record is invalid JSON")?;
+        let current =
+            self.private_record(&path, "reading the private record before linking its fix")?;
         let current = current[field]
             .as_str()
             .context("the private record has no description")?;
@@ -477,6 +470,23 @@ impl GitHub {
         Ok(())
     }
 
+    /// Retry only the observation before a private record's mutation. Raw
+    /// diagnostics stay available to classification, never to progress/errors.
+    fn private_record(&self, path: &str, operation: &str) -> Result<Value> {
+        self.retry_read(operation, || {
+            let output = self.output(&["api", path])?;
+            if !output.status.success() {
+                bail!(check_read::ReadFailure {
+                    command: operation.to_string(),
+                    stderr: String::from_utf8_lossy(&output.stderr).trim().to_string(),
+                    private_status: Some(output.status),
+                });
+            }
+            serde_json::from_slice(&output.stdout)
+                .with_context(|| format!("{operation} returned invalid JSON"))
+        })
+    }
+
     pub fn security_fix_text(&self, issue: &IssueUrl) -> Result<String> {
         let value = self.issue_view(issue, "title,body")?;
         let title = value["title"]
@@ -599,15 +609,10 @@ impl GitHub {
                 SecurityRecords::issues(Vec::new()),
             ),
         };
-        let observed = self.output_with_input(&["api", &path], None)?;
-        if !observed.status.success() {
-            bail!(
-                "reading a private Security finding record before its update failed ({})",
-                observed.status
-            );
-        }
-        let observed: Value = serde_json::from_slice(&observed.stdout)
-            .context("reading a private Security finding record returned invalid JSON")?;
+        let observed = self.private_record(
+            &path,
+            "reading a private Security finding record before its update",
+        )?;
         let current = storage.decode_state(&observed)?;
         let (description, untriaged) = match &current {
             RecordData::Advisory {

src/github/check_read.rs

@@ -1,31 +1,47 @@
-//! Classification for the CI watch's read-only recovery policy. Unknown
+//! Classification for GitHub read-only recovery policies. Unknown
 //! failures remain causal errors, never successful empty observations.
 
 use std::fmt;
+use std::process::ExitStatus;
 
 /// A non-success API response, retaining gh's original diagnostics. This
-/// carries evidence only: the CI watch owns whether and when to retry it.
+/// carries evidence only: the caller owns whether and when to retry it.
 #[derive(Debug)]
-pub(super) struct ApiFailure {
-    pub path: String,
+pub(super) struct ReadFailure {
+    pub command: String,
     pub stderr: String,
+    /// Private record diagnostics may contain evidence. Classify the raw
+    /// stderr, but expose only this status in errors and retry progress.
+    pub private_status: Option<ExitStatus>,
 }
 
-impl fmt::Display for ApiFailure {
+impl fmt::Display for ReadFailure {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "gh api {} failed: {}", self.path, self.stderr)
+        match self.private_status {
+            Some(status) => write!(f, "{} failed ({status})", self.command),
+            None => write!(f, "{} failed: {}", self.command, self.stderr),
+        }
     }
 }
 
-impl std::error::Error for ApiFailure {}
+impl std::error::Error for ReadFailure {}
 
 pub(crate) fn is_transient(error: &anyhow::Error) -> bool {
     if let Some(error) = error.downcast_ref::<serde_json::Error>() {
         return error.is_eof();
     }
-    let Some(failure) = error.downcast_ref::<ApiFailure>() else {
+    let Some(failure) = error.downcast_ref::<ReadFailure>() else {
         return false;
     };
+    let stderr = failure.stderr.trim().to_ascii_lowercase();
+    // Quota exhaustion belongs to quota-aware deferral, not a short retry.
+    // gh can report GraphQL quota errors with or without an HTTP status.
+    if stderr.contains("api rate limit")
+        || stderr.contains("rate_limited")
+        || stderr.contains("rate_limit_exceeded")
+    {
+        return false;
+    }
     // gh reports HTTP status in stderr. Prefer it to incidental prose, so
     // an authentication or validation failure mentioning EOF stays permanent.
     if let Some(status) = failure
@@ -36,7 +52,6 @@ pub(crate) fn is_transient(error: &anyhow::Error) -> bool {
     {
         return status == 429 || (500..600).contains(&status);
     }
-    let stderr = failure.stderr.trim().to_ascii_lowercase();
     if stderr == "eof" || stderr.ends_with(": eof") {
         return true;
     }