| 1 | # Review brief: S3 change index |
| 2 | |
| 3 | You are an independent second reader of a design spec for the CARLOS |
| 4 | platform (this repository). Read cold and adversarially. |
| 5 | |
| 6 | **Spec:** `docs/superpowers/specs/2026-10-21-s3-change-index-design.md` |
| 7 | at the commit named in the prompt. |
| 8 | |
| 9 | ## Fixed rulings, do not re-argue |
| 10 | |
| 11 | §1 of the spec lists the product owner's rulings: the bucket stays the |
| 12 | store of record (no database, cache tier or DynamoDB), and the fix is a |
| 13 | change index in the bucket with a backstop walk. Do not propose |
| 14 | alternatives to them. Report only mechanisms that **fail to honour** a |
| 15 | ruling, fail the spec's own goals (§2), or break something else in the |
| 16 | platform. Cost estimates are context, not something to audit to the cent. |
| 17 | |
| 18 | ## What to check |
| 19 | |
| 20 | Verify against the code, not the spec's description of it. Cite |
| 21 | `file:line` for every claim about existing code. The places that matter: |
| 22 | |
| 23 | - `internal/release/store.go` (Store, prefixStore, ModLister, S3Store |
| 24 | Put/Get/ListMod, DirStore), `internal/release/serial.go`, |
| 25 | `internal/console/serialbump.go`. |
| 26 | - `internal/provision/reconcile.go` (`Reconcile`, `knownAccounts`, |
| 27 | `ListAllRecords*`, `listAppSubtree`, the per-app trash and manifest |
| 28 | reads), `sidecar_reconcile.go`, `record_sequence.go` (`WriteRecord`), |
| 29 | `provision.go` (`DeleteRecord`). |
| 30 | - `internal/host/agent.go` (`reconcileMu`, `fastTick`, `fullPass`, |
| 31 | `fastMembers`, `PassFilter` call, `configGenLoop`, `genPollPass`, |
| 32 | `instancesLoop`), `movespass.go`, `moveshold.go`, `movesoffline*.go`, |
| 33 | `steering_loop.go`, `telemetry_pass.go`, `runs.go`, `seedwiring.go`. |
| 34 | - `internal/console/runsweep.go`, `seedsweep.go`, `apiruns*.go`. |
| 35 | - `internal/edge/certsync.go`, `homemap.go`, `storedoor.go`. |
| 36 | - `cli.go` (`openStore`), `appconfig_cli.go`, every |
| 37 | `release.NewS3Store`/`NewS3StoreIn` call site. |
| 38 | - `internal/hostops` mutation boundary and classifier. |
| 39 | |
| 40 | Questions worth your time: |
| 41 | |
| 42 | 1. **Missed changes.** Is there any write that a reader acts on whose key |
| 43 | §3.2 maps to no scope, or to the wrong scope? Any reader input outside |
| 44 | the scopes (for example something Reconcile or the moves pass reads |
| 45 | that is not under `apps/<a>/<b>/` or `accounts/<a>/`)? Any data prefix |
| 46 | wrongly excluded? |
| 47 | 2. **Ordering.** Does bump-after-write plus re-list hold under every |
| 48 | interleaving with concurrent writers, the backstop walk (§4.4) and the |
| 49 | moves pass's separate lock (§4.3)? Can a view ever move backwards, or a |
| 50 | strict consumer get an incomplete view as complete? |
| 51 | 3. **Removal safety.** Reconcile removes rows whose host is missing; |
| 52 | moves and seed sweep act on absence. Can a stale or partially refreshed |
| 53 | view cause a removal, a lost grant, a wrong move step or a snapshot |
| 54 | delete that today's walk would not? |
| 55 | 4. **The writer hook.** Does wrapping at the store level actually catch |
| 56 | every writer (hostops boundary, prefix stores, store doors for fleet |
| 57 | boxes, writers that use `S3Store` directly, CAS helpers)? Does the |
| 58 | wrapper break any caller that type-asserts optional extensions? What |
| 59 | happens with contention on one index object under bursts (deploys, |
| 60 | purges, run churn)? |
| 61 | 5. **The GET loops** (§5): can gating `configGenLoop`, canary |
| 62 | `PassFilter`, telemetry, the cert mirror or the home map on generations |
| 63 | lose a change, a challenge token or a canary adoption that today's |
| 64 | polling catches? |
| 65 | 6. **Rollout** (§7): is a mixed fleet (old writers, new readers armed) |
| 66 | safe given the flag, and is disarm really a clean fallback? |
| 67 | 7. Anything the spec relies on that does not exist, or exists with a |
| 68 | different shape. |
| 69 | |
| 70 | ## Output |
| 71 | |
| 72 | A numbered list of findings, each with severity (P1 = correctness or |
| 73 | data loss, P2 = goal failure or significant gap, P3 = minor), the spec |
| 74 | section, the code evidence with `file:line`, and the change you would |
| 75 | require. End with a verdict: accept, or reject with the P1s that block. |
| 76 | |
amadan