fix(github): route every engine GitHub call through the breaker

Merged
BN
6d ago
+1242-234

Stacks on preloopdev/preloop#685; retarget to main after it merges.

What & why

Thirteen engine GitHub call sites still sent straight through the shared HTTP
client, so a GitHub outage cost one round trip per caller for the whole
outage instead of one probe per credential. Every one of them now goes through
crates/preloop-runner-server/src/github_breaker.rs, on the credential bucket
the call actually spends.

The breaker applied to the webhook path only; the rest of the engine discovered
the same outage independently. This routes them all through one funnel:

  • send_github(state, credential, subsystem, request) — breaker + per-subsystem
    accounting for a caller-built request (streaming relays, header-only reads,
    per-request timeouts).
  • send_github_json(state, credential, subsystem, method, url, body) — the
    engine's standard GitHub JSON call, decoding with the failure text callers
    already classify on.
  • GithubCredential — token and bucket travel together, so a call cannot be
    admitted or accounted under a credential it does not spend; authorize()
    attaches the bearer and contributes nothing for an anonymous call.
  • GithubApiError — the non-success status stays typed (dispatch auth branches
    on it) while Display keeps status {code} for the push-back classifier.

Per-site routing

#SiteCallBucketSubsystem
G1dispatch_auth.rs fetch_json (/installation, /installation/repositories, /user, /app)installation-token round-trip, PAT and App-JWT actor lookupsInstallationUnknown for /installation, then Installation(id) for the listing; Pat; App(id)dispatch
G2github_push.rs github_json + github_pr.rscommit/tree verification, repo metadata, PR list/createInstallation(id) from the mint ledger, else InstallationUnknown; Patpush-back
G3snapshots.rs forge_relay_forward (git + tarball + api/v3 relay, 3 callers)anonymous forge relayAnonymoussnapshots
G4dispatch.rs resolve_default_branchGET /repos/{repo}Installation(id) from the mint ledger, else InstallationUnknown; Pat (env PAT)dispatch
G5runs.rs pat_oauth_scopesGET {api}/ PAT scope probePatcredentials
G6github.rs exchange_manifest_codePOST /app-manifests/{code}/conversionsAnonymous (the code is the credential)app-auth
G7scheduler.rs scan_remoterepo metadata + commit lookupPatworkflows
G8snapshots.rs resolve_github_repo_metaGET /repos/{slug}Installation(id) / Pat / Anonymous, per the credential it already selectedsnapshots

Every call keeps its current bound and error mapping: dispatch auth still fails
closed (502) on a refusal while the breaker is open and still turns a 401/403
into a rejected credential; push-back answers 502 for it (the classifier now
recognises the breaker's own text as transient); the forge relay, the schedule
scan and the repository-metadata lookup still degrade quietly; the manifest
exchange still maps everything to 500. Per-call client timeouts are unchanged
(the 10 s snapshot metadata bound, the 300 s relay client).

Two new GithubCredentialBucket/GithubSubsystem shapes were needed: the
installation:unknown bucket (a third-party installation token whose id only
arrives in the response being made — inventing an id would charge another
installation, and the PAT's bucket belongs to the operator) and the dispatch,
push-back, credentials subsystems.

Deliberately outside the breaker

  • G9 — presigned cross-host LFS blob downloads (snapshots.rs) already
    bypass it behind a via_github check, since they are not GitHub traffic and
    spend no GitHub API budget; the credential is stripped for them too. Unchanged.
  • G10 — orchestrator golden metadata lookups are not in this PR: the
    orchestrator has no breaker/AppState handle (it would need plumbing through
    RunnerPoolConfig). Reported to the parent workstream.

Protocol surface

  • I confirm this change touches the runner protocol interface: NO
    (no registration/broker/Twirp/check-run wire shapes; GitHub API calls only)

Verification performed

cargo fmt --all --check
cargo clippy --locked -p preloop-runner-server --all-targets -- -D warnings
cargo test --locked -p preloop-runner-server --lib -- github_breaker dispatch_tests snapshots
cargo test --locked -p preloop-runner-server --lib -- scheduler:: runs:: github_push:: github_pr:: dispatch github::
  • fmt --check and clippy -D warnings are clean, re-run on the rebase onto
    fix/bound-shared @ 793573744.
  • The required run (github_breaker, dispatch_tests, snapshots) is green on
    that head: 147 passed / 0 failed, and the five new tests pass individually.
  • Before the rebase, the adjacent modules were also green: 208 passed /
    1 failed (the fixture fixed in the second commit) and 73 passed / 0 failed for
    bootstrap/broker/distributed_task/github_app/webhook_watchdog/
    webhook_health.

New tests (github_breaker.rs), all green:

  • an_open_breaker_refuses_a_json_call_without_touching_github — three 5xx from a
    stub open the PAT bucket; the next call is refused with a typed BreakerOpen,
    the stub's hit count stays at 3, and breaker_blocked counts the refusal.
  • a_reachable_answer_closes_the_breaker_and_buckets_stay_independent — an answer
    resets the failure count, a 2xx under Anonymous flows while the PAT bucket has
    failures, and the anonymous request carries no Authorization header.
  • transport_failures_open_the_credential_breaker — connection-refused ×3 opens
    the bucket it was called under.
  • api_errors_carry_status_and_body — a non-2xx is typed and still reads as
    status 404 in the message.

New test (dispatch_tests.rs):

  • open_breaker_refuses_a_third_party_dispatch_without_touching_github — with the
    installation:unknown bucket open, a third-party (online-validated) token
    fails closed with 502 and the stub sees zero requests.

The one adapted test

github::tests::pat_throttle_on_actions_parks_check_run_sender stubbed a 429
for every path. The submit-time PAT scope probe now shares the PAT's breaker,
so the fixture's own submit parked the bucket before the test parked it, and
the test's first call failed with GitHub circuit breaker is open (retry after ~50s). The stub now throttles only the actions path; the probe's API root
answers 200 with no x-oauth-scopes header, which is the same Unverifiable
verdict the 429 produced. That is the only test whose fixture changed.

Checklist

  • Tests added/updated for the observable contract (breaker routing, fail-fast,
    bucket partitioning, credential never sent to an anonymous call)
  • Docs updated: docs/control-plane/github/webhook-resilience.mdx (breaker
    coverage now states the whole engine and the cross-host LFS exception; the
    old docs/webhook-resilience.md was retired by main's docs restructure)
  • Changelog entry under Unreleased → Fixed