carlos / platform Public

Reviewed fcf15ae9 against spec 9ccfaca6 and the worktree, including the full 4604a146 → 9ccfaca6 spec diff first. R1–R9 remain settled. Fourteen round-1 findings are resolved; four are partly resolved.

Round-1 dispositions

| # | Finding | Status | Why | |---|---|---|---| | 1 | GeoIP errors disclose IP | Resolved | Fixed-category logging and an actual reader-error regression test. | | 2 | Deploy continues after failed region read | Resolved | Transport/decode errors now stop; successful legacy responses retain compatibility. | | 3 | Old-console fleet canary inconsistency | Resolved | Missing app_default now refuses the flag consistently before mutation. | | 4 | Proxy defeats service gate | Resolved | Proxy/config suppression and connected-address verification are included. | | 5 | postJSON map treated as bytes | Resolved | Tests now inspect the decoded map. | | 6 | Replacement fixtures use create-only writes | Resolved | Helpers replace using the current ETag and check errors. | | 7 | Missing account in TestNonHomeAppRegions | Resolved | The fixture now runs the account backfill. | | 8 | Incorrect HTML assertions | Resolved | Positive assertions escape text; suppression uses an element ID. | | 9 | Migration preservation unproved | Partly | Nonempty values and rename/transfer checks were added, but the new fixture fails before those checks complete; see new finding 6. | | 10 | Vacuous guess property | Resolved | Expected existence is calculated independently and the real allow-list predicate is used. | | 11 | Tests miss their claimed boundaries | Resolved | Reader invocation, service-principal HTTP wiring, failed claims and successful excluded creates are now exercised. | | 12 | Missing integration coverage | Partly | Most cases were added; existing-canary refusal still lacks the required zero-write proof. | | 13 | Copy-review ordering and completeness | Partly | Ordering is fixed, but new diagnostics such as plan:3165 remain outside the inventory; checking approved-string presence cannot detect unreviewed additions. | | 14 | Unreadable default rendered as available | Resolved | C7 now requires confirmed availability; unreadable cases suppress the line. | | 15 | Missing-coordinate diagnostic absent | Resolved | Implementation and once-per-code test added. | | 16 | Moving fixture downloads | Resolved | One immutable revision is resolved before fetching all fixture material. | | 17 | Incomplete release/cross-build gates | Partly | Targets and rollout prerequisites are supplied, but plan:6849 still mandates /tmp/g and incorrectly calls it disk-backed, contrary to the supplied instructions. | | 18 | C25 displays code instead of place | Resolved | Existing regions are named through nameOf, with a code fallback. |

New findings

New implementation files do not yet exist; their code locations below refer to the numbered plan blocks, alongside relevant worktree code.

1. P1 — Task 10: incomplete edge reads can falsely prove a region keyless.

Plan/code: plan:2954, plan:3077; existing best-effort reader instancesplacement.go:98.

readEdgesFilesStrict silently skips failed GETs and malformed files. NoKeyProven then quantifies over only the surviving files. If a readable UK box is keyless while a matching-key or old-writer UK box is unreadable, an explicit create receives C31 despite the mismatch being unknown. R9 requires proof about every reachable hosting box.

Fix: preserve inventory completeness and suppress negative proof when relevant files cannot be assessed. Add failed-GET and malformed-file tests alongside a readable keyless box. Positive automatic eligibility and proof of universal key absence need different treatment.

1. P1 — Task 10: ineligible boxes determine a candidate’s location.

Plan/code: plan:3055; topology fields instancesplacement.go:35.

The AWS-region tally includes every box with an EIP, before eligibility is calculated. One eligible London box and two keyless or no-inbound Sydney boxes sharing placement uk therefore locate the eligible candidate in Sydney. This changes both its displayed place and nearest-region selection. Spec §6.1 explicitly takes coordinates from eligible boxes.

Fix: tally non-home coordinates only from individually eligible boxes; retain the home’s exemption separately. Test mixed eligibility and eligible-box ties.

1. P1 — Tasks 15, 21 and 22: HTTP 200 with policy-unreadable bypasses CLI protection.

Plan/code: plan:4840, plan:5893, plan:6260; fleets.go:479, apiinstances.go:798.

appDefault returns a nonnil block marked policy-unreadable, but both preflights treat its fields as authoritative. A failed app-region read manufactures home-valued region, allowing deploy --region ie without knowing the stored region. A failed policy/placement read leaves a non-fleet effective, allowing canary creation even though fleet placement was never established.

Fix: distinguish confirmed stored-region and fleet-placement facts from unavailable ones. Refuse the corresponding flag check when its required fact is unknown. Add command tests using successful HTTP responses with unreadable state, asserting zero writes/uploads.

