fix(orchestrator): route the golden-release GitHub calls through the engine's circuit breaker
Stacks on #693 (fix/bound-orchestrator) → #685 (fix/bound-shared); retarget to main after they merge.
Closes G10, the last row of the "no GitHub call may bypass the breaker" audit: the orchestrator's golden-release calls. #693 bounded their deadlines (30 s metadata, 1 h downloads) but left them outside the breaker — the pool had no handle on AppState's registry, so a GitHub outage discovered by the webhook path was rediscovered by the golden lookup, at the cost of its own 30-second wait.
What changed
One funnel, carried to both sides. New GithubFunnel in github_breaker.rs bundles the breaker registry and the per-subsystem consumption recorder — the pair AppState already held separately — so it can be cloned into a component that is not the server:
ServerConfig::github: Option<GithubFunnel>is adopted byAppState::new_with_storebefore the App registry attaches its accounting, so app-auth traffic and the pool share one registry rather than two.RunnerPoolConfig::github: Option<GithubFunnel>carries the same value (cloned) into the pool.preloop servecreates it once and passes it to both sides.None(a one-shotpreloop golden build, tests, the standalonepreloop-serverbinary) means "no engine to share with": the pool keeps a private funnel rather than letting a GitHub call go unobserved. The funnel is per pool, not a process-wide static, so parallel tests cannot trip each other's breakers.
Every golden call now goes through send_observed_labeled on the anonymous bucket (all of them are unauthenticated) under a new golden subsystem label, via one golden_send helper in the orchestrator so the funnel sees the whole dependency:
| Site | Call | Bucket | Subsystem | Deadline |
|---|---|---|---|---|
resolve_latest_golden_tag | GET api.github.com/repos/preloopdev/preloop/releases (the tag lookup) | Anonymous | golden | 30 s metadata client |
oci::get_manifest | OCI manifest, index follow-up | Anonymous | golden | 30 s metadata client |
oci::request_bearer_token | ghcr.io/token bearer handshake | Anonymous | golden | 30 s metadata client |
download_release_asset | <asset>.sha256 sidecar | Anonymous | golden | 30 s client + 15 s request timeout (unchanged) |
download_release_asset | release-asset probe and every resume attempt (Range) | Anonymous | golden | 1 h download client (unchanged) |
download_oci_golden | blob probe and every resume attempt (registry_get) | Anonymous | golden | 1 h download client (unchanged) |
oci's public API gained an Option<&GithubFunnel> gate (registry_get, request_bearer_token, get_manifest, fetch_json); preloop init's one-shot manifest probe passes None and sends plainly.
Behaviour
- Breaker open ⇒ the metadata lookup fails fast with a clear error. The refusal comes back as
BreakerOpen("GitHub circuit breaker is open (retry after 30s)"), nothing reaches the network, and none of the 30 s metadata deadline is spent.resolve_latest_golden_taglogs it atwarnand — deliberately — does not write its tag cache: a cached negative answer would outlive the breaker's own window (5 min TTL against a 30 s base open) and park the lookup after recovery. The caller still falls through to thelatestredirect, as it always did for a failed lookup. - Downloads change only in their admission decision. A refused transfer attempt fails like any other failed attempt (logged, next candidate, then local build); an admitted one keeps its 1 h budget, its resume offsets and its retry loop.
- Refusals are accounted as
breaker_blockedon thegoldenrow of the consumption snapshot (and successful exchanges asrequests), so the ops snapshot and the webhooks health endpoint show the pool's GitHub spend next to every other subsystem.
Deviations, called out
- Based on #693, not #685. #693 restructures exactly these functions (the
golden_http_clientsplit,GOLDEN_METADATA_TIMEOUT, the.sha256/manifest clients) and the 30 s metadata deadline this PR must keep exists only there. Stacking one level up keeps the diff minimal and avoids re-implementing #693's bound. send_observed_labeled, notsend_observed. The golden calls run where an engine funnel exists, and the audit asks for a subsystem label; the labeled funnel is what makes the label real (and the refusal visible in the snapshot). The neutralsend_observedis still what the pre-AppStatebootstrap paths use.- The plumbing is
Option<GithubFunnel>rather thanOption<Arc<GithubBreakerRegistry>>:send_observed_labeledneeds the recorder too, and creating the pair in the CLI is what letsAppStateand the pool share one registry (the pool config is built beforeserveconstructs the state).
Tests
open_breaker_refuses_the_golden_release_lookup_before_it_spends_the_deadline(new): trips the anonymous bucket, then asserts the lookup returnsNonein well underGOLDEN_METADATA_TIMEOUT, that a counting stub server received zero connections, that the refusal is reported asbreaker_blockedundergolden, and that after the open window lapses the same lookup reaches the stub, answersv9.9.9, and closes the breaker.subsystem_labels_are_a_small_closed_setextended withgolden.- Every existing golden-download/orchestrator test passes the funnel explicitly (
test_github()), so the download path is exercised with a closed breaker exactly as before.
Checklist
- Tests added/updated for the observable contract (fail-fast with the breaker open, no request sent, breaker closes on a reachable answer,
goldenaccounting) - Docs updated:
docs/webhook-resilience.md(breaker coverage now names the golden path) - Changelog entry under Unreleased → Fixed
- Gates:
cargo fmt --all --check,cargo clippy --locked -p preloop-orchestrator -p preloop-runner-server -p preloop-cli --all-targets -- -D warnings,cargo test --locked -p preloop-orchestrator,cargo test --locked -p preloop-runner-server github_breaker,cargo test --locked -p preloop-runner-server bootstrap
Summary by cubic
Routes the runner pool's golden-release GitHub calls through the engine's circuit breaker — the last GitHub traffic that bypassed it — and fixes a spurious 408 that dropped the upload-complete event behind queued progress frames.
- A new
GithubFunnelbundles the breaker registry and per-subsystem accounting soAppStateand the runner pool share one registry. All golden calls — release listing, OCI manifest and token lookups,.sha256sidecars, and transfers — go through the anonymous bucket under a newgoldensubsystem label; one-shot CLI paths (golden build,preloop initprobes, the standalone server) keep a private funnel rather than leaving a call unobserved. - Breaker state is keyed by
(service origin, credential), so a mirror, non-GHCR registry, token realm, or GHES instance keeps its own availability and rate-limit state;golden_sendconsults the target origin's cross-bucket state first, so an outage the webhook path already parked refuses the lookup before a connection opens. The anonymousBearer401 handshake no longer clears the failure streak; it is accounted as an expected protocol step and the retried token-bearing request keeps its old semantics. - The
goldenrow reports bytes moved: streamed chunks are charged as they arrive and metadata bodies by advertised length. - With the breaker open, the metadata lookup fails fast: no connection is opened, none of the 30 s deadline is spent, and the refusal is deliberately not cached as a negative tag, whose 5-minute TTL would outlive the breaker's own window. Downloads keep their 1-hour budget, resume offsets, and retry loop — only their admission decision changes. Refusals are accounted as
breaker_blockedon thegoldenrow, so ops and health snapshots show the pool's GitHub spend.
Bug Fixes
- The upload-complete event is no longer dropped behind progress, and progress no longer queues memory on the unauthenticated ingress: the gate reports the upload's phase over a
watchchannel, so a one-byte-frame burst replaces one phase instead of enqueueing ~33.5M events andCompletecan never sit behind aProgress. A small successful upload cannot turn into a spurious 408 under load.
Tests trip the anonymous bucket and assert the lookup returns None well under the deadline, a counting stub server sees zero connections, the refusal is accounted, and the same lookup answers and closes the breaker once the window lapses. The fail-fast test builds its client before tripping the breaker so the OS certificate-store load (up to five seconds on the process's first client) cannot race the open window and admit the request the test needs refused. Additional tests cover origin partitioning, foreign-host naming, the expected-challenge streak, a parked App bucket refusing across buckets, and transfer byte accounting.