| 1 | # Whole-branch code review brief: Plans 2 (boxes read account limits) |
| 2 | |
| 3 | You are a senior Go reviewer on the carlos platform. Review branch |
| 4 | `plans-2-box-limits`, range `4a1e4b29a7196a2a0acb505cd4f8a3dcea054ad0..HEAD` (`git diff 4a1e4b29a7196a2a0acb505cd4f8a3dcea054ad0..HEAD`; the |
| 5 | docs under docs/superpowers/ are requirements, not code to review). The |
| 6 | review is read-only: do not modify the working tree, index, HEAD or |
| 7 | branches. You may run `go test` for packages you want to probe (set |
| 8 | GOFLAGS=-buildvcs=false). |
| 9 | |
| 10 | Requirements, in order of authority: |
| 11 | 1. Spec: docs/superpowers/specs/2026-10-20-plans-caps-packs-design.md |
| 12 | (§5.2 idle window, §7 limits reach the boxes and fail open, §10). Its |
| 13 | section 3 decisions are not to be argued with. |
| 14 | 2. Plan: docs/superpowers/plans/2026-10-21-plans-2-box-limits.md, especially |
| 15 | "Self-hosters": Paul's requirement that deployments without a plan |
| 16 | catalog are never interrupted. |
| 17 | 3. Executor rulings: `.superpowers/sdd/2026-10-21-plans-2-box-limits/progress.md`, |
| 18 | lines containing `Ruling:`. Weigh each. |
| 19 | |
| 20 | The spec is a vision document. For behaviour it is silent on, judge by what a |
| 21 | reasonable operator, self-hoster or customer would expect, and grade by effect. |
| 22 | |
| 23 | Check these deliberately (the plan's Review Focus): |
| 24 | 1. Plans off (marker absent): exactly one bucket read per pass, no limits. |
| 25 | 2. CARLOS_PLAN_LIMITS=off: zero bucket reads, cache cleared. |
| 26 | 3. Bucket blips: marker read error keeps the whole cache; a per-account |
| 27 | error keeps that account's last value. |
| 28 | 4. Plans turned off again: marker disabled clears the cache; windows revert. |
| 29 | 5. A plan idle longer than the box default: the box default stands. |
| 30 | |
| 31 | Also look hard at: any path where a self-hosted box (no catalog ever) changes |
| 32 | behaviour or cost; the activator sweep change (always-on workers, held-for-tick, |
| 33 | probe entries, sidecar workers); races on the package-level cache and the |
| 34 | atomic hook; the console marker publisher (once per start, disable path, |
| 35 | CAS); SetAccountPlan's live-account guard SQL; the hostops and storedoor |
| 36 | entries for control/plans.json; the census overlay rows' honesty; and |
| 37 | darwin/windows builds. |
| 38 | |
| 39 | Output: findings most severe first, each Critical / Important / Minor with |
| 40 | file:line, the concrete failure scenario and a suggested fix; then "Declined |
| 41 | to judge" (every behaviour set aside, one line each with the reason); then a |
| 42 | verdict: ready to merge, ready with fixes, or not ready. |
| 43 | |
amadan