Share machine-local GitHub Quota waits with the Inbox

GLOSSARY.md

@@ -107,6 +107,9 @@ _Avoid_: invocation, job
 A **Command** that stopped at a plan-limit warning or rejection, with outcome `paused`: an **Experiment** or **Scoring run**. Its **Command record** keeps the stopping Harness and window, utilization and reset time when reported, and the command that resumes it. It waits in **Needs you** as Paused until a later Command resumes the same work or the Day shift makes a **Dismissal**. Resuming continues the same work with finished Trials and Scores kept and nothing finished repeated. This is separate from interruption, is neither success nor failure, and does not resume automatically.
 _Avoid_: failed Command (for a plan-limit pause)
 
+**Quota wait**:
+The time until GitHub's quota for a GitHub user resets, during which **Passes** are skipped and running **Commands** wait instead of failing.
+
 **Ticket run**:
 One invocation of the factory on a single issue that is not a **Spec**, from launch to cleanup, normally ending in one pull request ready for review, or a **Self-merge** when its command or the **User config** asks for one. A Ticket run on an issue labelled `bug` reproduces the bug before fixing it, in a fresh Ticket run or a **Continuation**, except for **Flaky-test Tickets**, Security fixes and **Base fixes**, which have their own flow. A Continuation whose **Issue branch** has commits beyond the **Base branch** reproduces at their merge base, then checks the fix on the branch; a fix already there still goes on to review and the pull request. If the bug is not reproduced, including when it is absent at that merge base, the Ticket run ends successfully with triage notes and `needs-info` on the issue, without opening or merging a pull request. Started directly on an **Issue URL**, by a **Spec run** for one of its **Tickets**, by an **Architect run** for the Ticket its **Architecture review** published, by a **Pickup run** for the **Ready issue** it took, or by a **Security run** for a **Security finding** it fixes. Every Ticket run carries out a **Process**: the **Built-in process** by default, or a **Custom process** selected by the Process that starts it. That Process owns its sequence of Steps. Dispatch chooses Spec or Ticket and makes any Claim before work starts; child Ticket runs inherit their Command's choices and limits.
 _Avoid_: Run, Merge run (former names), auto-merge (GitHub's own feature)

src/github.rs

@@ -20,9 +20,9 @@ pub const GENERATED: &str = "This comment was generated by AI.";
 
 mod check_read;
 pub(crate) mod evals;
-mod quota;
+pub(crate) mod quota;
 pub(crate) use check_read::is_transient as check_read_is_transient;
-pub(crate) use quota::{Quota, Quotas};
+pub(crate) use quota::{Quota, QuotaWait};
 
 mod advisories;
 mod base_ci;
@@ -775,15 +775,12 @@ impl GitHub {
     }
 
     /// What GitHub last showed for one batch of up to a hundred issues and pull requests.
-    /// The Inbox checks the returned quota before asking for another batch. Each
+    /// The Inbox checks the shared quota before asking for another batch. Each
     /// alias asks for state and settlement times (plus issue triage labels),
     /// and for the query's own `rateLimit`. A subject GitHub can't
     /// resolve answers null and stays out of the map, without failing the
     /// rest of the read. Only reads; nothing here writes to GitHub.
-    pub(crate) fn inbox_states(
-        &self,
-        urls: &[String],
-    ) -> Result<(BTreeMap<String, InboxState>, Quota)> {
+    pub(crate) fn inbox_states(&self, urls: &[String]) -> Result<BTreeMap<String, InboxState>> {
         let mut states = BTreeMap::new();
         let mut query = String::from("query {");
         let mut aliases = Vec::new();
@@ -841,6 +838,7 @@ impl GitHub {
         })?;
         let data = &json["data"];
         let quota = Quota::graphql(&data["rateLimit"])?;
+        self.observe_quota(quota::Resource::Graphql, quota)?;
         for (alias, url, subject) in aliases {
             if json["errors"]
                 .as_array()
@@ -855,7 +853,7 @@ impl GitHub {
             }
             states.insert(url.clone(), subject.read(&url, pull)?);
         }
-        Ok((states, quota))
+        Ok(states)
     }
 
     /// Open a draft pull request from `head` into `base`, and return its URL.
@@ -1278,6 +1276,16 @@ impl GitHub {
     }
 
     fn output_with_input(&self, args: &[&str], input: Option<&[u8]>) -> Result<Output> {
+        let output = self.raw_output_with_input(args, input)?;
+        if !output.status.success() && self.record_quota_failure(args, &output.stderr).is_err() {
+            // The original failure remains the caller's error, even when the
+            // machine cannot persist its wait or identify its signed-in user.
+            eprintln!("thirdshift: could not record GitHub Quota wait");
+        }
+        Ok(output)
+    }
+
+    fn raw_output_with_input(&self, args: &[&str], input: Option<&[u8]>) -> Result<Output> {
         process::output(
             Command::new("gh").args(args),
             input,

src/github/advisories.rs

@@ -735,7 +735,7 @@ impl GitHub {
 
     pub(crate) fn inbox_findings(&self, repo: &str) -> Result<BTreeMap<String, bool>> {
         self.list_security_records(repo, || {
-            if let Some(warning) = self.inbox_quotas()?.warning() {
+            if let Some(warning) = super::QuotaWait::warning(&self.inbox_quotas()?) {
                 bail!(warning);
             }
             Ok(())

src/github/execution_tests.rs

@@ -58,6 +58,209 @@ use std::fs;
 use std::path::PathBuf;
 use std::time::{Duration, Instant};
 
+#[test]
+fn primary_quota_failure_records_a_shared_wait() {
+    with_recorded_signal(
+        "github::execution_tests::primary_quota_failure_records_a_shared_wait",
+        |_| {
+            let fixture = Fixture::new("");
+            // SAFETY: this test runs in an isolated child before any workers.
+            unsafe { std::env::set_var("HOME", fixture.temp.path()) };
+            fixture.script(
+                r#"case "$*" in
+  'api user --hostname github.com') echo '{"login":"octocat"}' ;;
+  'api rate_limit --hostname github.com')
+    echo '{"resources":{"core":{"remaining":0,"reset":4102444800}}}' ;;
+  *) echo 'API rate limit exceeded for user ID 123. (HTTP 403)' >&2; exit 1 ;;
+esac"#,
+            );
+            let error = GitHub::new().issue_title(&issue()).unwrap_err();
+            assert!(format!("{error:#}").contains("API rate limit exceeded"));
+            let path = fixture.path(".thirdshift/github-quota.json");
+            assert!(
+                path.exists(),
+                "primary quota failure left no shared Quota wait"
+            );
+            let record: Value = serde_json::from_slice(&fs::read(path).unwrap()).unwrap();
+            assert_eq!(record["waits"][0]["host"], "github.com");
+            assert_eq!(record["waits"][0]["login"], "octocat");
+            assert_eq!(record["waits"][0]["resource"], "core");
+            assert_eq!(record["waits"][0]["reset_at"], "2100-01-01T00:00:00Z");
+            assert_eq!(record["waits"][0]["remaining"], 0);
+        },
+    );
+}
+
+#[test]
+fn primary_quota_failure_scopes_graphql_and_defaults_only_unreadable_resets() {
+    with_recorded_signal(
+        "github::execution_tests::primary_quota_failure_scopes_graphql_and_defaults_only_unreadable_resets",
+        |_| {
+            for (index, response) in [
+                "{}",
+                r#"{"resources":{"graphql":{"reset":"bad"}}}"#,
+                r#"{"resources":{"graphql":{"reset":4102444800}}}"#,
+            ]
+            .into_iter()
+            .enumerate()
+            {
+                let fixture = Fixture::new("");
+                // SAFETY: isolated test process, before workers start.
+                unsafe {
+                    std::env::set_var("GH_HOST", "git.example.com");
+                    std::env::set_var("GH_ENTERPRISE_TOKEN", "test-secret-never-stored");
+                }
+                fixture.script(&format!(
+                    "case \"$1 $2\" in\n  'api user') echo '{{\"login\":\"enterprise-user\"}}' ;;\n  'api rate_limit') echo '{response}' ;;\n  *) echo 'GraphQL: RATE_LIMIT_EXCEEDED (HTTP 429)' >&2; exit 1 ;;\nesac"
+                ));
+                let before = Utc::now();
+                assert!(GitHub::new().issue_title(&issue()).is_err());
+                let after = Utc::now();
+                let bytes = fs::read(fixture.path(".thirdshift/github-quota.json")).unwrap();
+                assert!(!String::from_utf8_lossy(&bytes).contains("test-secret-never-stored"));
+                let record: Value = serde_json::from_slice(&bytes).unwrap();
+                let wait = &record["waits"][0];
+                assert_eq!(wait["host"], "git.example.com");
+                assert_eq!(wait["login"], "enterprise-user");
+                assert_eq!(wait["resource"], "graphql");
+                let reset = DateTime::parse_from_rfc3339(wait["reset_at"].as_str().unwrap())
+                    .unwrap()
+                    .to_utc();
+                if index == 2 {
+                    assert_eq!(reset.timestamp(), 4102444800);
+                } else {
+                    assert!(reset >= before + chrono::Duration::minutes(15));
+                    assert!(reset <= after + chrono::Duration::minutes(15));
+                }
+                assert!(
+                    GitHub::new()
+                        .quota_waits_for("github.com")
+                        .unwrap()
+                        .is_empty()
+                );
+                assert_eq!(
+                    GitHub::new()
+                        .quota_waits_for("git.example.com")
+                        .unwrap()
+                        .len(),
+                    1
+                );
+            }
+        },
+    );
+}
+
+#[test]
+fn a_rest_quota_failure_in_a_repository_named_graphql_keeps_the_core_resource() {
+    with_recorded_signal(
+        "github::execution_tests::a_rest_quota_failure_in_a_repository_named_graphql_keeps_the_core_resource",
+        |_| {
+            let fixture = Fixture::new("");
+            fixture.script(
+                r#"case "$1 $2" in
+  'api user') echo '{"login":"octocat"}' ;;
+  'api rate_limit') echo '{"resources":{"core":{"remaining":0,"reset":4102444800}}}' ;;
+  *) echo 'HTTP 403: API rate limit exceeded (https://api.github.com/repos/acme/graphql/issues/7/comments)' >&2; exit 1 ;;
+esac"#,
+            );
+            let issue = IssueUrl::parse("https://github.com/acme/graphql/issues/7").unwrap();
+            assert!(GitHub::new().comment_issue(&issue, "hello").is_err());
+            let waits = GitHub::new().quota_waits().unwrap();
+            assert_eq!(waits.len(), 1);
+            assert!(matches!(waits[0].resource, quota::Resource::Core));
+        },
+    );
+}
+
+#[test]
+fn changing_credentials_does_not_reuse_another_users_wait() {
+    with_recorded_signal(
+        "github::execution_tests::changing_credentials_does_not_reuse_another_users_wait",
+        |_| {
+            let fixture = Fixture::new("");
+            fixture.script(
+                r#"case "$1 $2" in
+  'api user') printf '{"login":"%s"}\n' "$GH_TOKEN" ;;
+  'api rate_limit') echo '{"resources":{"core":{"remaining":0,"reset":4102444800}}}' ;;
+  *) echo 'API rate limit exceeded' >&2; exit 1 ;;
+esac"#,
+            );
+            // SAFETY: isolated test process, before workers start.
+            unsafe { std::env::set_var("GH_TOKEN", "alice") };
+            assert!(GitHub::new().issue_title(&issue()).is_err());
+            assert_eq!(GitHub::new().quota_waits().unwrap().len(), 1);
+            unsafe { std::env::set_var("GH_TOKEN", "bob") };
+            assert!(GitHub::new().quota_waits().unwrap().is_empty());
+            assert!(GitHub::new().issue_title(&issue()).is_err());
+            assert_eq!(GitHub::new().quota_waits().unwrap().len(), 1);
+            unsafe { std::env::set_var("GH_TOKEN", "alice") };
+            assert_eq!(GitHub::new().quota_waits().unwrap().len(), 1);
+        },
+    );
+}
+
+#[test]
+fn quota_failure_does_not_treat_a_close_comment_as_a_host_selector() {
+    with_recorded_signal(
+        "github::execution_tests::quota_failure_does_not_treat_a_close_comment_as_a_host_selector",
+        |_| {
+            let fixture = Fixture::new("");
+            fixture.script(
+                r#"case "$1 $2" in
+  'api user') echo '{"login":"octocat"}' ;;
+  'api rate_limit') echo '{"resources":{"core":{"remaining":0,"reset":4102444800}}}' ;;
+  *) echo 'API rate limit exceeded' >&2; exit 1 ;;
+esac"#,
+            );
+            assert!(
+                GitHub::new()
+                    .close_issue(&issue(), "--hostname=docs.example.net")
+                    .is_err()
+            );
+            assert_eq!(
+                GitHub::new().quota_waits_for("github.com").unwrap().len(),
+                1
+            );
+            assert!(
+                GitHub::new()
+                    .quota_waits_for("docs.example.net")
+                    .unwrap()
+                    .is_empty()
+            );
+        },
+    );
+}
+
+#[test]
+fn quota_failure_uses_repository_host_instead_of_comment_url() {
+    with_recorded_signal(
+        "github::execution_tests::quota_failure_uses_repository_host_instead_of_comment_url",
+        |_| {
+            let fixture = Fixture::new("");
+            fixture.script(
+                r#"case "$1 $2" in
+  'api user') echo '{"login":"octocat"}' ;;
+  'api rate_limit')
+    echo '{"resources":{"core":{"remaining":0,"reset":4102444800}}}' ;;
+  *) echo 'API rate limit exceeded for user ID 123. (HTTP 403)' >&2; exit 1 ;;
+esac"#,
+            );
+            assert!(
+                GitHub::new()
+                    .comment_issue(&issue(), "https://docs.example.net/")
+                    .is_err()
+            );
+            let record: Value = serde_json::from_slice(
+                &fs::read(fixture.path(".thirdshift/github-quota.json")).unwrap(),
+            )
+            .unwrap();
+            assert_eq!(record["waits"][0]["host"], "github.com");
+            assert_eq!(record["waits"][0]["login"], "octocat");
+            assert_eq!(record["waits"][0]["resource"], "core");
+        },
+    );
+}
+
 /// PATH changes happen only in the isolated process. Cleanup uses the
 /// executable's recorded PID, including after a failed assertion.
 struct Fixture {
@@ -77,7 +280,11 @@ impl Fixture {
         );
         // SAFETY: this isolated process runs one test; no environment readers
         // or execution workers have started.
-        unsafe { std::env::set_var("PATH", path) };
+        unsafe {
+            std::env::set_var("PATH", path);
+            std::env::set_var("HOME", fixture.temp.path());
+            std::env::remove_var("GH_HOST");
+        }
         fixture
     }
 
@@ -424,7 +631,7 @@ fn permanent_and_quota_read_failures_are_not_retried() {
                 "API rate limit exceeded for user ID 123. (HTTP 429)",
             ] {
                 fixture.script(&format!(
-                    "echo read >> '{}'\ncat >&2 <<'ERROR'\n{stderr}\nERROR\nexit 1",
+                    "case \"$1 $2\" in\n  'api user') echo '{{\"login\":\"runner\"}}'; exit 0 ;;\n  'api rate_limit') echo '{{}}'; exit 0 ;;\nesac\necho read >> '{}'\ncat >&2 <<'ERROR'\n{stderr}\nERROR\nexit 1",
                     fixture.path("reads").display()
                 ));
                 let error = GitHub::new().issue_title(&issue()).unwrap_err();