| 1 | | Previous finding | Disposition at `a10120de` | |
| 2 | |---|---| |
| 3 | | R1-1 — Readiness without established database state | **Resolved.** Unknown pointers block readiness; off and expired pointers require empty mirrors (`internal/console/apisystemgeoip.go:140,345,366`). | |
| 4 | | R1-2 — `off` waits beyond caller timeout | **Resolved.** `Off` uses `TryLock`; concurrent publication returns 409 (`internal/console/geoippublish.go:343`). | |
| 5 | | R1-3 — Unbounded console pointer reads | **Resolved.** Pointer reads remain streamed and capped, retaining the ETag (`internal/console/geoippublish.go:163`). | |
| 6 | | R1-4 — Recovery generation deleted after failed synchronization | **Partly resolved; P2 remains.** A → B → C now retains every potentially recoverable generation. However, republishing retained A while directory synchronization still fails can reach `internal/edge/geoipmirror.go:429–432`: a failure writing `current` unconditionally deletes A. A crash can then restore durable `current=A` without its database. Protect retained generations on installation-error paths too, and test A → B → A with both directory-sync sites failing and the final `current` write failing. | |
| 7 | | R1-5 — Rejected-upload test does not establish zero reads | **Resolved at the reservation boundary.** The panicking-reader test detects reads before slot reservation (`internal/console/geoippublish_test.go:375`). | |
| 8 | | R1-6 — Countries overflow ordering | **Resolved.** `_other` survives sorting and is relabelled during rendering (`internal/console/analyticsview.go:415,425`). | |
| 9 | | R2-1 — Feed status writes bypass the read cap | **Resolved.** The writer uses the ETag from the initial capped read (`internal/console/geoipfeed.go:146,161`). | |
| 10 | | R2-2 — Timed-out checks never persist failures | **Resolved.** Status persistence has a separate bounded context detached from the expired check (`internal/console/geoipfeed.go:131`). | |
| 11 | | R2-3 — Unreadable feed status clears an alert | **Resolved in the mirror.** Read and parse failures preserve the previous state; confirmed deletion clears it (`internal/edge/geoipmirror.go:497`). | |
| 12 | | R2-4 — Unusable directory prevents “not loaded” alert | **Resolved.** Control-plane state is retained before directory preparation (`internal/edge/geoipmirror.go:245,315`). | |
| 13 | | R2-5 — Published alert reasons never change while unhealthy | **Resolved.** A changed reason updates `Value` and emits an event for publication without restarting sustain (`internal/metrics/alert.go:269`). | |
| 14 | |
| 15 | New findings: |
| 16 | |
| 17 | 1. **P2 — The console follower can keep serving an expired database.** |
| 18 | `internal/console/geoipfollower.go:48–61`: expiry is checked against the newly read `current`, not the reader already loaded. Load GeoLite2 A, then make `current` unreadable—or have it select an unopenable replacement B. Once A passes its 30-day limit, every tick still returns while retaining A, so `/geo` and region guesses continue using expired data. Check the loaded reader’s own source/build on every tick, independently of local-state errors, and validate a candidate’s own metadata before swapping. Add expiry tests covering both failure paths. |
| 19 | |
| 20 | 2. **P2 — A transient status-read failure erases the feed’s failure history.** |
| 21 | `internal/console/geoipfeed.go:146–168`: a read error becomes an empty status and empty ETag. If the subsequent create-only write conflicts, `writeStatus` deletes the existing object and replaces it. With seven failures already recorded, one transient read failure followed by another failed download therefore replaces the count with one. The mirror reads a valid non-failing status and clears the alert despite no successful check. Distinguish confirmed absence from read failure; retry a capped read before updating, or preserve the existing object when its history is unknown. Test a failed status read followed by successful writes against an existing failing status. |
| 22 | |
| 23 | Validation was static. Targeted tests could not start because Go’s work directory was read-only; cross-compilation remains unverified. |
| 24 | |
| 25 | Verdict: revise. |
| 26 | |
amadan