1. P1 — Task 22: existing-canary mismatch is checked after a write.

Plan/code: plan:6323; canary.go:381, apimode.go:2827.

The first check has no records. The second runs after instancesEnable, which sends a PUT. An existing UK canary requested with --region ie therefore mutates before C25. This directly violates §16’s “before any write or upload.” The new command test covers only refusals from the first check.

Fix: obtain the domain and existing records through reads, and complete the mismatch check before enabling instances. Add an existing-canary mismatch case asserting zero mutation requests and unchanged stored state.

1. P1 — Tasks 12, 20 and 22: the prescribed edits leave three compile failures.

- Task 12: plan:3522, proposed regionclaim_test.go, imports net without using it. Remove the import. - Task 20: plan:5715 changes offerCreateApp’s signature, but deploywizardtest.go:104 retains the old arguments. The task’s createApp( search does not find offerCreateApp. Update that caller and include it in the file/commit list. - Task 22: plan:6308 inserts an appRegions(ctx, …) call directly after resolvedApp; ctx is declared on the following line at canary.go:375. Move its declaration before the inserted block.

1. P1 — Task 8: the new migration test cannot pass its rename step.

Plan/code: plan:2174, plan:2205; store.go:1804, store.go:1720.

The legacy database contains an app but no source account. Open migrates without running the account backfill. RenameAppIn consequently returns sql.ErrNoRows when looking up the source sqid; transfer also requires that account. The region-preservation implementation cannot make this test green.

Fix: seed the source account or explicitly run BackfillOpsMemberships before rename/transfer, while retaining the nonempty pre-rebuild region values.

1. P2 — Task 14: R9’s addr-backed exemption has no regression test.

Plan/code: plan:4442; shape predicate integration apiinstances.go:828.

The keyless-region test checks static exemption, socket/replica refusal and unknown-writer compatibility, but never creates an addr-backed record in the keyless region. The earlier addr-backed test uses an eligible region without an explicit region, so it cannot detect an incorrectly widened C31 gate.

Fix: add an explicit region:"uk" addr-backed create against ukNoKey, require 201, and confirm a mutation that includes addr-backed records in the C31 gate fails this test.

1. P2 — Task 9: the missing-key startup diagnostic is incorrectly switch-gated.

Plan/code: plan:2549; existing optional-key configuration main.go:467.

logRegionConfig returns immediately when app-region assignment is off. However, the guess endpoint remains operational with that switch off, and R9 requires the missing console key to be logged once. Non-home guesses silently disappear during the staged console rollout.

Fix: gate only the unnamed-home diagnostic on AppRegions; independently report the missing secrets public key. Test the switch-off configuration.

1. P2 — Task 11: the second mutation check does not prove its invariant.

Plan/code: plan:3455, proposed regionguess.go; policy predicate apiinstances.go:1797.

The instructions first make the function always return null, then change the home condition without restoring the first mutation. The second run is already red and never reaches the mutated home branch. It proves nothing about disallowed-home fallback.

Fix: restore and verify the passing baseline between mutations. Require each independent mutation to fail its intended assertion.

1. P2 — Tasks 22 and 25: cross-build failure returns success and permits continuation.

Plan/code: plan:6413, plan:6846; authoritative gate 20-cross@5:25.

go build … || { echo "FAILED $t"; break; } exits the loop successfully. Task 22 then proceeds directly to staging and committing; Task 25 continues into native tests. I confirmed the loop returns status 0 when its build command fails.

Fix: exit nonzero on failure, or accumulate failures and return a failing status before any subsequent commit. Apply this to every copied cross-build loop.

The parser reuse in Task 1 correctly handles CARLOS_POOLS=",tito". The migration SQL carries both new fields through migrateAppsKey; the fixture is the problem. The main create-door precedence and additive old-client wire compatibility remain consistent with the spec. All specified C identifiers are represented, subject to the outstanding copy-review completeness issue.

No new dependency-selection finding: the pinned MaxMind v1.13.1 module exists, and IANA lists the 2026b archive and signature. I did not execute the proposed Go tests, cross-builds or signature verification.

---

FIRST PROMPT: Independent read-only round-2 review of the nearest-region implementation plan against its amended spec and code.

WHAT: Check prior dispositions, implementation correctness, compatibility and required proof.

STATUS: Review complete; worktree unchanged. Four prior findings remain partly resolved, with ten new findings above.

NEEDED: Nothing for this review.

RECOMMENDED: Revise the identified tasks and re-review the affected implementation blocks and tests.

SESSION: Safe to close; no changes or running work.

Verdict: not ready to execute. The plan still contains incorrect placement/refusal behavior, a write-before-refusal path, compile blockers, a failing migration fixture, and incomplete validation gates.