carlos / platform Public

Not ready to execute. I reviewed commit 9513d52c, the rev-5 spec, and the worktree code without changing files. R1–R8 remain settled.

Findings below distinguish implementation failures from missing proof. New files do not yet exist; their code locations are the numbered implementation blocks in the plan.

1. P1 — Task 5: GeoIP errors can disclose the client’s IP. Plan/code: plan:1315–1319, proposed internal/geo/mmdb.go. Locate logs the dependency’s error verbatim. In the pinned v1.13.1 reader, looking up IPv6 in an IPv4-only database produces an error containing ip.String(). This violates §6.5 even though the error is rate-limited. See the pinned reader, lines 233–237. Fix: log a fixed, non-sensitive error category. Add a public-IPv6/IPv4-database test that captures logs and asserts the address never appears.

1. P1 — Task 21: deploy proceeds when its region check fails. Plan: plan:4899. Code: deploy.go:120, apimode.go:775. Any timeout, 500, authentication failure, or malformed response prints C15b and continues toward shipping. A request for ie can therefore ship to an existing uk app without checking the mismatch. §11 permits skipping against an old console with no app_default, not after an arbitrary read failure. Fix: stop on read/decode errors; retain compatibility for a successful legacy response lacking app_default. Test the complete command and assert zero uploads after each failure.

1. P1 — Task 22: old consoles break the fleet-first canary rule. Plan: plan:5026. Code: apiregions.go:52, apiinstances.go:798, canary.go:405. An old console supplies no app_default, so the first canary run passes the fleet check. Its create door then overrides --region uk with fleet/acme. On repetition, the record comparison produces C25 and suggests removal. This reproduces the inconsistent first/repeat behavior R4-2 explicitly closed. Fix: resolve fleet placement through a compatible read, or refuse the flag when that fact cannot be established. Test both runs against the current console response shape, asserting identical refusal and no mutation.

1. P1 — Task 23: the service gate can check the wrong edge and pass. Plan/code: plan:5212, proposed scripts/region-service-gate.sh; relevant service path: edge.go:2699. The script inherits proxy settings. Through an HTTPS proxy, --resolve does not establish that the connection reached the nominated edge: the proxy handles the destination connection. Multiple “via edge” checks can therefore exercise the same public ingress while another edge is broken. Fix: disable proxy use for these checks, disable implicit curl configuration, and verify the connected address for explicit-edge requests. Add a proxy-environment regression test. Curl documents --noproxy '*' for disabling proxies.

1. P1 — Task 12: the proposed tests do not compile against postJSON. Plan: plan:2748, plan:2784. Code: authcli_test.go:20. postJSON returns map[string]any, but the plan converts its result to []byte and passes it to strings.Contains. Implementing the feature cannot resolve these type errors. Fix: return the decoded map directly from createApp; inspect its error field, or use rawReq where raw response text is required.

1. P1 — Tasks 13 and 15: replacement fixtures use create-only writes. Plan: plan:3178, plan:3799. Code: placementdoors_test.go:12, store.go:328. putEdges uses Put(..., ""), which refuses overwriting uk-box.json; the stale-picker test aborts before POST. The corrupt-manifest test also uses an empty ETag, ignores the conflict, and leaves the valid pooled manifest intact. Neither test reaches its intended condition. Fix: read the current ETag and replace conditionally; check every fixture-write error.

1. P1 — Task 8: TestNonHomeAppRegions expects an account its fixture never creates. Plan: plan:1697. Code: store_test.go:21, store.go:623. openTestStore runs Open/migration only. The bab account is created by BackfillOpsMemberships, normally called from NewServer. The proposed inner join therefore returns zero rows. I reproduced that result using the migration SQL in an in-memory database. Fix: create the account explicitly or run the backfill before inserting the apps.

1. P1 — Tasks 13 and 16: HTML assertions reject correctly rendered copy. Plan: plan:3183, plan:4053. Code: server.go:1693, fleets_test.go:823. C5, C7, and C8 contain apostrophes. html/template renders them as ', but these tests search raw HTML for unescaped strings. Conversely, the fleet suppression assertion can pass despite an incorrectly displayed region line. Fix: compare parsed text content or escaped expected strings, including the negative assertion.

1. P2 — Task 8: the migration test does not prove preservation through migrateAppsKey. Plan: plan:1711. Code: store.go:487, store.go:1709, store.go:1791. The proposed DDL and INSERT … SELECT correctly carry both columns. However, the test rebuilds only a row whose new fields are empty. It would pass if the rebuilt table had the columns but the copy silently dropped their values. Rename and transfer preservation also have no new assertions. Fix: seed a pre-rebuild database already carrying uk/timezone; verify those values after rebuilding and reopening, then after rename and transfer.

1. P2 — Task 11: the property test permits an implementation that returns no guesses. Plan/code: plan:2578, proposed regionguess_test.go; actual policy predicate: apiinstances.go:1797. Every ok == false result is skipped. The property therefore omits §16’s second invariant: a guess exists whenever the resolution order can reach a filtered candidate. It also substitutes an arbitrary map predicate for the actual normalized manifest predicate. Fix: generate manifests and use instanceRegionAllowed; independently determine expected existence. Confirm an always-null mutation fails.

