Recover WireGuard on selected network handovers

[kasnder]

Wi-Fi/mobile handovers could leave WireGuard using an unusable path while TrackerControl remained the default VPN. Track physical-network callback snapshots and default-network transports, then recreate WireGuard during the existing 1.5-second debounced reload when the selected path changes, even if the configuration and TUN descriptor are reused.

Standby-network validation, suspension, metered, DNS, and address changes do not restart the active tunnel. On the selected path, identity/transport and address/route changes request a restart; metered and DNS changes only reload policy. Ignore TrackerControl's own replacement VPN identity and link properties. Physical callbacks remain enabled on API 23, with snapshots outside callbacks where older Android versions need them.

Carry the handover indication in the reload intent through to startOrUpdate, so disabled, failed, or dropped dispatches cannot leave a global restart flag suppressing the watchdog. Remove obsolete network-policy helpers and keep the connectivity setting consistent across supported Android versions.

This deliberately trades a brief reconnection during a handover for a fresh tunnel, using the existing reload and watchdog mechanisms. It adds no probes, native interfaces, or independent recovery timer. VPN transport information does not always identify the exact underlying network; retain a matching known selection and otherwise require an unambiguous candidate.

Validation: GitHub debug JVM tests, :app:lintGithubDebug, assembleGithubDebug, and git diff --check. Tests cover standby-network chatter, VPN-default handovers, API 23/24/28 snapshots, private-DNS active/name changes, debounce accumulation, and dropped reload commands. The reported Wi-Fi-to-4G incident has not been reproduced on a real device; real-device handover validation remains outstanding.

[kasnder]

Reviewed the diff against the surrounding code. The diagnosis is convincing — filtering on VALIDATED and calling isActiveNetwork() (a synchronous ConnectivityManager lookup) from inside a callback really does lose Wi-Fi↔mobile changes while our own VPN is the default network, and callback-owned snapshots are the right shape for the fix. PhysicalNetworkState is clean and readable, and the VPN-identity-churn handling is a nice touch.

Two things I think need to change before merge, plus some smaller points. Details inline.

Blocking-ish

1. Every physical network, not just the default one, now restarts WireGuard. The isActiveNetwork filter is gone and nothing replaced it, so background cellular chatter while on Wi-Fi (suspension during a voice call, validation flaps, carrier metered changes, RA/DNS refreshes) each cost a native VPN restart and a fresh handshake. This is the tradeoff the PR description mentions, but it lands on far more events than a handover. See the comment on onPhysicalCapabilitiesChanged.

2. forceRestartPending is set outside the path that consumes it. reload() is a silent no-op when enabled is false and swallows a failed foreground-service start; if that happens the flag sticks, and onMonitorBroken early-returns on it — so the watchdog's cheap recovery goes dark until the next startOrUpdate. See the comment on onUnderlyingNetworkChanged.

Smaller

3. NetworkReloadPolicy is now largely dead: onNetworkAvailable, onNetworkLost, onCapabilitiesChanged, onLinkPropertiesChanged and same() have no remaining callers in app/src/main (only NetworkReloadPolicyTest), and nothing can produce REASON_CONNECTED_CHANGED or REASON_METERED_CHANGED any more. Worth deleting the dead members and their tests in this PR rather than leaving a policy class whose tests assert behaviour the app no longer has.

4. The reload_onconnectivity preference is no longer read anywhere. ActivitySettings force-checks and disables it only on API ≥ O, so an API 24–25 user who turned it off now silently gets reload-on-every-change. Either extend that setEnabled(false) to API ≥ N, or drop the preference.

5. API 23 is downgraded from NetworkCallback to the CONNECTIVITY_ACTION broadcast — see the comment on the new SDK_INT < N guard. Only registerDefaultNetworkCallback needs 24.

6. entries can leak Network keys inserted via the default-network callback — inline comment on onDefaultNetworkLinkPropertiesChanged.

7. requestPrivateDnsWarningUpdate() is now called from both callbacks for the same event on the default network. Harmless if it's idempotent, but it's redundant work on a hot path.

Tests

• setPrivateDns(properties, name, active) ignores its active parameter and only sets mPrivateDnsServerName, so Entry.privateDnsActive — half the private-DNS change detection — is never exercised.
• Both reflection helpers (LinkProperties.mPrivateDnsServerName, NetworkCapabilities.mSignalStrength) reach into AOSP-internal fields and will break silently on a Robolectric bump. A comment saying why there's no public setter would help the next person.
• The two halves are tested separately but not the wiring between them: nothing covers "physical change → shouldRestartWireGuard → pendingWireGuardRestart → onUnderlyingNetworkChanged before reload", which is the actual behaviour change. The debounce accumulation (a REASON_PRIVATE_DNS_CHANGED arriving after a REASON_NETWORK_CHANGED in the same burst must not cancel the restart) is covered in NetworkReloadPolicyTest at the policy level but not through reloadAfterNetworkChange.

Agreed that real-device handover validation is still needed, particularly to confirm that unconditional recreation actually fixes the reported Wi-Fi→4G incident rather than just changing its timing.

---

Generated by Claude Code

[kasnder]

This is the part I'd push back on hardest: there is no longer any notion of which network changed.

