-
-
Notifications
You must be signed in to change notification settings - Fork 290
fix(remote): auto-recover the remote connection on network change #637
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| /** | ||
| * Network Reconnect Watcher | ||
| * | ||
| * Fixes the "connected at the office, changed WiFi, must manually rescan" drop. The remote-model | ||
| * (HTTP gateway) path has no liveness check: after the desktop's LAN IP moves or the phone changes | ||
| * network, the saved endpoint is stale but the app still believes it is connected and only fails on | ||
| * the next message. This watcher detects a network change and asks the service to recover. | ||
| * | ||
| * Native-dep-free by design: it reuses `getIpAddress()` (already a dependency via | ||
| * react-native-device-info) and AppState, rather than adding @react-native-community/netinfo (a new | ||
| * native module + rebuild). The device's own IP changing is a reliable proxy for "the network | ||
| * changed"; the actual re-discover / re-select decision lives in `remoteServerManager`, not here. | ||
| */ | ||
|
|
||
| import { AppState, AppStateStatus } from 'react-native'; | ||
| import { getIpAddress } from 'react-native-device-info'; | ||
| import { remoteServerManager } from './remoteServerManager'; | ||
| import logger from '../utils/logger'; | ||
|
|
||
| /** How often to poll the device IP while the app is foregrounded. */ | ||
| const IP_POLL_MS = 15_000; | ||
| /** Wait for a WiFi handoff to settle before acting, and coalesce rapid changes into one recovery. */ | ||
| const DEBOUNCE_MS = 2_500; | ||
|
|
||
| let appStateSub: { remove: () => void } | null = null; | ||
| let pollTimer: ReturnType<typeof setInterval> | null = null; | ||
| let debounceTimer: ReturnType<typeof setTimeout> | null = null; | ||
| let lastIp: string | null = null; | ||
| let started = false; | ||
|
|
||
| function isUsableIp(ip: string | null | undefined): ip is string { | ||
| return !!ip && ip !== '0.0.0.0'; | ||
| } | ||
|
|
||
| function scheduleRecovery(reason: string): void { | ||
| if (debounceTimer) clearTimeout(debounceTimer); | ||
| debounceTimer = setTimeout(() => { | ||
| debounceTimer = null; | ||
| logger.log(`[NetReconnect] recovering after ${reason}`); | ||
| remoteServerManager | ||
| .recoverActiveConnection() | ||
| .catch((err) => logger.warn('[NetReconnect] recovery failed:', (err as Error).message)); | ||
| }, DEBOUNCE_MS); | ||
| } | ||
|
|
||
| async function checkIpChanged(): Promise<void> { | ||
| let ip: string; | ||
| try { | ||
| ip = await getIpAddress(); | ||
| } catch { | ||
| return; | ||
| } | ||
| if (!isUsableIp(ip)) return; | ||
| if (isUsableIp(lastIp) && ip !== lastIp) { | ||
| logger.log(`[NetReconnect] device IP changed ${lastIp} -> ${ip}`); | ||
| scheduleRecovery('network change'); | ||
| } | ||
| lastIp = ip; | ||
|
Comment on lines
+53
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift Detect recovery conditions that keep the device IP unchanged. Line 54 only schedules recovery when the usable IP string changes. A remote server can move while the device IP stays unchanged. A network rejoin can also assign the same private IP address. In both cases, Trigger a bounded active-connection validation independently of an IP-string difference. 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| function startPoll(): void { | ||
| if (pollTimer) return; | ||
| pollTimer = setInterval(() => { checkIpChanged().catch(() => { /* checkIpChanged never rejects */ }); }, IP_POLL_MS); | ||
| } | ||
|
|
||
| function stopPoll(): void { | ||
| if (pollTimer) { | ||
| clearInterval(pollTimer); | ||
| pollTimer = null; | ||
| } | ||
| } | ||
|
|
||
| function handleAppState(state: AppStateStatus): void { | ||
| if (state === 'active') { | ||
| // The network may have changed while backgrounded; compare current IP to the last one we saw. | ||
| checkIpChanged().catch(() => { /* checkIpChanged never rejects */ }); | ||
| startPoll(); | ||
| } else { | ||
| stopPoll(); | ||
| } | ||
| } | ||
|
|
||
| /** Start watching for network changes. Idempotent — safe to call once at app boot. */ | ||
| export function startNetworkReconnectWatcher(): void { | ||
| if (started) return; | ||
| started = true; | ||
| // Seed the baseline IP without triggering a recovery on first launch. | ||
| getIpAddress().then((ip) => { if (isUsableIp(ip)) lastIp = ip; }).catch(() => { /* no network yet */ }); | ||
| appStateSub = AppState.addEventListener('change', handleAppState); | ||
| if (AppState.currentState === 'active') startPoll(); | ||
| logger.log('[NetReconnect] watcher started'); | ||
| } | ||
|
|
||
| /** Stop watching. Used on teardown; the watcher is otherwise app-lifetime. */ | ||
| export function stopNetworkReconnectWatcher(): void { | ||
| appStateSub?.remove(); | ||
| appStateSub = null; | ||
| stopPoll(); | ||
| if (debounceTimer) { | ||
| clearTimeout(debounceTimer); | ||
| debounceTimer = null; | ||
| } | ||
| started = false; | ||
|
Comment on lines
+95
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win Invalidate pending IP checks during teardown.
Use a lifecycle generation token, or an equivalent cancellation guard, before updating 🤖 Prompt for AI Agents |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,8 +9,11 @@ | |
|
|
||
| import { RemoteServer, RemoteModel, ServerTestResult } from '../types'; | ||
| import { useRemoteServerStore } from '../stores/remoteServerStore'; | ||
| import { useAppStore } from '../stores/appStore'; | ||
| import { OpenAICompatibleProvider } from './providers/openAICompatibleProvider'; | ||
| import { providerRegistry } from './providers/registry'; | ||
| import { discoverLANServers, DiscoveredServer } from './networkDiscovery'; | ||
| import { shouldAutoDiscoverRemoteModels } from '../utils/remoteAutoDiscovery'; | ||
| import logger from '../utils/logger'; | ||
| import { | ||
| storeApiKeyImpl, | ||
|
|
@@ -22,6 +25,18 @@ import { | |
| initializeProvidersImpl, | ||
| } from './remoteServerManagerUtils'; | ||
|
|
||
| /** Normalize an endpoint for identity comparison (lowercase, no trailing slashes). */ | ||
| const trimSlash = (url: string): string => { | ||
| let s = url.toLowerCase(); | ||
| while (s.endsWith('/')) s = s.slice(0, -1); | ||
| return s; | ||
| }; | ||
|
|
||
| /** Extract the port from an endpoint, or null if it cannot be parsed. */ | ||
| const portOf = (endpoint: string): string | null => { | ||
| try { return new URL(endpoint).port; } catch { return null; } | ||
| }; | ||
|
|
||
| class RemoteServerManager { | ||
| /** | ||
| * Add a new remote server | ||
|
|
@@ -192,6 +207,88 @@ class RemoteServerManager { | |
| return initializeProvidersImpl(() => this.getServers()); | ||
| } | ||
|
|
||
| /** | ||
| * Scan the LAN and reconcile the result against saved servers. For each discovered endpoint that | ||
| * matches a saved server on the same port but a new IP, update it in place and re-select the active | ||
| * model. Returns the genuinely-new servers (not remaps) so a caller can surface them, plus the ids | ||
| * of servers that moved. Does NOT gate on settings — the caller decides whether a scan is allowed. | ||
| * This is the single owner of the "server moved to a new IP" reconciliation; UI callers delegate here. | ||
| */ | ||
| async scanAndReconcile(): Promise<{ moved: string[]; found: DiscoveredServer[] }> { | ||
| let discovered: DiscoveredServer[]; | ||
| try { | ||
| discovered = await discoverLANServers(); | ||
| } catch (error) { | ||
| logger.warn('[RemoteServerManager] LAN scan failed:', (error as Error).message); | ||
| return { moved: [], found: [] }; | ||
| } | ||
| if (discovered.length === 0) return { moved: [], found: [] }; | ||
|
|
||
| const store = useRemoteServerStore.getState(); | ||
| const existingServers = store.servers; | ||
| const existingEndpoints = new Set(existingServers.map((s) => trimSlash(s.endpoint))); | ||
| const moved: string[] = []; | ||
| const found: DiscoveredServer[] = []; | ||
|
|
||
| for (const d of discovered) { | ||
| if (existingEndpoints.has(trimSlash(d.endpoint))) continue; | ||
|
|
||
| const dPort = portOf(d.endpoint); | ||
| const samePortServer = dPort | ||
| ? existingServers.find((s) => portOf(s.endpoint) === dPort) | ||
| : null; | ||
|
|
||
| if (samePortServer) { | ||
| await this.applyMovedServer(samePortServer, d.endpoint, d.name); | ||
| moved.push(samePortServer.id); | ||
|
Comment on lines
+236
to
+243
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Do not identify a moved server by port alone. If a scan finds an existing reachable server and another server on the same port, Line 238 selects the existing server and Line 242 overwrites its endpoint. The new server is also omitted from Build candidates from saved endpoints absent from the scan. Remap only one unambiguous candidate. Otherwise, return the discovered server for user selection. 🤖 Prompt for AI Agents |
||
| } else { | ||
| found.push(d); | ||
| } | ||
| } | ||
|
|
||
| return { moved, found }; | ||
| } | ||
|
|
||
| /** Update a saved server that has moved to a new endpoint, and re-select it if it was active. */ | ||
| private async applyMovedServer(server: RemoteServer, endpoint: string, name: string): Promise<void> { | ||
| logger.log('[RemoteServerManager] Server moved to new IP, updating:', server.name, '->', endpoint); | ||
| await this.updateServer(server.id, { endpoint, name }); | ||
| try { await this.discoverModels(server.id); } catch { /* offline — models repopulate on next reach */ } | ||
| const store = useRemoteServerStore.getState(); | ||
| if (store.activeServerId === server.id && store.activeRemoteTextModelId) { | ||
| try { | ||
| await this.setActiveRemoteTextModel(server.id, store.activeRemoteTextModelId); | ||
| } catch { /* user can re-select from the picker */ } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Recover the active remote connection after a network change. Cheap-first: if there is an active | ||
| * server and it is still reachable at its known endpoint, do nothing. Only when it is unreachable | ||
| * (or the user has enabled auto-discovery) do we scan the LAN to find where it moved and reconnect. | ||
| * This keeps LAN scanning off unless the user is actually relying on a remote server, and makes the | ||
| * "connected" state honest again after the peer's IP changes. | ||
| */ | ||
| async recoverActiveConnection(): Promise<void> { | ||
| const activeId = useRemoteServerStore.getState().activeServerId; | ||
|
|
||
| if (activeId) { | ||
| const result = await this.testConnection(activeId).catch(() => ({ success: false })); | ||
| if (result.success) { | ||
| logger.log('[RemoteServerManager] Active server still reachable; no rescan needed'); | ||
| return; | ||
| } | ||
| logger.log('[RemoteServerManager] Active server unreachable; rescanning to recover'); | ||
| } | ||
|
|
||
| const allowScan = | ||
| shouldAutoDiscoverRemoteModels(useAppStore.getState().settings) || !!activeId; | ||
| if (!allowScan) return; | ||
|
Comment on lines
+275
to
+286
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Run the scan when auto-discovery is enabled. When the active server is reachable, Line 279 returns before Evaluate the setting before the early return. Return early only when the server is reachable and auto-discovery is disabled. 🤖 Prompt for AI Agents |
||
|
|
||
| const { moved, found } = await this.scanAndReconcile(); | ||
| logger.log(`[RemoteServerManager] Recovery scan complete: ${moved.length} moved, ${found.length} new`); | ||
| } | ||
|
|
||
| /** | ||
| * Clear all servers | ||
| */ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: off-grid-ai/OGAM
Length of output: 38463
🏁 Script executed:
Repository: off-grid-ai/OGAM
Length of output: 11542
🏁 Script executed:
Repository: off-grid-ai/OGAM
Length of output: 36970
Start the reconnect watcher after provider initialization settles
initializeProviders()performs asynchronous provider creation, model discovery, and active-model restoration. The watcher can runrecoverActiveConnection()during these operations. Both paths mutate the provider registry and remote-server store without shared serialization, so recovery can update an endpoint while initialization later registers a provider from an earlier server snapshot. Start the watcher ininitializeProviders().finally(...), or serialize both operations.🤖 Prompt for AI Agents