[JacobStephens2]

Summary

A rate-limited request whose gh api rate_limit probe already shows the window reset recorded a Quota wait until the next reset. Every Pass on the machine then skipped, and the Command slept, for up to an hour with a full quota. The probe is now an ordinary observation.

 GitHub::output_with_input  (rate-limited request)
   record_quota_failure
     probe rate_limit
-    Store::exhausted(quota)            # always waiting = true
-    return quota.reset_at
+    Store::observe(quota)              # waits only while quota.low(); else clears the stored wait
+    return Some(reset) if remaining < 500, else None
-  quota::command::wait(reset)          # sleeps a whole window at 4995 left
+  Some(reset) -> quota::command::wait(reset)    (unchanged)
+  None        -> retry the call once, immediately (Commands only)

 Store::waiting                         # Pass admission, Inbox, status
-  keep waits with reset_at > now
+  keep waits with quota.low(now)       # a stored 4995-left wait is not a wait

• A failed probe still falls back to remaining 0, reset in 15 minutes, so it still records a wait (src/github/execution_tests.rs unchanged and passing).
• The 500 reserve is now one RESERVE constant. The Inbox warning always says "(fewer than 500)", since only low waits reach it.
• GLOSSARY.md (Quota wait), docs/guide.md (+ regenerated site/docs/index.html) and site/how-it-works/ describe the single immediate retry.
• Two existing fixtures modelled a primary rate limit with a probe still showing the reserve: security_run::within_cap_quota_wait_keeps_pre_audit_github_diagnostics_local and status::github_failures_warn_with_last_answer_and_clear_after_recovery. Under this fix a probe like that means the window already reset. Their later probes now report the quota spent, via the fake's next_rate_limits, so they keep testing a real Quota wait.

Evidence

Execution evidence

Confirmed cause: record_quota_failure recorded every probe via Store::exhausted (waiting = true unconditionally) and returned its reset. pass_quota_wait then trusted any stored wait without probing.

• Before (fix not applied, original form of the test): cargo test --test quota_wait a_rate_limit_whose_probe_shows_the_reserve_retries_without_a_quota_wait

  thirdshift: gh pr ready 1 --repo acme/widgets failed: GraphQL: API rate limit exceeded (HTTP 403)
    left: Some(1)
   right: Some(0)
  test result: FAILED. 0 passed; 1 failed
  

  With a probe of 4995 left and a reset beyond the two-hour cap, the Command tried to wait for that reset and failed with the original error. With an hour's reset, it sleeps an hour.
  After: test a_rate_limit_whose_probe_shows_the_reserve_retries_without_a_quota_wait ... ok. pr ready is called twice and the PR is made ready. The test now also seeds a real stored wait (graphql: 400 left) first and asserts the probe cleared it.

• Before: cargo test --test pass_quota a_stored_wait_with_the_reserve_available_does_not_skip_a_pass

  ["pickup"]: thirdshift: Pickup run starting, 2026-11-12 +0000
  thirdshift: GitHub Quota wait until 2026-11-12T19:26:47Z (graphql: 4995 left)
  test result: FAILED. 0 passed; 1 failed
  

  This is the exact message the fleet logged.
  After: test a_stored_wait_with_the_reserve_available_does_not_skip_a_pass ... ok. status shows neither the wait nor "Inbox reads paused", and Pickup does GitHub work.

• New: a_rate_limit_whose_probe_shows_the_reserve_retries_only_once ... ok. If the retry is rate-limited too, the call fails with the original error after 2 calls, with no wait.

• Mutation checks, each reverted afterwards:

  • Skipping observe when not low fails the clearing assertion.
  • Removing the reset_retried bound fails retries_only_once.
  • Filtering Store::waiting on reset_at only fails the Pass test.

• Other test runs:

  • cargo test --test quota_wait --test pass_quota: 10 + 7 passed.
  • cargo test --bin thirdshift github::: 74 passed.
  • cargo clippy --all-targets -- -D warnings and cargo fmt --check are clean.
  • npm test --prefix site: 69/69, including docs:check.

• Full suite: scripts/affected-tests main selects the full Rust suite plus site. A clean cargo test --no-fail-fast on 4aab052 passed 4468 tests and failed 8. None of the 8 touches quota code:

  • 7 pass when rerun individually: activity_log::an_activity_log_that_cannot_be_written…, architect_run::a_failed_send…, launch_pull::an_up_to_date_launch_directory…, merge_run::a_merge_run_ends_with_the_issue_closed…, merge_run::an_issue_branch_already_deleted…, pickup_run::ready_labels_are_read…, run_notification::a_failed_send…. Under full parallel load in the container they logged could not read process …'s environment: Permission denied sweep warnings.
  • trial_scope::trial_sessions_keep_their_cap_and_clean_detached_work fails because the container has no systemctl.

• After merging the latest main (41744de): quota_wait, pass_quota, status and security_run (10 + 7 + 79 + 130) pass, and so do the github:: units (74), clippy, fmt and docs:check.

• Environment note: this host has no C compiler. Rust builds and tests ran in the local node:22 container (--init, with host /etc/passwd mounted) using the repo's pinned 1.99.1 toolchain from the host.

Visual evidence

site/how-it-works/ gains one sentence in the "Deliver against the latest Base branch" paragraph. Captures are at 1440×900 (Before from origin/main, After from this branch; the paragraph is outlined): /tmp/before-desktop.7sdLyF.png, /tmp/after-desktop.sM3B1T.png. Uploading them to a secret Gist was denied by this session's permission policy, so they are not embedded. Layout is unchanged; it is a copy edit inside an existing <p>.

Merge Danger

Door: two-way

Revertible. Existing ~/.thirdshift/github-quota.json records stay readable; a stored wait with ≥ 500 left is simply ignored.

Blast Radius: quota

Affects every Pass's admission and every Command's handling of a primary rate limit. A misclassified rate limit (probe full, request still limited) now fails after one immediate retry, where it used to wait a window.

Unaddressed findings

Standards

• 2. Duplicated predicate self.wait_for_quota && quota::command::active() in the reset-retry and secondary-limit guards (src/github.rs, output_with_input). Declined: the two guards bound different counters (!reset_retried, secondary_retries < 5). A helper would name only two short field reads next to each other in one match. Judgement call; behaviour is unaffected.
• 3. record_quota_failure returns Option<DateTime<Utc>> (src/github/quota.rs) rather than a Recovery enum. Declined: None means "no reset to wait for", which the doc comment states. The retry bound lives where the request is retried, beside the secondary-limit retry counter in the same loop. Judgement call; behaviour is unaffected.

Spec

None. All three Spec findings were addressed: stored-wait clearing is now tested, the Inbox text is asserted, and the guide states that a second rate limit after the retry fails the call.

Security review outcome

• Security review incomplete: session failed, ended early or omitted a valid final line; see Session trace

Review coverage

Both reviewers read every changed file except site/docs/index.html, generated from docs/guide.md. Both confirmed that exclusion on the evidence that npm run docs:check --prefix site and npm test --prefix site (69/69) pass.

Left unread by the review: tests/security_run.rs and tests/status.rs. They changed in 4aab052 after the review, in the fixture-only change described in the Summary.

Closes #2128

Built with codex codex-cli 0.167.1 · gpt-6.1-sol · xhigh

[JacobStephens2]

Delivery completed on agent-authored or reviewed head 41744de.