fix(net): survive edge connection-table flushes — DERP/disco resilience - #94
Code review found 1 important issue
Found 6 candidates, confirmed 5. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 4 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | components/microlink/src/ml_wg_mgr.c:1219-1226 |
[quality] Safety-peer membership test duplicated 4 more times by this PR sweep:(ml->config.priority_peer_ip != 0 && .* |
| 🟡 Nit | firmware/components/dcs_support/src/dcs_pstop_ring.c:139-152 |
LED brightness has no floor — pct=0 can fully blank the PSTOP safety ring |
| 🟡 Nit | components/microlink/src/ml_derp.c:1649-1653 |
Aux DERP connect: home-rx pumping stops after TLS handshake, leaving HTTP-upgrade/DERP-handshake reads unpumped |
Annotations
Check warning on line 1226 in components/microlink/src/ml_wg_mgr.c
claude / Claude Code Review
[quality] Safety-peer membership test duplicated 4 more times by this PR sweep:\(ml->config\.priority_peer_ip != 0 && .*== ml->config\.priority_peer_ip\) \|\| is_health_tracked
sweep:`(ml->config\.priority_peer_ip != 0 && .*== ml->config\.priority_peer_ip\) \|\| is_health_tracked`
This PR adds 4 more independently-maintained copies of the safety-peer membership test (e.g. `teardown_vetoed` at ml_wg_mgr.c:928 and `safety_rekey_inflight` at :1225), on top of 2 pre-existing ones — 6 total in this file. Extract a single `is_safety_peer(ml, vpn_ip)` helper (the agent running the sweep expression will find and fix the remaining sites) so a future change to the safety-peer de
Check warning on line 152 in firmware/components/dcs_support/src/dcs_pstop_ring.c
claude / Claude Code Review
LED brightness has no floor — pct=0 can fully blank the PSTOP safety ring
The new master LED brightness control clamps only the upper bound (>100), so setting brightness to 0% (e.g. `POST /api/led_brightness?pct=0`) fully blanks the entire PSTOP safety ring — including the RED STOP, GREEN OK, and PURPLE lockstep-MISMATCH indications — and the value persists across reboots with no auto-recovery. This contradicts the file's own established invariant (locate mode auto-expires via `DCS_RING_LOCATE_TIMEOUT_MS` specifically so a diagnostic feature can never mask STOP/OK ind
Check warning on line 1653 in components/microlink/src/ml_derp.c
claude / Claude Code Review
Aux DERP connect: home-rx pumping stops after TLS handshake, leaving HTTP-upgrade/DERP-handshake reads unpumped
In `ml_derp_connect()`, `derp_pump_home_rx()` is only called inside the TLS-handshake retry loop (ml_derp.c:1635); once the handshake completes the aux conn's read timeout is restored to the full 20s `DERP_CONNECT_TIMEOUT_MS` (line 1651-1653) and the subsequent HTTP-upgrade byte-read loop and DERP-handshake reads (ServerKey/ServerInfo) never pump home-rx again. A network stall during those phases can therefore still starve home-conn rx for up to ~20s during an aux (re)connect, reproducing the ex