Skip to content

feat(github): webhook-driven observer refresh - #6326

Open
schickling-assistant wants to merge 5 commits into
mainfrom
schickling-assistant/gh-webhook-refresh
Open

schickling-assistant wants to merge 5 commits into
mainfrom
schickling-assistant/gh-webhook-refresh

Conversation

@schickling-assistant

@schickling-assistant schickling-assistant commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Problem — GitHub observers learn PR head/check/queue facts only through GraphQL polls gated at 5 min (15 min when settled). Polling is both slow for the factory landing loop and the dominant GraphQL spend. REST listings are already conditional (ETag).

Goal — Let verified GitHub webhook deliveries push freshness through the existing refresh-claim machinery, and make polling a slow backstop while a repository is webhook-covered.

Contract

  • Config: [github] webhook_listen = "127.0.0.1:41871" (loopback, nonzero port, optional) and webhook_secret_file = "/abs/path" (absolute; required iff webhook_listen is set). The secret file is re-read per delivery with token-file custody rules (owner-owned regular file, no group/other bits, no symlink follow); absent/unreadable/empty → 503, never unsigned acceptance.
  • HTTP: only POST /fractal/github-webhook on that listener; everything else 404. Body > 5 MiB → 413. X-Hub-Signature-256 HMAC-SHA256 over the raw body, constant-time verify; missing/mismatch → 401. Missing X-GitHub-Event/X-GitHub-Delivery after auth → 400. ping → 200. Content type application/json.
  • Events that refresh: pull_request, pull_request_review, pull_request_review_comment, pull_request_review_thread, issue_comment, issues, push, status, merge_group, create, delete, and check_suite / check_run / workflow_run once completed. Others → 204. Payload read: repository.full_name (lowercased), ref for push, ref+ref_type for create/delete.
  • Refresh: every active github.repository observer whose locator equals full_name (ASCII case-insensitive) and, for a push/create/delete of branch B, every github.ref observer of OWNER/REPO@B, gets an observer.refresh-requested claim with attempt = "github-webhook:" + hex(sha256("fractal.github-webhook.v1\0" + delivery + "\0" + observer)), unless that attempt is already requested (redelivery). 202 {"refreshed": n}.
  • Coalescing (on the polling member): a poll still takes the oldest pending refresh, which names its arm, so deliveries never retire a running look. Under the look lock, right before its reads, a poll taken for a webhook refresh answers the newest pending webhook refresh from the same member; that answer also answers every earlier one from that member (each was written after its change and before the reads). A burst during one look costs one more look.
  • Cadence: an observer with a webhook refresh claim (attempt prefix github-webhook:) accepted within 30 min is webhook-covered — computed on the polling member from its replicated claims. Covered repositories use the 15-min settled interval as the GraphQL floor, and quiet standing polls go straight to the 10-min ceiling. Uncovered behaviour is unchanged.
  • Observability: OTel counter st.github.webhook.deliveries{github.event, outcome} (outcome refreshed|ignored|unauthorized|unavailable|invalid; unknown events labelled other), doctor line github-webhook (address, last verified delivery, counts) on daemons running the receiver.

Decisions

  • Claim body comes from one helper (store::observer_refresh::append_observer_refresh_tx), used by both declared refreshes and webhooks; active-observer listing is shared too. The pending-refresh query moved from fractal-missions to fractal-store (observer_refresh::pending_attempt), next to coalesced_attempt, which the reconciler calls under the look lock.
  • No schema change: N-1 readers reject unknown claim fields, so a webhook refresh is marked only by the github-webhook: prefix of its required attempt string. Registry digest unchanged.
  • Coverage reaches the provider as ObservationRequest.webhook_covered; reconcile computes it per poll via Store::github_webhook_covered.
  • Pure logic (event filter, matching, attempt, doctor state) lives in fractal-kernel::github_webhook; the axum receiver lives in fractal-daemon::github_webhook.

Verification (local, macOS, nix develop)

  • cargo test -p fractal-kernel --lib github_webhook — event filter, repo-case/ref matching, attempt binding, doctor line, config validation: pass
  • cargo test -p fractal-store --lib observer_refresh — claim with exactly attempt/revision, branch ref observers, redelivery writes nothing, deliveries during a poll coalesce into one answer, declared refreshes keep their place, coverage window and declared refreshes not counting: 3 pass
  • cargo test -p fractal-daemon --lib github_webhook — 503 on absent / group-readable secret, 401 wrong/unsigned, 202 + claims, redelivery {"refreshed":0}, ping 200, ignored 204, invalid 400, 413, 404 for other routes/methods, HMAC unit: 3 pass
  • cargo test -p fractal-github-base --lib webhook_coverage — covered GraphQL floor 15 min vs uncovered 5 min: pass
  • cargo test -p fractal-reconcile --lib standing_watch_cadence — uncovered 180→360→600 unchanged; webhook-covered quiet poll goes to 600 s immediately, change returns to 180 s, provider sees webhook_covered: pass
  • cargo test -p fractal-reconcile --lib ref_watch — incl. webhook_refreshes_during_a_look_keep_it_and_coalesce_into_one_more (a look held in flight while deliveries arrive keeps its answer; one more look answers the rest): 17 pass
  • cargo test -p fractal-schema --lib assistant_principal_registry_digest_transition (digest unchanged), cargo check -p fractal --bins --tests, clippy on touched crates (no findings on changed lines), scripts/check-sqlite-large-values: pass
  • Full suite: burst nextest run linked in the PR comments.

