| 1 | | Previous finding | Disposition at `034727d4` | |
| 2 | |---|---| |
| 3 | | R1-1 — Readiness without established database state | **Resolved.** Unknown pointers block readiness; off and expired pointers require empty mirrors. | |
| 4 | | R1-2 — `off` waits beyond caller timeout | **Resolved.** `TryLock` refuses concurrent publication with 409. | |
| 5 | | R1-3 — Unbounded console pointer reads | **Resolved for pointer reads.** `readPointerCapped` streams and caps the object. | |
| 6 | | R1-4 — Recovery generation deleted after failed synchronization | **Resolved.** Installation-error cleanup preserves loaded and retained generations through `removeGenerationLocked`. | |
| 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. | |
| 8 | | R1-6 — Countries overflow ordering | **Resolved.** `_other` remains available to the sorter and is relabelled during rendering. | |
| 9 | | R2-1 — Feed status writes bypass the read cap | **Partly resolved; P2 remains for `DirStore`.** `geoipfeed.go:174` calls `Put`, whose implementation at `internal/release/store.go:408` reads the entire existing object with `os.ReadFile`. An oversized `feed.json` therefore still reaches memory after its capped read rejects it, even before a create-only conflict. Apply the stat/streamed-hash conditional-write checks already used by `PutFrom`; test the underlying write path too. | |
| 10 | | R2-2 — Timed-out checks never persist failures | **Resolved.** Status writes receive a separate bounded context. | |
| 11 | | R2-3 — Unreadable feed status clears an alert | **Resolved within a running mirror.** Read and parse failures preserve its previous state. The cold-start case is new finding 2 below. | |
| 12 | | R2-4 — Unusable directory prevents “not loaded” alert | **Resolved.** Control-plane state is retained before directory preparation. | |
| 13 | | R2-5 — Published alert reasons never change while unhealthy | **Resolved for publication.** Reason changes update `Value` and emit an event. The email consumer has a separate defect described below. | |
| 14 | | R3-1 — Console follower keeps serving an expired database | **Resolved.** Every tick checks the loaded reader independently of `current`; candidates also receive a metadata expiry check. | |
| 15 | | R3-2 — Transient status-read failure erases feed history | **Resolved.** An unsuccessful status read marks history unknown and prevents replacement. | |
| 16 | |
| 17 | New findings: |
| 18 | |
| 19 | 1. **P2 — Unbounded bucket operations can prevent startup or permanently occupy the publish slot.** |
| 20 | `internal/console/server.go:1444,1448` and `internal/console/apisystemgeoip.go:53`: startup supplies `context.Background()` to synchronous feed-status deletion and to maintenance operations holding the publisher mutex. Manual staging also reads the pointer without an operation deadline; the HTTP read deadline only bounds the incoming connection. A bucket request that connects but stalls can therefore prevent console startup, or leave publication and `off` returning 409 indefinitely. Give these operations bounded contexts, keep optional cleanup off the synchronous startup path, and test cancellation with a context-aware stalled store. |
| 21 | |
| 22 | 2. **P2 — Restarting the agent can falsely clear a persisted GeoIP alert.** |
| 23 | `internal/edge/geoipmirror.go:542–558`: a new mirror initializes pointer and feed state as false, and returns reason 0 before establishing either state. Restart with reason 5 persisted, then make `feed.json` unreadable while alert publication still works: the restored firing rule receives zero and publishes a recovery despite no successful feed check. An expired-pointer alert has the same problem when its pointer cannot be read after restart. Represent unknown inputs explicitly and preserve the persisted alert until sufficient state is known to clear it. Test a cold mirror against a restored firing rule. |
| 24 | |
| 25 | 3. **P2 — Console email delivery suppresses changes to an active GeoIP reason.** |
| 26 | `internal/console/alertwatch.go:201,243`: delivery deduplication compares only `Firing`. After an operator receives reason 2, the newly emitted reason-3 event updates the bucket, but the watcher sees `true → true` and sends no expiry notification. Remember the delivered GeoIP reason per recipient, or an equivalent transition identifier, while retaining retry behavior. Test reason 2 → 3 through `alertPass`, beyond the evaluator test. |
| 27 | |
| 28 | 4. **P2 — Region guesses can carry another generation’s attribution.** |
| 29 | `internal/console/regionpicker.go:45–49`: `guessRegion` performs its lookup before `geoipAttribution` independently reads the current source. If the follower swaps DB-IP to GeoLite2 between those calls, the page credits GeoLite2 for a DB-IP-derived suggestion; swapping to nil removes the credit altogether. Return the source alongside the location under the same reader lock and use that source for attribution. Add a test that swaps between lookup and rendering. |
| 30 | |
| 31 | Validation was static. Targeted tests could not start because Go’s work directory was read-only; cross-compilation remains unverified. |
| 32 | |
| 33 | Verdict: revise. |
| 34 | |
amadan