amadan
carlos / platform Public
Reviewed plan 5600586a against spec 9ccfaca6, amendment 8baddbaa, the three earlier reviews, and this worktree. R1–R9 remain settled.
| Earlier finding still open after round 3 | Status | Why | |---|---|---| | R1-13 — copy-review completeness | Resolved | O19 is inventoried; new C30c/O20 are included, with review required before product-file writes. | | R1-17 — release/build/scratch gates | Partly; decline unjustified | Release and cross-build gates are complete. The supplied instructions contain no /tmp/g exception; socket-path limits do not authorize one. The tzdata and probe scripts also retain /tmp fallbacks. | | R2-4 — canary mismatch checked after writing | Resolved | The exact record is now checked before instancesEnable. | | R3-1 — capped/partial listing bypasses refusal | Resolved | Direct record GET replaces listing-based inference; capped-list and failed-read cases assert zero writes. | | R3-2 — pooled availability ignores allow-list | Resolved | Availability now applies pol.allows("") to the effective home. | | R3-3 — fixture provenance breaks approved-copy equality | Resolved | The immutable SHA is resolved before review; approved F1 is copied without substitution. | | R3-4 — missing same-box eligibility proof | Resolved | Split-capability fixtures and real reconciler pairing cover the required distinction. | | R3-5 — missing persisted-record compatibility proof | Resolved | Home/non-home stored-byte goldens and the steering-stamp mutation cover it. | | R3-6 — replicas lose their place name | Resolved | Every non-fleet response receives region_name, with endpoint and CLI coverage. |
For R1-17, replace the exception at plan:107 with an allowed on-disk directory and shorten test socket paths where necessary. Remove the /tmp fallbacks too. A passing test under /tmp/g establishes functionality, not authorization.
New findings
- P1 — Tasks 15 and 22: pooled/fleet branches violate R9a when the stored-region read fails.
Plan: 5324, 6944. Code: proposed internal/console/regiondefault.go:56 and :62 within Task 15’s file block; integration at apiregions.go:31 and canary.go:381.
With rerr != nil, a known fleet returns region_known:false but effective:"fleet/acme". A known pooled manifest returns a home-valued effective and nonnil available. R9a explicitly requires both fields to remain unknown: empty effective, omitted available.
TestAppDefaultKnownFlags exercises the failed read only without either overriding placement, so it misses both cases.
Fix: condition Effective and Available on a successful region read. Preserve canary’s refusal independently: simply clearing Effective would defeat Task 22’s fleet-prefix check. Use independently confirmed placement information, or conservatively refuse that combined state. Add failed-region-read × pooled/fleet endpoint cases and a fleet-canary zero-write test.
- P2 — Tasks 12 and 14: refusal messages recommend choices the corresponding door rejects.
Plan: 4041, 4044, 4859. Code: proposed internal/console/regionclaim.go:35 and :38; existing rejection predicates at apiinstances.go:799 and apiinstances.go:827.
There are three concrete cases:
- C18 lists every published code, including non-hosting nz. The Task 12 test explicitly expects that suggestion, then verifies that choosing nz produces C19. - C19 filters hosting capability but still suggests regions that NoKeyProven makes fail with C31. - C16 lists eligible regions without the manifest allow-list. An unavailable UK default with Regions:["uk"] can recommend home ie, which the next request rejects.
This misses §8.1’s requirement that refusals list what would pass, and makes C16’s recovery advice unreliable.
Fix: construct alternatives using the relevant door’s predicates, including normalized allow-lists for instance requests and the explicit R9 refusal for app requests. Keep explicit-choice rules distinct from automatic eligibility. Add tests that submit every advertised alternative under the same fixture and verify acceptance.
- P2 — Task 13: the unchanged-form contract is neither preserved nor tested.
Plan: 4265, 4505. Code: app_new.html:14, server.go:3586.
TestNewAppPageWithoutPickerIsUnchanged checks only that name="region" is absent, and only with the switch off. It would pass if existing form content changed or disappeared.
The proposed untrimmed {{with .RegionPicker}} block also leaves additional whitespace when the picker is nil. That fails the explicit byte-identical requirement in §§10 and 16, even though the rendered appearance remains unchanged.
Fix: preserve the existing no-picker output exactly and compare the rendered form against a baseline golden with deterministic CSRF data. Cover both switch-off and switch-on/one-eligible-region cases. The proposed insertion should fail that comparison before correction.
Other requested checks
The additive instance_domain field is sound. It supplies the authoritative hostname suffix needed for a direct preflight read, without changing legacy regions or default. Checking the subsequent enable response against the preflight hostname is appropriate.
I found no further mismatch in the interface blocks, migration carry through migrateAppsKey, create-door precedence, or old-CLI/old-console handling. Task 1 correctly reuses reconciliation parsing: CARLOS_POOLS=",tito" does not advertise shared-pool eligibility. All specified C identifiers, five Review Focus cases, and rollout stages have task coverage; the behavioral gaps above remain despite that coverage.
The dependency choices have no identified blocker: MaxMind v1.13.1 declares Go 1.21, compatible with this repository’s Go 1.26 declaration, and IANA publishes tzdata 2026b and its signature. The plan includes signature verification and all five required cross-build targets with failure propagation.
This was static, read-only review. I did not run the proposed implementation’s tests or cross-builds; its implementation files do not yet exist. The worktree remains clean.
---
FIRST PROMPT: Independent round-4 review of the nearest-region implementation plan.
WHAT: Check prior dispositions, spec coverage, interfaces, compatibility and execution gates.
STATUS: Review complete: three new findings; R1-17 remains partly unresolved.
NEEDED: No additional user input.
RECOMMENDED: Correct the findings and scratch instructions before implementation.
SESSION: Safe to close; no files changed.
Verdict: not ready to execute. R9a is violated, refusal alternatives and form-preservation tests need correction, and the declined scratch-rule finding remains unresolved.