Skip to content

fix(lan-discovery): restore IPv6 fallback subnet scan broken in 56790cf#365

Merged
dishit-wednesday merged 1 commit into
mainfrom
fix/lan-discovery-ipv6-fallback-subnet
May 18, 2026
Merged

fix(lan-discovery): restore IPv6 fallback subnet scan broken in 56790cf#365
dishit-wednesday merged 1 commit into
mainfrom
fix/lan-discovery-ipv6-fallback-subnet

Conversation

@dishit-wednesday

@dishit-wednesday dishit-wednesday commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Problem

LAN server discovery stopped working for users whose phones return an IPv6 address from getIpAddress() — which is the default on iOS 26+ devices even when connected to WiFi.

Commit 56790cf introduced a gateway probe step: before scanning the fallback subnets (192.168.1.x, 192.168.0.x), it would first ping the router at .1 on port 80 and 11434. If neither responded, discovery would bail out entirely with return [].

Most home routers do not serve HTTP on port 80 or 11434. So the gateway probe always timed out, and discovery silently returned nothing — even though the device was on WiFi and servers were reachable.

Root Cause Confirmed

Added diagnostic logging to verify. The logs showed:

Device IP: ::2401:4900:8f56:87a5:a0af:3531 | IPv6: true
Gateway 192.168.1.1 did not respond on :80 or :11434
Gateway 192.168.0.1 did not respond on :80 or :11434
No gateway responded — skipping scan ← old broken behaviour

After the fix:

No gateway responded — scanning all fallback subnets anyway
Found Ollama at 192.168.1.114:11434
Scan complete — found 3 server(s)

Fix

Restore the pre-56790cf behaviour: always scan fallback subnets when the device has an IPv6 address, regardless of gateway response. The gateway probe is kept as an optimisation — if it responds we scan only that subnet and skip the other — but it no longer gates the entire scan.

// Before (broken)
if (!reachableSubnet) return [];
subnetsToScan = [reachableSubnet];

// After (fixed)
subnetsToScan = reachableSubnet ? [reachableSubnet] : FALLBACK_SUBNETS;
Impact
IPv4 users: no change, different code path entirely
Cellular users: still bailed early by the !ip check before this runs
Enterprise networks (10.x, 172.16.x): fallback subnets won't match, same as before
Home networks (192.168.1.x, 192.168.0.x): discovery works again

Thanks to Aman for helping out in debugging.

Co-Authored-By: Aman Kumar [amankumar020003@gmail.com] )
github.com/pirate329



Thank you Saiganesh Menon for helping to think this through and debugging with me.

Co-Authored-By: Saiganesh Menon [saiganesh.menon@wednesday.is]



Commit 56790cf added a gateway probe before scanning fallback subnets
when the device returns an IPv6 address (common on iOS 26+). If the
gateway did not respond on :80 or :11434 — which most home routers
don't — discovery silently returned [] and no servers were found.

The fix restores the pre-56790cf behaviour: always scan the fallback
subnets (192.168.1.x, 192.168.0.x) when on IPv6, regardless of
whether the gateway responds. The gateway probe is kept as an
optimisation — if it responds we scan only that subnet, saving time —
but it no longer gates the scan entirely.

Root cause confirmed via in-app diagnostic logging: device IP was
IPv6 (2401:4900:8f56:87a5:a0af:3531), both gateways timed out, scan
was skipped. After fix, all three LAN servers were discovered correctly.

Co-Authored-By: Dishit <hanmadishit74@gmail.com>

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@sonarqubecloud

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request enhances the LAN server discovery process by introducing a logging mechanism through an optional onLog callback and a shared log helper. It also updates the IPv6 discovery logic to attempt scanning fallback subnets even when initial gateway probes fail. Feedback focuses on improving the logging implementation by supporting distinct log levels for informational messages versus warnings and ensuring that errors caught during environment checks are logged with the correct severity.

* Uses a short timeout so we bail fast when on cellular.
*/
async function findReachableSubnet(subnets: string[]): Promise<string | null> {
async function findReachableSubnet(subnets: string[], log: (msg: string) => void): Promise<string | null> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Update the log parameter type to support an optional log level. This allows the function to pass severity information back to the caller's logging helper.

Suggested change
async function findReachableSubnet(subnets: string[], log: (msg: string) => void): Promise<string | null> {
async function findReachableSubnet(subnets: string[], log: (msg: string, level?: 'log' | 'warn') => void): Promise<string | null> {

Comment on lines +111 to 126
const log = (msg: string) => {
logger.warn('[Discovery]', msg);
onLog?.(msg);
};

let runningOnEmulator: boolean;
try {
runningOnEmulator = await isEmulator();
} catch (err) {
logger.warn('[Discovery] isEmulator() threw:', (err as Error).message);
log(`isEmulator() threw: ${(err as Error).message} — assuming not emulator`);
runningOnEmulator = false;
}
if (runningOnEmulator) {
logger.warn('[Discovery] Running on emulator — skipping LAN scan (emulator network stack cannot handle concurrent probes)');
log('Running on emulator — skipping scan (emulator network stack cannot handle concurrent probes)');
return [];
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The log helper currently unifies all discovery logs under logger.warn, which incorrectly marks informational progress as warnings. This change introduces an optional log level. Furthermore, it ensures that any errors caught during the isEmulator check are logged rather than swallowed, which aids in debugging and prevents silent failures.

Suggested change
const log = (msg: string) => {
logger.warn('[Discovery]', msg);
onLog?.(msg);
};
let runningOnEmulator: boolean;
try {
runningOnEmulator = await isEmulator();
} catch (err) {
logger.warn('[Discovery] isEmulator() threw:', (err as Error).message);
log(`isEmulator() threw: ${(err as Error).message} — assuming not emulator`);
runningOnEmulator = false;
}
if (runningOnEmulator) {
logger.warn('[Discovery] Running on emulator — skipping LAN scan (emulator network stack cannot handle concurrent probes)');
log('Running on emulator — skipping scan (emulator network stack cannot handle concurrent probes)');
return [];
}
const log = (msg: string, level: 'log' | 'warn' = 'log') => {
logger[level]('[Discovery]', msg);
onLog?.(msg);
};
let runningOnEmulator: boolean;
try {
runningOnEmulator = await isEmulator();
} catch (err) {
log('isEmulator() threw: ' + (err as Error).message + ' — assuming not emulator', 'warn');
runningOnEmulator = false;
}
if (runningOnEmulator) {
log('Running on emulator — skipping scan (emulator network stack cannot handle concurrent probes)', 'warn');
return [];
}
References
  1. When catching errors, log them instead of swallowing them to ensure failures are visible and to aid in debugging.

@codecov

codecov Bot commented May 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 53.33333% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.85%. Comparing base (2682e05) to head (00def5a).

Files with missing lines Patch % Lines
src/services/networkDiscovery.ts 53.33% 13 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (53.33%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #365      +/-   ##
==========================================
- Coverage   82.86%   82.85%   -0.01%     
==========================================
  Files         235      235              
  Lines       12003    12017      +14     
  Branches     3279     3283       +4     
==========================================
+ Hits         9946     9957      +11     
- Misses       1199     1203       +4     
+ Partials      858      857       -1     
Files with missing lines Coverage Δ
src/services/networkDiscovery.ts 61.76% <53.33%> (+1.53%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dishit-wednesday dishit-wednesday merged commit ecbfcd7 into main May 18, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant