Conversation
Show Noble BLE RSSI and HTTP/TCP RTT link quality on Meshtastic/MeshCore Connection panels, plus Reticulum BLE RNode and TCP Client rows. Linux BLE shows Unavailable (Web Bluetooth).
|
Warning Review limit reached
Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: QUIET Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds connected-link quality monitoring. Noble BLE sessions emit RSSI updates, while HTTP and TCP probes measure RTT. Typed IPC APIs, renderer hooks, Reticulum maps, and connection meters expose these measurements. ChangesHost Link Quality Monitoring
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ConnectedRadio
participant UseHostLinkMeter
participant ElectronAPI
participant MainProcess
participant HostOrPeripheral
ConnectedRadio->>UseHostLinkMeter: provide connection state
UseHostLinkMeter->>ElectronAPI: subscribe to RSSI or request RTT
ElectronAPI->>MainProcess: forward IPC operation
MainProcess->>HostOrPeripheral: poll RSSI or probe HTTP/TCP
HostOrPeripheral-->>MainProcess: measurement or failure
MainProcess-->>ElectronAPI: nullable result
ElectronAPI-->>UseHostLinkMeter: update link quality state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
src/main/noble-ble-manager.test.ts-149-167 (1)
149-167: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd behavioral coverage for link RSSI polling.
The current tests only assert source-string containment. Add a fake-timer/mocked peripheral test that exercises
linkRssiPollInflight, theNOBLE_LINK_RSSI_UPDATE_TIMEOUT_MSbound, cleared links onlinkRssiPollTimer, and connect-time + periodiclinkRssiemission behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/noble-ble-manager.test.ts` around lines 149 - 167, Extend the link RSSI polling tests around NobleBleManager to use fake timers and a mocked peripheral, exercising linkRssiPollInflight and enforcing the NOBLE_LINK_RSSI_UPDATE_TIMEOUT_MS bound. Verify linkRssiPollTimer cleanup clears tracked links, and cover both the initial connect-time linkRssi emission and subsequent periodic emissions instead of relying only on SOURCE string assertions.
🧹 Nitpick comments (8)
src/renderer/hooks/useReticulumTcpLinkQualityMap.ts (1)
39-52: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the target key encoding collision-safe.
encodeTcpProbeTargetKeyjoins records with|.hostcomes from user-editable interface configuration and is only trimmed, so it can contain|. In that casedecodeTcpProbeTargetKeysplits one record into two, and the effect probes a wrong host withportNaN. UseJSON.stringifyfor the key instead.♻️ Proposed change
function encodeTcpProbeTargetKey(targets: readonly TcpProbeTarget[]): string { - return targets - .map((t) => `${t.id}\0${t.host}\0${t.port}`) - .sort() - .join('|'); + const sorted = [...targets].sort((a, b) => + `${a.id}\u0000${a.host}\u0000${a.port}`.localeCompare(`${b.id}\u0000${b.host}\u0000${b.port}`), + ); + return JSON.stringify(sorted); } function decodeTcpProbeTargetKey(targetKey: string): TcpProbeTarget[] { if (!targetKey) return []; - return targetKey.split('|').map((part) => { - const [id, host, portStr] = part.split('\0'); - return { id, host, port: Number(portStr) }; - }); + return JSON.parse(targetKey) as TcpProbeTarget[]; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/hooks/useReticulumTcpLinkQualityMap.ts` around lines 39 - 52, Update encodeTcpProbeTargetKey and decodeTcpProbeTargetKey to serialize and deserialize the target array with JSON.stringify and JSON.parse instead of delimiter-based joining and splitting. Preserve deterministic key ordering by sorting targets before serialization, and keep an empty key decoding to an empty array.src/renderer/hooks/useReticulumBleRnodeRssiMap.test.ts (1)
128-143: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that no scan request is issued.
The test name states that no scan occurs, but the assertion only checks map size. An empty map also occurs when a scan runs and returns no matching device. Add a call assertion on
proxyGetto test the stated behavior.♻️ Proposed change
expect(result.current.size).toBe(0); + expect(window.electronAPI.reticulum.proxyGet).not.toHaveBeenCalled(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/hooks/useReticulumBleRnodeRssiMap.test.ts` around lines 128 - 143, Add an assertion in the test around useReticulumBleRnodeRssiMap verifying that proxyGet is not called when all BLE RNode targets are disabled. Keep the existing empty-map assertion, and use the test’s existing proxyGet mock rather than introducing a separate request mechanism.src/renderer/lib/hostLinkQuality.ts (1)
10-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
HOST_LINK_RTT_PROBE_TIMEOUT_MSdefinition.
HOST_LINK_RTT_PROBE_TIMEOUT_MSis defined here and also insrc/main/host-link-rtt.ts(export const HOST_LINK_RTT_PROBE_TIMEOUT_MS = 3 * MS_PER_SECOND;). Two independent declarations of the same cross-process timing contract can drift out of sync if one file changes and the other does not. Define the constant once in a shared module (for examplesrc/shared/timeConstants.ts) and import it in both the main-process probe and this renderer module.As per coding guidelines, "Reuse shared validation helpers, including
clampTcpPort()and time constants derived fromMS_PER_SECOND; do not duplicate inline parsers or clamps."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/lib/hostLinkQuality.ts` around lines 10 - 11, Move HOST_LINK_RTT_PROBE_TIMEOUT_MS into a shared time-constants module, then import and reuse that symbol in both hostLinkQuality and the main-process host-link RTT probe. Remove the duplicate local declarations while preserving the existing 3-second timeout contract and MS_PER_SECOND-based definition.Source: Coding guidelines
src/renderer/hooks/useHostLinkMeter.ts (1)
94-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDirect
protocol === 'meshcore'comparisons instead ofProtocolCapabilities. Both files route logic by hardcoded protocol string comparison rather than a capability-driven check, against the stated guideline to gate onProtocolCapabilities/useRadioProvider(protocol).
src/renderer/hooks/useHostLinkMeter.ts#L94-L103:run()branches onprotocol === 'meshtastic'andprotocol === 'meshcore'to pick the HTTP vs. TCP probe; replace with a per-protocol probe-config lookup (e.g., a map keyed by protocol providing the probe function and default port).src/renderer/hooks/useHostLinkMeter.ts#L53-L56:activeexcludes reticulum viaprotocol !== 'reticulum'; derive this from a capability flag instead of a protocol-name comparison.src/renderer/hooks/useHostLinkMeter.ts#L68:sessionIdis derived viaprotocol === 'meshcore' ? 'meshcore' : 'meshtastic'; source the session id from protocol metadata instead of an inline ternary.src/renderer/lib/hostLinkQuality.ts#L57-L71:parseTcpProbeTargetselectsparseTcpAddressvs.parseMeshtasticTcpAddressviaprotocol === 'meshcore'; route through the same protocol-config lookup used above.As per coding guidelines, "Use
ProtocolCapabilitiesanduseRadioProvider(protocol)for feature gating; do not compareprotocol === 'meshcore'."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/hooks/useHostLinkMeter.ts` around lines 94 - 103, Replace hardcoded protocol comparisons with a shared protocol configuration/capability lookup. In src/renderer/hooks/useHostLinkMeter.ts:94-103, select the probe and default port from that configuration; at :53-56, derive active from the reticulum capability; and at :68, obtain sessionId from protocol metadata. In src/renderer/lib/hostLinkQuality.ts:57-71, use the same configuration to select the TCP target parser, preserving the existing per-protocol behavior without inline protocol-name checks.Source: Coding guidelines
src/main/noble-ble-manager.ts (1)
128-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
MS_PER_SECOND-derived values for the new time constants.
NOBLE_LINK_RSSI_POLL_MSandNOBLE_LINK_RSSI_UPDATE_TIMEOUT_MSuse raw millisecond literals (4_000,5_000).src/main/host-link-rtt.tsderives its timeout fromMS_PER_SECOND(3 * MS_PER_SECOND) in the same PR. Use the same pattern here for consistency.♻️ Proposed fix
+import { MS_PER_SECOND } from '../shared/timeConstants'; + /** Host↔radio BLE RSSI poll while GATT is connected (Connection panel meter). */ -export const NOBLE_LINK_RSSI_POLL_MS = 4_000; +export const NOBLE_LINK_RSSI_POLL_MS = 4 * MS_PER_SECOND; /** Bound a single updateRssiAsync so a hung stack cannot stall the poll loop. */ -const NOBLE_LINK_RSSI_UPDATE_TIMEOUT_MS = 5_000; +const NOBLE_LINK_RSSI_UPDATE_TIMEOUT_MS = 5 * MS_PER_SECOND;As per coding guidelines, "Reuse shared validation helpers, including
clampTcpPort()and time constants derived fromMS_PER_SECOND; do not duplicate inline parsers or clamps."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/noble-ble-manager.ts` around lines 128 - 131, Update the time constants near the BLE RSSI poll declarations, NOBLE_LINK_RSSI_POLL_MS and NOBLE_LINK_RSSI_UPDATE_TIMEOUT_MS, to derive their values from the shared MS_PER_SECOND constant instead of raw millisecond literals, preserving the existing 4-second and 5-second durations.Source: Coding guidelines
src/renderer/components/ConnectionPanel.tsx (1)
419-438: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSimplify the duplicated host-address ternary.
hostAddressin theuseHostLinkMetercall recomputes the same branching logic asactiveHostAddress(lines 419-424), just keyed offstate.connectionTypeinstead of the localconnectionTypeselector. Because the connection-type selector is hidden once connected, andstate.connectionTypealways matches the localconnectionTypeused to establish the connection, these two expressions resolve to the same value in every reachable case:
state.connectionType === 'tcp'→ both usetcpAddress.state.connectionType === 'http'→ both use${tcpHost}:${tcpPort}for meshcore,httpAddressfor meshtastic.Keep one source of truth to avoid the two ternaries silently diverging after a future edit.
Confirm the equivalence holds across the auto-connect and reconnect flows before applying, since this component has many state-transition paths.
♻️ Proposed simplification
const hostLinkMeter = useHostLinkMeter({ protocol, connectionType: state.connectionType, status: state.status, - hostAddress: - state.connectionType === 'tcp' - ? tcpAddress - : state.connectionType === 'http' - ? protocol === 'meshcore' - ? `${tcpHost}:${tcpPort}` - : httpAddress - : activeHostAddress, + hostAddress: activeHostAddress, platform: window.electronAPI.getPlatform() as NodeJS.Platform, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/components/ConnectionPanel.tsx` around lines 419 - 438, Use the existing activeHostAddress value as the hostAddress passed to useHostLinkMeter, removing the duplicated state.connectionType/protocol ternary. Confirm auto-connect and reconnect flows preserve the established address selection, including TCP, HTTP, and meshcore cases.src/renderer/components/reticulum/ReticulumInterfacesPanel.tsx (2)
2185-2226: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd role="group" to match the established accessible-name pattern.
Both status
<span>elements setaria-labelwithout an ARIA role. A<span>defaults to role "generic" in the accessibility tree, and generic-role elements are not guaranteed to exposearia-labelas their accessible name to assistive technology.ConnectionLinkMeter.tsxusesrole="group"together witharia-labelfor the equivalent BLE/RTT status indicator. Match that pattern here so the accessible name is reliably exposed.Confirm actual screen-reader behavior for aria-label on role-less spans before deciding this is required, since browser/AT handling of this case varies.
♿ Proposed fix
{showBleRnodeSignal ? ( <span + role="group" className="text-muted flex shrink-0 items-center gap-1 text-xs" aria-label={t('connectionPanel.hostSignal')} data-testid={`reticulum-ble-signal-${iface.id}`} >{showTcpLinkQuality ? ( <span + role="group" className="text-muted flex shrink-0 items-center gap-1 text-xs" aria-label={t('connectionPanel.linkQuality')} data-testid={`reticulum-tcp-link-${iface.id}`} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/components/reticulum/ReticulumInterfacesPanel.tsx` around lines 2185 - 2226, Confirm the current screen-reader behavior for aria-label on role-less status spans, then update the BLE and TCP status spans in the Reticulum interfaces panel only if needed to ensure their labels are exposed. If required, add role="group" alongside aria-label to both spans, matching the established ConnectionLinkMeter pattern.
2185-2226: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated link-meter markup instead of reusing ConnectionLinkMeter.
This block reimplements the same "SignalBars + rounded value, or SignalBars noData + unavailable label" pattern that
ConnectionLinkMeter.tsxalready defines for BLE RSSI and IP RTT. Both implementations independently callMath.round(), the sameconnectionPanel.bleRssiDbm/connectionPanel.linkQualityMs/connectionPanel.hostSignalUnavailable/connectionPanel.linkQualityUnavailablekeys, andrttToSignalLevel. A future change to rounding, level mapping, or i18n keys in one location can silently diverge from the other.Extract the inner "bars + label" content (independent from ConnectionLinkMeter's outer
flex justify-betweenrow wrapper, which doesn't fit this compact list-row layout) into a small shared presentational helper that bothConnectionLinkMeterand this panel can call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/components/reticulum/ReticulumInterfacesPanel.tsx` around lines 2185 - 2226, Extract the shared “SignalBars plus value or unavailable label” rendering from ConnectionLinkMeter into a small presentational helper, preserving its existing rounding, rttToSignalLevel mapping, and translation keys. Update both ConnectionLinkMeter and the ReticulumInterfacesPanel block using showBleRnodeSignal/showTcpLinkQuality to call the helper, while keeping ConnectionLinkMeter’s outer flex/justify-between wrapper separate from the compact panel layout.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/index.ts`:
- Around line 6358-6365: The hostLink:probeTcpRtt handler currently duplicates
TCP port validation inline. Replace that condition with the shared
clampTcpPort() helper, preserving rejection of invalid inputs and passing the
clamped valid port to probeTcpRttMs.
In `@src/renderer/hooks/useHostLinkMeter.ts`:
- Around line 80-117: Update the polling effect’s run function to use a
generation counter for probe requests: increment it whenever a probe is issued,
capture that generation, and only apply the result when it matches the latest
generation and the effect is not cancelled. Keep the existing interval, cleanup,
and transport-specific probe selection unchanged.
---
Other comments:
In `@src/main/noble-ble-manager.test.ts`:
- Around line 149-167: Extend the link RSSI polling tests around NobleBleManager
to use fake timers and a mocked peripheral, exercising linkRssiPollInflight and
enforcing the NOBLE_LINK_RSSI_UPDATE_TIMEOUT_MS bound. Verify linkRssiPollTimer
cleanup clears tracked links, and cover both the initial connect-time linkRssi
emission and subsequent periodic emissions instead of relying only on SOURCE
string assertions.
---
Nitpick comments:
In `@src/main/noble-ble-manager.ts`:
- Around line 128-131: Update the time constants near the BLE RSSI poll
declarations, NOBLE_LINK_RSSI_POLL_MS and NOBLE_LINK_RSSI_UPDATE_TIMEOUT_MS, to
derive their values from the shared MS_PER_SECOND constant instead of raw
millisecond literals, preserving the existing 4-second and 5-second durations.
In `@src/renderer/components/ConnectionPanel.tsx`:
- Around line 419-438: Use the existing activeHostAddress value as the
hostAddress passed to useHostLinkMeter, removing the duplicated
state.connectionType/protocol ternary. Confirm auto-connect and reconnect flows
preserve the established address selection, including TCP, HTTP, and meshcore
cases.
In `@src/renderer/components/reticulum/ReticulumInterfacesPanel.tsx`:
- Around line 2185-2226: Confirm the current screen-reader behavior for
aria-label on role-less status spans, then update the BLE and TCP status spans
in the Reticulum interfaces panel only if needed to ensure their labels are
exposed. If required, add role="group" alongside aria-label to both spans,
matching the established ConnectionLinkMeter pattern.
- Around line 2185-2226: Extract the shared “SignalBars plus value or
unavailable label” rendering from ConnectionLinkMeter into a small
presentational helper, preserving its existing rounding, rttToSignalLevel
mapping, and translation keys. Update both ConnectionLinkMeter and the
ReticulumInterfacesPanel block using showBleRnodeSignal/showTcpLinkQuality to
call the helper, while keeping ConnectionLinkMeter’s outer flex/justify-between
wrapper separate from the compact panel layout.
In `@src/renderer/hooks/useHostLinkMeter.ts`:
- Around line 94-103: Replace hardcoded protocol comparisons with a shared
protocol configuration/capability lookup. In
src/renderer/hooks/useHostLinkMeter.ts:94-103, select the probe and default port
from that configuration; at :53-56, derive active from the reticulum capability;
and at :68, obtain sessionId from protocol metadata. In
src/renderer/lib/hostLinkQuality.ts:57-71, use the same configuration to select
the TCP target parser, preserving the existing per-protocol behavior without
inline protocol-name checks.
In `@src/renderer/hooks/useReticulumBleRnodeRssiMap.test.ts`:
- Around line 128-143: Add an assertion in the test around
useReticulumBleRnodeRssiMap verifying that proxyGet is not called when all BLE
RNode targets are disabled. Keep the existing empty-map assertion, and use the
test’s existing proxyGet mock rather than introducing a separate request
mechanism.
In `@src/renderer/hooks/useReticulumTcpLinkQualityMap.ts`:
- Around line 39-52: Update encodeTcpProbeTargetKey and decodeTcpProbeTargetKey
to serialize and deserialize the target array with JSON.stringify and JSON.parse
instead of delimiter-based joining and splitting. Preserve deterministic key
ordering by sorting targets before serialization, and keep an empty key decoding
to an empty array.
In `@src/renderer/lib/hostLinkQuality.ts`:
- Around line 10-11: Move HOST_LINK_RTT_PROBE_TIMEOUT_MS into a shared
time-constants module, then import and reuse that symbol in both hostLinkQuality
and the main-process host-link RTT probe. Remove the duplicate local
declarations while preserving the existing 3-second timeout contract and
MS_PER_SECOND-based definition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Pro Plus
Run ID: 971df5f9-fa2f-414c-8745-c00283f4348f
⛔ Files ignored due to path filters (16)
src/renderer/locales/cs/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/de/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/en/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/es/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/fr/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/id/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/it/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ja/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ko/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/nl/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/pl/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/pt-BR/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ru/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/tr/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/uk/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/zh/translation.jsonis excluded by!src/renderer/locales/**
📒 Files selected for processing (28)
src/main/host-link-rtt.test.tssrc/main/host-link-rtt.tssrc/main/index.contract.test.tssrc/main/index.tssrc/main/noble-ble-manager.test.tssrc/main/noble-ble-manager.tssrc/preload/index.tssrc/renderer/App.test.tsxsrc/renderer/components/ConnectionLinkMeter.test.tsxsrc/renderer/components/ConnectionLinkMeter.tsxsrc/renderer/components/ConnectionPanel.hostLinkMeter.test.tsxsrc/renderer/components/ConnectionPanel.tsxsrc/renderer/components/SignalBars.test.tsxsrc/renderer/components/SignalBars.tsxsrc/renderer/components/reticulum/ReticulumInterfacesPanel.test.tsxsrc/renderer/components/reticulum/ReticulumInterfacesPanel.tsxsrc/renderer/hooks/hostLinkQuality.probes.test.tssrc/renderer/hooks/useHostLinkMeter.test.tssrc/renderer/hooks/useHostLinkMeter.tssrc/renderer/hooks/useReticulumBleRnodeRssiMap.test.tssrc/renderer/hooks/useReticulumBleRnodeRssiMap.tssrc/renderer/hooks/useReticulumTcpLinkQualityMap.test.tssrc/renderer/hooks/useReticulumTcpLinkQualityMap.tssrc/renderer/lib/devElectronApiStub.tssrc/renderer/lib/hostLinkQuality.test.tssrc/renderer/lib/hostLinkQuality.tssrc/renderer/vitest.electronApiMock.tssrc/shared/electron-api.types.ts
| ipcMain.handle('hostLink:probeTcpRtt', async (event, host: unknown, port: unknown) => { | ||
| assertIpcSender(event, 'hostLink:probeTcpRtt'); | ||
| validateHttpHost(host); | ||
| if (!Number.isInteger(port) || (port as number) < 1 || (port as number) > 65535) { | ||
| throw new Error('Invalid port'); | ||
| } | ||
| return probeTcpRttMs(host, port as number); | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Locate clampTcpPort() and confirm renderer callers of hostLink.probeTcpRtt pass a bare host.
rg -n -C3 'function clampTcpPort|export.*clampTcpPort' --type=ts
rg -n -C5 'probeTcpRtt\(' src/rendererRepository: Colorado-Mesh/mesh-client
Length of output: 163
🏁 Script executed:
#!/bin/bash
set -u
echo "== files =="
git ls-files | sed -n '1,120p'
echo "== clampTcpPort candidates =="
rg -n "clampTcpPort" . || true
echo "== hostLink probe handlers and relevant helpers =="
rg -n -C4 "probeTcpRtt|probeHttpRtt|validateHttpHost|hostLink:probeTcpRtt|hostLink:probeHttpRtt|clampTcpPort" src/main src/shared src/renderer || true
echo "== renderer probeTcpRtt call sites =="
rg -n -C5 "\.probeTcpRtt|probeTcpRtt\(" . || trueRepository: Colorado-Mesh/mesh-client
Length of output: 50381
Use clampTcpPort() for the TCP probe port validation.
clampTcpPort() is the shared TCP-port helper in src/shared/tcpPort.ts; replace the inline Number.isInteger(port) || port < 1 || port > 65535 check with it to satisfy the shared-validation guideline.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from 'child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/index.ts` around lines 6358 - 6365, The hostLink:probeTcpRtt handler
currently duplicates TCP port validation inline. Replace that condition with the
shared clampTcpPort() helper, preserving rejection of invalid inputs and passing
the clamped valid port to probeTcpRttMs.
Source: Coding guidelines
Gate HTTP/TCP meter updates on a probe generation so a slower older poll cannot overwrite a newer RTT after overlapping interval runs.
Exclude Dependabot/Renovate auto-reviews and ignore common lockfile and generated paths alongside the existing Free-tier filters.
1 participant
Summary
Unavailable (Web Bluetooth)for Linux BLE—when RSSI is unknown)interfaces[]identity churn cannot restart probes in a loopTest plan
Unavailable (Web Bluetooth)(no fake bars)—or dBm); enabled TCP Client shows Link quality; disabled/serial rows hide meterspnpm exec vitest runon host-link / ConnectionPanel.hostLinkMeter / ReticulumInterfacesPanel meter testsSummary by CodeRabbit
New Features
Bug Fixes
Tests