From 60f9d83d88654b5033299ff382f0f23a772a6b80 Mon Sep 17 00:00:00 2001 From: Joe WB3IHY Date: Thu, 6 Aug 2026 12:48:32 -0400 Subject: [PATCH 1/3] fix(connection): disable Meshtastic TCP host-link RTT probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WiFi/TCP signal-bars meter opened a second, separate TCP connection to the exact same host:port as the live Meshtastic session every 4s for the life of the session, purely to time the connect. Confirmed via live tshark packet capture (5/5 reproduced samples across two builds, before and after upstream PR #808): the device sends a genuine RST+ACK on the *real* session within ~15-210ms of a probe cycle overlapping a real outbound write. Not the node (a stable Android-app connection to the same node holds for hours) and not the network (only this one client active). PR #808's setNoDelay/setKeepAlive do not affect this — they're on the main session's socket; the probe opens its own. Disable the probe for Meshtastic raw TCP specifically. rttMs naturally stays null (the probe effect now skips this combo), so the UI falls through to the existing ip-rtt no-data rendering ("—") rather than a dedicated 'unavailable' kind, which would incorrectly show ConnectionLinkMeter's Web-Bluetooth-specific copy on a WiFi/TCP-only transport. HTTP and MeshCore probing are untouched (unproven, though MeshCore's http transport uses the same underlying pattern and may warrant the same scrutiny separately). Deliberate interim tradeoff: this drops the signal-bars feature for *every* Meshtastic TCP session, not only ones that would hit the collision, since there's no cheap signal for "is a competing probe currently unsafe" short of deriving RTT from the already-open session instead of a second connection — flagged in code for a proper follow-up. --- .../ConnectionPanel.hostLinkMeter.test.tsx | 10 +++--- src/renderer/hooks/useHostLinkMeter.test.ts | 25 ++++++++++++--- src/renderer/hooks/useHostLinkMeter.ts | 31 ++++++++++++++++--- src/renderer/lib/hostLinkQuality.ts | 6 ++++ 4 files changed, 59 insertions(+), 13 deletions(-) diff --git a/src/renderer/components/ConnectionPanel.hostLinkMeter.test.tsx b/src/renderer/components/ConnectionPanel.hostLinkMeter.test.tsx index 233e80247..867f41c7d 100644 --- a/src/renderer/components/ConnectionPanel.hostLinkMeter.test.tsx +++ b/src/renderer/components/ConnectionPanel.hostLinkMeter.test.tsx @@ -141,7 +141,10 @@ describe('ConnectionPanel host link meter', () => { }); }); - it('shows Link quality for Meshtastic TCP', async () => { + it('shows Link quality with no data for Meshtastic TCP and never probes (device RST hazard)', () => { + // Meshtastic raw-TCP RTT probing opens a second connection to the same host:port as + // the live session — confirmed via live packet capture to get the device to RST the + // real session. See useHostLinkMeter.ts's meshtasticTcpProbeUnsafe comment. vi.mocked(window.electronAPI.getPlatform).mockReturnValue('darwin'); vi.mocked(window.electronAPI.hostLink.probeTcpRtt).mockResolvedValue(120); render( @@ -161,8 +164,7 @@ describe('ConnectionPanel host link meter', () => { />, ); expect(screen.getByText('Link quality')).toBeInTheDocument(); - await waitFor(() => { - expect(window.electronAPI.hostLink.probeTcpRtt).toHaveBeenCalled(); - }); + expect(screen.getByText('—')).toBeInTheDocument(); + expect(window.electronAPI.hostLink.probeTcpRtt).not.toHaveBeenCalled(); }); }); diff --git a/src/renderer/hooks/useHostLinkMeter.test.ts b/src/renderer/hooks/useHostLinkMeter.test.ts index fb2d050bf..87fbcacda 100644 --- a/src/renderer/hooks/useHostLinkMeter.test.ts +++ b/src/renderer/hooks/useHostLinkMeter.test.ts @@ -77,7 +77,19 @@ describe('useHostLinkMeter', () => { }, ); - it('returns ip-rtt for Meshtastic TCP via probeTcpRtt', async () => { + it('returns ip-rtt with no data for Meshtastic raw TCP and never opens a competing probe connection', async () => { + // Meshtastic raw-TCP RTT probing used to open a second, separate connection to the + // exact same host:port as the live session every poll tick — confirmed via live + // packet capture to intermittently get the *real* session RST'd by the device + // (5/5 reproduced samples; unaffected by upstream PR #808's setNoDelay/setKeepAlive, + // which only touches the main session's socket). Do not re-enable this probe for + // 'meshtastic' + 'tcp' without deriving RTT from the already-open session instead. + // + // kind stays 'ip-rtt' (not 'unavailable') with null rttMs/level — same rendering as + // an HTTP probe failure ("—" + no-data bars). A dedicated 'unavailable' kind would + // incorrectly show ConnectionLinkMeter's Web-Bluetooth-specific copy on this + // WiFi/TCP-only transport. + vi.useFakeTimers(); const { result } = renderHook(() => useHostLinkMeter({ protocol: 'meshtastic', @@ -88,11 +100,14 @@ describe('useHostLinkMeter', () => { }), ); expect(result.current.kind).toBe('ip-rtt'); - await waitFor(() => { - expect(result.current.rttMs).toBe(80); - expect(result.current.level).toBe(3); + expect(result.current.rttMs).toBeNull(); + expect(result.current.level).toBeNull(); + + // Advance well past several poll intervals to confirm no deferred/interval probe fires. + await act(async () => { + await vi.advanceTimersByTimeAsync(20_000); }); - expect(window.electronAPI.hostLink.probeTcpRtt).toHaveBeenCalledWith('10.0.0.5', 4403); + expect(window.electronAPI.hostLink.probeTcpRtt).not.toHaveBeenCalled(); }); it('returns ip-rtt for MeshCore TCP/IP (http transport) via probeTcpRtt', async () => { diff --git a/src/renderer/hooks/useHostLinkMeter.ts b/src/renderer/hooks/useHostLinkMeter.ts index 8f647fa06..88e4c314a 100644 --- a/src/renderer/hooks/useHostLinkMeter.ts +++ b/src/renderer/hooks/useHostLinkMeter.ts @@ -55,6 +55,23 @@ export function useHostLinkMeter(opts: { isConnectedStatus(status) && (connectionType === 'ble' || connectionType === 'http' || connectionType === 'tcp'); + // Meshtastic raw-TCP RTT probing opens a second, separate socket to the exact same + // host:port as the live protocol session every poll tick. Captured via live packet + // capture: the device intermittently RSTs the *real* session within ~15-210ms of a + // probe cycle overlapping a real outbound write (5/5 reproduced samples, both before + // and after PR #808's setNoDelay/setKeepAlive change — unaffected, since that only + // touches the main session's socket). Meshtastic WiFi/TCP firmware likely tracks very + // few concurrent API connections; a second churn-y connection to the same port + // destabilizes it. Do not reintroduce a competing connect for this transport without + // deriving RTT from the already-open session instead. + // + // Deliberate blunt/interim tradeoff: this disables the probe (and the signal-bars UI) + // for *every* Meshtastic TCP session, not only ones that hit the collision, because + // there is no cheap signal here for "is a competing probe currently unsafe" short of + // the real fix above. A previously-working, cosmetic-only feature regressing is an + // acceptable cost against dropping the live connection. + const meshtasticTcpProbeUnsafe = protocol === 'meshtastic' && connectionType === 'tcp'; + // BLE RSSI via Noble (macOS / Windows) useEffect(() => { if (!active || connectionType !== 'ble') { @@ -78,7 +95,11 @@ export function useHostLinkMeter(opts: { // HTTP / TCP RTT probe useEffect(() => { - if (!active || (connectionType !== 'http' && connectionType !== 'tcp')) { + if ( + !active || + (connectionType !== 'http' && connectionType !== 'tcp') || + meshtasticTcpProbeUnsafe + ) { setRttMs(null); return; } @@ -98,8 +119,6 @@ export function useHostLinkMeter(opts: { let next: number | null = null; if (protocol === 'meshtastic' && connectionType === 'http') { next = await probeHttpLinkRttMs(address); - } else if (protocol === 'meshtastic' && connectionType === 'tcp') { - next = await probeTcpLinkRttMs(address, 'meshtastic'); } else if (protocol === 'meshcore' && connectionType === 'http') { // MeshCore "http" transport is TCP/IP host:port next = await probeTcpLinkRttMs(address, 'meshcore'); @@ -117,7 +136,7 @@ export function useHostLinkMeter(opts: { if (timer) clearInterval(timer); setRttMs(null); }; - }, [active, connectionType, hostAddress, protocol]); + }, [active, connectionType, hostAddress, protocol, meshtasticTcpProbeUnsafe]); if (!active || !connectionType) return IDLE; @@ -129,6 +148,10 @@ export function useHostLinkMeter(opts: { } if (connectionType === 'http' || connectionType === 'tcp') { + // meshtasticTcpProbeUnsafe: rttMs never gets set (probe effect above skips this + // combo entirely), so this naturally renders the same "—" / no-data state as an + // HTTP probe failure — not a separate 'unavailable' kind, which would incorrectly + // show ConnectionLinkMeter's Web-Bluetooth-specific copy for a WiFi/TCP transport. const level = rttMs != null ? rttToSignalLevel(rttMs) : null; return { kind: 'ip-rtt', rssi: null, rttMs, level }; } diff --git a/src/renderer/lib/hostLinkQuality.ts b/src/renderer/lib/hostLinkQuality.ts index e15842bb8..a1a8ba95e 100644 --- a/src/renderer/lib/hostLinkQuality.ts +++ b/src/renderer/lib/hostLinkQuality.ts @@ -53,6 +53,12 @@ export interface ParsedTcpProbeTarget { * Parse a TCP probe target. * - `meshtastic`: default port 4403 (`parseMeshtasticTcpAddress`) * - `meshcore`: default port 5000 (`parseTcpAddress`) + * + * The `'meshtastic'` branch is currently unreachable from any production call site — + * `useHostLinkMeter.ts` deliberately never probes Meshtastic raw TCP (opening a second + * connection to the same host:port as the live session has been confirmed to get the + * device to RST the real connection). Do not wire a new call site for it without reading + * that hook's `meshtasticTcpProbeUnsafe` comment first. */ export function parseTcpProbeTarget( address: string, From 3fe2e732b374670448cd009a269c57fa1d5308f1 Mon Sep 17 00:00:00 2001 From: Joe WB3IHY Date: Thu, 6 Aug 2026 13:41:59 -0400 Subject: [PATCH 2/3] fix(connection): address CodeRabbit review on #811 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Derive the displayed RTT from meshtasticTcpProbeUnsafe directly instead of trusting rttMs state: on the render right after Meshtastic switches from HTTP to TCP, the render body ran before the probe effect's cleanup had cleared rttMs, so a prior HTTP RTT value could render as if it were the (disabled) TCP link quality. Add a covering test. - Parameterize the two new/updated Meshtastic-TCP-no-data tests across linux/darwin/win32 (it.each) per project convention — the disabled-probe behavior does not depend on platform, so a single-platform case wasn't sufficient coverage. --- .../ConnectionPanel.hostLinkMeter.test.tsx | 56 ++++++------ src/renderer/hooks/useHostLinkMeter.test.ts | 91 +++++++++++++------ src/renderer/hooks/useHostLinkMeter.ts | 17 ++-- 3 files changed, 105 insertions(+), 59 deletions(-) diff --git a/src/renderer/components/ConnectionPanel.hostLinkMeter.test.tsx b/src/renderer/components/ConnectionPanel.hostLinkMeter.test.tsx index 867f41c7d..9f0bdbe7a 100644 --- a/src/renderer/components/ConnectionPanel.hostLinkMeter.test.tsx +++ b/src/renderer/components/ConnectionPanel.hostLinkMeter.test.tsx @@ -141,30 +141,34 @@ describe('ConnectionPanel host link meter', () => { }); }); - it('shows Link quality with no data for Meshtastic TCP and never probes (device RST hazard)', () => { - // Meshtastic raw-TCP RTT probing opens a second connection to the same host:port as - // the live session — confirmed via live packet capture to get the device to RST the - // real session. See useHostLinkMeter.ts's meshtasticTcpProbeUnsafe comment. - vi.mocked(window.electronAPI.getPlatform).mockReturnValue('darwin'); - vi.mocked(window.electronAPI.hostLink.probeTcpRtt).mockResolvedValue(120); - render( - , - ); - expect(screen.getByText('Link quality')).toBeInTheDocument(); - expect(screen.getByText('—')).toBeInTheDocument(); - expect(window.electronAPI.hostLink.probeTcpRtt).not.toHaveBeenCalled(); - }); + it.each(['linux', 'darwin', 'win32'] as const)( + 'shows Link quality with no data for Meshtastic TCP and never probes on %s (device RST hazard)', + (platform) => { + // Meshtastic raw-TCP RTT probing opens a second connection to the same host:port as + // the live session — confirmed via live packet capture to get the device to RST the + // real session. See useHostLinkMeter.ts's meshtasticTcpProbeUnsafe comment. + // Platform-independent behavior, so covered on all three platforms per convention. + vi.mocked(window.electronAPI.getPlatform).mockReturnValue(platform); + vi.mocked(window.electronAPI.hostLink.probeTcpRtt).mockResolvedValue(120); + render( + , + ); + expect(screen.getByText('Link quality')).toBeInTheDocument(); + expect(screen.getByText('—')).toBeInTheDocument(); + expect(window.electronAPI.hostLink.probeTcpRtt).not.toHaveBeenCalled(); + }, + ); }); diff --git a/src/renderer/hooks/useHostLinkMeter.test.ts b/src/renderer/hooks/useHostLinkMeter.test.ts index 87fbcacda..12ec2e833 100644 --- a/src/renderer/hooks/useHostLinkMeter.test.ts +++ b/src/renderer/hooks/useHostLinkMeter.test.ts @@ -77,37 +77,74 @@ describe('useHostLinkMeter', () => { }, ); - it('returns ip-rtt with no data for Meshtastic raw TCP and never opens a competing probe connection', async () => { - // Meshtastic raw-TCP RTT probing used to open a second, separate connection to the - // exact same host:port as the live session every poll tick — confirmed via live - // packet capture to intermittently get the *real* session RST'd by the device - // (5/5 reproduced samples; unaffected by upstream PR #808's setNoDelay/setKeepAlive, - // which only touches the main session's socket). Do not re-enable this probe for - // 'meshtastic' + 'tcp' without deriving RTT from the already-open session instead. - // - // kind stays 'ip-rtt' (not 'unavailable') with null rttMs/level — same rendering as - // an HTTP probe failure ("—" + no-data bars). A dedicated 'unavailable' kind would - // incorrectly show ConnectionLinkMeter's Web-Bluetooth-specific copy on this - // WiFi/TCP-only transport. - vi.useFakeTimers(); - const { result } = renderHook(() => - useHostLinkMeter({ - protocol: 'meshtastic', - connectionType: 'tcp', - status: 'configured', - hostAddress: '10.0.0.5:4403', - platform: 'darwin', - }), + it.each(['linux', 'darwin', 'win32'] as const)( + 'returns ip-rtt with no data for Meshtastic raw TCP and never opens a competing probe connection on %s', + async (platform) => { + // Meshtastic raw-TCP RTT probing used to open a second, separate connection to the + // exact same host:port as the live session every poll tick — confirmed via live + // packet capture to intermittently get the *real* session RST'd by the device + // (5/5 reproduced samples; unaffected by upstream PR #808's setNoDelay/setKeepAlive, + // which only touches the main session's socket). Do not re-enable this probe for + // 'meshtastic' + 'tcp' without deriving RTT from the already-open session instead. + // Platform-independent behavior (no OS-specific mechanism involved), so covered on + // all three platforms per project convention rather than a single-platform case. + // + // kind stays 'ip-rtt' (not 'unavailable') with null rttMs/level — same rendering as + // an HTTP probe failure ("—" + no-data bars). A dedicated 'unavailable' kind would + // incorrectly show ConnectionLinkMeter's Web-Bluetooth-specific copy on this + // WiFi/TCP-only transport. + vi.useFakeTimers(); + const { result } = renderHook(() => + useHostLinkMeter({ + protocol: 'meshtastic', + connectionType: 'tcp', + status: 'configured', + hostAddress: '10.0.0.5:4403', + platform, + }), + ); + expect(result.current.kind).toBe('ip-rtt'); + expect(result.current.rttMs).toBeNull(); + expect(result.current.level).toBeNull(); + + // Advance well past several poll intervals to confirm no deferred/interval probe fires. + await act(async () => { + await vi.advanceTimersByTimeAsync(20_000); + }); + expect(window.electronAPI.hostLink.probeTcpRtt).not.toHaveBeenCalled(); + }, + ); + + it('clears stale HTTP RTT when Meshtastic switches from HTTP to TCP', async () => { + // Render logic derives the displayed RTT from meshtasticTcpProbeUnsafe directly + // rather than trusting rttMs state, so a prior HTTP probe result can never render + // as if it were the (disabled) TCP link quality, without depending on the probe + // effect's cleanup (setRttMs(null)) having committed first. Verifies the settled + // end state; RTL's act()-wrapped rerender flushes that cleanup synchronously in + // this environment, so this does not exercise the specific single-render flash + // the derivation also guards against in a real browser (passive effects commit + // after paint there) — the render-time guard is defense in depth regardless. + const { result, rerender } = renderHook( + (props: { connectionType: 'http' | 'tcp' }) => + useHostLinkMeter({ + protocol: 'meshtastic', + connectionType: props.connectionType, + status: 'configured', + hostAddress: 'meshtastic.local', + platform: 'darwin', + }), + { initialProps: { connectionType: 'http' } }, ); + + await waitFor(() => { + expect(result.current.rttMs).toBe(40); + }); + + rerender({ connectionType: 'tcp' }); + expect(result.current.kind).toBe('ip-rtt'); expect(result.current.rttMs).toBeNull(); expect(result.current.level).toBeNull(); - - // Advance well past several poll intervals to confirm no deferred/interval probe fires. - await act(async () => { - await vi.advanceTimersByTimeAsync(20_000); - }); - expect(window.electronAPI.hostLink.probeTcpRtt).not.toHaveBeenCalled(); }); it('returns ip-rtt for MeshCore TCP/IP (http transport) via probeTcpRtt', async () => { diff --git a/src/renderer/hooks/useHostLinkMeter.ts b/src/renderer/hooks/useHostLinkMeter.ts index 88e4c314a..378f5abdf 100644 --- a/src/renderer/hooks/useHostLinkMeter.ts +++ b/src/renderer/hooks/useHostLinkMeter.ts @@ -148,12 +148,17 @@ export function useHostLinkMeter(opts: { } if (connectionType === 'http' || connectionType === 'tcp') { - // meshtasticTcpProbeUnsafe: rttMs never gets set (probe effect above skips this - // combo entirely), so this naturally renders the same "—" / no-data state as an - // HTTP probe failure — not a separate 'unavailable' kind, which would incorrectly - // show ConnectionLinkMeter's Web-Bluetooth-specific copy for a WiFi/TCP transport. - const level = rttMs != null ? rttToSignalLevel(rttMs) : null; - return { kind: 'ip-rtt', rssi: null, rttMs, level }; + // meshtasticTcpProbeUnsafe: force the displayed RTT to null rather than trusting + // rttMs state directly — on the render right after switching from Meshtastic HTTP + // to TCP, this branch runs before the probe effect's cleanup has cleared rttMs + // (effects commit after render), so a stale HTTP RTT value could otherwise flash + // as if it were the (disabled) TCP link quality. Same "—" / no-data rendering as + // an HTTP probe failure — not a separate 'unavailable' kind, which would + // incorrectly show ConnectionLinkMeter's Web-Bluetooth-specific copy on a + // WiFi/TCP-only transport. + const displayedRttMs = meshtasticTcpProbeUnsafe ? null : rttMs; + const level = displayedRttMs != null ? rttToSignalLevel(displayedRttMs) : null; + return { kind: 'ip-rtt', rssi: null, rttMs: displayedRttMs, level }; } return IDLE; From 984455b948b79d96e70cb55cb008256cf1884c53 Mon Sep 17 00:00:00 2001 From: Joe WB3IHY Date: Thu, 6 Aug 2026 14:10:16 -0400 Subject: [PATCH 3/3] docs(connection): cross-reference RST hazard note on probeTcpLinkRttMs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applying CodeRabbit's own configured review lens (.coderabbit.yaml + AGENTS.md) proactively to catch anything before its next pass: the hazard note about the unreachable 'meshtastic' TCP-probe branch was only on parseTcpProbeTarget (the inner helper); probeTcpLinkRttMs (the outer, actually-imported function) is the one a future reader would land on first. Also reviewed the diff for the same-shape stale-render race CodeRabbit found on the http/tcp connectionType switch, applied to a protocol switch (meshcore<->meshtastic) instead: not reachable in practice — ConnectionPanel/useHostLinkMeter is mounted once per protocol tab with a fixed protocol prop, confirmed via its three App.tsx call sites, so no fix needed there. --- src/renderer/lib/hostLinkQuality.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/renderer/lib/hostLinkQuality.ts b/src/renderer/lib/hostLinkQuality.ts index a1a8ba95e..f185c5d87 100644 --- a/src/renderer/lib/hostLinkQuality.ts +++ b/src/renderer/lib/hostLinkQuality.ts @@ -94,7 +94,12 @@ export async function probeHttpLinkRttMs(httpAddress: string): Promise