Repository navigation
github-backup(drain): webhook events are acked even when the fetch failed or the mirror is missing — freshness signal silently dropped #3219
Description
Bug: github-backup drainer acks webhook events even when the fetch failed or the mirror is missing — the freshness signal is silently dropped
Where: infra/github-backup/bin/drain_queue.py (landed in #3214; system tracked by #3215)
Evidence
In main() (drain_queue.py:56-64), the per-repo fetch result is discarded:
if repo in planned:
fetch_mirrors.fetch_repo(root, repo, state) # return value ignored
fetched.append(repo) # reported as fetched either wayfetch_repo (bin/fetch_mirrors.py:23-42) returns "ok", "fail" (git fetch non-zero), or "missing" (no valid mirror at the path). The drainer:
- Appends every event id to
ackup front (drain_queue.py:45) and POSTs the full ack list to the Worker at the end (drain_queue.py:67) — including events whose fetch just failed or whose mirror is missing. The Worker deletes them from KV (worker/src/index.js/queue/ack), so the event can never be redelivered. - Sets the 15-minute per-repo coalesce stamp
drain_last::<repo>(drain_queue.py:52) before the fetch, so even a hypothetical redelivery inside the window would be skipped. - Writes nothing to
fetch.logfor the failure (only unplanned repos get a fetch.log line, drain_queue.py:61-64). The dailyfetch_mirrors.pymain loop logs failures; the drain path bypasses that logging entirely. - Prints the repo in the run summary's
fetchedlist regardless of outcome, so the systemd journal shows a successful drain.
Reproduced (laptop probe, main @ 96d1349)
Probe ~/git/fleet/probe-2026-10-31-laneB-drainack/probe.py drives the real drain_queue.main() with a stub Worker returning one push event for a planned repo and fetch_repo stubbed to return "fail":
{"events": 1, "fetched": ["owner/repo"]}
ack calls: [('/queue/ack', {'ids': ['ev1']})]
fetch.log exists: False
state keys: ['drain_last::owner/repo']
VERDICT: event acked despite fetch failure: True
The "missing" return takes the same code path (return value ignored), so a planned repo whose clone previously failed also loses its events until the weekly reconcile notices the missing mirror.
Why it matters
The webhook layer exists to keep mirrors fresh between weekly reconciles (Worker header: "Events are a freshness signal only; the weekly reconcile sweep is the correctness floor"). A transient fetch failure (network blip, GitHub 5xx, auth hiccup) currently converts into up-to-a-week of silent staleness for that repo: the event is deleted, the failure is in no log, and the run summary claims success. The correctness floor still catches the data eventually, but the failure is invisible — the opposite of this system's own observability pattern elsewhere (fetch_mirrors exits non-zero on 3-day chronic failures precisely so systemd surfaces them).
Acceptance criteria
- Drain only acks an event for a planned repo when its fetch returned
"ok"; on"fail"/"missing"the event stays queued for the next drain (or is acked only after an explicit retry policy), and the coalesce stamp is not advanced on failure. - Fetch failures/missing mirrors on the drain path are appended to
fetch.login the same format the daily pass uses, and the run summary distinguishes fetched vs failed repos. - A test (stub Worker + stub fetch, as in the probe) covers: success → acked; failure → not acked + logged; unplanned repo → acked + noted for reconcile (current intended behavior preserved).
Activity
dezren39 commentedon Oct 31, 2026
🤖 Agent note · filed
dezren39 commentedon Nov 1, 2026
🤖 Design-architect pass 29 (2026-10-31) — agent note: this issue is ADOPTED as a phase of design #3282 ("a mirror is what it can prove — one validity, outcome and alarm contract for the github-backup estate mirror", design doc: files/designs/code-github-backup-mirror-contract.md in the keep-the-four-repos-healthy goal workspace), linked as a sub-issue of #3282 and labelled source/design-architect. Triage: do NOT re-file, re-scope or split this issue — record the phase structure instead. Phase mapping: Phase 1 (validity & destinations) = #3238 + #3233; Phase 2 (recorded outcomes & the alarm) = #3250 + #3219 + #3270 + #3257; Phase 3 (durable state & plan retention) = #3240 + #3229 + closing audit #3283 (gated on Phases 1–2). The design records the pins that span the phases: validity = bare + HEAD resolves, with no emptiness exception inside the plan (build_plan excludes zero-commit repos); destinations are absent / valid / skeleton (remove + clone) / debris (quarantine to
.partial-<timestamp>+ clone); every fetch attempt ends in one recorded outcome shared by the daily loop and the drainer, and the drainer acks only onok; the chronic alarm is a streak of 3 consecutive unavailable days (fail, timeout and missing all count); generated state is written atomically only throughghb_commonhelpers; reconcile retains a failed owner's previously planned entries. Where an origin's acceptance left a choice open (the streak-vs-window rule, missing-counts-vs-immediate, report-vs-retain, skeleton handling), the design doc records the decision — triage follows the design's pin, not the open alternative. Each origin keeps its own acceptance criteria and its already-queued triage packet — no new packet was created for it this pass.