Skip to content

Notify Go engine on network change for posture check re-evaluation #154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MichaelUray wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Notify Go engine on network change for posture check re-evaluation #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a5a9e21
fix: notify Go engine on network change for posture check re-evaluation
MichaelUray Apr 6, 2026
b324342
fix: skip initial onAvailable callback to prevent login cancellation
MichaelUray Apr 9, 2026
489bf16
fix: update test assertions to match new any-network-change notificat…
MichaelUray Apr 16, 2026
7d64176
fix: use Log.e for exception logging in notifyNetworkChanged
MichaelUray Apr 16, 2026
c07433c
docs: document initialCallbackReceived multi-network limitation
MichaelUray Apr 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions tool/src/main/java/io/netbird/client/tool/EngineRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,19 @@ public synchronized void stop() {
goClient.stop();
}

/**
* Notify the Go client that the underlying network changed (WiFi/cellular).
* This triggers an immediate re-sync of NetworkAddresses with the management
* server for posture check re-evaluation.
*/
public void notifyNetworkChanged() {
try {
goClient.onUnderlyingNetworkChanged();
} catch (Exception e) {
Log.e(LOGTAG, "notifyNetworkChanged: " + e.getMessage());
}
}

public PeerInfoArray peersInfo() {
return goClient.peersList();
}
Expand Down
6 changes: 6 additions & 0 deletions tool/src/main/java/io/netbird/client/tool/VPNService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ public void onCreate() {
networkChangeDetector = new NetworkChangeDetector(
(ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE));
networkChangeDetector.subscribe(networkAvailabilityListener);
// Notify Go layer on any network change for posture check re-evaluation
networkChangeDetector.setNetworkChangedCallback(() -> {
if (engineRunner != null) {
engineRunner.notifyNetworkChanged();
}
});
networkChangeDetector.registerNetworkCallback();

// Register broadcast receiver for stopping engine (e.g., during profile switch)
Expand Down
20 changes: 9 additions & 11 deletions tool/src/main/java/io/netbird/client/tool/networks/ConcreteNetworkAvailabilityListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,24 @@ public ConcreteNetworkAvailabilityListener() {

@Override
public void onNetworkAvailable(@Constants.NetworkType int networkType) {
boolean isWifiAvailable = Boolean.TRUE.equals(availableNetworkTypes.get(Constants.NetworkType.WIFI));
Boolean wasAvailable = availableNetworkTypes.put(networkType, true);

availableNetworkTypes.put(networkType, true);

// if wifi is available and wasn't before, notifies listener.
// Android prioritizes wifi over mobile data network by default.
if (!isWifiAvailable && networkType == Constants.NetworkType.WIFI) {
// Always notify on any network change so the engine re-syncs
// NetworkAddresses with the management server. This ensures
// posture checks see the current network (e.g. WiFi subnet)
// immediately after a network switch.
if (!Boolean.TRUE.equals(wasAvailable)) {
notifyListener();
}
}
Comment on lines 14 to 25
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the MockNetworkChangeDetector in tests implements the same skip logic
ast-grep --pattern $'class MockNetworkChangeDetector {
  $$$
}'

# Also check the test setup
rg -n "MockNetworkChangeDetector" --type java -A 10

Repository: netbirdio/android-client

Length of output: 7390

🏁 Script executed:

# Get the full ConcreteNetworkAvailabilityListener implementation including onNetworkLost
find . -name "ConcreteNetworkAvailabilityListener.java" -type f | head -1 | xargs cat -n

# Also get the full test file to see all test cases
find . -name "ConcreteNetworkAvailabilityListenerUnitTest.java" -type f | head -1 | xargs wc -l

Repository: netbirdio/android-client

Length of output: 2150

🏁 Script executed:

# Get the full test file to understand all test cases
cat -n tool/src/test/java/io/netbird/client/tool/ConcreteNetworkAvailabilityListenerUnitTest.java

Repository: netbirdio/android-client

Length of output: 5200

🏁 Script executed:

# Let's verify if these tests are actually passing or failing
# First, check if there's a test runner/gradle setup
find . -name "build.gradle" -o -name "pom.xml" | head -5

# Also search for any test configurations
find . -name "*.gradle" -o -name "*.xml" | grep -E "(test|gradle)" | head -10

Repository: netbirdio/android-client

Length of output: 115

🏁 Script executed:

# Let me trace through the logic more carefully
# Create a simple analysis script to verify test expectations
python3 << 'EOF'
# Test 1: shouldNotifyListenerNetworkUpgraded
print("Test 1: shouldNotifyListenerNetworkUpgraded")
available = {}
notifications = 0

# activateMobile()
was_available = available.get("MOBILE")
available["MOBILE"] = True
if not (was_available is True):
    notifications += 1
    print(f"  activateMobile: was_available={was_available}, notified [count: {notifications}]")

# activateWifi()
was_available = available.get("WIFI")
available["WIFI"] = True
if not (was_available is True):
    notifications += 1
    print(f"  activateWifi: was_available={was_available}, notified [count: {notifications}]")

print(f"  Expected: 1, Actual: {notifications}, PASS: {notifications == 1}\n")

# Test 2: shouldNotifyListenerNetworkDowngraded
print("Test 2: shouldNotifyListenerNetworkDowngraded")
available = {}
notifications = 0

# activateMobile()
was_available = available.get("MOBILE")
available["MOBILE"] = True
if not (was_available is True):
    notifications += 1
    print(f"  activateMobile: was_available={was_available}, notified [count: {notifications}]")

# activateWifi()
was_available = available.get("WIFI")
available["WIFI"] = True
if not (was_available is True):
    notifications += 1
    print(f"  activateWifi: was_available={was_available}, notified [count: {notifications}]")

# deactivateWifi()
available.pop("WIFI", None)
if len(available) > 0:  # not empty
    notifications += 1
    print(f"  deactivateWifi: available={available}, notified [count: {notifications}]")

print(f"  Expected: 2, Actual: {notifications}, PASS: {notifications == 2}\n")

# Test 3: shouldNotNotifyListenerNetworkDidNotUpgrade
print("Test 3: shouldNotNotifyListenerNetworkDidNotUpgrade")
available = {}
notifications = 0

# activateWifi()
was_available = available.get("WIFI")
available["WIFI"] = True
if not (was_available is True):
    notifications += 1
    print(f"  activateWifi: was_available={was_available}, notified [count: {notifications}]")

# reset counter
notifications = 0

# activateMobile()
was_available = available.get("MOBILE")
available["MOBILE"] = True
if not (was_available is True):
    notifications += 1
    print(f"  activateMobile: was_available={was_available}, notified [count: {notifications}]")

# deactivateMobile()
available.pop("MOBILE", None)
if len(available) > 0:  # not empty
    notifications += 1
    print(f"  deactivateMobile: available={available}, notified [count: {notifications}]")

print(f"  Expected: 0, Actual: {notifications}, PASS: {notifications == 0}\n")

# Test 4: shouldNotNotifyListenerNoNetworksAvailable
print("Test 4: shouldNotNotifyListenerNoNetworksAvailable")
available = {}
notifications = 0

# activateWifi()
was_available = available.get("WIFI")
available["WIFI"] = True
if not (was_available is True):
    notifications += 1
    print(f"  activateWifi: was_available={was_available}, notified [count: {notifications}]")

# reset counter
notifications = 0

# deactivateWifi()
available.pop("WIFI", None)
if len(available) > 0:  # not empty
    notifications += 1
    print(f"  deactivateWifi: available={available}, notified [count: {notifications}]")
else:
    print(f"  deactivateWifi: available={available}, NOT notified (no networks left)")

print(f"  Expected: 0, Actual: {notifications}, PASS: {notifications == 0}\n")
EOF

Repository: netbirdio/android-client

Length of output: 1009

🏁 Script executed:

# The tests are failing with the current code. Let me check git history to see what changed
# First, check if there are any recent changes to this file
git log --oneline tool/src/main/java/io/netbird/client/tool/networks/ConcreteNetworkAvailabilityListener.java | head -5

# Also check the diff to see what was changed in this PR
git diff HEAD~1 tool/src/main/java/io/netbird/client/tool/networks/ConcreteNetworkAvailabilityListener.java

Repository: netbirdio/android-client

Length of output: 350

🏁 Script executed:

# Check if there's any de-duplication logic or additional filtering in the code
# Search for any logic that might prevent duplicate notifications
rg -n "notifyListener" --type java -B 3 -A 3

# Also check if there's a previous version or any commented-out code
git show b324342 --stat | head -20

Repository: netbirdio/android-client

Length of output: 3255

🏁 Script executed:

# Get the full diff for that commit to see what changed
git show b324342 -- tool/src/main/java/io/netbird/client/tool/networks/ConcreteNetworkAvailabilityListener.java tool/src/test/java/io/netbird/client/tool/ConcreteNetworkAvailabilityListenerUnitTest.java

Repository: netbirdio/android-client

Length of output: 7798

🏁 Script executed:

# The tests were newly created in this commit. Let me verify if they're actually passing
# Check if there's a test runner configured
find . -name "*.gradle" -type f | head -5 | xargs grep -l "test"

# Also check the Android project structure
ls -la tool/

Repository: netbirdio/android-client

Length of output: 553

🏁 Script executed:

# Let me check if there are any other versions or branches that might have different code
# Search for any conditionals that might disable notifications in certain cases
rg -n "networkTypeChanged|notify" tool/src/main/java/io/netbird/client/tool/networks/ --type java -B 2 -A 2

# Also check if the NetworkToggleListener has any deduplication
rg -n "class.*NetworkToggleListener" --type java -A 20

Repository: netbirdio/android-client

Length of output: 8345

🏁 Script executed:

# Let me verify my understanding by checking if there's any logic that might
# prevent duplicate notifications or handle "network upgrade" scenarios
# Search for anything that might be comparing network types

rg -n "WIFI|MOBILE" tool/src/main/java/io/netbird/client/tool/networks/ConcreteNetworkAvailabilityListener.java

# Also check if there's a Constants class that defines network quality/priority
find . -name "Constants.java" -type f -path "*/networks/*"

Repository: netbirdio/android-client

Length of output: 135

🏁 Script executed:

# Let me check the Constants file to see if there's network quality/priority defined
cat tool/src/main/java/io/netbird/client/tool/networks/Constants.java

# And let me look at test 1 more carefully - maybe there's a comment that explains the expected behavior
rg -n "shouldNotifyListenerNetworkUpgraded" -A 20 tool/src/test/java/io/netbird/client/tool/ConcreteNetworkAvailabilityListenerUnitTest.java

Repository: netbirdio/android-client

Length of output: 1403

🏁 Script executed:

# Let me verify my understanding one more time by looking at the test comments/intentions
# The test "shouldNotifyListenerNetworkUpgraded" might suggest a different expected behavior
# Let me trace through the logic with fresh eyes

python3 << 'EOF'
# The key insight: "Network Upgraded" typically means:
# MOBILE → WIFI (upgraded)
# WIFI → MOBILE (downgraded)

# Test 1: "shouldNotifyListenerNetworkUpgraded"
# Sequence: activateMobile() then activateWifi()
# This looks like a real network upgrade scenario
# But does the implementation only notify on quality upgrades?

# Let me check the implementation logic again:
# onNetworkAvailable: notifies if !Boolean.TRUE.equals(wasAvailable)
#   - This notifies on ANY new network, not just upgrades
# onNetworkLost: notifies if !availableNetworkTypes.isEmpty()
#   - This notifies on ANY loss as long as others remain

print("Test 1 expectations suggests:")
print("- activateMobile() → should notify (1st notification)")
print("- activateWifi() → should NOT notify? (WiFi is added but already have MOBILE)")
print()
print("But the implementation notifies on EVERY new network being added.")
print("This is a mismatch between test expectations and implementation.")
EOF

Repository: netbirdio/android-client

Length of output: 361

🏁 Script executed:

# Since this is a new file in the commit, let me check if there was an older implementation
git log --all --full-history -- tool/src/main/java/io/netbird/client/tool/networks/ConcreteNetworkAvailabilityListener.java

# Also check if there are any other network-related listeners
find . -name "*NetworkAvailabilityListener*" -o -name "*NetworkToggleListener*" | grep -v ".git"

Repository: netbirdio/android-client

Length of output: 3133

🏁 Script executed:

# Let me see what the previous version of this file looked like before commit b324342
git show d277cf6:tool/src/main/java/io/netbird/client/tool/networks/ConcreteNetworkAvailabilityListener.java

Repository: netbirdio/android-client

Length of output: 1856

🏁 Script executed:

# Perfect! Now I understand the issue. Let me verify the test expectations match the OLD implementation
python3 << 'EOF'
print("=== OLD IMPLEMENTATION (d277cf6) ===")
print("Notifies ONLY on WiFi availability changes:")
print()

print("Test 1: shouldNotifyListenerNetworkUpgraded")
print("  activateMobile(): isWifiAvailable=false, adding MOBILE (not WIFI) → no notify")
print("  activateWifi(): isWifiAvailable=false, adding WIFI, and WIFI wasn't available → notifies [1]")
print("  Expected: 1 ✓ MATCHES")
print()

print("Test 2: shouldNotifyListenerNetworkDowngraded")
print("  activateMobile(): isWifiAvailable=false, adding MOBILE → no notify")
print("  activateWifi(): isWifiAvailable=false, adding WIFI, WIFI wasn't available → notifies [1]")
print("  deactivateWifi(): isMobileAvailable=true, removing WIFI, MOBILE is available → notifies [2]")
print("  Expected: 2 ✓ MATCHES")
print()

print("Test 3: shouldNotNotifyListenerNetworkDidNotUpgrade")
print("  activateWifi(): WiFi becomes available → notifies [1, then reset to 0]")
print("  activateMobile(): isWifiAvailable=true, adding MOBILE (not WiFi) → no notify")
print("  deactivateMobile(): isMobileAvailable=true, removing MOBILE (not WiFi) → no notify")
print("  Expected: 0 ✓ MATCHES")
print()

print("=== NEW IMPLEMENTATION (b324342) ===")
print("Notifies on ANY network change:")
print()
print("Test 1 would produce 2 notifications (not 1)")
print("Test 2 would produce 3 notifications (not 2)")
print("Test 3 would produce 2 notifications (not 0)")
print()
print("CONCLUSION: Tests were written for OLD behavior but code was completely rewritten!")
EOF

Repository: netbirdio/android-client

Length of output: 1321

Tests do not match the implementation — they will fail.

The code was rewritten to notify on any network change (lines 18-24 and 31-35), but the unit tests still expect the old WiFi-priority behavior (notify only on WiFi changes). All four tests will fail:

  • shouldNotifyListenerNetworkUpgraded: expects 1 but gets 2 (both MOBILE and WIFI notify)
  • shouldNotifyListenerNetworkDowngraded: expects 2 but gets 3 (MOBILE, WIFI, then WIFI loss all notify)
  • shouldNotNotifyListenerNetworkDidNotUpgrade: expects 0 but gets 2 (MOBILE and MOBILE loss notify while WIFI is present)
  • shouldNotNotifyListenerNoNetworksAvailable: expects 0 and gets 0 ✓ (only passes because no networks remain)

Update the test assertions to match the new behavior: any new network triggers a notification, and any lost network triggers a notification if others remain.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@tool/src/main/java/io/netbird/client/tool/networks/ConcreteNetworkAvailabilityListener.java`
around lines 14 - 25, Update the unit tests to reflect the new behavior in
ConcreteNetworkAvailabilityListener: onNetworkAvailable(int networkType) now
notifies on any added network and the corresponding onNetworkLost/logic notifies
when a network is lost while others remain. Adjust the assertions in
shouldNotifyListenerNetworkUpgraded, shouldNotifyListenerNetworkDowngraded,
shouldNotNotifyListenerNetworkDidNotUpgrade to expect notifications for both
MOBILE and WIFI additions (increment counts accordingly) and to expect
notifications when a network is removed but other networks still present; leave
shouldNotNotifyListenerNoNetworksAvailable unchanged. Ensure the test
expectations match the notifyListener() calls emitted by onNetworkAvailable and
the network-loss handler.


@Override
public void onNetworkLost(@Constants.NetworkType int networkType) {
boolean isMobileAvailable = Boolean.TRUE.equals(availableNetworkTypes.get(Constants.NetworkType.MOBILE));

availableNetworkTypes.remove(networkType);

// if wifi is lost and mobile data is available, notifies listener.
// No use to notify it if there's no other type of network available.
if (isMobileAvailable && networkType == Constants.NetworkType.WIFI) {
// Notify on any network loss if another network is still available,
// so the engine re-syncs with updated NetworkAddresses.
if (!availableNetworkTypes.isEmpty()) {
notifyListener();
}
}
Expand Down
40 changes: 36 additions & 4 deletions tool/src/main/java/io/netbird/client/tool/networks/NetworkChangeDetector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,23 @@ public class NetworkChangeDetector {
private final ConnectivityManager connectivityManager;
private ConnectivityManager.NetworkCallback networkCallback;
private volatile NetworkAvailabilityListener listener;
private volatile Runnable networkChangedCallback;
private volatile boolean initialCallbackReceived;

public NetworkChangeDetector(ConnectivityManager connectivityManager) {
this.connectivityManager = connectivityManager;
initNetworkCallback();
}

/**
* Set a callback that fires on every network availability/loss event,
* regardless of type. Used to notify the Go layer about underlying
* network changes for posture check re-evaluation.
*/
public void setNetworkChangedCallback(Runnable callback) {
this.networkChangedCallback = callback;
}

private void checkNetworkCapabilities(Network network, Consumer<Integer> operation) {
var capabilities = connectivityManager.getNetworkCapabilities(network);
if (capabilities == null) return;
Expand All @@ -37,16 +48,36 @@ private void initNetworkCallback() {
networkCallback = new ConnectivityManager.NetworkCallback() {
@Override
public void onAvailable(@NonNull Network network) {
// Skip the very first onAvailable after registerNetworkCallback().
// Android fires this immediately for each already-connected network —
// it is an initial state report, not an actual network change.
// Note: if both WiFi and cellular are connected at registration time,
// Android fires onAvailable for each, but we only skip the first one.
// The second fires a spurious notification, which is acceptable because
// the EngineRestarter debounces rapid network change callbacks anyway.
if (!initialCallbackReceived) {
initialCallbackReceived = true;
Log.d(LOGTAG, "ignoring initial onAvailable (not a real network change)");
return;
}
Log.d(LOGTAG, "onAvailable: " + network);
NetworkAvailabilityListener localListener = listener;
if (localListener == null) return;
checkNetworkCapabilities(network, localListener::onNetworkAvailable);
if (localListener != null) {
checkNetworkCapabilities(network, localListener::onNetworkAvailable);
}
Runnable cb = networkChangedCallback;
if (cb != null) cb.run();
}

@Override
public void onLost(@NonNull Network network) {
Log.d(LOGTAG, "onLost: " + network);
NetworkAvailabilityListener localListener = listener;
if (localListener == null) return;
checkNetworkCapabilities(network, localListener::onNetworkLost);
if (localListener != null) {
checkNetworkCapabilities(network, localListener::onNetworkLost);
}
Runnable cb = networkChangedCallback;
if (cb != null) cb.run();
}

@Override
Expand All @@ -59,6 +90,7 @@ public void onCapabilitiesChanged(@NonNull Network network, @NonNull NetworkCapa
}

public void registerNetworkCallback() {
initialCallbackReceived = false;
NetworkRequest.Builder builder = new NetworkRequest.Builder();
builder.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
connectivityManager.registerNetworkCallback(builder.build(), networkCallback);
Expand Down
28 changes: 14 additions & 14 deletions tool/src/test/java/io/netbird/client/tool/ConcreteNetworkAvailabilityListenerUnitTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ public void shouldNotifyListenerNetworkUpgraded() {
var networkChangeDetector = new MockNetworkChangeDetector(networkAvailabilityListener);

// Act:
networkChangeDetector.activateMobile();
networkChangeDetector.activateWifi();
networkChangeDetector.activateMobile(); // new network type -> notify
networkChangeDetector.activateWifi(); // new network type -> notify

// Assert:
assertEquals(1, networkToggleListener.totalTimesNetworkTypeChanged);
// Assert: both mobile and wifi becoming available trigger notifications
assertEquals(2, networkToggleListener.totalTimesNetworkTypeChanged);
}

@Test
Expand All @@ -70,16 +70,16 @@ public void shouldNotifyListenerNetworkDowngraded() {
var networkChangeDetector = new MockNetworkChangeDetector(networkAvailabilityListener);

// Act:
networkChangeDetector.activateMobile();
networkChangeDetector.activateWifi(); // upgraded, network changes.
networkChangeDetector.deactivateWifi(); // downgraded, network changes.
networkChangeDetector.activateMobile(); // new network type -> notify
networkChangeDetector.activateWifi(); // new network type -> notify
networkChangeDetector.deactivateWifi(); // lost, mobile still available -> notify

// Assert:
assertEquals(2, networkToggleListener.totalTimesNetworkTypeChanged);
// Assert: each event triggers a notification
assertEquals(3, networkToggleListener.totalTimesNetworkTypeChanged);
}

@Test
public void shouldNotNotifyListenerNetworkDidNotUpgrade() {
public void shouldNotifyListenerOnAnyNetworkChange() {
// Assemble:
var networkToggleListener = new MockNetworkToggleListener();
var networkAvailabilityListener = new ConcreteNetworkAvailabilityListener();
Expand All @@ -92,11 +92,11 @@ public void shouldNotNotifyListenerNetworkDidNotUpgrade() {

networkToggleListener.resetCounter();

networkChangeDetector.activateMobile();
networkChangeDetector.deactivateMobile();
networkChangeDetector.activateMobile(); // new network type -> notify
networkChangeDetector.deactivateMobile(); // lost, wifi still available -> notify

// Assert:
assertEquals(0, networkToggleListener.totalTimesNetworkTypeChanged);
// Assert: every network change notifies for posture check re-evaluation
assertEquals(2, networkToggleListener.totalTimesNetworkTypeChanged);
}

@Test
Expand Down