| 1 | Reviewed plan `b7c5b60d` against spec `9ccfaca6` and the worktree, read-only. R1–R9 remain settled. |
| 2 | |
| 3 | **Round-1 findings** |
| 4 | |
| 5 | | # | Finding | Status | Why | |
| 6 | |---|---|---|---| |
| 7 | | 1 | GeoIP errors disclose IP | Resolved | Fixed-category logging; actual IPv6 reader-error regression test. | |
| 8 | | 2 | Deploy continues after failed region read | Resolved | Read/decode errors stop deployment; successful legacy responses remain compatible. | |
| 9 | | 3 | Old-console fleet canary inconsistency | Resolved | Missing `app_default` consistently refuses the flag before mutation. | |
| 10 | | 4 | Proxy defeats service gate | Resolved | Proxy/config suppression and connected-address verification are present. | |
| 11 | | 5 | `postJSON` map treated as bytes | Resolved | Tests inspect the decoded map. | |
| 12 | | 6 | Replacement fixtures use create-only writes | Resolved | Replacement helpers use current ETags and check errors. | |
| 13 | | 7 | Missing account fixture | Resolved | The fixture now runs `NewServer`’s account backfill. | |
| 14 | | 8 | Incorrect HTML assertions | Resolved | Assertions escape copy; suppression checks the element ID. | |
| 15 | | 9 | Migration preservation unproved | Resolved | Nonempty values survive rebuild, reopen, rename and transfer; source account is seeded. | |
| 16 | | 10 | Vacuous guess property | Resolved | Expected existence is independently calculated; always-null mutation must fail. | |
| 17 | | 11 | Tests miss claimed boundaries | Resolved | Reader invocation, service-principal HTTP wiring and failed claims are exercised. | |
| 18 | | 12 | Missing integration coverage | Resolved | Previously enumerated cases now have named tests, including command-level zero-write checks. | |
| 19 | | 13 | Copy-review completeness/order | Partly | Task 17’s diagnostic at [plan:5403](/home/paulca/amadan.net/carlos/platform-nearest-region/docs/superpowers/plans/2026-10-02-nearest-region-default.md:5403) remains outside the inventory; the final scan catches it only after writing. | |
| 20 | | 14 | Unreadable defaults displayed as available | Resolved | C7 requires confirmed availability. | |
| 21 | | 15 | Missing-coordinate diagnostic absent | Resolved | Once-per-code diagnostic and test supplied. | |
| 22 | | 16 | Moving fixture downloads | Resolved | One SHA is resolved before fetching every fixture and licence. | |
| 23 | | 17 | Incomplete release/cross-build gates | Partly | Gates are completed, but [plan:104](/home/paulca/amadan.net/carlos/platform-nearest-region/docs/superpowers/plans/2026-10-02-nearest-region-default.md:104) still invents a `/tmp/g` exception contrary to the supplied scratch rule. | |
| 24 | | 18 | C25 displays code instead of place | Resolved | `nameOf` supplies the place, falling back to the code. | |
| 25 | |
| 26 | **Round-2 findings** |
| 27 | |
| 28 | | # | Finding | Status | Why | |
| 29 | |---|---|---|---| |
| 30 | | 1 | Incomplete reads falsely prove keylessness | Resolved | `NoKeyProven` requires snapshot completeness; failed GETs and malformed files are tested. | |
| 31 | | 2 | Ineligible boxes determine coordinates | Resolved | Non-home coordinates come only from individually eligible boxes. | |
| 32 | | 3 | HTTP 200 with unreadable facts bypasses preflight | Resolved | Explicit knowledge flags gate the corresponding CLI decisions. | |
| 33 | | 4 | Canary mismatch checked after writing | Partly | Ordering is fixed for returned records; an incomplete listing still bypasses it—new finding 1 below. | |
| 34 | | 5 | Three compile failures | Resolved | Import, caller arities and `ctx` placement are corrected. | |
| 35 | | 6 | Migration fixture cannot rename | Resolved | Source account seeded before rename/transfer. | |
| 36 | | 7 | Addr-backed R9 exemption untested | Resolved | Explicit keyless-region addr-backed create must succeed. | |
| 37 | | 8 | Missing-key diagnostic switch-gated | Resolved | Missing-key logging runs independently of the assignment switch. | |
| 38 | | 9 | Second mutation check is meaningless | Resolved | Passing baseline restored between mutations. | |
| 39 | | 10 | Cross-build failure returns success | Resolved | All loops accumulate failures and exit nonzero before continuation. | |
| 40 | |
| 41 | **New findings** |
| 42 | |
| 43 | New implementation files do not exist yet. Where relevant, “code” below identifies their exact proposed block. |
| 44 | |
| 45 | 1. **P1 — Task 22: an incomplete instance list bypasses the canary refusal.** |
| 46 | |
| 47 | **Plan:** [6587](/home/paulca/amadan.net/carlos/platform-nearest-region/docs/superpowers/plans/2026-10-02-nearest-region-default.md:6587). **Code:** [apimode.go:3159](/home/paulca/amadan.net/carlos/platform-nearest-region/apimode.go:3159), [apiinstances.go:416](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/console/apiinstances.go:416), [provision.go:1188](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/provision/provision.go:1188), [canary.go:381](/home/paulca/amadan.net/carlos/platform-nearest-region/canary.go:381). |
| 48 | |
| 49 | The preflight discards `truncated`. The server lists at most 500 records. If an existing UK canary sorts after those records, `canary --region ie` passes preflight and sends the `instancesEnable` PUT before subsequently encountering a conflict. Individual failed record reads are also silently omitted by `ListRecords`. |
| 50 | |
| 51 | **Fix:** establish the target record’s state through an authoritative read, or refuse when completeness cannot be established. Checking `truncated` is necessary but does not cover skipped GET failures. Add command tests with the canary beyond the cap and with its read failing; both must produce zero mutations. |
| 52 | |
| 53 | 2. **P1 — Task 15: pooled defaults are declared available even when the create door refuses them.** |
| 54 | |
| 55 | **Plan/code:** proposed `appDefault`, [plan:5041](/home/paulca/amadan.net/carlos/platform-nearest-region/docs/superpowers/plans/2026-10-02-nearest-region-default.md:5041). **Existing door:** [apiinstances.go:799](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/console/apiinstances.go:799), [apiinstances.go:1797](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/console/apiinstances.go:1797). |
| 56 | |
| 57 | For `Pool:"tito", Regions:["uk"]`, the plan returns `effective:"ie", available:true, reason:"pooled"`. A no-region create skips the app fill, normalizes to `""`, and fails that allow-list. This contradicts §9.3’s contract that availability describes the effective placement. |
| 58 | |
| 59 | **Fix:** apply the normalized allow-list to the pooled effective home before asserting availability. Preserve pooled UI suppression. Add an endpoint/create-door consistency test using a pooled manifest that excludes `""`. |
| 60 | |
| 61 | 3. **P2 — Tasks 0, 5 and 25: the fixture provenance substitution contradicts the approved-copy contract.** |
| 62 | |
| 63 | **Plan:** [199](/home/paulca/amadan.net/carlos/platform-nearest-region/docs/superpowers/plans/2026-10-02-nearest-region-default.md:199), [1571](/home/paulca/amadan.net/carlos/platform-nearest-region/docs/superpowers/plans/2026-10-02-nearest-region-default.md:1571). **Code:** proposed `approved_copy_test.go`, [plan:7137](/home/paulca/amadan.net/carlos/platform-nearest-region/docs/superpowers/plans/2026-10-02-nearest-region-default.md:7137). |
| 64 | |
| 65 | Task 0 records F1 verbatim, including `<sha>`. Task 5 replaces `<sha>` with the resolved commit. Task 25 requires the complete recorded string to occur byte-for-byte in the resulting README. With unchanged approved wording, that assertion necessarily fails. |
| 66 | |
| 67 | **Fix:** resolve and record the immutable fixture SHA before reviewing F1, then use it throughout Task 5. Alternatively, explicitly define and verify template substitutions instead of claiming literal equality. |
| 68 | |
| 69 | 4. **P2 — Tasks 1, 10 and 11: tests do not prove that eligibility facts belong to one box.** |
| 70 | |
| 71 | **Plan:** [2831](/home/paulca/amadan.net/carlos/platform-nearest-region/docs/superpowers/plans/2026-10-02-nearest-region-default.md:2831), [2855](/home/paulca/amadan.net/carlos/platform-nearest-region/docs/superpowers/plans/2026-10-02-nearest-region-default.md:2855). **Code:** proposed `regionCandidates`; relevant mint predicates are [reconcile.go:925](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/provision/reconcile.go:925) and [reconcile.go:1339](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/provision/reconcile.go:1339). |
| 72 | |
| 73 | The implementation correctly evaluates each box, but the tests lack a same-region case where different boxes collectively advertise every fact while none satisfies all facts. An implementation that combines capabilities across boxes could pass the supplied cases. §16 also asks for the guess refusal paired with the corresponding reconcile refusal; helper-predicate equality alone does not provide that proof. |
| 74 | |
| 75 | **Fix:** add split-capability fixtures, including an ordinary-capable keyless box beside a keyed named-pool-only box. Assert ineligibility and exclusion from guesses, and exercise the relevant reconcile refusals. Require an aggregate-across-boxes mutation to fail. |
| 76 | |
| 77 | 5. **P2 — Tasks 14 and 25: the required persisted-record compatibility tests are absent.** |
| 78 | |
| 79 | **Plan:** [4486](/home/paulca/amadan.net/carlos/platform-nearest-region/docs/superpowers/plans/2026-10-02-nearest-region-default.md:4486), [4508](/home/paulca/amadan.net/carlos/platform-nearest-region/docs/superpowers/plans/2026-10-02-nearest-region-default.md:4508), [7197](/home/paulca/amadan.net/carlos/platform-nearest-region/docs/superpowers/plans/2026-10-02-nearest-region-default.md:7197). **Code:** [apiinstances.go:935](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/console/apiinstances.go:935), [apiinstances_test.go:2411](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/console/apiinstances_test.go:2411). |
| 80 | |
| 81 | `TestFillWithAHomeAppRegionChangesNothing` checks only two response fields. No proposed test compares the persisted legacy record with its pre-change golden, or checks `Steering` on an app-defaulted non-home record. The existing no-steering test creates a home record. A textual search for added `Steering` lines cannot prove either §16 requirement. |
| 82 | |
| 83 | **Fix:** add deterministic persisted-record comparisons for legacy home and app-defaulted non-home creates. Explicitly assert no steering stamp; verify that introducing one fails the test. |
| 84 | |
| 85 | 6. **P2 — Task 14: successful replica creates omit the promised place name.** |
| 86 | |
| 87 | **Plan/code:** [plan:4727](/home/paulca/amadan.net/carlos/platform-nearest-region/docs/superpowers/plans/2026-10-02-nearest-region-default.md:4727), modifying [apiinstances.go:1066](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/console/apiinstances.go:1066). |
| 88 | |
| 89 | `rec.Role == ""` suppresses `region_name` for every replica, including a successful explicit UK replica on an eligible box. The CLI consequently uses its old-console output fallback instead of C14. §§7, 9.2 and 11 specify the response/output addition without this replica exclusion. |
| 90 | |
| 91 | **Fix:** name non-fleet replica placements too, and test a successful replica create’s response and CLI output. |
| 92 | |
| 93 | The additional **`region_known` and `placement_known` fields are sound**. They separate stored-region certainty from placement-policy certainty, are additive, and make absent knowledge fail closed. Their exact semantics should be incorporated into §9.3. |
| 94 | |
| 95 | The migration SQL carries both columns correctly. Task 1 reuses the actual parser: `CARLOS_POOLS=",tito"` produces `["tito"]` and correctly publishes shared-pool eligibility as false. The principal create-door precedence remains correct. All C identifiers and rollout steps have task homes, subject to the findings above. |
| 96 | |
| 97 | No new dependency-selection blocker: [MaxMind v1.13.1](https://raw.githubusercontent.com/oschwald/maxminddb-golang/v1.13.1/go.mod) exists, and [IANA publishes the 2026b archive and signature](https://data.iana.org/time-zones/releases/). The five cross-build targets now match CI and propagate failures. |
| 98 | |
| 99 | I did not execute the proposed Go implementation, tests, downloads or cross-builds. The recorded scratch results are the plan author’s evidence, not independently reproduced results. The worktree remains clean. |
| 100 | |
| 101 | --- |
| 102 | |
| 103 | **FIRST PROMPT:** |
| 104 | Independent read-only round-3 review of the nearest-region plan. |
| 105 | |
| 106 | **WHAT:** |
| 107 | Check earlier dispositions, spec coverage and implementation correctness. |
| 108 | |
| 109 | **STATUS:** |
| 110 | Complete: two new P1 and four new P2 findings; no files changed. |
| 111 | |
| 112 | **NEEDED:** |
| 113 | Nothing for this review. |
| 114 | |
| 115 | **RECOMMENDED:** |
| 116 | Revise the affected tasks and close the remaining prior findings. |
| 117 | |
| 118 | **SESSION:** |
| 119 | Safe to close; nothing running. |
| 120 | |
| 121 | **Verdict: not ready to execute. The canary preflight can still write before refusing, pooled availability can contradict the create door, and the copy-verification interface and required regression coverage remain incomplete.** |
amadan