carlos / platform Public

Clone
git clone https://amadan.net/carlos/platform

Plain git — no account needed to clone.

Download

Download this file

Review brief: S3 change index, whole-branch code review

You are the final, independent reviewer of branch s3-polling-336 before it is merged. Read cold and adversarially. Verify against the code, cite file:line for every claim.

Range: git diff 5f341f33..<commit named in the prompt> (5f341f33 is the merge base with main; the branch already contains main up to it). The same diff with commit list and stat is in .superpowers/sdd/2026-10-21-s3-change-index/review-5f341f33..26b9e850.diff.

Spec (binding): docs/superpowers/specs/2026-10-21-s3-change-index-design.md (rev 3 plus the §3.2 schedule-status row and the §4.4 confirm-in-bucket note). Plan: docs/superpowers/plans/2026-10-21-s3-change-index.md. Execution ledger (rulings, deferred minors, carried findings): .superpowers/sdd/2026-10-21-s3-change-index/progress.md.

Fixed rulings, do not re-argue

Spec §1 R1 to R4, and every line in the ledger that starts with Ruling:. Report only mechanisms that fail to honour them, or that break something.

What matters most

The feature is dormant until CARLOS_CHANGE_INDEX=read is set; writers always bump. In priority order:

  1. Disarmed equivalence. With the variable unset, is every box, edge, console and CLI path behaviourally identical to main, apart from the writers' best-effort bump of control/changes.json? Any new read, goroutine, changed store type, changed error, or changed timing.
  2. Destructive decisions on stale data, when armed. Reconcile and sidecar removals, trash removals, moves steps, seed-sweep snapshot deletes, orphan-run deletes, claim install and retire. Each must still be proved against the live bucket, or by a snapshot that is provably at least as fresh as the walk it replaces.
  3. Lost changes beyond the 30-minute backstop. Any write a reader acts on that can be missed indefinitely: scopes, gates, the cert mirror, the home map, the console sweeps, the fast tick's last-acted map.
  4. Concurrency. Lock order w.pass then reconcileMu then RecordView.mu (a leaf); the watch's poll slot; the edge and agent sharing one watch; anything that can stall the health heartbeat.
  5. Cost. The point of the work. Any steady-state loop that still reads per app or per box when nothing changed, once armed.
  6. Census and root. Task 12's overlays (internal/provision/testdata/plan14-root-reads-s3-change-index.tsv, internal/provision/s3_change_index_census_test.go): are the classes right, and is root's new read-increment-CAS of control/changes.json (via runSystemClaims) acceptable?

Also triage the ledger's minor (deferred) lines: name any that must be fixed before merge, and why. One is flagged by the controller as likely: RecordView's index poll runs under the caller's unbounded context, and a lock give-up leaves ReadGate readers trusting a frozen observation.

Output

Numbered findings, each with severity (P1 = correctness, data loss or an outage; P2 = a goal failure or significant gap; P3 = minor), file:line evidence, and the change you would require. Then the deferred-minor triage (must-fix list). End with a verdict: accept, or reject with the P1s that block.