| 1 | Round 6 review of `00ace13c`. |
| 2 | |
| 3 | | Previous finding | Disposition | Evidence from revision 6 | |
| 4 | |---|---|---| |
| 5 | | R3-1. Expiry/install races | Resolved | §5.3 retains expiry checks before download and installation, with `genMu` serializing selection, expiry and cleanup. | |
| 6 | | R3-2. Unprotected console `Locate` | Resolved | §5.4 retains locked `Swappable.Locate` and puts the wrapper in `Config.GeoIP`. | |
| 7 | | R3-3. Coordinate-only results | Resolved | §8.1a accepts coordinate-only hits; §9 counts their country as `_unknown`. | |
| 8 | | R3-4. Heartbeat freshness | Resolved | §5.5 retains `status.BoxStale` and the jittered-heartbeat test. | |
| 9 | | R1-4, carried forward: orphan destruction | Resolved | §3 retains startup and daily bucket sweeps, followed by §15’s noncurrent-version expiration. | |
| 10 | | R1-16, carried forward: rollout verification | Partly resolved | §5.5 now reconciles active fleet registrations and uses `LoadBoxesComplete` everywhere. However, every platform edges record remains expected forever, while retired boxes retain those records: [instancesplacement.go:86](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/console/instancesplacement.go:86). A retired box therefore permanently blocks “ready.” Define an explicit retirement mechanism that removes it from expected inventory; stale heartbeats alone must not establish retirement. | |
| 11 | | R4-1. Duplicate tunnel analytics | Resolved | §8.4 now requires zero remote observations and exactly one at the hub, matching §9. | |
| 12 | | R4-2. Publication deadline across backhaul | Resolved | §4.2 retains staging followed by a 202 response and a polled publication job. | |
| 13 | | R5-1. Concurrent upload staging | Resolved | §4.2 reserves the mutex before reading the body, transfers mutex and directory ownership to the job, and tests refusal while the first upload is stalled. | |
| 14 | |
| 15 | 1. **P2 — §§8.2–8.4, 9: plaintext tunnel suppression breaks the hub’s cache contract.** The hub computes geo headers, but the plaintext tunnel receiver discards them. An app returning `Vary: X-Carlos-Geo-Country` therefore produces a response without receiving a country, while the hub caches that response under, for example, `IE`. After the channel reconnects over TLS, the hub can replay it although the app would now receive `IE`. Fleet fallback between plaintext and TLS connections has the same problem. Hub-only analytics also retains the ingress country despite §9’s plaintext-`off` rule. |
| 16 | |
| 17 | **Evidence:** [edge.go:1657](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/edge/edge.go:1657) matches cached variants against the hub’s request; [edge.go:1776](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/edge/edge.go:1776) stores its `Vary` values. Connection selection happens later at [tunnel.go:393](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/edge/tunnel.go:393), and reconnect replaces the channel without cache invalidation at [tunnel.go:209](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/edge/tunnel.go:209). |
| 18 | |
| 19 | **Fix:** define how the hub determines and pins the destination’s geo capability before cache matching, keeping app headers, variants and analytics consistent. Cover reconnect and fleet-selection races. Test plaintext→TLS, TLS→plaintext and mixed-transport fleet fallback with an app varying on country. |
| 20 | |
| 21 | 2. **P2 — §§4.1–4.2: console crashes leave staged uploads without cleanup.** Cleanup on handler or job exit does not run after abrupt process termination. A crash during staging or publication can leave up to 1 GiB under the console’s data directory. Repeated crashes and retries accumulate abandoned files despite the single-upload slot. |
| 22 | |
| 23 | **Evidence:** §4.1 specifies exit-path cleanup; §4.2 transfers that responsibility to the job. The reused registry stores jobs only in process-local maps at [apisystemupdate.go:374](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/console/apisystemupdate.go:374); its sweep removes finished map entries, not abandoned files. |
| 24 | |
| 25 | **Fix:** use a dedicated GeoIP staging directory and remove abandoned staging directories at console startup, before admitting uploads or starting the feed. Test restart and retry after abrupt termination during both staging and job execution. |
| 26 | |
| 27 | Verdict: revise. |
amadan