1. P2 — Tasks 5, 12 and 14: several tests do not exercise the claimed protection. Plan: plan:1210, plan:2829, plan:2845, plan:3490. Code: apicli.go:349, server.go:3478, server.go:3515. Specifically: - The private-address test calls Locatable directly; removing its use from Locate would not fail it. - The “service principal” test supplies an empty regionReq; it never tests principal-to-IP wiring. - Review Focus 5 tests a taken-name precheck, not a failure after resolution, and omits the held-apex case. - Static/addr-backed exclusion tests ignore HTTP status, so refusals can count as success. Fix: exercise the real call boundaries, assert successful responses before examining records, and add the missing failure scenarios.

1. P2 — Tasks 14, 15, 20–23: required integration coverage is missing. Plan: plan:3432, plan:3705, plan:4792, plan:5150. Code: apiinstances.go:763, canary.go:381, homemap_test.go:866. §16 requires, but the tasks do not supply: - app-region store-error C20; pooled/fleet precedence with a non-home app default; - failed placement-read C27 and every app_default.reason; - old-body person-IP inference through the HTTP door; - command-level proof that deploy/canary refusals precede writes and uploads; - the service gate exercised against the actual backhaul/missing-mesh-key path. Task 23’s canned 404 tests the shell checker only. Existing backhaul tests are useful, but are not connected to that checker. Fix: add these named cases and mutation checks rather than claiming all §16 tests are covered.

1. P2 — Tasks 4, 6, 7, 18, 24 and 25: copy gates are ordered incorrectly and incompletely checked. Plan: plan:1098, plan:1467, plan:5386. Code targets: the proposed copy files and README.md:227. The tasks write and commit drafts before invoking review. The copy-review skill explicitly requires “Only then write copy into files.” Approval is also inferred from five matching commit subjects across unrestricted history, which proves neither approval nor application. New operator messages, usage additions, the fixture README and tz-refresh script output fall outside the stated inventories. Fix: review the complete string inventories before product-file writes, then apply approved text verbatim. Verify review results against the actual changed strings, not commit-message counts.

1. P2 — Task 16: unreadable defaults are presented as available. Plan: plan:4093. Code: server.go:2956. For a non-home app, edges-unreadable or policy-unreadable leaves Available == nil; the next branch nevertheless renders C7, “New instances start in…”. The corresponding create cannot proceed. Fix: handle unreadable reasons before the affirmative branch. Add explicit UI tests for both reasons and keep C7 conditional on confirmed availability.

1. P2 — Tasks 10–11: missing-coordinate diagnostics have no implementation. Plan: plan:527, plan:2645. Code target: proposed nearest.go/regionguess.go; topology source: instancesplacement.go:173. Unknown AWS regions are silently excluded. §§6.3, 6.5 and 14 require a once-per-code log. Only AWS-region disagreement logging is implemented. Fix: add the diagnostic at the console boundary and test its throttling without logging caller coordinates.

1. P2 — Task 5: fixture downloads are not pinned to the recorded revision. Plan: plan:1133. Code target: proposed internal/geo/testdata/*; dependency declarations: go.mod:5. Two files are fetched from moving main, then a separate ls-remote HEAD supplies the claimed pin. A branch update between requests can produce mixed fixtures or incorrect provenance. Fix: resolve one immutable SHA first; download both fixtures and provenance/license material using that SHA.

1. P2 — Tasks 24–25: release and cross-compile gates are incomplete. Plan: plan:5310, plan:5375, plan:5420. Code: 20-cross@5:41, README.md:227. The local gate omits Darwin/arm64 and Windows/arm64 on this amd64 machine, and builds only the root package for Darwin/Windows. Operational omissions include the dev service gate, recording the rollback interval, and an explicit pre-activation check that the exact floor version exists in all three required documents. The tzdata refresh is described as a checklist step but never added to that checklist. /tmp/g and the replacement Go cache also conflict with the supplied scratch/cache instructions. Fix: run all five existing CI targets with CGO_ENABLED=0 go build ./...; complete those rollout prerequisites and use the approved scratch/cache locations.

1. P2 — Task 22: C25 receives a code instead of the required place name. Plan: plan:5038. Code: canary.go:387, apiinstances.go:547. The helper passes have, such as uk, to C25 even when a current console can supply “UK · London”. The test expressly locks in the bare code. Fix: retain region names from /regions and resolve the existing record’s region through that map, falling back to the code only when unavailable.

The Task 1 parser reuse, CARLOS_POOLS=",tito" result, additive publication fields, Task 8 migration SQL, and the principal create-door fill conditions are consistent with the approved design. Every specified C identifier is represented, but the findings above prevent complete behavioral and review coverage. The pinned MaxMind module and IANA 2026b release exist; neither is an invented dependency.

I did not run the proposed Go suite or cross-builds because their implementation files do not exist and this review was read-only. The worktree remains clean.

Verdict: not ready to execute. Fix the P1 failures and add the missing P2 proof and gates before implementation begins.