amadan
S3 change index: spec rev 1 and review brief (#336)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+419
−0
docs/superpowers/specs/2026-10-21-s3-change-index-design.md+344 −0docs/superpowers/specs/CODEX-REVIEW-BRIEF-S3-CHANGE-INDEX.md+75 −0
CI cancelled — superseded by a newer push — run details
diff --git a/docs/superpowers/specs/2026-10-21-s3-change-index-design.md b/docs/superpowers/specs/2026-10-21-s3-change-index-design.md| new file mode 100644 |
| index 000000000..eefba204b |
| --- /dev/null |
| +++ b/docs/superpowers/specs/2026-10-21-s3-change-index-design.md |
| @@ -0,0 +1,344 @@ |
| +# S3 change index: stop re-walking the deployment bucket |
| + |
| +Amadan discussion: carlos/platform #336. Branch `s3-polling-336`. |
| +Status: draft rev 1, 2026-10-21. |
| + |
| +## 0. Problem and evidence |
| + |
| +Flagship S3 cost $211 for October by the 20th and runs at ~$16/day, up |
| +from ~$2/day in early September. S3 server access logs (2026-10-20 21:14Z |
| +to 2026-10-21 11:00Z, 5.5M requests, matching Cost Explorer) put nearly all |
| +of it on the deployment bucket, made by the box roles: |
| + |
| +| Family (per day, whole fleet) | Count | ≈ /month | |
| +|---|---|---| |
| +| LIST `apps/<acct>/<app>/instances/` | 1.17M | $175 | |
| +| LIST `apps/<acct>/` (delimiter) | 0.49M | $73 | |
| +| LIST `apps/<acct>/<app>/sidecars/` | 0.34M | $52 | |
| +| GET `control/certs/*` | 1.55M | $19 | |
| +| GET `accounts/<sqid>/*` | 1.41M | $17 | |
| +| GET `apps/*/*/config-gen.json` | 1.23M | $15 | |
| +| GET `apps/*/*/channels/canary` | 0.60M | $7 | |
| + |
| +The cost scales with apps × boxes × passes, not with use: about $3 per app |
| +per month. The causes, all on main at 3d444a1b: |
| + |
| +- `provision.listAppSubtree` (`internal/provision/reconcile.go:2022`) lists |
| + every account and every app in the bucket; `knownAccounts` (`:1760`) |
| + re-GETs every `account.json`. |
| +- Every box runs that walk three times per full pass: `provision.Reconcile` |
| + and `ReconcileSidecars` under `reconcileMu` |
| + (`internal/host/agent.go:1542,1570`), then `ListAllRecordsStrict` in the |
| + moves pass (`internal/host/movespass.go:1079`). `refreshOfflineView` can |
| + add a fourth. |
| +- Full passes fire on any change to the global `control/serial` (any |
| + tenant's console action, read every 2s by `fastTick`) and on the ungated |
| + 5–6 minute `instancesLoop` (`agent.go:3358`). Fitted: ~570 passes per box |
| + per day. |
| +- The console's `StartRunSweep` (`internal/console/runsweep.go:17`) walks |
| + every app every 30s. |
| +- `configGenLoop` (`agent.go:1979`) GETs each hosted app's |
| + `config-gen.json` every 3–4s; `PassFilter` (`agent.go:2628`) GETs every |
| + canary channel pointer on every 2s tick; the edge cert mirror |
| + (`internal/edge/certsync.go:411`) GETs every cert every 30s. |
| + |
| +## 1. Rulings (Paul, 2026-10-21) |
| + |
| +- **R1.** The bucket stays the store of record. No database, cache tier or |
| + DynamoDB in this work (weighed on #336: DynamoDB is ~20x cheaper per |
| + read but is a second required component and a large migration; a cache |
| + needs the same change signal this spec adds). |
| +- **R2.** Fix the change detection: writers record which app changed in |
| + the bucket, readers re-read only what changed, a slow full walk remains |
| + as the backstop. |
| + |
| +## 2. Goals and non-goals |
| + |
| +Goals: |
| +- Steady-state request rate independent of the number of apps when |
| + nothing changes; work proportional to what changed when something does. |
| +- No loss of correctness: every reader converges to the bucket's state; |
| + the bound on staleness for writers that bypass the index is the backstop |
| + interval. |
| +- Provider-neutral: plain S3 API (conditional PUT, conditional GET, |
| + ListObjectsV2), works on MinIO and `DirStore`. |
| +- Target: flagship S3 below ~$100/month, roughly storage, litestream and a |
| + polling floor. |
| + |
| +Non-goals (follow-ups, named so they are not lost): `domains.Reconcile`'s |
| +flat `List("apps/")` (~60 pages every 5–6 min on the housekeeper box, ~$3 |
| +a month), console `materialize`/`settle` `List("apps/")`, the litestream |
| +data bucket, and the `control/serial` mechanism itself, which stays. |
| + |
| +## 3. The change index |
| + |
| +### 3.1 Object |
| + |
| +One object per deployment bucket: `control/changes.json`. |
| + |
| +```json |
| +{"schema": 1, |
| + "entries": {"apps/bab/shop": 41, "accounts/bab": 3, |
| + "control/certs": 977, "control/edges": 12}} |
| +``` |
| + |
| +Each entry is a **scope** and a generation, a positive integer that only |
| +increases. The object is unsigned (like `control/serial`), carries no |
| +authority and is only ever a hint to re-read: a forged or rolled-back index |
| +can delay convergence until the backstop, never change what a reader |
| +believes about a record (the reader always re-reads the records |
| +themselves). Size is ~40 bytes per app; entries are not removed in v1. |
| + |
| +### 3.2 Which writes bump which scope |
| + |
| +A pure function `ChangeScope(key) (scope string, ok bool)` in |
| +`internal/release`: |
| + |
| +| Key | Scope | |
| +|---|---| |
| +| `apps/<a>/<b>/<rest>` where the first segment of `<rest>` is not a data prefix | `apps/<a>/<b>` | |
| +| `apps/<a>/<b>/{logs,errors,analytics,runs,metrics}/…` | none | |
| +| `accounts/<a>/…` | `accounts/<a>` | |
| +| `control/certs/…` | `control/certs` | |
| +| `control/edges/…` | `control/edges` | |
| +| everything else (including `control/changes.json` and `control/serial`) | none | |
| + |
| +The data-prefix list is a denylist on purpose: a missing exclusion costs |
| +an extra re-read, a missing inclusion costs staleness until the backstop. |
| +`runs/` is excluded because run state that readers act on lives in the |
| +instance record (`ActiveRun`), which is in scope. The exact data-prefix set |
| +is fixed in the plan from the writer map (#336 thread) and pinned by a |
| +table test. |
| + |
| +### 3.3 Write path |
| + |
| +The bump lives in the store, not at call sites. A wrapper |
| +`release.WithChangeIndex(s Store) Store` decorates the deployment store; |
| +after a successful `Put` or `Delete` whose key has a scope, it bumps that |
| +scope: |
| + |
| +1. `Get control/changes.json` (absent = empty, schema 1). |
| +2. Set `entries[scope] = old + 1` (or 1). |
| +3. `Put` with `If-Match` on the ETag read, or create-only if absent. |
| +4. On a precondition failure, retry from 1, at most 4 attempts with |
| + jitter, all inside a 2s budget detached from the caller's cancellation |
| + (the `bumpSerial` pattern, `internal/console/serialbump.go:31`). |
| + |
| +Properties: |
| +- **Bump after write, never before.** A reader that sees the new |
| + generation and re-lists is guaranteed (S3 strong read-after-write) to see |
| + the write. Bumping first would let a reader re-list before the write and |
| + record the new generation, missing the write until the backstop. |
| +- **Best effort.** A failed bump is logged and never fails the write. The |
| + write is then visible at the next backstop walk, or sooner if anything |
| + else bumps the same scope. |
| +- **Coalescing** (plan detail): a process may batch several scopes into one |
| + read-modify-write, provided each scope's bump still follows its write. |
| +- The wrapper forwards the optional extensions (`ModLister`, the new |
| + `ETagLister`, `ConditionalGetter`) so wrapping never removes a |
| + capability (today `NewPrefixStore` drops them, |
| + `internal/release/store.go:215-217`). |
| + |
| +Every opener of the deployment store wraps it: the console |
| +(`cmd/console/main.go`), the box agent and edge (`internal/host/agent.go`, |
| +`internal/edge/edge.go`), the CLI's direct-bucket `openStore()` |
| +(`cli.go:2065`) and every other `release.NewS3Store` call site that opens |
| +the deployment bucket. A test walks the AST for `release.NewS3Store`/ |
| +`NewS3StoreIn` call sites and requires each to be listed as either |
| +"deployment, wrapped" or "other bucket" with a reason, so a new opener |
| +cannot silently skip the index. |
| + |
| +Writes that still bypass the index: binaries built before this change |
| +(old CLIs in direct-bucket mode, boxes mid-roll), and any tool writing the |
| +bucket directly. The backstop (§4.4) covers them. |
| + |
| +### 3.4 Read path |
| + |
| +Optional extension `ConditionalGetter.GetIfChanged(ctx, key, etag)` returns |
| +`(body, etag, changed, err)`; S3 answers 304 Not Modified for an unchanged |
| +object. A reader polls `control/changes.json` with it and diffs `entries` |
| +against the generations it last acted on. A 304 costs one GET ($0.40 per |
| +million), so a 2s poll is ~43k GETs per process per day. |
| + |
| +## 4. The record view (box agent and console) |
| + |
| +### 4.1 State |
| + |
| +A `RecordView` in `internal/provision` holds, for the whole deployment: |
| + |
| +- per account: known/soft-deleted, from `account.json`, with the |
| + generation it was read at; |
| +- per app: the `instances/` and `sidecars/` key listings with each key's |
| + ETag, the parsed record bodies, the trash marker and the `manifest.json` |
| + facts Reconcile reads per pass (`reconcile.go:983-1004`), and the app's |
| + generation; |
| +- `complete` and `unreadable` flags with today's meanings (strict |
| + consumers fail on an incomplete view exactly as they fail on a partial |
| + walk today). |
| + |
| +It produces the same `[]Record` / sidecar slices and the same |
| +`known`/`readFailed` results the walk produces today, so `Reconcile`, |
| +`ReconcileSidecars`, the moves pass, `steeringLoop` and the console sweeps |
| +consume it without changing their logic. |
| + |
| +### 4.2 Refresh |
| + |
| +`Refresh(ctx)`: |
| +1. Conditional GET of the index. Unchanged: done. |
| +2. For each scope whose generation moved (or is new): re-read that scope. |
| + An app scope re-lists `instances/` and `sidecars/` with |
| + `ETagLister.ListETags` and GETs only keys that are new or whose ETag |
| + changed; it re-reads the trash marker and manifest. An account scope |
| + re-reads `account.json`. |
| +3. Record the new generations only for scopes re-read successfully. A |
| + failed scope keeps its old generation (so the next refresh retries) and |
| + marks the view incomplete. |
| + |
| +`ETagLister` is a new optional extension: S3 returns the ETag with every |
| +ListObjectsV2 entry; `DirStore` returns the same ETag its `Get` returns. |
| +Using ETags instead of LastModified avoids the one-second resolution trap |
| +that `ClaimCache` needs a settle window for. |
| + |
| +### 4.3 Ordering and monotonicity |
| + |
| +- The view only moves forward: each scope is replaced by a newer listing, |
| + never an older one. |
| +- In the agent, `Refresh` for a Reconcile pass runs inside `reconcileMu`, |
| + so the snapshot a pass uses is never older than the previous pass's (the |
| + reason `reconcileMu` exists, `agent.go:1529-1542`). |
| +- One full pass refreshes once and feeds Reconcile, ReconcileSidecars and |
| + the moves pass from the same view, replacing three walks with one |
| + refresh. The moves pass, which runs under its own `w.pass` lock, reads a |
| + copy taken after the Reconcile refresh; it is at least as fresh as its |
| + own walk would have been when it started. |
| + |
| +### 4.4 Backstop walk |
| + |
| +A full strict walk (today's code path, with ETag reuse for bodies) rebuilds |
| +the view from scratch every `changeIndexBackstop` (default 30 minutes, |
| +jittered ±10%) per process, and at start. Its result replaces the view |
| +wholesale and resets every generation to the index's current values read |
| +**before** the walk began (so a bump that lands during the walk triggers a |
| +re-read afterwards, never a skip). |
| + |
| +The backstop is what bounds staleness for writers that bypass the index. |
| +Today those writers (box run/seed/move writes, direct-bucket CLI writes) |
| +already rely on the 5-minute floor; with the index they converge in |
| +seconds, and only failures or old binaries wait for the backstop. |
| + |
| +### 4.5 What triggers a pass |
| + |
| +- `fastTick` keeps reading `control/serial` every 2s as today and also |
| + polls the index (§3.4). Either change triggers a full pass, which is now |
| + a `Refresh` plus the existing reconcile logic. The 5s clamp stays. |
| +- `instancesLoop`'s 5–6 minute floor stays, now as a `Refresh` (one |
| + conditional GET when nothing changed) rather than a walk. |
| +- The backstop is separate (§4.4). |
| + |
| +### 4.6 Console sweeps |
| + |
| +- **Run sweep** (30s): keeps its interval and reads the console's own |
| + `RecordView` after a `Refresh`. Run state the boxes write |
| + (`SetRecordRun`) is an instance-record write and bumps the app's scope |
| + once the boxes run this code. Minutes-stale is already safe here |
| + (`KeepFor` 24h, CAS on every change). |
| +- **Seed sweep** (5 min): keeps its strict walk, because its snapshot |
| + deletes are proved by absence and must see records listed after the |
| + snapshot listing (`seedsweep.go:126-131`). It now skips the walk |
| + entirely when the snapshot listing is empty, since there is nothing to |
| + delete; stale-seed abandonment reads the view. |
| + |
| +## 5. GET loops |
| + |
| +- **`configGenLoop`**: GETs an app's `config-gen.json` only when the app's |
| + scope generation moved, or when its last read is older than the backstop |
| + interval. `genCache` (`agent.go:1641`) stays as the ETag comparison. |
| +- **Canary pointers** (`fastMembers` → `PassFilter`): canary routes stay |
| + members of every tick, but `PassFilter` reads a channel pointer only when |
| + its app's generation moved or its last read is older than the backstop. |
| + Direct-bucket promotes from new CLIs bump the app scope, so canaries keep |
| + adopting in seconds; old CLIs fall back to the backstop. |
| +- **`telemetryPass`**: app and account `telemetry.json` reads are gated on |
| + their scope generations, and the account file is read once per pass, not |
| + once per app. |
| +- **Cert mirror** (`certsync.go:627`): polls the index; when |
| + `control/certs` moves, `ListETags` and GET only new or changed certs; the |
| + full rebuild that drops spent challenge tokens runs at the backstop and |
| + whenever a key disappears from the listing. The boot refresh before the |
| + listeners bind is unchanged. |
| +- **Home map** (`internal/edge/homemap.go`): the same rule for |
| + `control/edges`. |
| + |
| +## 6. Provider boundary |
| + |
| +Everything is core: conditional PUT and GET and ListObjectsV2 ETags are |
| +plain S3 API, supported by MinIO and implemented by `DirStore`. No AWS SDK |
| +outside `internal/release`'s existing S3 client. A deployment that never |
| +arms the readers behaves exactly as today. `TestDeliveredContractMinio` |
| +gains coverage for `GetIfChanged` and `ListETags`. |
| + |
| +## 7. Rollout |
| + |
| +Writers ship first and always bump (cost: ~one extra GET and PUT per |
| +relevant write, ~$3/month). Readers are armed per process by |
| +`CARLOS_CHANGE_INDEX=read` (box `host.env`, console environment), off by |
| +default, so a mixed fleet behaves as today. |
| + |
| +1. Release with the writers and dormant readers. Roll dev boxes, dev |
| + console, flagship boxes, flagship console (roll order rule). |
| +2. Arm dev (boxes then console). Check: passes still converge (instance |
| + create, delete, config set, promote, a run) within seconds; the |
| + backstop log line appears every ~30 minutes; request rate on dev's |
| + bucket drops (Cost Explorer daily, usage type by operation). |
| +3. Arm flagship boxes, then the console. Measure Cost Explorer daily |
| + ListBucket/GetObject for three days against the 2026-10-19 baseline |
| + (2.26M LIST, 7.5M GET). |
| +4. Disarm is removing the variable and restarting: readers fall back to |
| + walking, writers keep bumping harmlessly. |
| + |
| +The census overlay for the touched packages is owed per the plan14 |
| +process (#280). |
| + |
| +## 8. Testing |
| + |
| +- **Production-wired witnesses**: the writer wrapper is exercised through |
| + the real openers (console, agent, CLI direct mode) against `DirStore`, |
| + not through a fake; the AST opener test from §3.3. |
| +- **Invariant simulation**: random interleavings of writers (some |
| + bumping, some bypassing, some failing their bump), concurrent readers |
| + refreshing, and backstop walks, on `DirStore`. Invariants: (a) after a |
| + `Refresh` that observed generation g for a scope, every write to that |
| + scope that bumped to ≤ g is visible in the view; (b) after a backstop |
| + walk, the view equals a fresh strict walk; (c) the view never moves |
| + backwards; (d) a strict consumer never receives an incomplete view as |
| + complete. One 3000-seed sweep per revision, niced, per the CPU budget. |
| + Mutation checks: remove bump-after-write ordering, record generations |
| + for failed scopes, skip the before-walk index read in §4.4; each must |
| + fail the simulation. |
| +- Table tests for `ChangeScope`. |
| +- Unit tests for `GetIfChanged` and `ListETags` on both stores, plus MinIO |
| + in the delivered-contract test. |
| + |
| +## 9. Expected effect |
| + |
| +| Family (per day) | Today | After (estimate) | |
| +|---|---|---| |
| +| Record walks (LIST) | ~2.0M | ~70k (backstop 48/day/process + changed apps) | |
| +| Index + serial polls (GET) | 0.13M | ~0.35M | |
| +| Record and manifest bodies (GET) | ~2M | changed only | |
| +| config-gen + canary + certs + accounts (GET) | ~4.8M | changed only + backstop | |
| + |
| +Expected flagship S3: ~$50–90/month (storage $16, transfer ~$25, |
| +litestream ~$26, requests ~$10–25), flat as apps are added. |
| + |
| +## 10. Questions for Paul |
| + |
| +1. **Backstop interval.** 30 minutes bounds staleness for old binaries and |
| + failed bumps at ~$5/month. 15 minutes doubles that cost; 60 halves it. |
| + Recommendation: 30. |
| +2. **Old CLIs in direct-bucket mode** (promote, env set, trash): their |
| + writes converge at the backstop (up to 30 minutes) instead of within |
| + seconds as today (`configGenLoop`'s 3s poll, canary pointers read every |
| + tick) until operators update the CLI. Console and new-CLI writes are |
| + unaffected. |
| + Recommendation: accept, and note it in the release notes. |
diff --git a/docs/superpowers/specs/CODEX-REVIEW-BRIEF-S3-CHANGE-INDEX.md b/docs/superpowers/specs/CODEX-REVIEW-BRIEF-S3-CHANGE-INDEX.md| new file mode 100644 |
| index 000000000..cb4759e6d |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-REVIEW-BRIEF-S3-CHANGE-INDEX.md |
| @@ -0,0 +1,75 @@ |
| +# Review brief: S3 change index |
| + |
| +You are an independent second reader of a design spec for the CARLOS |
| +platform (this repository). Read cold and adversarially. |
| + |
| +**Spec:** `docs/superpowers/specs/2026-10-21-s3-change-index-design.md` |
| +at the commit named in the prompt. |
| + |
| +## Fixed rulings, do not re-argue |
| + |
| +§1 of the spec lists the product owner's rulings: the bucket stays the |
| +store of record (no database, cache tier or DynamoDB), and the fix is a |
| +change index in the bucket with a backstop walk. Do not propose |
| +alternatives to them. Report only mechanisms that **fail to honour** a |
| +ruling, fail the spec's own goals (§2), or break something else in the |
| +platform. Cost estimates are context, not something to audit to the cent. |
| + |
| +## What to check |
| + |
| +Verify against the code, not the spec's description of it. Cite |
| +`file:line` for every claim about existing code. The places that matter: |
| + |
| +- `internal/release/store.go` (Store, prefixStore, ModLister, S3Store |
| + Put/Get/ListMod, DirStore), `internal/release/serial.go`, |
| + `internal/console/serialbump.go`. |
| +- `internal/provision/reconcile.go` (`Reconcile`, `knownAccounts`, |
| + `ListAllRecords*`, `listAppSubtree`, the per-app trash and manifest |
| + reads), `sidecar_reconcile.go`, `record_sequence.go` (`WriteRecord`), |
| + `provision.go` (`DeleteRecord`). |
| +- `internal/host/agent.go` (`reconcileMu`, `fastTick`, `fullPass`, |
| + `fastMembers`, `PassFilter` call, `configGenLoop`, `genPollPass`, |
| + `instancesLoop`), `movespass.go`, `moveshold.go`, `movesoffline*.go`, |
| + `steering_loop.go`, `telemetry_pass.go`, `runs.go`, `seedwiring.go`. |
| +- `internal/console/runsweep.go`, `seedsweep.go`, `apiruns*.go`. |
| +- `internal/edge/certsync.go`, `homemap.go`, `storedoor.go`. |
| +- `cli.go` (`openStore`), `appconfig_cli.go`, every |
| + `release.NewS3Store`/`NewS3StoreIn` call site. |
| +- `internal/hostops` mutation boundary and classifier. |
| + |
| +Questions worth your time: |
| + |
| +1. **Missed changes.** Is there any write that a reader acts on whose key |
| + §3.2 maps to no scope, or to the wrong scope? Any reader input outside |
| + the scopes (for example something Reconcile or the moves pass reads |
| + that is not under `apps/<a>/<b>/` or `accounts/<a>/`)? Any data prefix |
| + wrongly excluded? |
| +2. **Ordering.** Does bump-after-write plus re-list hold under every |
| + interleaving with concurrent writers, the backstop walk (§4.4) and the |
| + moves pass's separate lock (§4.3)? Can a view ever move backwards, or a |
| + strict consumer get an incomplete view as complete? |
| +3. **Removal safety.** Reconcile removes rows whose host is missing; |
| + moves and seed sweep act on absence. Can a stale or partially refreshed |
| + view cause a removal, a lost grant, a wrong move step or a snapshot |
| + delete that today's walk would not? |
| +4. **The writer hook.** Does wrapping at the store level actually catch |
| + every writer (hostops boundary, prefix stores, store doors for fleet |
| + boxes, writers that use `S3Store` directly, CAS helpers)? Does the |
| + wrapper break any caller that type-asserts optional extensions? What |
| + happens with contention on one index object under bursts (deploys, |
| + purges, run churn)? |
| +5. **The GET loops** (§5): can gating `configGenLoop`, canary |
| + `PassFilter`, telemetry, the cert mirror or the home map on generations |
| + lose a change, a challenge token or a canary adoption that today's |
| + polling catches? |
| +6. **Rollout** (§7): is a mixed fleet (old writers, new readers armed) |
| + safe given the flag, and is disarm really a clean fallback? |
| +7. Anything the spec relies on that does not exist, or exists with a |
| + different shape. |
| + |
| +## Output |
| + |
| +A numbered list of findings, each with severity (P1 = correctness or |
| +data loss, P2 = goal failure or significant gap, P3 = minor), the spec |
| +section, the code evidence with `file:line`, and the change you would |
| +require. End with a verdict: accept, or reject with the P1s that block. |