The old callbacks filtered on isActiveNetwork(network), so only the network actually carrying traffic could trigger a reload. Now any network matching the request (INTERNET + NOT_VPN) produces REASON_NETWORK_CHANGED, and NetworkReloadPolicy.shouldRestartWireGuard maps that onto a full tunnel teardown and re-handshake via the new forceRestartPending path.

On a typical device the non-default network is not quiet. With Wi-Fi as the default, the cellular network stays registered and emits capability changes (NOT_SUSPENDED toggling for the duration of a voice call, validation flapping, carrier metered changes) and link-property changes (IPv6 RA refresh, DNS server updates, route churn). Each of those now costs a native VPN restart plus a WireGuard re-handshake, where before it cost nothing. onPhysicalLost has the same shape: Wi-Fi dropping while you are already settled on cellular restarts the tunnel for no reason.

The 1.5 s debounce doesn't help here — it coalesces a burst, not events minutes apart.

That collides with the standing "battery is a first-class constraint" rule, and with the comment this PR removes from WgEgress ("so we don't redo the handshake on every DHCP/connectivity blip"). Suggestion: keep the broad snapshotting (it is genuinely useful for DNS and rule reloads), but gate shouldRestartWireGuard on changes to the egress path only — a default-network switch, a transport change on the default network, or a link-address/route change on the default network. Non-default networks would then update their snapshot and return null.

Related question on the tradeoff: commit 1 was "Verify and recover WireGuard data paths after network handovers" and commit 2 narrowed it to unconditional recreation. What made the verify-then-restart approach unworkable? That reads like the narrower fix for the reported incident (rebind succeeded, path was dead) without paying a re-handshake on every blip.

---

Generated by Claude Code

[kasnder]

The comment says "route-relevant values", but three of these five aren't:

• NET_CAPABILITY_VALIDATED flips on captive-portal revalidation and on transient cellular validation loss. The old code deliberately requested VALIDATED rather than comparing it.
• NET_CAPABILITY_NOT_SUSPENDED flips for the duration of a voice call on 2G/3G.
• NOT_METERED / TEMPORARILY_NOT_METERED change when the carrier grants a zero-rating window — the routes are identical either side.

None of these changes the path a WireGuard packet takes, but each one now recreates the tunnel. Metered state does need to reach reload() (it used to, via REASON_METERED_CHANGED), so the fix is probably to split the snapshot: a route-relevant part that may restart WireGuard (transports, addresses, routes) and a policy part that only triggers a reload.

---

Generated by Claude Code

[kasnder]

Setting forceRestartPending here makes the flag depend on a reload that the caller cannot guarantee will happen.

reloadAfterNetworkChange calls this and then ServiceSinkhole.reload(...), which is a silent no-op when the enabled preference is false, and which swallows the failure if startForegroundService throws (ForegroundServiceStartNotAllowedException → startService → also throws in the background on 8+ → logged and dropped). Network callbacks fire while the app is backgrounded, so that is not purely theoretical.

If the reload is dropped, forceRestartPending stays set with nothing to consume it, and onMonitorBroken returns early on exactly that flag:

// A full restart is already queued; it will rebind everything anyway.
if (forceRestartPending) return

So the watchdog's cheap recovery is disabled indefinitely until some other startOrUpdate runs — the opposite of what this PR is trying to achieve. Every other writer of this flag (requestFullRestart) pairs it with a queued reload and a pendingRestartRunnable that can re-fire; this path has neither.

Two options: set the flag from inside the reload command handler (i.e. on the path that definitely reaches startOrUpdate), or have reloadAfterNetworkChange check the return of the reload attempt and clear the flag when the reload could not be dispatched.

---

Generated by Claude Code

[kasnder]

minSdk is 23, so this branch is live, and it is a regression for API 23 devices. Before this change they got the NetworkCallback path (registerNetworkCallback is API 21); now they fall back to the deprecated CONNECTIVITY_ACTION broadcast, which is a much coarser signal and whose REASON_CONNECTIVITY_CHANGED also restarts WireGuard on every broadcast.

Only registerDefaultNetworkCallback needs API 24. Registering nc unconditionally and skipping only dnc below 24 would keep API 23 on the better path — the default-network tracking degrades gracefully (defaultSeen stays false, so acceptDefaultIfPhysical never reports a switch).

Side note on the fallback: if this branch runs, listenConnectivityChanges() registers the receiver, and the caller's catch in onCreate would register it a second time should anything after it ever throw. Harmless today because it's the last statement, but worth making listenConnectivityChanges idempotent on registeredConnectivityChanged.

---

Generated by Claude Code

[kasnder]

Minor, but entries can grow without bound. onDefaultNetworkCapabilitiesChanged routes into onPhysicalCapabilitiesChanged, which calls entry(network) and creates a map entry. The default-network callback is unfiltered, so it can insert a Network that never matched the physical request (a default network lacking NET_CAPABILITY_INTERNET, for instance). onPhysicalLost will never fire for such a network, and onDefaultNetworkLost deliberately removes nothing, so the key leaks until reset().

Since Network keys are per-connection, a device that cycles through many of these over a long service lifetime accumulates them. Either have onDefaultNetworkLost do entries.remove(network) for keys the physical callback never registered, or make onPhysicalCapabilitiesChanged use a lookup that doesn't create (the physical callback always delivers onAvailable first, so the create-on-demand isn't needed there either).

---

Generated by Claude Code