fix(tether): no retransmit of ambiguous USB hand-offs; monotonic WireGuard handshake timestamps (#157) by iliabaranov · Pull Request #160 · polymathrobotics/protective-stop

iliabaranov · 2026-09-20T18:24:21Z

Fixes #157 (two 9-minute tether outages, 2026-09-19 19:18 and 2026-09-20 09:06). Two independent, minimal commits.

Root cause (from tether-side captures)

A routine host-initiated WireGuard rekey; the device's handshake response left the device three times (identical bytes, within 1 ms). The host's parallel handshake workers each consumed a copy and derived three sessions from one handshake (three keepalives, each with counter 0), keeping the last and discarding the first — the one whose receiver index the device uses. From then on the host transmitted but no longer accepted the device's packets; the device's keys expired at 180 s; its ~70 re-initiations were dropped as replays (uptime-based timestamps, see below) until the host trimmed the idle peer ~5 min later. Recovery at exactly 540 s both times.

The duplication is ours: ml_usb_tx.c retried the head frame on any non-OK result from tinyusb_net_send_sync(), whose ESP_ERR_TIMEOUT is ambiguous (the deferred xmit may have completed just after the wait). The retry counter rose by exactly 2 in the second of each broken rekey. With a split counter, steady state showed 0 genuine busy refusals and ~10 timeouts/min — every "busy retry" was a timeout.

Commit 1 — `fix(usb)` (3 files, 24+/10−)

Retry only ESP_FAIL (TinyUSB ran and refused the frame: definitely unsent). A timed-out offer is given up — possibly lost, never doubled — and counted in the new usb_tx_timeout_uncertain (/state.json).
Hand-off wait 20 → 200 ms, capped to the frame's remaining TTL (400 ms), so timeouts become rare instead of frequent.

Claim kept narrow: this removes the timeout-driven retransmission. The vendor's stale deferred-callback path (a timed-out offer's callback can still copy the next frame) remains and is bounded only by the now-rare timeout — tracked separately (upstream esp_tinyusb).

Commit 2 — `fix(wg)` (4 files, 42+/2−)

The handshake TAI64N was built from uptime, so after a reboot every device-initiated handshake was older than the last one the host had accepted from our key and was silently dropped as a replay until the host forgot the peer. Now seconds = TAI(0) + (epoch << 32) + uptime_s with a per-boot epoch read-increment-committed once at startup (before the WG netif, off the handshake path). wireguard-go compares the 12 bytes lexicographically, never against wall time.

Availability trade-off, stated explicitly: on NVS failure it logs, exposes wg_epoch = 0 in /state.json (degraded: device-initiated recovery may fail; host-initiated handshakes still work), and keeps the tunnel up rather than failing init. Erasing NVS / downgrading re-introduces the pre-fix behaviour for hosts that retain replay state (~5 min). The degraded case has not been bench-tested against a host with retained state.

Bench screening (tethered remote, boot events included)

	before (v1.3-31)	commit 1 alone	commits 1+2, 200 ms wait
duplicated frames device→host	2.8 % (535/h, incl. 6 handshake responses)	1.0 %	0 / 2144
`timeout_uncertain`	n/a (≈10/min hidden in busy_retries)	≈10/min	2 at boot, then 0 (6 min)
heartbeat loss	0.1 %	1.4 % (withdrawn frames lost)	0 (1654 sent; replies 1655 = +1 BOND ack)
device-initiated handshakes accepted	never (host always initiated)	—	first post-boot initiation answered in 1 ms; device now runs its own 120 s rekeys

A 1 h steady-state tether soak on this head is running; results will be posted here. Final qualification of the merged result is a separate step.

Both roles build; pre-commit clean. Public repo: no captures attached.

