| index 999bbf135..dd5fee0c7 100644 |
| --- a/docs/superpowers/plans/2026-08-23-multi-region-steering.md |
| +++ b/docs/superpowers/plans/2026-08-23-multi-region-steering.md |
| @@ -42,7 +42,10 @@ zones are Route53 — dated spec note in Task 8). |
| **AMENDED (controller, 2026-08-23, on Task 2's review):** only |
| `SnapshotFingerprint` rides routeCols — pin it. `Fingerprint` is a |
| hand-curated reload-relevance set with NO steering consumer on the |
| - edge (the converger is a host duty reading the registry directly): |
| + edge (the converger is a host duty reading bucket records directly — |
| + **corrected 2026-08-22, T8**: Task 6's review ruled the converger |
| + reads `provision.ListAllRecords`, never the registry; see that task's |
| + carry below and the spec's Part 4 amendment): |
| steering stays OUT of `Fingerprint`, recorded as a deliberate |
| exclusion in the unit-column idiom. Likewise `shapeMismatch` does NOT |
| compare steering — it is a record-owned column (Channel/Environment |
| @@ -288,7 +291,10 @@ func desiredState(routes []registry.Route, edges []edgesFileView, appsDomain str |
| **Interfaces:** |
| - Consumes: Tasks 4-5; `CARLOS_STEERING_ZONE_ID` (unset → the loop arms, |
| logs once "steering: zone unset — dormant", duty returns nil |
| - immediately); the registry for routes; the store for edges files. |
| + immediately); the store for edges files and (**corrected 2026-08-22, |
| + T8, per the review ruling below**) for routes too — the registry is |
| + NOT consulted; `provision.ListAllRecords` walks bucket records |
| + instead, because routes are not uniformly provisioned across boxes. |
| - Dormancy statement: on today's boxes the env is unset → one log line |
| at arm, zero API calls, zero writes. With the env set but one box: the |
| steering hostname + any steered hosts get single-edge latency records |
| @@ -334,7 +340,7 @@ func desiredState(routes []registry.Route, edges []edgesFileView, appsDomain str |
| dated housekeeper-not-console DNS-writer amendment; Route53-only |
| note; the pool-rail v1 note) + this plan's rollout notes. |
| |
| -- [ ] Fill rollout notes from the ledger + these knowns: arming order |
| +- [x] Fill rollout notes from the ledger + these knowns: arming order |
| per deployment = tofu apply (steering_zone_ids) → set |
| `CARLOS_STEERING_ZONE_ID` on the box → verify the housekeeper's |
| converge log → steer one guinea-pig host → dig from two vantage |
| @@ -344,13 +350,69 @@ func desiredState(routes []registry.Route, edges []edgesFileView, appsDomain str |
| commands ready-to-run); edges-file `role` over-claim note (steering |
| reads the ARMED-EDGE set, not roles — unaffected, but read-local |
| still wants the replica role first, restated). |
| -- [ ] Full gate; commit `docs: steering rollout notes`. |
| +- [x] Full gate; commit `docs: steering rollout notes`. |
| |
| ## Rollout notes (filled by Task 8 at execution) |
| |
| -- (filled at execution) |
| - |
| -Pre-filled by Task 7 (fold into the list above at Task 8): |
| +This plan ships stage 4 (spec Part 4) dormant on single-box deployments — |
| +nothing below is armed by merging this branch. It is the checklist and |
| +open-questions list for the stage-5/6 rollout that follows, synthesized |
| +from the SDD ledger (`progress.md`) across all seven implementation |
| +tasks and their reviews, reconciling plan-2's (multi-region-backhaul) |
| +rollout notes and constraints where they name Part 4 directly. |
| + |
| +### Arming checklist (per deployment, in order) |
| + |
| +1. **Confirm plan-2's stage-5 checklist items 1–2 already hold for this |
| + deployment before arming steering at all** (consumed here, not |
| + repeated in full — see `2026-08-22-multi-region-backhaul`'s rollout |
| + notes): every box meant to be armed has published |
| + `control/edges/<box>.json` (`aws s3 ls |
| + s3://<control-bucket>/control/edges/` — one object per armed box, |
| + no gaps), and `control/edge-mesh-key` exists and every armed box's |
| + mirror has loaded it. Steering's desired-state fold only ever sees a |
| + box through its edges file (Part 4's `armedEdges`), so a box missing |
| + one is invisible to the converger exactly as it is invisible to the |
| + home map — it gets no latency records and answers no traffic, silently. |
| +2. `tofu apply` with `steering_zone_ids` (and, per the IAM note below, |
| + `steering_record_names`) set for the deployment's region-host module — |
| + **Paul's applies rule**: tofu apply sessions are blocked on Paul |
| + running them; this plan does not run one. Have the exact command |
| + ready (module path + var values) so the apply is a single approved |
| + step, not a design conversation at apply time. |
| +3. Set `CARLOS_STEERING_ZONE_ID` on **both units on both armed boxes** — |
| + the agent unit (converger duty + edge probe + wildcard parent, one |
| + process) and the console unit (custom-domain CNAME guidance). See the |
| + "two units" mechanism note below; a unit that misses the env stays |
| + silently on old behaviour, not broken, so this step has no error |
| + signal of its own — the next two steps are how you catch a miss. |
| +4. Verify the housekeeper's converge log on whichever box holds the |
| + lease (`journalctl -u carlos -g "^steering:"` — this is the |
| + canonical operator surface, not the housekeeper's own error log; see |
| + the duty-status mechanism note below) — expect the "steering loop |
| + armed... converges only while this box holds the housekeeper lease" |
| + line, then an actual convergence pass once records exist. |
| +5. **Per-EIP wildcard verification BEFORE treating steering as live — |
| + this is a hard gate, not a nice-to-have.** For every armed EIP: |
| + `curl --resolve edge.<apps-domain>:443:<EIP> https://edge.<apps-domain>/` |
| + and expect the steering-probe 200 body |
| + (`carlos steering probe ok`), or confirm the wildcard line in that |
| + box's journal (`edge: wildcard *.<apps-domain> obtained` on the |
| + issuer, `loaded from cache` on the others). Until every EIP passes |
| + this, some of the health checks Route53 is about to run WILL fail at |
| + the handshake — see the wildcard-propagation mechanism note. |
| +6. Steer one guinea-pig host (`carlos steering -app X -host H latency`) |
| + — pick something low-stakes, per the spec's flagship-AU-first |
| + rationale (a mistake here "burns nobody's ticket sales"). |
| +7. Dig from two vantage points (different geographic resolvers — e.g. a |
| + local machine plus a cloud shell in another region) and confirm each |
| + gets a DIFFERENT armed EIP back, consistent with its own latency to |
| + each edge. A single vantage point cannot distinguish "latency routing |
| + worked" from "there is only one record." |
| +8. Only after 1–7 hold for the first armed pair: repeat for the next |
| + region, then move to the next deployment (tito UK/SE, stage 6). |
| + |
| +### Mechanism notes |
| |
| - **A dead box is never retired by anything but the health check.** A box |
| writes `control/edges/<box>.json` and nothing ever deletes it: an |
| @@ -403,3 +465,221 @@ Pre-filled by Task 7 (fold into the list above at Task 8): |
| armed**, not just the local box's: the DNS check that stamps a custom |
| domain `pointing` compares the resolved address against that list, and a |
| steered name resolves to whichever edge the resolver is nearest. |
| +- **The desired set comes from BUCKET RECORDS, never the lease-holder's |
| + local registry.** `provision.ListAllRecords` (Task 6's review fix, |
| + `dc30c28`) walks `accounts/` + `apps/` FAIL-CLOSED: any unreadable |
| + account manifest or any record `Get` failure aborts the whole read with |
| + no partial result, because a short list here is indistinguishable from |
| + records having been deleted and would NXDOMAIN a live host. This is why |
| + a housekeeper lease migration does not flap home-boxed steered hosts — |
| + every box reads the same bucket truth regardless of which one holds |
| + the lease. |
| +- **A refused pass reports green.** `steeringDuty` returns `nil` whether |
| + the tick actually converged or was refused by one of the two safety |
| + guards (empty desired-set, or the shrink guard below) — deliberately, |
| + so a routine 5-minute error cadence does not bury the one log line that |
| + explains what happened. The housekeeper's own success/failure surface |
| + is therefore NOT where steering health lives: read the journal |
| + (`journalctl -u carlos -g "^steering:"`) after any arming or |
| + retirement action, don't trust a clean housekeeper status alone. |
| +- **A GENUINE >half-record withdrawal has no escape hatch.** The |
| + proportional shrink guard (Task 5, hardened in the fix rounds) refuses |
| + to delete more than half the zone's managed records — or more than a |
| + small floor for tiny zones — on EVERY tick, forever, once triggered, |
| + with no override flag. This is deliberate (an accidental multi-box |
| + retirement should never silently wipe a zone), but it means a REAL |
| + multi-box retirement needs an operator action, not a wait: either |
| + delete the excess records by hand in Route53, or retire boxes in |
| + batches that each stay under the limit. Threading a status flag back |
| + through the housekeeper duty contract to make this self-healing was |
| + considered and deliberately left out (Task 6) — it would change the |
| + contract for every housekeeper duty, not just this one. |
| +- **"Zero API calls" when dormant is honest about Route53, not about |
| + every credential path.** The steering duty's own dormancy log says |
| + "the duty reads nothing and calls no AWS API," which is true for |
| + Route53 — `steering.NewClient` is only constructed inside the |
| + zone-armed branch. It is NOT a claim that a dormant box touches |
| + nothing at all: `openDeployStore` (common to every housekeeper duty, |
| + not steering-specific) resolves AWS credentials through the default |
| + chain, which may reach IMDS regardless of whether steering is armed. |
| + Read "dormant" as "no Route53 calls, no DNS writes" — not literally |
| + zero network activity. |
| +- **An empty accounts listing is a theoretical failure mode, not a |
| + designed-for one.** If `provision.ListAllRecords` ever returned |
| + success with zero accounts (every account manifest simultaneously |
| + unreadable in a way that somehow does not trip the fail-closed path — |
| + not reachable by any known code path today, but named here because the |
| + consequence would be severe if it ever happened), every tenant's |
| + steered records would look managed-but-unwanted and the ONLY thing |
| + standing between that and mass deletion is the shrink guard above. |
| + Worth an eye during arming (does the first real convergence log a |
| + record count that matches expectations?), not a blocker. |
| +- **A soft-deleted account's records are a deliberate, silent drop — |
| + correct, not a bug.** `knownAccounts` (`internal/provision/reconcile.go`) |
| + skips any account manifest with `m.Deleted() == true`, so that |
| + account's records fail `ParseRecordKey`'s known-account guard and |
| + never reach `ListAllRecords`'s output. DNS follows the same retirement |
| + `Reconcile` already applies elsewhere — an account a member deleted |
| + should not keep steering traffic. `ListAllRecords`'s own doc comment |
| + does not currently name this behaviour explicitly (Task 6 minor, |
| + triaged below as a fix-wave candidate); the behaviour itself is |
| + correct and needs no change. |
| + |
| +### Constraints from plan 2 (multi-region-backhaul), checked against what shipped |
| + |
| +- **Pool narrowing (backhaul plan's constraint #6): honored, and more |
| + strictly than asked.** The constraint required that a steered |
| + non-shared-pool route only get latency records for the armed edges |
| + that actually serve its pool. Task 3's v1 pool rail goes further: the |
| + console door and CLI REFUSE `steering:"latency"` outright on any route |
| + whose `Pool != ""` (409), so no pool-scoped route ever reaches the |
| + desired-state fold in the first place; the fold's own defensive |
| + exclusion (logged) is the belt-and-suspenders backstop for a route |
| + that reaches the bucket by some other path. Per-pool record scoping — |
| + the constraint's original, narrower ask — is recorded as future work |
| + (spec "Anticipated, not built" §3). |
| +- **Edges-file `role` over-claim (backhaul plan's item 9): resolved by |
| + construction, not by a role change.** The concern was that Part 4 |
| + might read the `role: "home"` field a static-route adopter over-claims |
| + and make a wrong read-local decision. Confirmed by code |
| + (`internal/steering/steering.go`'s `edgesFileView`): the steering |
| + converger's edges-file reader carries only `V`/`Box`/`EIP`/`Region` — |
| + `Hosts`/`role` is deliberately absent, with a doc comment stating why |
| + (steering is any-edge-any-app; the fold never needs to know which |
| + hosts a box homes). Steering is therefore unaffected today. The |
| + underlying want — a `"replica"` role before the read-local |
| + anticipation can trust this map — is restated in the spec (Part 3 |
| + amendment) as still open for whoever builds that feature, not solved |
| + here. |
| +- **T7 egress ruling (backhaul plan's item 5): stays open, no issue |
| + number yet.** Under-metered, abuse-surface-shaped egress for |
| + backhauled bytes — billed on the sending box's AWS account, never |
| + attributed in CARLOS usage — needs Paul's ruling on the containment |
| + approach (a per-home-box in-flight semaphore is the option on the |
| + table) before it gets a numbered issue, per the backhaul plan's own |
| + note. That plan flagged this as needing resolution BEFORE steering |
| + sends real, sustained cross-region traffic — which stage 5 of this |
| + plan is about to start doing. **Recommendation: raise this with Paul |
| + before or alongside the stage-5 arming decision, not after** — the |
| + window the backhaul plan called out ("today it is dormant/low-volume |
| + by construction... which is the window in which to decide, not |
| + after") ends the moment stage 5 begins. |
| + |
| +### Deferred-minor triage (every task's ledger entry, included or excluded with a reason) |
| + |
| +**Task 1** (stage-5 gates): |
| +- seed degrade-path untested (no-prior/corrupt → nil) — RECORDED. Has a |
| + behavioural precedent (the snapshot loop's analogous case) to pin |
| + against later; no known defect. |
| +- backhaul throttle key-isolation unpinned package-wide — FIX-WAVE |
| + CANDIDATE. The suggested pin (stub `logThrottled` + erroring |
| + `RoundTripper`) is cheap and non-flaky; a future constant-key refactor |
| + could silently collapse per-box throttle buckets with no test to catch |
| + it. |
| +- edgesFileView needs `region` when Task 4 consumes it — DONE. Task 4 |
| + added it (`Region` field, `edgesFileView`); closed, no action. |
| +- spec Part 3 prose lacks region — DONE, this task (spec amendment |
| + above, Part 3's Home-resolution section). |
| + |
| +**Task 3** (console door + CLI): |
| +- pool 409 on an addrs-only patch over a hand-written already-steered |
| + +pooled bucket object — RECORDED. Unreachable via any supported |
| + writer; a wording note only if a hand-edited bucket object ever |
| + produces it in practice. |
| +- steering-verb refusal branch untested (`os.Exit` path) — FIX-WAVE |
| + CANDIDATE. Same-shaped precedent already exists elsewhere in the CLI |
| + test suite to copy from; cheap to add. |
| +- `provision.go:113`'s "Task 3" prose reads as forward-looking — |
| + RECORDED, not fixed here. Verified: it refers to a DIFFERENT plan's |
| + Task 3 (`2026-08-04-instance-provisioning-design`), not this plan's — |
| + stale-sounding but not factually wrong, and outside this task's |
| + declared file scope (spec + this plan only). One-line doc-comment |
| + polish for a future docs pass. |
| + |
| +**Task 4** (desired state): |
| +- 3-way collision subtest wanted — FIX-WAVE CANDIDATE. Correct by trace |
| + through `dedupeCaseCollisions`/`dedupeRecords`, but unpinned by a test |
| + that actually exercises three colliding rows at once. |
| +- a member steering a route literally named `edge.<apps-domain>` — |
| + RECORDED, verified this session. `dedupeRecords`'s (Name, Box) key |
| + collision catches it (first kept, collapse logged); confirmed by |
| + reading `internal/steering/steering.go`'s `desiredState`/`dedupeRecords` |
| + directly. No code change needed. |
| + |
| +**Task 5** (Route53 converger) — the largest minors list, and the one |
| +most already self-documented as deliberate scope notes in the code |
| +itself (`internal/steering/route53.go`), verified by direct reading this |
| +session rather than assumed from the ledger: |
| +- `steeringHostname` matches any `edge.*` prefix (order-dependence on |
| + the synthetic record being first) — RECORDED. Documented in code; |
| + `desiredState` always emits the synthetic record first, deterministically. |
| +- malformed record wedges-vs-skip-loudly — RECORDED. Matches |
| + `ListAllRecords`'s documented skip-and-log policy; consistent with |
| + every other defensive layer in this package, not a gap. |
| +- duplicate-check self-heal takes two passes (comment reads optimistic) — |
| + RECORDED. Self-heals correctly; a comment-accuracy nit, not a defect. |
| +- GC's foreign-surviving-reference gate untested — FIX-WAVE CANDIDATE. |
| + Safety-relevant code path (`gcHealthChecks`'s "still referenced, leave |
| + it" branch) with no direct test. |
| +- 32KB batch limit unmentioned — RECORDED. `changeBatchLimit` is already |
| + conservative relative to Route53's real cap; a documentation nicety. |
| +- wanted-key self-defense in the delete loop (relies on an upstream |
| + guarantee) — RECORDED, see the `wantKey`/`recordKey` item next; same |
| + underlying invariant. |
| +- `wantKey` builds `rec.Name` unlowercased while `recordKey` explicitly |
| + lowercases — the ledger flagged this for "final review" to triage. |
| + Verified this session: `recordKey`'s own comment already states the |
| + invariant honestly (`desiredRecord.Name` is lowercase by contract, via |
| + `steering.go`'s `fqdn`) — the risk is a silent miss ONLY if some future |
| + caller ever handed `wantKey` an unfolded name, which nothing in this |
| + package does today. Triage: RECORDED as an accepted, documented risk |
| + (matches this file's own "structural backstop" pattern elsewhere); |
| + FIX-WAVE CANDIDATE for defense-in-depth (a one-line |
| + `strings.ToLower(rec.Name)` in `wantKey` would remove the invariant |
| + dependency entirely) — not urgent, since the fold that feeds it cannot |
| + currently produce a counterexample. |
| + |
| +**Task 6** (housekeeper duty): |
| +- "zero API calls" dormancy overclaim — ADDRESSED, this task (mechanism |
| + note above corrects the wording: no Route53 calls, not literally zero |
| + network activity). |
| +- edges files have no retirement path — ALREADY CARRIED (Task 7's |
| + pre-filled dead-box mechanism note above; Task 6 independently flagged |
| + the same fact). |
| +- keyed parse-log variant when the `[][]byte` pin lifts (~6 lines) — |
| + RECORDED. A forward-looking hook for a refactor that has not happened; |
| + no current need. |
| +- duty-status surface + >half-withdrawal no-escape — ADDRESSED, this |
| + task (spec's Failure-modes bullet + the two mechanism notes above). |
| +- soft-deleted account doc naming on `ListAllRecords` — FIX-WAVE |
| + CANDIDATE. Cheap, correctness-adjacent doc-comment addition (behaviour |
| + itself is verified correct, see the constraint section above). |
| +- `ListAllRecords` wants its own package-level table test — FIX-WAVE |
| + CANDIDATE. New exported surface with only indirect test coverage today |
| + (via the steering loop's tests). |
| +- successful-but-empty accounts listing (theoretical) — ADDRESSED, this |
| + task (mechanism note above). |
| +- stale no-registry-reads test comment — FIX-WAVE CANDIDATE. Trivial |
| + wording fix in a test file, no functional impact. |
| + |
| +**Task 7** (IAM + guidance): no standalone minors — every fix-round item |
| +(the autocert reasoning correction, the trailing-dot normalization, the |
| +IAM name bound, the two-units correction, the disclosure sentence) was |
| +folded into either the code or the pre-filled mechanism notes above by |
| +the task's own two fix rounds. Nothing left to triage. |
| + |
| +### For Paul |
| + |
| +- **The tofu applies** (arming checklist step 2): `steering_zone_ids` + |
| + `steering_record_names` for each deployment's region-host module — |
| + commands ready, not run by any agent session per the applies rule. |
| +- **The env sets** (arming checklist step 3): `CARLOS_STEERING_ZONE_ID` |
| + on both units on both boxes, per deployment, alongside the apply. |
| +- **The T7 egress ruling** (cross-ref above): a decision on the |
| + containment approach (or an explicit "ship steering first, decide |
| + later") is needed before or alongside the stage-5 arming decision — |
| + raised here rather than left buried in plan 2's own notes, since this |
| + is the plan whose stage-5 work makes the ruling time-sensitive. |
| +- **PR routing**: this branch merges to `main` directly — no stack, no |
| + base branch other than `main` (see the PR body, drafted in the task |
| + report). |
amadan