amadan
docs: Astra's round-4 findings on the nearest-region spec
Codex (GPT-6) review of rev 4.1 (4ad64f7c), copied unedited: no P1s, three bounded P2s.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 file changed,
+54
−0
docs/superpowers/specs/CODEX-FINDINGS-NEAREST-REGION-R4.md+54 −0
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-NEAREST-REGION-R4.md b/docs/superpowers/specs/CODEX-FINDINGS-NEAREST-REGION-R4.md| new file mode 100644 |
| index 000000000..077b81c84 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-NEAREST-REGION-R4.md |
| @@ -0,0 +1,54 @@ |
| +Reviewed rev 4.1 at `4ad64f7c`, after AGENTS.md and rounds 1–3. The worktree is clean. In this revision, dispositions are **§21**; §19 contains copy. |
| + |
| +| Round-1 finding | Status | Why | |
| +|---|---|---| |
| +| 1. Picker bypasses pool exclusion | Resolved | A pooled manifest yields no candidates or guess. | |
| +| 2. Browser adoption changes the default | Resolved | `@auto` is distinct from an explicit choice; inference follows adoption detection. | |
| +| 3. Unnamed home prevents override | Resolved | Non-home app defaults require a named home, providing an explicit-home selector. | |
| +| 4. Mixed versions and rollback unspecified | Resolved | Read-first rollout, switch behavior, and the binary floor are specified; R7 settles enforcement. | |
| +| 5. Existing-canary `--region` undefined | Resolved | Equality and refusal are defined; a separate fleet interaction remains below. | |
| +| 6. Manifest absence versus failure unspecified | Resolved | Absent, known, and unreadable policies have distinct behavior. | |
| +| 7. Unavailable-default explanation disappears | Resolved | `app_default` represents absent regions; C8 is independent of alternative-region count. | |
| +| 8. Query hints reflected by redirects | Resolved | Hints use POST bodies, with privacy assertions covering redirects, errors, logs, and audit. | |
| + |
| +I found **three new P2 findings**, and no new P1 findings. |
| + |
| +1. **P2 — Fleet-placed apps still receive guesses that cannot determine their placement.** |
| + **Spec:** §6.1, §9.4, §12. |
| + |
| + Candidate selection considers the manifest’s pool and allow-list, but does not exclude fleet placement. These are independent properties: placement comes from the app row or routes in [fleets.go:479](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/console/fleets.go:479), and the create door unconditionally replaces the requested region at [apiinstances.go:798](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/console/apiinstances.go:798). |
| + |
| + For an app on `fleet/acme` with an unrestricted, shared-pool manifest, `/regions/guess` can suggest `uk`. A picker submits `uk`, but the instance lands on `fleet/acme`. The corrected `app_default.effective` does not fix this separate endpoint. |
| + |
| + **Fix:** Resolve `appPlacement` at the instance-guess endpoint and return `guess: null` for fleet-placed apps. Preserve app-creation inference for the stored, dormant app region. Test guess → submitted region → resulting record for a fleet-placed app. |
| + |
| +2. **P2 — Identical `canary --region` commands behave differently before and after creation on a fleet.** |
| + **Spec:** §8.2, §11, §12, C25. |
| + |
| + Canary creation and reuse are separate branches at [canary.go:405](/home/paulca/amadan.net/carlos/platform-nearest-region/canary.go:405). Creation reaches the fleet override at [apiinstances.go:798](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/console/apiinstances.go:798). |
| + |
| + On a fleet-placed app, the first `canary --region uk` creates a record with region `fleet/acme`. The next identical command follows §11’s comparison, finds `fleet/acme != uk`, and refuses. C25 recommends removing and recreating the canary, which reproduces the mismatch. |
| + |
| + **Fix:** Apply fleet precedence consistently to both creation and reuse. Resolve effective fleet placement before the existing-record comparison and report that placement. Add a repeated-command test; removing a canary must not be the proposed remedy for this case. |
| + |
| +3. **P2 — Phase P’s verification can pass while non-home instances remain unreachable.** |
| + **Spec:** §13.3, §16, §17 Phase P steps 4–5. |
| + |
| + Phase P verifies the app row and instance record, while relying on existing backhaul for service. Published placement capability establishes configuration, not a working request path: [instancesplacement.go:185](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/console/instancesplacement.go:185) treats a nonempty address as reachable. Backhaul separately requires a mirrored mesh key; without one it refuses to dial at [edge.go:2699](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/edge/edge.go:2699), and the request receives the unknown-host response at [edge.go:1428](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/edge/edge.go:1428). |
| + |
| + Thus all eligibility fields and stored values can pass the prescribed checks while requests through the deployment’s existing DNS fail. Deferring latency measurements is consistent with R8; deferring basic service verification leaves Phase P’s acceptance incomplete. |
| + |
| + **Fix:** Before production activation, require a successful request through the existing DNS entry edge to a freshly created instance in each candidate region. Verify the expected application/version, and include a missing-mesh-key case in the integration test. This needs neither DNS steering nor Phase L. |
| + |
| +The other requested checks support the design: |
| + |
| +- **Create precedence:** The proposed insertion preserves fleet precedence and the pool, replica, static, and address-backed exclusions. `DerivesPaths` has the required meaning at [provision.go:343](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/provision/provision.go:343); sidecars retain their separate door. |
| +- **Migration:** ALTERs precede `migrateAppsKey` at [store.go:402](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/console/store.go:402). Updating both the rebuild DDL and copy columns at [store.go:487](/home/paulca/amadan.net/carlos/platform-nearest-region/internal/console/store.go:487) addresses the migration trap. |
| +- **Compatibility and privacy:** Permissive decoding and client unmarshalling support additive fields. POST-body hints and request-local coordinates introduce no identified retention requirement. The specified rollout order and R7 floor address old-console behavior. |
| +- **Policy and unavailable defaults:** Normalized guesses remain subject to the allow-list and create-time checks; I found no new bypass. Migrated apps retain `""` and avoid the new positive-eligibility refusal. For non-home defaults, an explicit override remains subject to policy—if every allowed region is unavailable, capacity or policy repair is necessary. |
| + |
| +There are **no open questions to recommend answers to** in rev 4.1: §17 is rollout, and §18 explicitly says none. R5, R7, and R8 settle the earlier questions. |
| + |
| +All supporting code citations above were checked in this worktree. No files changed, no tests ran, and production configuration was not inspected. |
| + |
| +**Verdict: not ready for planning.** Specify the two fleet behaviors and add Phase P’s service-verification gate. These are bounded corrections; none requires reopening R1–R8 or expanding into Phase L implementation. |
| \ No newline at end of file |