…200 ms hand-off wait (#157) tinyusb_net_send_sync() returning ESP_ERR_TIMEOUT is ambiguous: the deferred xmit may have completed just after the wait, so the head-retry of #149 could send the frame twice. Duplicating a WireGuard handshake RESPONSE makes the host derive one session per copy and discard the one the device uses — the two 9-min tether outages of 2026-09-19/20 (retry counter +2 in the very second of each broken rekey; three response copies and three counter-0 host keepalives on the wire). Two changes: - Retry only ESP_FAIL (the TinyUSB task ran and the endpoint refused the frame: definitely unsent). A timed-out offer is given up — possibly lost, never doubled — and counted in the new usb_tx_timeout_uncertain. - Hand-off wait 20 -> 200 ms: on the bench every "busy retry" was in fact a timeout (~10/min; equal-priority passes of 70+ ms starve the TinyUSB task), and each timeout also leaves a stale deferred callback that can double the NEXT frame (vendor race). 200 ms (half the frame TTL) makes timeouts rare. Bench (tethered remote, 6 min each): before 2.8 % duplicated frames, 6 duplicated handshake responses/h; after: 0 duplicates, 0 timeouts after boot, 0 heartbeat loss (1654 sent / 1655 replies). The vendor's stale-callback path remains a separate (upstream) exposure for the rare timeout.

… epoch (#157) The timestamp was uptime-based, so after a reboot every device-initiated handshake was older than the last one the host had accepted from our key and was silently dropped as a replay until the host forgot the peer (~5 min idle trim). seconds = TAI(0) + (epoch << 32) + uptime_s, epoch read-increment- committed once at startup (before the WG netif is up, off the handshake path); on NVS failure it logs and falls back to uptime-only rather than taking the tunnel down. wireguard-go compares the 12 bytes lexicographically and never against wall time. Bench: the device's post-reboot first initiation is answered in 1 ms, and it now initiates its own 120 s rekeys (previously never accepted; the host was always the initiator).

graphify-labs

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 4 advisory finding(s) below merit a look before merge.

Formal verification. No changes could be formally verified in this run.

Graphify review — findings

Adds reboot-monotonic WireGuard handshake timestamps: wireguard_tai64n_epoch_init reads-increments-commits a per-boot epoch in NVS once before the WG netif comes up and offsets wireguard_tai64n_now by (epoch << 32) seconds, so a rebooted device isn't rejected as a replay; if NVS can't persist it logs and falls back to uptime-only timestamps rather than dropping the tunnel, and wireguard_tai64n_epoch (0 = degraded) is exposed on the admin state page as wg_epoch. Stops the USB TX drain from duplicating frames on an ambiguous hand-off: usb_drain now retries only ESP_FAIL (definitely unsent) and gives up on TIMEOUT — counted in the new timeout_uncertain diag — since a doubled handshake response made the host derive a second session, and bumps TX_SYNC_WAIT_MS from 20 ms to 200 ms (capped to the remaining TTL) to make timeouts rare.

Worth a look

Timed-out USB handoff frees ring slot while deferred send may still use it — components/ml_dev_tether/src/ml_usb_tx.c:121 · Escalate · high
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
NVS epoch read errors are ignored and can reset the monotonic boot epoch — components/microlink/components/wireguard_lwip/src/wireguard-platform-esp32.c:44 · Escalate · high
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
TIMEOUT frame no longer retried — potential frame loss on transient starvation — components/ml_dev_tether/src/ml_usb_tx.c:108 · Escalate · medium
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
New diagnostic field shifts existing public struct offsets — components/ml_dev_tether/include/ml_usb_tx.h:35 · Escalate · medium
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 238 functions depend on the 153 functions this change touches.

Health — this change adds coupling hotspots:

new: disco_send_ping_to_peer() — 8 callers, 8 callees
new: dcs_support_init() — 2 callers, 30 callees
new: add_peer() — 2 callers, 22 callees
new: wireguardif_network_rx() — 3 callers, 11 callees
new: disco_send_call_me_maybe() — 6 callers, 4 callees
new: neg_apply_target() — 2 callers, 10 callees
new: disco_periodic_probes() — 1 callers, 16 callees
new: process_disco_pong() — 1 callers, 15 callees
…and 35 more — each is listed as a finding

Verification — 238 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

verification_scope: 236 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify wireguard\_tai64n\_now.

The verifier did not have enough to check wireguard\_tai64n\_now, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: void

Could not verify: Could not verify wg\_init\_interface.

The verifier did not have enough to check wg\_init\_interface, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: esp_err_t

Could not verify: Could not verify ml\_usb\_tx\_get\_diag.

The verifier did not have enough to check ml\_usb\_tx\_get\_diag, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: void

Could not verify: Could not verify usb\_drain.

The verifier did not have enough to check usb\_drain, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported parameter type(s)

Could not verify: Could not verify page\_state.

The verifier did not have enough to check page\_state, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: esp_err_t

· 43 more finding(s) on lines outside this diff (see the check run).

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding on the double NVS epoch write, I also checked two related concerns and ruled them out: the new wireguard_tai64n_epoch_init() NVS write in wg_init_interface() runs before wireguardif_init()/netif setup rather than while holding the lwIP TCPIP core lock, so it does not stall RX/TX during WG bring-up; and in ml_usb_tx.c's usb_drain(), tinyusb_net_send_sync() only returns ESP_OK, ESP_FAIL, or ESP_ERR_TIMEOUT in practice, so the else-branch handling of "any non-OK/non-FAIL result" as timeout_uncertain does not silently swallow a distinct definite-failure code.

Extended reasoning...

A CONFIRMED finding (double invocation of the new epoch-init NVS write when the early cold-bond wg_init_interface() call fails and the normal path retries it later in the same boot) is already queued to be posted as an inline comment, so per the workflow rules I do not restate it in the body. Beyond that, I independently examined two other candidate issues that were investigated and ruled out this run: (1) whether the new blocking NVS flash write added to wg_init_interface() executes while the global lwIP TCPIP core lock is held, which would stall all netif RX/TX (including pstop safety heartbeats) device-wide — reading ml_wg_mgr.c shows the call is placed immediately before wireguardif_init()/netif state assignment, i.e., during interface setup rather than inside a per-packet or lock-held hot path, so this is not a live concern in this diff; and (2) whether usb_drain()'s new two-way branch (ESP_FAIL retried, everything else counted as timeout_uncertain) could misclassify a definite non-timeout failure code as an uncertain timeout — tinyusb_net_send_sync()'s documented/observed return surface is limited to ESP_OK, ESP_FAIL, and ESP_ERR_TIMEOUT, so the else branch in practice only catches the intended ambiguous-timeout case. Given the CONFIRMED finding already flagged for inline posting, and the security/availability-sensitive nature of both the WireGuard replay-timestamp logic and the USB tether retry logic (per the PR's own stated root-cause analysis of two production outages), a human should still review this PR; I am not approving it.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

🟡 components/microlink/src/ml_wg_mgr.c — If the early cold-bond wg_init_interface() call at ml_wg_mgr.c:4610 fails (e.g. calloc or wireguardif_init failure), wg_up_early stays false and wg_init_interface() runs again at line 4657, calling wireguard_tai64n_epoch_init() (ml_wg_mgr.c:461) a second time in the same boot. Each call does an NVS read-increment-commit, so a single boot burns two epoch values/flash writes instead of one, and the admin page's wg_epoch jumps by 2 that boot (misleading diagnostics), though monotonicity for handshakes is unaffected. Fix: make epoch persistence idempotent per boot (call it once before either wg_init_interface() attempt, or guard with an already-initialized flag) so retries of wg_init_interface() within one boot never re-run the NVS commit.

Extended reasoning...

wg_init_interface() at ml_wg_mgr.c:429 unconditionally calls wireguard_tai64n_epoch_init() at line 461 before touching the netif. It is invoked from two call sites in ml_wg_mgr_task: line 4610 (cold-bond early preseed, gated on vpn_ip!=0 && cached>0) and line 4657 (normal path, gated on !wg_up_early). If the line-4610 call returns non-ESP_OK (calloc failure at line 437, or wireguardif_init failure at line 463), wg_up_early stays false. Control then reaches line 4655's if (!wg_up_early) and calls wg_init_interface() again at line 4657, re-running wireguard_tai64n_epoch_init(). Each call does nvs_open+nvs_get_u32+increment+nvs_set_u32+nvs_commit against the 'epoch' key in namespace 'ml_wg', so the counter advances twice for one physical boot. This is new: before this diff wg_init_interface() had no epoch side effect, so retrying it on failure was free.

Verification: nit. The mechanism is real but the consequences are cosmetic/wear-level, so nothing actually breaks. Reachability: wireguard_tai64n_epoch_init() is called unconditionally inside wg_init_interface() at ml_wg_mgr.c:461, positioned AFTER the calloc-failure return (line 437-440) but BEFORE wireguardif_init (line 463) whose failure returns ESP_FAIL at line 470. wireguard_tai64n_epoch_init…

…leaves the epoch untouched Review round 1 (#160): wg_init_interface() can be retried within one boot (early cold-bond attempt, then the normal path), which would have burnt two epoch values per boot; and an NVS read error other than ESP_ERR_NVS_NOT_FOUND would have restarted the counter at 1 — a rollback of exactly the value the fix exists to keep monotonic. Now: once per boot, and only not-found starts a fresh counter; any other read/write/commit error leaves NVS untouched and reports degraded (wg_epoch = 0).

iliabaranov · 2026-09-20T18:57:57Z

Review round 1 → 60bed691 (both roles build, pre-commit clean):

claude[bot] 🟡 — epoch init runs twice if wg_init_interface() is retried in one boot. Correct; fixed — wireguard_tai64n_epoch_init() is now once-per-boot (static guard), so a retried interface init cannot burn a second epoch or make wg_epoch jump by 2.

Graphify — "NVS epoch read errors are ignored and can reset the monotonic boot epoch" (high). Correct and the more important of the two: an nvs_get_u32 error other than not-found would have restarted the counter at 1 — a rollback of exactly the value this fix keeps monotonic. Now only ESP_ERR_NVS_NOT_FOUND starts a fresh counter; any other read/write/commit error leaves NVS untouched and reports degraded (wg_epoch = 0, log line).

Graphify — "timed-out USB hand-off frees the ring slot while the deferred send may still use it" (high). Not reachable: tinyusb_net_send_sync() waits for the buffer semaphore before returning on timeout (so a copy in progress completes first), then NULLs its global packet pointer — after it returns, nothing in the vendor code references our slot; NCM copies the frame synchronously into its NTB inside do_send_sync. The stale-callback hazard that does remain (#161) is the callback picking up the next published packet, not a freed one — and the pre-fix retry path advanced the head under exactly the same conditions after the retry.

Graphify — "TIMEOUT frame no longer retried — potential loss on transient starvation" (medium). That is the deliberate trade-off, stated in the PR: a duplicated frame broke the tunnel for 9 minutes twice; a lost heartbeat costs nothing at 5 Hz. And it is why the wait went to 200 ms (TTL-capped): with it, timeouts were 2 at boot and then 0 in 20 min, heartbeat loss 0 (5865/5865) — a frame is lost only if the TinyUSB task is starved longer than 200 ms, at which point the 400 ms TTL would have expired it shortly anyway.

Graphify — "new diagnostic field shifts struct offsets" (medium). ml_usb_tx_diag_t is firmware-internal (one producer, one consumer, rebuilt together); no ABI boundary.

The 1 h combined soak (this head's predecessor 7d7e47d3 + #162) is running; results to follow here. 60bed691 differs only by the two guards above, neither on a path the soak exercises.

graphify-labs

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 4 advisory finding(s) below merit a look before merge.

Formal verification. No changes could be formally verified in this run.

Graphify review — findings

Makes WireGuard handshake timestamps monotonic across reboots by adding wireguard_tai64n_epoch_init, which reads-increments-commits a per-boot epoch in NVS (namespace ml_wg) and shifts it into the high bits of the TAI64N seconds so a rebooted device can re-initiate instead of being dropped as a replay for up to 9 minutes (#157); on NVS failure it logs and falls back to uptime-only timestamps rather than blocking the tunnel, and wg_init_interface calls it once before the netif comes up. Reworks the USB TX drain hand-off so usb_drain only retries on ESP_FAIL (endpoint definitely refused) and gives up on TIMEOUT — counting it in the new timeout_uncertain diag — to avoid double-sending a WireGuard handshake response that would make the host derive and keep the wrong session; the sync wait grows from 20 ms to 200 ms (capped to remaining TTL) to make starvation timeouts rare. Exposes usb_tx_timeout_uncertain and the current handshake epoch in the admin state JSON.

Worth a look

Timed-out USB send releases a ring slot while TinyUSB may still use its buffer — components/ml_dev_tether/src/ml_usb_tx.c:120 · Escalate · high
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Public diagnostics struct inserts a field before existing members — components/ml_dev_tether/include/ml_usb_tx.h:35 · Escalate · medium
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
wireguard_tai64n_epoch() return truncated to 32 bits in JSON output — firmware/components/dcs_support/src/dcs_admin_pages.c:371 · Escalate · medium
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
left_ms clamp to 1ms when TTL nearly expired still allows a frame past TTL — components/ml_dev_tether/src/ml_usb_tx.c:110 · Escalate · medium
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 238 functions depend on the 153 functions this change touches.

Health — this change adds coupling hotspots:

new: disco_send_ping_to_peer() — 8 callers, 8 callees
new: dcs_support_init() — 2 callers, 30 callees
new: add_peer() — 2 callers, 22 callees
new: wireguardif_network_rx() — 3 callers, 11 callees
new: disco_send_call_me_maybe() — 6 callers, 4 callees
new: neg_apply_target() — 2 callers, 10 callees
new: disco_periodic_probes() — 1 callers, 16 callees
new: process_disco_pong() — 1 callers, 15 callees
…and 35 more — each is listed as a finding

Verification — 238 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

verification_scope: 236 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify wireguard\_tai64n\_now.

The verifier did not have enough to check wireguard\_tai64n\_now, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: void

Could not verify: Could not verify wg\_init\_interface.

The verifier did not have enough to check wg\_init\_interface, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: esp_err_t

Could not verify: Could not verify ml\_usb\_tx\_get\_diag.

The verifier did not have enough to check ml\_usb\_tx\_get\_diag, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: void

Could not verify: Could not verify usb\_drain.

The verifier did not have enough to check usb\_drain, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported parameter type(s)

Could not verify: Could not verify page\_state.

The verifier did not have enough to check page\_state, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: esp_err_t

· 43 more finding(s) on lines outside this diff (see the check run).

claude

I reviewed this PR (through the latest pushed commit) and didn't find any bugs. Because it changes WireGuard handshake anti-replay timestamp logic and boot-time TCPIP-core locking behavior, a human look is still worthwhile.

What was reviewed: wireguard_tai64n_epoch_init()'s single NVS read-increment-commit running under LOCK_TCPIP_CORE() in wg_init_interface() — blocking but boot-time-only, before live traffic; the "done" latch that leaves the epoch permanently at 0 for the rest of the boot after a transient NVS failure, even though wg_init_interface() can be retried — falls back to the documented uptime-only degraded mode rather than blocking the tunnel; and the USB TX ring slot for a timed-out (ESP_TIMEOUT) frame being freed for producer reuse immediately rather than pinned for the frame's remaining TTL.

Extended reasoning...

Overview

This PR (bug #157) makes two independent fixes plus diagnostics: (1) wireguard-platform-esp32.c/.h, its CMakeLists.txt (adds nvs_flash dep), and ml_wg_mgr.c add a persisted per-boot epoch counter mixed into the WireGuard TAI64N handshake timestamp so timestamps stay monotonic across reboots instead of being uptime-only (which peers could reject as stale/replayed); (2) ml_usb_tx.c/.h raises the TinyUSB hand-off wait from 20ms to 200ms (capped to the frame's remaining TTL) and stops retrying on ESP_TIMEOUT (ambiguous outcome), retrying only on ESP_FAIL (definitely unsent), tracking gone-not-retried timeouts in a new timeout_uncertain counter; (3) dcs_admin_pages.c exposes both new counters (wg_epoch, usb_tx_timeout_uncertain) in the admin status JSON. I read the full diff directly (not just the PR description) and confirmed the code matches the stated behavior, including that the epoch-init call sits inside the existing LOCK_TCPIP_CORE() critical section in wg_init_interface(), and that a timed-out USB frame now falls through to advance s_head (dropped, not retried) while an ESP_FAIL frame short-circuits back for retry.

Security risks

The change is squarely in anti-replay/handshake-timestamp territory for WireGuard, which is security-relevant even though it's not introducing new crypto primitives — it changes what value is fed into the replay-timestamp comparison the peer trusts. A subtly wrong epoch/monotonicity guarantee (e.g., an epoch that could roll backward, or overlap across two devices sharing an NVS namespace) would reintroduce exactly the outage class this PR fixes, or worse, weaken replay protection. The graceful degrade-on-NVS-failure path (falls back to uptime-only, epoch=0) is a deliberate availability/security trade-off that's explicitly documented but worth a human's sign-off. No injection, auth-bypass, or data-exposure risk was found in the diagnostics additions (dcs_admin_pages.c just serializes two more counters).

Level of scrutiny

This warrants above-baseline scrutiny: it touches a security-relevant handshake timestamp path, changes locking timing at boot (a blocking NVS commit now runs while a shared TCPIP-core lock the codebase treats as latency-critical is held), and changes retry/duplicate-suppression semantics for USB frames including WireGuard handshake responses — exactly the mechanism blamed for the original outage. The PR's own description acknowledges the degraded-epoch case "has not been bench-tested against a host with retained state," which is a meaningful residual unknown for a fix targeting a specific reproduced outage.

Other factors

A prior review from this bug-hunting system was already posted on an earlier pushed version of this PR, and a further commit landed afterward; this run re-examined the current HEAD and turned up no confirmed bugs, only the ruled-out items noted in the "what was reviewed" summary above. Given the security/reliability sensitivity of the touched paths and that a new commit was pushed since the last review, a fresh human look is still the safer path even though this automated pass found nothing to block on.

iliabaranov · 2026-09-20T19:47:51Z

1 h combined soak — `main` + this PR (`7d7e47d3`) + #162 (`e042e589`) on all three bench remotes

2026-09-20 11:45:50 → 12:46:48 PDT, steady state, no E-stop cycling. Image v1.3-8-gb09ee3bd, ELF 9f14b8f3e, verified on DUT/7a60 (Ethernet, polling) and eed0 (USB tether). Zero fails, zero machine state changes, every heartbeat answered on all three units.

metric	DUT	7a60	eed0
status	OK	OK	OK
W5500 mode	poll10	poll10	tether
link / path to machine	eth / direct	eth / direct	usb / direct
bond (Δ rebonds)	BONDED (Δ0)	BONDED (Δ0)	BONDED (Δ0)
uptime / resets in window	1h01 / 0	1h02 / 0	1h02 / 0
traffic Δ sent / replies / send-fail	18252 / 18252 / 0	18192 / 18192 / 0	18199 / 18199 / 0
reply age 1 Hz: p99 / max	268 / 466 ms	315 / 434 ms	252 / 379 ms
INT gauge max / eth recoveries	280 ms / 0	320 ms / 0	— (tether)
lockstep mismatch Δ timeout/content (total)	0/0 (0/2)	0/0 (0/0)	0/0 (0/0)
NVS flush max in window	142 ms	119 ms	131 ms
internal heap free / LFB / boot-min	58 / 29 / 13 KB (window min 24 KB)	59 / 28 / 13 KB (window min 23 KB)	11 / 5 / 3 KB (window min 6 KB)
machine-side pcap ≥600 ms (window)	0 gaps, 0 unans, lat≤4 ms	0 gaps, 0 unans, lat≤11 ms	n/a
unreachable samples / stale crash record	0 / 1	0 / 1	0 / 1

This PR's claims, on the wire (tether capture, device ↔ machine host):

duplicated frames: 0 / 20 730 device→host, 0 / 20 697 host→device (before: 2.8 %, incl. 6 duplicated handshake responses/h)
rekeys: 33 initiated by the device, 33 answered by the host, 0 host-initiated (before this PR the device had never had an initiation accepted; the host was always the initiator)
usb_tx_timeout_uncertain: 2 at boot, then 0 for the hour; busy_retries 0; heartbeat loss 0 (18 199 / 18 199)
wg_epoch = 4 on eed0 (its fourth boot on this counter), 1 on the two freshly flashed Ethernet units

Versus the 8 h baseline (run 4, pre-fix image): eed0 rebonds 1 → 0, send failures 72 → 0, reply-age max 2193 → 379 ms; all other metrics unchanged within noise (control-plane reconnects 1.0/h vs 2.7–3.6/h).

Preceded by a 20 min screen of this head alone on eed0 (0 duplicates / 6393 frames, 0 loss). The current head 60bed691 adds only the two review guards (once-per-boot init, read-error handling), off the exercised paths. Final qualification of the merged result remains a separate step.

iliabaranov · 2026-09-20T19:49:51Z

Graphify round on 60bed691 → 8b786d42:

"diagnostics struct inserts a field before existing members" — moved timeout_uncertain to the end of ml_usb_tx_diag_t (internal struct anyway, but now offset-stable).
"wireguard_tai64n_epoch() truncated to 32 bits in JSON" — it is a uint32_t (the epoch counter); the (unsigned long) cast is a widening for %lu. No truncation.
"left_ms clamp to 1 ms allows a frame past TTL" — the TTL is checked before the offer (>= TX_TTL_US → expired, not offered), so at the clamp left_ms > 0 except for the few µs between the two reads; the clamp then permits at most ~1 ms of TTL granularity. Intentional: a 0-tick wait re-creates the fix(usb): give the NCM offer a real wait — zero-tick send_sync never transmits #149 zero-wait bug (packet reclaimed before the TinyUSB task can run it).
"timed-out send releases a ring slot while TinyUSB may still use its buffer" — as answered above: the vendor call waits for its buffer semaphore before returning on timeout and NULLs its packet pointer; NCM copies synchronously; nothing references the slot after return.

Both roles build; CI green on the previous head, re-running on this one.

graphify-labs

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 4 advisory finding(s) below merit a look before merge.

Formal verification. No changes could be formally verified in this run.

Graphify review — findings

Adds reboot-monotonic WireGuard handshake timestamps: wireguard_tai64n_epoch_init bumps a per-boot epoch in NVS once at startup and folds it into the high bits of every TAI64N stamp so post-reboot initiations aren't rejected as replays (#157), falling back to uptime-only stamps (with a logged error) rather than dropping the tunnel if NVS can't persist. Stops the USB TX drain from doubling frames on ambiguous hand-offs — only ESP_FAIL (definitely unsent) is retried while a TIMEOUT is given up and counted in the new timeout_uncertain diag, the sync wait grows from 20 ms to 200 ms (capped to remaining TTL) to make timeouts rare. Surfaces wg_epoch and usb_tx_timeout_uncertain in the admin state JSON.

Worth a look

Timed-out USB send releases ring slot while deferred TinyUSB callback may still reference it — components/ml_dev_tether/src/ml_usb_tx.c · Escalate · high
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
wg_epoch is truncated on 32-bit targets — firmware/components/dcs_support/src/dcs_admin_pages.c:368 · Escalate · medium
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
TIMEOUT frames no longer retried may drop legitimately-unsent frames — components/ml_dev_tether/src/ml_usb_tx.c:118 · Escalate · medium
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Dynamic USB send wait can become a zero-tick timeout — components/ml_dev_tether/src/ml_usb_tx.c:109 · Escalate · medium
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 238 functions depend on the 153 functions this change touches.

Health — this change adds coupling hotspots:

new: disco_send_ping_to_peer() — 8 callers, 8 callees
new: dcs_support_init() — 2 callers, 30 callees
new: add_peer() — 2 callers, 22 callees
new: wireguardif_network_rx() — 3 callers, 11 callees
new: disco_send_call_me_maybe() — 6 callers, 4 callees
new: neg_apply_target() — 2 callers, 10 callees
new: disco_periodic_probes() — 1 callers, 16 callees
new: process_disco_pong() — 1 callers, 15 callees
…and 35 more — each is listed as a finding

Verification — 238 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

verification_scope: 236 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify wireguard\_tai64n\_now.

The verifier did not have enough to check wireguard\_tai64n\_now, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: void

Could not verify: Could not verify wg\_init\_interface.

The verifier did not have enough to check wg\_init\_interface, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: esp_err_t

Could not verify: Could not verify ml\_usb\_tx\_get\_diag.

The verifier did not have enough to check ml\_usb\_tx\_get\_diag, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: void

Could not verify: Could not verify usb\_drain.

The verifier did not have enough to check usb\_drain, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported parameter type(s)

Could not verify: Could not verify page\_state.

The verifier did not have enough to check page\_state, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: esp_err_t

· 43 more finding(s) on lines outside this diff (see the check run).

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

…ose log shows uptime and epoch apart Review round 2 (#160): the one-time NVS commit sat inside LOCK_TCPIP_CORE() in wg_init_interface(), so its flash write would have stalled every lwIP caller for the duration; it now runs before the lock is taken. The wg_verbose_logging TAI64N line printed the epoch-inflated seconds; it now prints uptime and the epoch separately.

iliabaranov · 2026-09-20T22:36:54Z

Graphify round on 8b786d42: the four advisories are the same items as the previous rounds (slot release after timeout — not reachable, see above; wg_epoch is a uint32_t, no truncation; TIMEOUT frames not retried — the stated trade-off, measured 0 loss; "dynamic wait can become zero" — the clamp floors it at 1 tick precisely so it cannot, that being the #149 bug). No further change.

graphify-labs

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 4 advisory finding(s) below merit a look before merge.

Formal verification. No changes could be formally verified in this run.

Graphify review — findings

Fixes post-reboot WireGuard handshake rejections (#157) by persisting a per-boot epoch in NVS and folding it into the high bits of wireguard_tai64n_now, so handshake timestamps stay monotonic across reboots; wireguard_tai64n_epoch_init does the single read-increment-commit before the TCPIP core lock is taken, and on NVS failure falls back to uptime-only timestamps (device-initiated handshakes may be rejected as replays) rather than dropping the tunnel. Reworks the USB TX drain to retry only definitively-unsent frames (ESP_FAIL) and give up on ambiguous timeouts — counted in the new timeout_uncertain diag — since a doubled handshake response makes the host derive a second session and drop ours; also raises TX_SYNC_WAIT_MS from 20ms to 200ms (capped to remaining TTL) to make those timeouts rare. Surfaces the boot epoch on the admin health page via wireguard_tai64n_epoch.

Worth a look

Timeout path frees TX slot while TinyUSB send may still reference it — components/ml_dev_tether/src/ml_usb_tx.c:113 · Escalate · high
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Public diagnostic struct size changed — components/ml_dev_tether/include/ml_usb_tx.h:37 · Escalate · medium
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Epoch init once-guard is unsynchronized and published before initialization completes — components/microlink/components/wireguard_lwip/src/wireguard-platform-esp32.c:34 · Escalate · medium
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
wg_epoch is truncated on ESP32 when formatting the TAI64N epoch — firmware/components/dcs_support/src/dcs_admin_pages.c:368 · Escalate · medium
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 238 functions depend on the 153 functions this change touches.

Health — this change adds coupling hotspots:

new: disco_send_ping_to_peer() — 8 callers, 8 callees
new: dcs_support_init() — 2 callers, 30 callees
new: add_peer() — 2 callers, 22 callees
new: wireguardif_network_rx() — 3 callers, 11 callees
new: disco_send_call_me_maybe() — 6 callers, 4 callees
new: neg_apply_target() — 2 callers, 10 callees
new: disco_periodic_probes() — 1 callers, 16 callees
new: process_disco_pong() — 1 callers, 15 callees
…and 35 more — each is listed as a finding

Verification — 238 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

verification_scope: 236 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify wireguard\_tai64n\_now.

The verifier did not have enough to check wireguard\_tai64n\_now, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: void

Could not verify: Could not verify wg\_init\_interface.

The verifier did not have enough to check wg\_init\_interface, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: esp_err_t

Could not verify: Could not verify ml\_usb\_tx\_get\_diag.

The verifier did not have enough to check ml\_usb\_tx\_get\_diag, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: void

Could not verify: Could not verify usb\_drain.

The verifier did not have enough to check usb\_drain, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported parameter type(s)

Could not verify: Could not verify page\_state.

The verifier did not have enough to check page\_state, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: esp_err_t

· 43 more finding(s) on lines outside this diff (see the check run).

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding, I also checked whether usb_drain()'s new r == ESP_FAIL retry gate could silently drop a frame TinyUSB actually never sent, for some non-OK result other than the documented ESP_ERR_TIMEOUT/ESP_FAIL pair — tinyusb_net_send_sync()'s call chain (per the surrounding comment) only ever returns those two plus ESP_OK, so the drop path stays bounded to the genuinely ambiguous timeout case and doesn't widen to cover definitely-unsent frames.

Extended reasoning...

This run's remaining candidate beyond the confirmed inline nit was whether the ESP_FAIL-only retry gate in ml_usb_tx.c could now silently drop frames that TinyUSB definitely never transmitted (not just the intentionally-abandoned ambiguous-timeout case). Reading the code and its accompanying comment, tinyusb_net_send_sync() only returns ESP_OK, ESP_FAIL (endpoint definitely refused), or ESP_ERR_TIMEOUT (ambiguous) in this call path, so the else branch that now gives up instead of retrying is correctly scoped to the ambiguous case only — ruled out as a real gap. I did not re-raise the two lock-ordering/logging nits from my prior review since commit adde5e4 addresses both directly (epoch NVS commit now runs before LOCK_TCPIP_CORE(), and the verbose TAI64N log now prints uptime and epoch separately).

…162) Closes the ~1 s Ethernet receive-stall class discussed in #128. ## Problem The ESP-IDF W5500 driver configures the chip's INT pin as a **falling-edge** interrupt although INT is a **level** signal. If a frame completes in the sub-µs window between the driver clearing `Sn_IR` and INT re-asserting, no new edge is produced and the frame waits for the driver task's 1000 ms `ulTaskNotifyTake` re-check (the vendor's own workaround, espressif/esp-idf#6233; polling mode was added later in esp-idf#12692). On a 5 Hz heartbeat with a 1.6 s stop-on-silence budget that eats up to 1 s of margin, receive-side only. ## Change (93 lines, 5 files) - **`CONFIG_DCS_ETH_W5500_POLL_MS`** (new, default **10**, range 0–100; 0 = interrupt mode as before). Non-zero selects the SDK's polling mode (`eth_w5500_config_t.poll_period_ms`, `int_gpio_num = -1`) — public API, no vendor-driver patch. The recovery ladder's rungs 2/3 re-install through the same `dcs_eth_start()`, so a reinstall never falls back to interrupt mode. - **INT-line gauge**: the Ethernet watchdog task now ticks every 20 ms (health check keeps its 1 Hz cadence) and samples the INT pin: `eth_int_low_ticks` and `eth_int_low_max_ms` (longest *sampled* continuous low span) in `/state.json` and the admin page. Telemetry only. Note: in polling mode INT legitimately stays low ≤10 ms per frame, so the gauge is **not comparable across modes** — it is the instrument for interrupt-mode diagnosis; the machine-side RX-freeze metric is the cross-mode one. Cost of polling: one 4-byte SPI read (~1.6 µs bus, ~50 µs incl. task wake) per 10 ms when idle ≈ 0.5 % of one core; added RX latency uniform 0–10 ms. 10 ms is also ESP-IDF's own example default for this mode. ## Evidence **8 h crossover (2026-09-19/20, two Ethernet remotes on one machine):** | | 7a60 — interrupt mode | DUT — polling 10 ms | |---|---|---| | INT-line stalls of 960–1000 ms | **8** (19:19, 20:15, 20:31, 20:46, 21:00, 22:59, 00:16, 01:19 — ≈1/h) | 0 | | machine-side RX freezes ≥600 ms (retained 4.8 h of capture) | **3**, each 1006–1009 ms and coincident to the second with a gauge stall | 0 | | Ethernet recoveries / resets | 0 / 0 | 0 / 0 | | reply age p99 / max | 310 / 555 ms | 263 / 432 ms | Poisson at ≈1/h: P(0 in 8.25 h) ≈ 3·10⁻⁴. **Then 1 h with both units polling (run 5):** 0 stalls, 0 RX freezes, reply-age p99 249/336 ms, no other metric moved. A further 1 h soak of this head together with #160 follows in this PR. Relevance: none of the 8 stalls broke a bond — this restores ~1 s of the 1.6 s budget rather than fixing a disconnect cause; it also matches the ~790 ms local SYN→SYN-ACK delays in #128's captures. Both roles build; pre-commit clean. Companion finding #159 (small-frame RX saturation) is unaffected by the mode.

graphify-labs

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Formal verification. No changes could be formally verified in this run.

Graphify review — findings

No blocking issues surfaced.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 238 functions depend on the 153 functions this change touches.

Health — this change adds coupling hotspots:

new: disco_send_ping_to_peer() — 8 callers, 8 callees
new: dcs_support_init() — 2 callers, 30 callees
new: add_peer() — 2 callers, 22 callees
new: wireguardif_network_rx() — 3 callers, 11 callees
new: disco_send_call_me_maybe() — 6 callers, 4 callees
new: neg_apply_target() — 2 callers, 10 callees
new: disco_periodic_probes() — 1 callers, 16 callees
new: process_disco_pong() — 1 callers, 15 callees
…and 35 more — each is listed as a finding

Verification — 238 functions in the blast radius were not formally verified this run (proofs are advisory here).

Health delta baseline: last indexed commit c030740, 1 commit(s) behind this PR's base.

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

verification_scope: 236 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify wireguard\_tai64n\_now.

The verifier did not have enough to check wireguard\_tai64n\_now, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: void

Could not verify: Could not verify wg\_init\_interface.

The verifier did not have enough to check wg\_init\_interface, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: esp_err_t

Could not verify: Could not verify ml\_usb\_tx\_get\_diag.

The verifier did not have enough to check ml\_usb\_tx\_get\_diag, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: void

Could not verify: Could not verify usb\_drain.

The verifier did not have enough to check usb\_drain, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported parameter type(s)

Could not verify: Could not verify page\_state.

The verifier did not have enough to check page\_state, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: esp_err_t

· 43 more finding(s) on lines outside this diff (see the check run).

claude

Code review completed

Nothing new to post: everything this review found is already covered by existing comments on this pull request or didn't merit a separate one.

Still open from earlier reviews (1):

Unresolved: 1 minor or pre-existing.

rajasimman-madhivanan

Sol's finding is harmless if the callback publishes idempotent messages like heartbeat. If not that needs to be fixed since there is a chance that old callback might still get invoked.

rajasimman-madhivanan · 2026-09-21T01:41:13Z

+      int64_t left_ms = (TX_TTL_US - (esp_timer_get_time() - s->queued_us)) / 1000;
+      uint32_t wait_ms =
+        (left_ms < (int64_t)TX_SYNC_WAIT_MS) ? (left_ms > 0 ? (uint32_t)left_ms : 1u) : TX_SYNC_WAIT_MS;
+      esp_err_t r = tinyusb_net_send_sync(s->bytes, s->len, NULL, pdMS_TO_TICKS(wait_ms));


From Sol - tinyusb_net_send_sync() may return timeout while its deferred do_send_sync callback remains queued. This branch then retires the current slot and immediately publishes the next frame; because Espressif’s callback reads the shared packet_to_send pointer rather than a request-specific pointer, the stale callback can transmit the next frame before that frame’s own callback runs, duplicating it. If the next frame is a WireGuard handshake response, the same session-split outage can recur. The 200 ms wait reduces probability but does not eliminate the race. Can we drain/cancel the timed-out callback before advancing, or patch the wrapper to bind each deferred callback to a specific request/generation?

Agreed — a handshake response is exactly the non-idempotent case, so the 200 ms wait alone was a probability reduction, not a fix. 5e89d706 closes it from our side without touching the component: deferred callbacks and USB events share TinyUSB's single FIFO (usbd_defer_func() → _usbd_q), and tud_task_event_ready() reports whether that queue is non-empty. After a timed-out offer the drain now waits (bounded by the 200 ms offer wait) until the queue has been observed empty — at which point every earlier deferred callback, including the stale one, has already executed. It executes harmlessly because the vendor's packet pointer is NULL and its buffer semaphore is still held (the vendor only gives it at the start of the next offer), so the callback's xSemaphoreTake(…, 0) fails and it returns. The next frame is published only after that, so it can no longer be picked up by a stale callback. Past the bound (TinyUSB task starved > 200 ms) we proceed regardless — the ring's frames are expiring at 400 ms anyway. Request-scoped callback ownership in the component itself remains the proper upstream fix (#161).

…e publishing the next frame Review (#160, reviewer + #161): a timed-out offer's deferred callback can still sit in the TinyUSB event queue; it reads the vendor's SHARED packet pointer, so publishing the next frame before it has run can transmit that frame twice — a doubled handshake response is the #157 outage. Deferred callbacks and USB events share one FIFO (usbd_defer_func -> _usbd_q), so once tud_task_event_ready() reports the queue empty, every earlier callback has executed — harmlessly, because the vendor pointer is NULL and its semaphore is held until the next offer. The drain now waits for that (bounded by the 200 ms offer wait) before advancing. This closes the stale- callback duplication path from our side without touching the component. Also: wg_epoch distinguishes "not initialised yet" (0) from "init failed" (UINT32_MAX) from a persisted epoch.

iliabaranov · 2026-09-21T02:28:26Z

@rajasimman-madhivanan — the condition in your approval is addressed in 5e89d706 (stale deferred callback drained via tud_task_event_ready() before the next frame is published; details in the inline reply). Both roles build; CI running.

The 8 h soak now in progress (v1.3-12, started 15:40 PDT, 3 h clean so far) runs the previous head adde5e47; the new guard only changes the timed-out-offer path, which has fired twice (at boot) in the entire run. I'll post the 8 h table here when it completes at 23:55 PDT.

graphify-labs

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Formal verification. No changes could be formally verified in this run.

Graphify review — findings

No blocking issues surfaced.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 238 functions depend on the 153 functions this change touches.

Health — this change adds coupling hotspots:

new: disco_send_ping_to_peer() — 8 callers, 8 callees
new: dcs_support_init() — 2 callers, 30 callees
new: add_peer() — 2 callers, 22 callees
new: wireguardif_network_rx() — 3 callers, 11 callees
new: disco_send_call_me_maybe() — 6 callers, 4 callees
new: neg_apply_target() — 2 callers, 10 callees
new: disco_periodic_probes() — 1 callers, 16 callees
new: process_disco_pong() — 1 callers, 15 callees
…and 35 more — each is listed as a finding

Verification — 238 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

verification_scope: 236 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify wireguard\_tai64n\_now.

The verifier did not have enough to check wireguard\_tai64n\_now, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: void

Could not verify: Could not verify wg\_init\_interface.

The verifier did not have enough to check wg\_init\_interface, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: esp_err_t

Could not verify: Could not verify ml\_usb\_tx\_get\_diag.

The verifier did not have enough to check ml\_usb\_tx\_get\_diag, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: void

Could not verify: Could not verify usb\_drain.

The verifier did not have enough to check usb\_drain, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported parameter type(s)

Could not verify: Could not verify page\_state.

The verifier did not have enough to check page\_state, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: unsupported return type: esp_err_t

· 43 more finding(s) on lines outside this diff (see the check run).

claude

Code review found no issues

No high-confidence issues detected in this change.

iliabaranov added 2 commits September 20, 2026 11:22

graphify-labs Bot reviewed Sep 20, 2026

View reviewed changes

iliabaranov mentioned this pull request Sep 20, 2026

fix(eth): W5500 RX via SDK polling mode; INT-line stall gauge (#128) #162

Merged

claude Bot reviewed Sep 20, 2026

View reviewed changes

graphify-labs Bot reviewed Sep 20, 2026

View reviewed changes

claude Bot reviewed Sep 20, 2026

View reviewed changes

fix(usb): append the new diag field after the existing members

8b786d4

graphify-labs Bot reviewed Sep 20, 2026

View reviewed changes

claude Bot reviewed Sep 20, 2026

View reviewed changes

Comment thread components/microlink/src/ml_wg_mgr.c Outdated

Comment thread components/microlink/components/wireguard_lwip/src/wireguard-platform-esp32.c Outdated

graphify-labs Bot reviewed Sep 20, 2026

View reviewed changes

claude Bot reviewed Sep 20, 2026

View reviewed changes

Comment thread firmware/components/dcs_support/src/dcs_admin_pages.c

Merge branch 'main' into fix/usb-tx-no-timeout-retry

11e540e

graphify-labs Bot reviewed Sep 21, 2026

View reviewed changes

claude Bot reviewed Sep 21, 2026

View reviewed changes

rajasimman-madhivanan approved these changes Sep 21, 2026

View reviewed changes

graphify-labs Bot reviewed Sep 21, 2026

View reviewed changes

iliabaranov merged commit 595d172 into main Sep 21, 2026
11 checks passed

iliabaranov deleted the fix/usb-tx-no-timeout-retry branch September 21, 2026 02:32

claude Bot reviewed Sep 21, 2026

View reviewed changes

Conversation

iliabaranov commented Sep 20, 2026

Root cause (from tether-side captures)

Commit 1 — fix(usb) (3 files, 24+/10−)

Commit 2 — fix(wg) (4 files, 42+/2−)

Bench screening (tethered remote, boot events included)

Uh oh!

graphify-labs Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Graphify review — findings

Impact & health

Graphify review

Gate & verification

graphify gate

Formal verification

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

iliabaranov commented Sep 20, 2026

Uh oh!

graphify-labs Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Graphify review — findings

Impact & health

Graphify review

Gate & verification

graphify gate

Formal verification

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

iliabaranov commented Sep 20, 2026

1 h combined soak — main + this PR (7d7e47d3) + #162 (e042e589) on all three bench remotes

Uh oh!

iliabaranov commented Sep 20, 2026

Uh oh!

graphify-labs Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Graphify review — findings

Impact & health

Graphify review

Gate & verification

graphify gate

Formal verification

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

iliabaranov commented Sep 20, 2026

Uh oh!

graphify-labs Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Graphify review — findings

Impact & health

Graphify review

Gate & verification

graphify gate

Formal verification

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

graphify-labs Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Graphify review — findings

Impact & health

Commit 1 — `fix(usb)` (3 files, 24+/10−)

Commit 2 — `fix(wg)` (4 files, 42+/2−)

graphify-labs Bot left a comment •

edited

Loading

graphify-labs Bot left a comment •

edited

Loading

1 h combined soak — `main` + this PR (`7d7e47d3`) + #162 (`e042e589`) on all three bench remotes

graphify-labs Bot left a comment •

edited

Loading

graphify-labs Bot left a comment •

edited

Loading

graphify-labs Bot left a comment •

edited

Loading

graphify-labs Bot left a comment •

edited

Loading