| 1 | Found **seven P2 issues** at `ec91ac11`. This was a static, read-only review against the specified HEAD and locally available amadan `origin/main` (`e963691d`). No builds or tests ran. |
| 2 | |
| 3 | 1. **P2 — The no-capacity path leaves a registered operation behind.** |
| 4 | [Task 11’s “writes nothing” test](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-region-ha-two-box/docs/superpowers/plans/2026-10-16-region-ha-2a-core.md:4886) calls `StartCreate` directly. The real `beginClaimCreate` path first calls `RegisterStart` and `Bind`; on chooser failure it only stops renewal. That registration blocks app mutations until settlement. Recovery’s `judgeRegistration` can subsequently create a free claim to invalidate the saved observation. Thus the HTTP refusal still leaves recovery work and can cause a later claim write. Test the complete door through recovery, and resolve this against H27 while preserving the claim’s fencing guarantees. |
| 5 | |
| 6 | 2. **P2 — H27 is missing from other chooser doors.** |
| 7 | [Task 11’s sole integration point](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-region-ha-two-box/docs/superpowers/plans/2026-10-16-region-ha-2a-core.md:5033) covers only `handleAPIInstanceCreate`. Other callers are: |
| 8 | - `handleAPIDomainAttach` and browser `handleAppDomainAttach`, through `attachClaimFirstRoute → beginClaimCreate`. |
| 9 | - `handleAPISidecarCreate`, for standalone sidecar records, through `beginClaimCreate`. |
| 10 | - `handleAPIInstanceMoveRequest`, through `selectMoveBoxes → chooseBox`. |
| 11 | |
| 12 | First-route and standalone-sidecar chooser failures already produce a **generic 503**, rather than the plan’s claimed 409, but lack the capacity explanation and alert. An empty move destination produces **400**, without an alert. The standalone-sidecar path also contradicts the spec’s blanket statement that sidecars choose nothing. Cover these doors and distinguish standalone sidecars from instance-attached sidecars. |
| 13 | |
| 14 | 3. **P2 — Earlier instance validation bypasses the new placement/refusal policy.** |
| 15 | [Task 11](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-region-ha-two-box/docs/superpowers/plans/2026-10-16-region-ha-2a-core.md:5026) leaves `apiinstances.go:1239–1266` unchanged. For an app-derived region, stale status on every box produces **400 before the chooser**, without an alert. Moreover, `regionCandidates` requires every potentially serving fresh sibling to qualify: one misconfigured sibling can reject the region despite another eligible box. Use the HA eligible set for these decisions. Add HTTP tests for both scenarios; the proposed helper tests cannot catch them. |
| 16 | |
| 17 | 4. **P2 — Move role filtering remains active with region HA off.** |
| 18 | [Task 10, line 4459](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-region-ha-two-box/docs/superpowers/plans/2026-10-16-region-ha-2a-core.md:4459) gates the roster read on `HostClaim`, without checking `RegionHA`. After disabling HA, persisted roles still exclude destinations, and the new roster read can independently refuse a move. This violates the explicit dormant-behavior constraint. Test `HostClaim=true, RegionHA=false`; the proposed test only checks claims off. |
| 19 | |
| 20 | 5. **P2 — Private forwarding also ignores the HA switch.** |
| 21 | [Task 8’s `dialAddress`](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-region-ha-two-box/docs/superpowers/plans/2026-10-16-region-ha-2a-core.md:3057) uses private addresses whenever the published regions match. An HA-disabled reader will therefore switch to private dialing when a peer publishes `private_ip`, including during mixed configuration or rollback. That can break forwarding where private access has not been enabled. Carry the reader’s HA setting into address selection and test both settings. |
| 22 | |
| 23 | 6. **P2 — The hourly alert limit does not survive console restart.** |
| 24 | [Task 11’s limiter](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-region-ha-two-box/docs/superpowers/plans/2026-10-16-region-ha-2a-core.md:4962) stores timestamps only in `Server` memory. A refusal, restart, and retry sends two alerts within an hour. The plan acknowledges this exception, but H27 grants none. Persist the limit or obtain an explicit ruling relaxing it; test across server reconstruction. |
| 25 | |
| 26 | 7. **P2 — Pending copy is visible despite the claimed feature gate.** |
| 27 | [Task 1’s gate](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-region-ha-two-box/docs/superpowers/plans/2026-10-16-region-ha-2a-core.md:407) changes pending rows to “approved” for checking. Task 10 then adds unconditional CLI help and documentation. Running `carlos system help` exposes pending copy even with HA off. Require actual approval before wiring these surfaces; disabling HA does not hide them. |
| 28 | |
| 29 | The census instructions correctly identify Tasks 7 and 10 and require recalculation after the newer overlays. I found no additional census omission in the proposed changes by inspection. However, fixing the findings above may also change frozen units including `Server.beginClaimCreate`, `Server.attachClaimFirstRoute`, and `Server.handleAPIInstanceMoveRequest`; those need explicit census treatment. |
| 30 | |
| 31 | --- |
| 32 | |
| 33 | **FIRST PROMPT:** |
| 34 | Review plan 2a at `ec91ac11`, read-only, without heavy checks. |
| 35 | |
| 36 | **WHAT:** |
| 37 | Review implementation correctness, spec coverage, census handling, and tests. |
| 38 | |
| 39 | **STATUS:** |
| 40 | Review complete: seven P2 findings. Worktree unchanged. |
| 41 | |
| 42 | **NEEDED:** |
| 43 | Nothing for this review. |
| 44 | |
| 45 | **RECOMMENDED:** |
| 46 | Revise the plan and add complete request-path tests before implementation. |
| 47 | |
| 48 | **SESSION:** |
| 49 | Safe to close; nothing started remains running. |
| 50 | |
| 51 | **Verdict: not ready to build.** |
amadan