carlos / platform Public

Round 4 review of d06821cb.

| Previous finding | Disposition | Evidence from revision 4 | |---|---|---| | R3-1. Expiry/install races | Resolved | §5.3 checks expiry before download and installation; genMu serializes selection, expiry and cleanup. | | R3-2. Unprotected console Locate | Resolved | §5.4 adds locked Swappable.Locate, puts the wrapper in Config.GeoIP, and includes concurrent region guesses in testing. | | R3-3. Coordinate-only results | Resolved | §8.1a accepts a valid coordinate pair without country; §9 counts that result as _unknown. | | R3-4. Heartbeat freshness | Resolved | §5.5 uses status.BoxStale and tests the normal jittered heartbeat interval. | | R1-4, carried forward: orphan destruction | Resolved | §3 adds startup and daily orphan sweeps, making orphaned uploads eligible for §15’s noncurrent-version expiration. | | R1-16, carried forward: rollout verification | Partly resolved | §5.5 includes fleet prefixes through systemBoxes, but still excludes records older than seven days and has no expected-inventory comparison. Moreover, systemBoxes calls LoadBoxes, which discards unreadable records: apisystemupdate.go:233, fleet.go:219. “Ready” still needs expected-box reconciliation and explicit handling of incomplete reads. |

1. P2 — §§8.1a, 9: tunneled pageviews acquire two country counts. The spec adds country observations at every existing analytics observer and sums them across boxes. A successful tunneled request passes through both the hub’s observer and the remote edge’s observer. With analytics enabled on both, one pageview contributes twice to Countries and GeoSources. A hub cache hit contributes once, so the distortion depends on cache behavior.

Evidence: the observer runs for every non-status route, including tunnel and fleet routes, at edge.go:1551. Analytics switches do not exclude forwarding routes: telemetry_pass.go:35. Both boxes publish app analytics through usage_loop.go:30.

Fix: define one analytics observation per served response. Suppress the forwarding hub’s observation for responses served through the tunnel, while retaining observations for responses answered at the hub, including cache hits. Keep usage accounting separate. Test a real hub and remote edge with both meters enabled, flush both, and assert that the aggregated pageview, country and source counts each increase once.

1. P2 — §§4.1–4.2: the publication deadline does not survive backhaul. Extending the console’s request-body deadline to fifteen minutes does not extend the sending edge’s response-header deadline. After the upload finishes, verification and bucket publication can legitimately take more than thirty seconds. If the request arrived through another edge, that edge returns 502 before the console’s allowed operation finishes.

Evidence: edge.go:2674 sets backhaulHeaderTimeout to thirty seconds, and edge.go:2805 applies it as ResponseHeaderTimeout. The platform-update API already moves lengthy processing into a job at apisystemupdate.go:345.

Fix: specify an upload-and-job protocol: stage the bounded upload, return a job identifier promptly, then verify and publish under the existing publication mutex and operation deadline. Have the CLI wait for the job’s result. Test publication through a backhaul with processing delayed beyond its response-header timeout.

Verdict: revise.