Pushed with ST_SKIP_EVIDENCE=1 (no local evidence signer); hosted CI at the exact head is the proof.

Complexity — ~900 changed lines (three new modules with tests); above the usual 300-line target, kept as one PR because the receiver, claim path and cadence are one contract.

Concerns

  • N-1 pollers accept webhook refresh claims as ordinary refreshes but do not coalesce them (one poll per delivery) and do not slow their cadence; upgrade pollers before configuring a receiver to save GraphQL budget. [github] rejects unknown keys, so remove the two keys before rolling a member back.
  • Webhook refresh polls bypass the GraphQL floor, like declared refreshes. Coalescing bounds a burst during one look to one more look; busy repos (many check_run completions) still trigger roughly one refresh look per look duration. Compare st_github_responses before/after enabling.
  • Repository aliases of one resource each get a claim (same as declared refresh, which targets every observer of the resource).

Follow-ups — Operator wiring (secret custody, GitHub hook, proxy) is outside this repo.

References — docs/st3/github-observer-admission.md#webhook-delivery; release-notes/github-webhook-refresh.json.

@schickling-assistant

Copy link
Copy Markdown
Contributor Author

Burst nextest at d6671eb: https://github.com/compoundingtech/fractal/actions/runs/42914124868 — failed fast after 12/9753 tests; the 6 failures are all fractal::integration messaging_faults::* (fixture TimeoutError), outside this diff. Head 0b59911 adds the pinned-digest test fix (assistant_principal_registry_digest_transition; fractal-schema lib 153/153 locally). Full-suite rerun with --no-fail-fast: https://github.com/compoundingtech/fractal/actions/runs/42920551577. The second commit is unsigned because local 1Password SSH signing failed; it can be squashed or re-signed.

A loopback receiver ([github].webhook_listen + webhook_secret_file) verifies
X-Hub-Signature-256 per delivery and writes observer.refresh-requested claims
for matching github.repository / github.ref observers, coalescing on a pending
refresh. Their attempt is "github-webhook:" + a digest of delivery and
observer, so the claim schema is unchanged and N-1 readers accept it. A
webhook-covered repository (webhook refresh within 30 min, read from
replicated claims on the polling member) raises the GraphQL floor to the
15-minute settled interval and quiet standing polls go straight to the
10-minute ceiling.

agent-identity: unknown
agent-persona: unknown
agent-supervisor: unavailable
agent-tool: unknown
agent-runtime: unknown
agent-model: unknown
@schickling-assistant
schickling-assistant force-pushed the schickling-assistant/gh-webhook-refresh branch from 0b59911 to 5834933 Compare November 10, 2026 23:15
The poller answers refreshes oldest first, so the oldest unanswered refresh
may already be polling and have read GitHub before a new delivery's change.
A delivery now queues one more refresh in that case; only an observer with
two unanswered refreshes absorbs the delivery, because the second has not
started and its poll reads what the delivery announced.

agent-identity: unknown
agent-persona: unknown
agent-supervisor: unavailable
agent-tool: unknown
agent-runtime: unknown
agent-model: unknown
Absorbing a delivery on the receiver judged "a poll has not started" from
the receiver's replica, which lags the poller's answers, and counted a
redelivered attempt twice; either way a delivery whose change postdated the
running poll's reads could be dropped while coverage slowed the backstop.

The receiver now writes one request per delivery (a redelivery of an
already requested attempt writes nothing) and the poller coalesces: when the
oldest pending request is a webhook refresh, it takes the newest pending
webhook refresh from the same member, and answering it also answers every
earlier webhook refresh from that member, each written after its change and
before the poll started. Declared refreshes keep oldest-first order.

agent-identity: unknown
agent-persona: unknown
agent-supervisor: unavailable
agent-tool: unknown
agent-runtime: unknown
agent-model: unknown
…s reads

Choosing the newest webhook refresh as the pending attempt renamed the arm
on every delivery, so each delivery during a look retired it and discarded
its answer; steady deliveries could keep any look from answering.

The pending attempt is the oldest unanswered request again and names the
arm, so deliveries never retire a running look. Under the look lock, right
before the reads, an arm taken for a webhook refresh answers the newest
pending webhook refresh from the same member instead, which answers every
earlier one from that member. A reconciler test holds a look in flight
while deliveries arrive and checks that it keeps its answer and that one
more look answers the rest.

agent-identity: unknown
agent-persona: unknown
agent-supervisor: unavailable
agent-tool: unknown
agent-runtime: unknown
agent-model: unknown
@schickling-assistant

Copy link
Copy Markdown
Contributor Author

Exact head 0308776 (all commits signed, verified).

…nd_return)

agent-identity: unknown
agent-persona: unknown
agent-supervisor: unavailable
agent-tool: unknown
agent-runtime: unknown
agent-model: unknown

This branch was previously deployed

1 inactive deployment
evidence-signing — 46b9c452 Deployed Nov 11, 2026 by schickling-assistant via ci-script-tests #19767
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant