fix: MeshMapper host, MQTT channel map, and MeshCore BLE ghost nodes - #781
Conversation
Migrate mqtt.meshmapper.cc to .net (TLS), let radio topic indices win over manual LongFast@0 for Colorado layouts, expose channelNameToIndex in debug snapshots, and suppress Meshtastic last_heard for the connected MeshCore BLE MAC.
|
Warning Review limit reached
Next review available in: 41 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 (10)
📝 WalkthroughWalkthroughThe change updates MeshMapper host migration, MQTT channel attribution and diagnostics, MeshCore BLE node suppression, and MQTT TLS error classification. It adds IPC and renderer APIs, storage migration logic, runtime state tracking, debug snapshot fields, documentation, and regression tests. ChangesMQTT channel attribution and diagnostics
MeshMapper host migration
MeshCore BLE node suppression
MQTT TLS error hints
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Renderer
participant Preload
participant IPC
participant MQTTManager
participant DebugSnapshot
Renderer->>Preload: getChannelNameToIndex()
Preload->>IPC: invoke mqtt:getChannelNameToIndex
IPC->>MQTTManager: getChannelNameToIndex()
MQTTManager-->>IPC: sanitized channel mapping
IPC-->>Renderer: channel mapping
Renderer->>DebugSnapshot: store mapping in Meshtastic context
DebugSnapshot-->>Renderer: include mapping in debug snapshot
sequenceDiagram
participant MeshCoreRuntime
participant BLEState
participant MeshtasticRuntime
participant MeshtasticSideEffects
MeshCoreRuntime->>BLEState: store connected BLE MAC
MeshtasticRuntime->>BLEState: read connected BLE MAC
MeshtasticRuntime->>MeshtasticRuntime: compare node ID with BLE-derived ID
MeshtasticRuntime-->>MeshtasticSideEffects: suppress matching node event
MeshtasticSideEffects-->>MeshtasticRuntime: skip signal and persistence updates
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 (3)
src/main/mqtt-manager.test.ts-1369-1369 (1)
1369-1369: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the
anycasts from the direct MQTTManager private accesses.Inlines 1360, 1369, 1380, 1392, 1424, 1432, and 1465 cast through
anyto reach_doConnect,channelNameToIndex, andonMessage. Use test-only interfaces with narrowed member shapes and cast throughunknownfor these internal accesses. KeepgetChannelNameToIndex()for public channel-index reads.🤖 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/mqtt-manager.test.ts` at line 1369, Replace the any casts used for direct MQTTManager private accesses in the affected tests with test-only interfaces describing the needed _doConnect, channelNameToIndex, and onMessage members, casting through unknown to those interfaces. Preserve getChannelNameToIndex() for public channel-index reads and update all referenced access sites consistently.Source: Coding guidelines
src/renderer/lib/debugSnapshotMeshtasticContext.test.ts-1-1 (1)
1-1: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore the jsdom test environment.
The change removes the file-level jsdom directive. This file matches
src/renderer/**/*.test.{ts,tsx}. Keep the directive or configure jsdom for this test file.As per coding guidelines, renderer tests must use jsdom. As per path instructions, renderer tests must use jsdom.
🤖 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/debugSnapshotMeshtasticContext.test.ts` at line 1, Restore the file-level jsdom environment directive in debugSnapshotMeshtasticContext.test.ts, or otherwise configure this test file to run under jsdom, while leaving the existing Vitest imports and test behavior unchanged.Sources: Coding guidelines, Path instructions
src/renderer/lib/debugSnapshotMeshtasticContext.test.ts-25-29 (1)
25-29: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd coverage for the known-empty map state.
The contract distinguishes
nullfor an unknown map from{}for a known empty map. These tests covernulland a non-empty map only. Add a behavioral assertion that sets{}and verifies that the builder preserves{}.As per coding guidelines, behavioral changes must ship with a passing test. As per path instructions, tests should prefer behavioral assertions.
🤖 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/debugSnapshotMeshtasticContext.test.ts` around lines 25 - 29, Add a behavioral test alongside the existing mqttChannelNameToIndex tests that calls setDebugSnapshotMeshtasticContext with an empty map, then builds the context via buildDebugSnapshotMeshtasticContextFromRuntime and asserts mqttChannelNameToIndex remains {} rather than becoming null or another fallback.Sources: Coding guidelines, Path instructions
🧹 Nitpick comments (1)
src/renderer/lib/meshcoreMqttUserHint.test.ts (1)
38-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise each TLS matcher independently.
The test input contains all three patterns. Because the production condition uses
||,EPROTOmatches first. The test does not verifyTLSV1_ALERT_INTERNAL_ERRORorOPENSSL_internal:TLSV1_ALERTindependently.Add separate inputs for each pattern.
Suggested test change
it('returns tlsHandshake hint for EPROTO / TLSV1_ALERT_INTERNAL_ERROR', () => { - const out = meshcoreMqttUserFacingHint( - 'write EPROTO 30458176:error:10000438:SSL routines:OPENSSL_internal:TLSV1_ALERT_INTERNAL_ERROR:../../third_party/boringssl/src/ssl/tls_record.cc:486:SSL alert number 80', - ); - expect(meshcoreUserMessageKey(out)).toBe('meshcore.mqttHints.tlsHandshake'); + for (const message of [ + 'write EPROTO', + 'TLSV1_ALERT_INTERNAL_ERROR', + 'OPENSSL_internal:TLSV1_ALERT', + ]) { + expect( + meshcoreUserMessageKey(meshcoreMqttUserFacingHint(message)), + ).toBe('meshcore.mqttHints.tlsHandshake'); + } });As per path instructions,
**/*.test.tstests should prefer behavioral assertions.🤖 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/meshcoreMqttUserHint.test.ts` around lines 38 - 44, Update the test for meshcoreMqttUserFacingHint to exercise each TLS matcher independently: add separate behavioral cases whose inputs contain only EPROTO, only TLSV1_ALERT_INTERNAL_ERROR, and only OPENSSL_internal:TLSV1_ALERT, and assert each returns the tlsHandshake user-message key.Source: Path instructions
🤖 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/renderer/lib/meshtastic/meshtasticRawPacketSideEffects.ts`:
- Around line 82-84: Move the shouldSuppressMeshtasticNodeHear guard from
applySignalAndHops() into handleRawPacket(), after validating from and after
deps.touchLastData() but before raw-packet diagnostics, packet-path recording,
or setSignalTelemetry(). Ensure suppressed packets return before all packet-wide
side effects, and add assertions verifying diagnostics and setSignalTelemetry()
are not called for them.
In `@src/renderer/runtime/useMeshcoreRuntime.ts`:
- Line 3069: Update the MeshCore connection lifecycle around
attemptMeshcoreReconnect(), handleMeshcoreConnectionLost(), and successful BLE
attachment to consistently track the canonical BLE MAC or node identity across
Linux, macOS, and Windows. Set the identity after every successful BLE attach,
including Linux Web Bluetooth connections without blePeripheralId, and clear it
on connection loss or failed replacement so stale values cannot suppress
Meshtastic updates. Add coverage for loss, reconnect, failed replacement, and
Linux Web Bluetooth.
---
Other comments:
In `@src/main/mqtt-manager.test.ts`:
- Line 1369: Replace the any casts used for direct MQTTManager private accesses
in the affected tests with test-only interfaces describing the needed
_doConnect, channelNameToIndex, and onMessage members, casting through unknown
to those interfaces. Preserve getChannelNameToIndex() for public channel-index
reads and update all referenced access sites consistently.
In `@src/renderer/lib/debugSnapshotMeshtasticContext.test.ts`:
- Line 1: Restore the file-level jsdom environment directive in
debugSnapshotMeshtasticContext.test.ts, or otherwise configure this test file to
run under jsdom, while leaving the existing Vitest imports and test behavior
unchanged.
- Around line 25-29: Add a behavioral test alongside the existing
mqttChannelNameToIndex tests that calls setDebugSnapshotMeshtasticContext with
an empty map, then builds the context via
buildDebugSnapshotMeshtasticContextFromRuntime and asserts
mqttChannelNameToIndex remains {} rather than becoming null or another fallback.
---
Nitpick comments:
In `@src/renderer/lib/meshcoreMqttUserHint.test.ts`:
- Around line 38-44: Update the test for meshcoreMqttUserFacingHint to exercise
each TLS matcher independently: add separate behavioral cases whose inputs
contain only EPROTO, only TLSV1_ALERT_INTERNAL_ERROR, and only
OPENSSL_internal:TLSV1_ALERT, and assert each returns the tlsHandshake
user-message key.
🪄 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: 8a8346d2-7591-4c42-81e0-af81f39f139f
⛔ 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 (27)
docs/meshcore-meshtastic-parity.mddocs/troubleshooting.mdsrc/main/index.ipc-security.test.tssrc/main/index.tssrc/main/mqtt-manager.test.tssrc/main/mqtt-manager.tssrc/preload/index.tssrc/renderer/lib/connectedMeshcoreBleMac.tssrc/renderer/lib/connectionPanelStorageMigrations.test.tssrc/renderer/lib/connectionPanelStorageMigrations.tssrc/renderer/lib/debugSnapshot.test.tssrc/renderer/lib/debugSnapshot.tssrc/renderer/lib/debugSnapshotMeshtasticContext.test.tssrc/renderer/lib/debugSnapshotMeshtasticContext.tssrc/renderer/lib/letsMeshJwt.test.tssrc/renderer/lib/letsMeshJwt.tssrc/renderer/lib/meshcoreBleMacMeshtasticNodeId.test.tssrc/renderer/lib/meshcoreBleMacMeshtasticNodeId.tssrc/renderer/lib/meshcoreMqttPresets.test.tssrc/renderer/lib/meshcoreMqttUserHint.test.tssrc/renderer/lib/meshcoreMqttUserHint.tssrc/renderer/lib/meshtastic/meshtasticRawPacketSideEffects.test.tssrc/renderer/lib/meshtastic/meshtasticRawPacketSideEffects.tssrc/renderer/runtime/useMeshcoreRuntime.tssrc/renderer/runtime/useMeshtasticRuntime.tssrc/renderer/vitest.electronApiMock.tssrc/shared/electron-api.types.ts
Move MeshCore MAC suppress earlier in raw-packet handling, clear/restore the connected BLE identity across loss/reconnect/Linux Web BT, and tighten related tests (typed MQTT private access, empty channel map, per-matcher TLS hints).
1 participant
Summary
Fixes from a win32 5.25.0 developer support bundle (Nathan / Colorado Mesh layout) covering three product bugs plus triage tooling. There is no Meshtastic↔MeshCore chat bridge in mesh-client; “bridging” failures in that report were MeshCore MQTT uplink / packet logger blocked by MeshMapper TLS.
1. Meshtastic MQTT → wrong channel tab
Symptom: With radio layout
ch0=OnTrail(private),ch1=unnamed default public (AQ==/ LongFast),ch2=cm-west-slp, MQTT text landed on channel 0 from ~2026-08-01 onward (previously on ch1). Logs had zeromqtt-channel-topic-mismatch/mqtt-unknown-channel-name— topic…/e/LongFast/…resolved as LongFast→0, matching gatewayMeshPacket.channel0.Cause: Manual Connection Panel lines like
LongFast@0=…(or early hardcoded LongFast→0 before radio sync) could stick inchannelNameToIndex. Radio sync inupdateChannelKeyspreserved manual index mappings when applying radio keys, so Colorado’s public-on-slot-1 layout never overwrote the map.Fix: After applying manual PSK lines, always re-apply radio topic indices so local RF layout is source of truth for inbound MQTT attribution. Manual PSKs are still preserved; only the name→slot map for topics like LongFast is forced from the radio. Bare/
LongFast=without@indexand Garber-style named-key PSK preservation remain covered by regression tests.Triage: Expose sanitized main-process
channelNameToIndex(no PSKs) viamqtt:getChannelNameToIndexIPC andmqttChannelNameToIndexon Export for GitHub / developer debug snapshots (pushed afterupdateChannelKeysfromuseMeshtasticRuntime).2. MeshMapper MQTT TLS — migrate
.cc→.netSymptom: Exact screenshot error on
wss://mqtt.meshmapper.cc:443/ws(EPROTO/TLSV1_ALERT_INTERNAL_ERROR). Same session: LetsMesh + Colorado Mesh CONNACK OK; Meshtastic MQTT tomqtt.meshtastic.orgOK. Wiki hostmqtt.meshmapper.networks;.ccfails TLS (SNI/cert). Client still dialed.ccsince #265.Fix:
MESHMAPPER_HOST = 'mqtt.meshmapper.net'(legacy.cckept only as a migrate-from alias).server === mqtt.meshmapper.cc.TLSV1_ALERT_INTERNAL_ERROR/ EPROTO tomeshcore.mqttHints.tlsHandshake(points users at.net).3. Meshtastic “Blue” ghost while MeshCore BLE is connected
Symptom: Meshtastic Nodes showed Blue
!e3da2e2fJust now, hops 0, MQTT-, same long name as self/Blck. Hardware was MeshCore-flashed; BLE MACcc:2e:e3:da:2e:2f→ lower 32 bits =0xe3da2e2f. Not a wrong-tab / MeshCore-list mix-up — co-channel hear kept refreshing a stale Meshtastic NodeDB row.Fix:
meshcoreBleMacToMeshtasticNodeId/shouldSuppressMeshtasticNodeHear; moduleconnectedMeshcoreBleMacset/cleared fromuseMeshcoreRuntimeon BLE connect/disconnect.last_heardbumps when the node id matches the currently connected MeshCore BLE MAC:applySignalAndHopsinmeshtasticRawPacketSideEffectsonNodeUpdate/ ensure-node paths inuseMeshtasticRuntimeDocs / i18n
docs/troubleshooting.md— MQTT channel map + dual-radio MAC ghost notesdocs/meshcore-meshtastic-parity.md— MeshMapper hostOut of scope
Test plan
mqttChannelNameToIndex, MAC helper, raw-packet suppress / non-suppressvitest run --changed) green on this branchLongFast@0=→ inbound LongFast text on channel 1 after radio configure.ccsettings → dials.net, WSS connects; packet logger can publish when MQTT shows connectedmqttChannelNameToIndex(e.g.{ "LongFast": 1 }) and no PSKsSummary by CodeRabbit
New Features
Bug Fixes
.nethost, with automatic migration from.cc.