carlos / platform Public

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.

1. P2 — The no-capacity path leaves a registered operation behind. Task 11’s “writes nothing” test 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.

1. P2 — H27 is missing from other chooser doors. Task 11’s sole integration point covers only handleAPIInstanceCreate. Other callers are: - handleAPIDomainAttach and browser handleAppDomainAttach, through attachClaimFirstRoute → beginClaimCreate. - handleAPISidecarCreate, for standalone sidecar records, through beginClaimCreate. - handleAPIInstanceMoveRequest, through selectMoveBoxes → chooseBox.

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.

1. P2 — Earlier instance validation bypasses the new placement/refusal policy. Task 11 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.

1. P2 — Move role filtering remains active with region HA off. Task 10, line 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.

1. P2 — Private forwarding also ignores the HA switch. Task 8’s dialAddress 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.

1. P2 — The hourly alert limit does not survive console restart. Task 11’s limiter 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.

1. P2 — Pending copy is visible despite the claimed feature gate. Task 1’s gate 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.

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.

---

FIRST PROMPT: Review plan 2a at ec91ac11, read-only, without heavy checks.

WHAT: Review implementation correctness, spec coverage, census handling, and tests.

STATUS: Review complete: seven P2 findings. Worktree unchanged.

NEEDED: Nothing for this review.

RECOMMENDED: Revise the plan and add complete request-path tests before implementation.

SESSION: Safe to close; nothing started remains running.

Verdict: not ready to build.