From 1840d632a27e855c670d550bf457b38d7bb06384 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Wed, 2 Sep 2026 23:19:35 -0400 Subject: [PATCH] docs(reconcile): say what a refresh hook has to be, and that resync is not it `ProviderView` reads through bookkeeping, so it is a fresh view of a zone only to the extent its hook makes that bookkeeping authoritative; `Route53Dns::resync` hydrates and removes nothing, so the wiring this crate used to prescribe would have reported every zone in sync forever. Recorded against the open onboarding item that will do the wiring, along with the attempt-budget change. Co-Authored-By: Claude Opus 5 (1M context) --- crates/didbot-reconcile/src/lib.rs | 27 ++++++++++++++++++++++----- docs/zone-reconciliation.md | 23 +++++++++++++++++++---- plan/onboarding.md | 14 ++++++++++++++ 3 files changed, 55 insertions(+), 9 deletions(-) diff --git a/crates/didbot-reconcile/src/lib.rs b/crates/didbot-reconcile/src/lib.rs index 02be28c1..038a0820 100644 --- a/crates/didbot-reconcile/src/lib.rs +++ b/crates/didbot-reconcile/src/lib.rs @@ -308,11 +308,28 @@ where /// /// Without a refresh this reports what *this process* published, which is /// the whole truth for the in-process providers and only half of it for a -/// remote zone. `Route53Dns` is wired with -/// `.refreshed_by(|p| p.resync().map_err(|e| e.to_string()))`, so a failing -/// `ListResourceRecordSets` becomes [`ViewError::Unreadable`] instead of a -/// zone that looks empty. That hook is the only thing standing between this -/// crate and a `route53` feature flag it does not want. +/// remote zone. The hook is the only thing standing between this crate and a +/// `route53` feature flag it does not want. +/// +/// # What a refresh hook has to be +/// +/// [`DnsProvider::target`] answers from bookkeeping, so this is a +/// [`ZoneView`] — a *fresh* read — only to the extent the hook makes that +/// bookkeeping match the zone. The hook must therefore be **authoritative**, +/// not additive: after it returns, a name the zone no longer holds must be +/// gone from the bookkeeping and a name the zone answers differently for +/// must have been updated. Reporting the hook's own failure as an error, +/// which turns a failing list call into [`ViewError::Unreadable`] rather +/// than a zone that looks empty, is necessary and is not sufficient. +/// +/// **`Route53Dns::resync` does not meet that bar today.** It hydrates: it +/// inserts what it finds, discards a collision with what is already cached, +/// and removes nothing. Wired here it would leave a reconciler that reads a +/// zone forever and reports it in sync — blind to a record deleted at the +/// provider's console and blind to one repointed there, which are the two +/// drifts this crate exists to repair. Anything wiring a remote provider up +/// to a reconciler owes it an authoritative read first; see +/// `plan/onboarding.md`. pub struct ProviderView

{ provider: Arc

, zone: String, diff --git a/docs/zone-reconciliation.md b/docs/zone-reconciliation.md index 145098e2..0e8586f4 100644 --- a/docs/zone-reconciliation.md +++ b/docs/zone-reconciliation.md @@ -81,7 +81,7 @@ reconciler working from a five-minute snapshot would race it every run. | backoff | double to 1 hour | a tick whose gates failed waits longer; one clean tick clears it | | confirmations | 2 reads | drift must survive a second read before anything is written, so a change in flight is not fought | | repairs per tick | 25 | a wholesale-emptied zone costs a bounded number of writes per tick, not thousands at once | -| attempts per name | 5 | a name that cannot be repaired — *or that keeps drifting back* — is quarantined and reported. The counter clears the moment the name reads correct | +| accepted repairs per name | 5 | a name that keeps drifting back after a repair the provider *accepted* is quarantined and reported. The counter clears the moment the name reads correct, and a repair the provider refused never adds to it | Between the interval and the confirmation count, drift is repaired five to ten minutes after it happens. That is this epic's answer to `plan/onboarding.md`'s @@ -89,9 +89,24 @@ open question of how long drift is propagation and how long it is a fault: long enough that a propagating change is never fought, short enough that a withdrawn hostname is back before an agent's next session. -The attempt budget is the one aimed at people rather than at providers. A -human deliberately changing a record by hand gets argued with five times and -then reported, rather than forever. +The attempt budget is aimed at people rather than at providers, and only at +people. A human deliberately changing a record by hand gets argued with five +times and then reported, rather than forever. + +A repair the provider *refused* is not charged to it, and the two bounds are +not interchangeable. Quarantine is permanent for the life of the process — its +only exit is observing the name correct — so spending it on a provider fault +strands the name: nothing but this machine would ever put the record back, and +quarantine is what stopped it. A refused repair fails a gate instead, which is +what the backoff is computed from, so a provider outage costs one attempt per +tick at a rate falling to hourly and the name is repaired on the first tick +after the provider recovers. + +Every one of these counters lives in memory, so a restart discards all of +them — the backoff included. What keeps that safe rather than expensive is the +confirmation count: a repair needs two consecutive reads from one process, so a +process that crash-loops faster than its own interval reads the zone and writes +nothing at all. ## The gates diff --git a/plan/onboarding.md b/plan/onboarding.md index 7f601156..98050dfb 100644 --- a/plan/onboarding.md +++ b/plan/onboarding.md @@ -225,6 +225,20 @@ default above, restated as what an operator actually sees when it happens. published, and what each should point at" from the account store does not exist. Until it does, a deployment still has nobody whose job the zone is. +- [ ] **Give the reconciler an authoritative read of a remote zone.** Its + other half, and the one that will look done when it is not. + `ProviderView` reads through `DnsProvider::target`, which answers from + local bookkeeping, so it is a fresh view of the zone only to the extent + its refresh hook makes that bookkeeping match. `Route53Dns::resync` is + the obvious hook and does not: it inserts what it finds, discards a + collision with what is already cached, and removes nothing. Wired that + way the reconciler reads the zone every five minutes and reports it in + sync no matter what — blind to a record deleted at the AWS console and + blind to one repointed there, which are the two drifts it exists to + repair. Either `resync` becomes authoritative or the reconciler gets a + `ZoneView` that is a real read; the choice belongs with whoever wires + the intent above, because a reconciler that cannot see drift is worse + than no reconciler — it is a green light over a broken zone. - [ ] **An operator who has lost access to the DID that operates a server.** There is nothing to rotate — the server holds no credential — but the -- 2.51.2