Conversation
Upstream closed #32; stock firmware does not push 0x86 on bad credentials (client timeout), so stop carrying the dead local handler.
Skip GPS+hop distance anomalies (hop_goblin, close-in bad_route) when hasDistanceBasedHopAnomalies is false. MeshCore nearby multi-hop contacts are poorly connected, not Meshtastic-style over-hopping. Fixes #804
Stop precipitating stale OS bonds (graceful prepare-stop, sleep/wake detach, bond-aware reconnect) and recover cleaner (halt Noble, USB unpair, transport-aware TX hints).
…hrash Keep SoftAP/OpenHop configured after contacts FIN, reopen a live TCP window for user sends, and land the remaining 24h-review hardening (i18n, docs, Reticulum bounds).
|
Warning Review limit reached
Next review available in: 27 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 (17)
📝 WalkthroughWalkthroughThis PR adds orderly Reticulum shutdown, BLE bond recovery, transport and persistence safeguards, protocol-aware diagnostics, and MeshCore TCP dead-bridge recovery. It also updates related documentation, tests, scripts, and UI actions. ChangesReliability and recovery updates
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant ReticulumSidecarManager
participant ReticulumSidecar
participant LiveBridge
App->>ReticulumSidecarManager: stop sidecar
ReticulumSidecarManager->>ReticulumSidecar: POST /api/v1/stack/prepare-stop
ReticulumSidecar->>LiveBridge: prepare_stop()
LiveBridge-->>ReticulumSidecar: shutdown complete
ReticulumSidecar-->>ReticulumSidecarManager: success or error
ReticulumSidecarManager->>ReticulumSidecar: SIGTERM
sequenceDiagram
participant useSendMessage
participant MeshcoreSessionApi
participant useMeshcoreRuntime
participant MeshCoreTCP
useSendMessage->>MeshcoreSessionApi: ensureTcpLiveForUserTx()
MeshcoreSessionApi->>useMeshcoreRuntime: recover dead bridge
useMeshcoreRuntime->>MeshCoreTCP: reconnect and await self information
MeshCoreTCP-->>useMeshcoreRuntime: live bridge
useMeshcoreRuntime-->>MeshcoreSessionApi: ready for transmission
MeshcoreSessionApi-->>useSendMessage: refreshed handle
useSendMessage->>MeshCoreTCP: send message
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: 5
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/renderer/lib/reticulum/useReticulumNobleBleYieldWatcher.ts (1)
100-106: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftCancel or serialize stale Noble-yield synchronization before bond-desync cleanup.
A tick that started before
bondDesyncActivecan pass its cancellation check, awaitsyncReticulumNobleBleYield(), and later prepare or retain the Reticulum lease. A newer bond-desync tick releases that lease, but the older tick can reacquire it after the release. This violates the no-reacquire guarantee during stale-bond recovery.
src/renderer/lib/reticulum/useReticulumNobleBleYieldWatcher.ts#L100-L106: Give each active effect anAbortController, abort it during cleanup, and pass its signal into synchronization.src/renderer/lib/reticulum/reticulumNobleBleYield.ts#L46-L64: Serialize competing synchronizations or enforce cancellation after every awaited lease operation before mutatingyieldActiveor acquiring a lease.src/renderer/lib/reticulum/reticulumNobleBleYield.test.ts#L266-L301: Add a deferred-promise regression test where a normal tick starts first, bond desync activates, and the first tick resolves afterward.🤖 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/reticulum/useReticulumNobleBleYieldWatcher.ts` around lines 100 - 106, Prevent stale Noble-yield synchronizations from reacquiring the Reticulum lease after bond-desync cleanup. In src/renderer/lib/reticulum/useReticulumNobleBleYieldWatcher.ts:100-106, create an AbortController per active effect, abort it during cleanup, and pass its signal to syncReticulumNobleBleYield. In src/renderer/lib/reticulum/reticulumNobleBleYield.ts:46-64, serialize competing synchronizations or check cancellation after every awaited lease operation before changing yieldActive or acquiring a lease. In src/renderer/lib/reticulum/reticulumNobleBleYield.test.ts:266-301, add a deferred-promise regression test where a normal tick starts before bond desync and resolves afterward, verifying the stale tick cannot reacquire the lease.Source: Path instructions
reticulum-sidecar/src/stack/lxmf_delivery.rs (1)
284-323: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound the outbound backchannel ingress too.
spawn_lxmf_outbound_backchannel()builds an unboundedouter_txand then drops messages only after forwarding them to the 256-iteminner_tx. A producer that outpaces delivery will still build an unbounded backlog inouter_rx; use a bounded admission step and add a saturation test with delivery blocked and producer traffic flooded.🤖 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 `@reticulum-sidecar/src/stack/lxmf_delivery.rs` around lines 284 - 323, Bound admission in spawn_lxmf_outbound_backchannel instead of using an unbounded outer_tx, so producer traffic is dropped once the configured capacity is reached rather than accumulating in outer_rx. Preserve the existing drop-newest behavior and saturation warning, and add a test that blocks delivery, floods the producer, and verifies the ingress queue remains bounded.Source: Path instructions
src/renderer/runtime/useMeshcoreRuntime.ts (1)
2868-2876: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winSoftAP dead-bridge state is not cleared when switching away from TCP.
meshcoreTcpBridgeDeadRef.currentandsetMeshcoreTcpSoftApDeadAccepted(false)reset only insideif (type === 'tcp'). If a TCP SoftAP session reaches the dead-bridge-accepted state and the user then connects via BLE or serial,prepareRfConnect('ble'|'serial', ...)skips this block, so both flags stay stale.
useSendMessage.tsgates its SoftAP recovery branch only onisMeshcoreTcpSoftApDeadAccepted(), with no transport check. On the next send over the new BLE/serial session, that stale flag routes the send intoensureTcpLiveForUserTx(). SincemeshcoreConnectTypeRef.currentis no longer'tcp',ensureTcpLiveForUserTx()callshandleMeshcoreConnectionLostRef.current()— forcing an unnecessary reconnect of the healthy BLE/serial session — then waits onwaitForMeshcoreTcpLiveForUserTx(), which only resolves through the TCP-onlynotifyMeshcoreTcpLiveForUserTx()call ininitConn. The wait always times out afterMESHCORE_TCP_USER_TX_LIVE_TIMEOUT_MS(20s), and the message is marked failed. This repeats for every subsequent send until the app restarts.Move the TCP-specific flag resets outside the
type === 'tcp'guard so they clear for every transport.🐛 Proposed fix: reset SoftAP/dead-bridge state for every transport
- if (type === 'tcp') { - meshcoreTcpBridgeDeadRef.current = false; - meshcoreTcpInitBurstCapturedRef.current = false; - meshcoreTcpContactsDumpInFlightRef.current = false; - setMeshcoreTcpSoftApDeadAccepted(false); - if (!opts?.preserveReconnectState) { - meshcoreDeferredReconnectRef.current = false; - } - } + meshcoreTcpBridgeDeadRef.current = false; + meshcoreTcpInitBurstCapturedRef.current = false; + meshcoreTcpContactsDumpInFlightRef.current = false; + setMeshcoreTcpSoftApDeadAccepted(false); + if (type === 'tcp' && !opts?.preserveReconnectState) { + meshcoreDeferredReconnectRef.current = false; + }🤖 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/runtime/useMeshcoreRuntime.ts` around lines 2868 - 2876, Move the resets for meshcoreTcpBridgeDeadRef.current and setMeshcoreTcpSoftApDeadAccepted(false) out of the type === 'tcp' guard in prepareRfConnect so they execute for every transport. Keep the TCP-only burst, contacts, and deferred reconnect resets inside the TCP guard, preserving the existing preserveReconnectState behavior.
🟡 Other comments (7)
src/renderer/lib/reticulum/reticulumBleBondDesync.test.ts-14-24 (1)
14-24: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winExercise the unsubscribe contract.
After
unsub(), trigger another state transition. Assert thatseendoes not change. The current test passes ifunsub()does nothing.Proposed fix
setReticulumBleBondDesyncActive(true); setReticulumBleBondDesyncActive(true); setReticulumBleBondDesyncActive(false); unsub(); + setReticulumBleBondDesyncActive(true); expect(seen).toEqual([true, false]);As per coding guidelines, “Ship a passing test for behavioral changes before considering the work complete.” As per path instructions, “Prefer behavioral assertions; skip style-only test nits.”
🤖 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/reticulum/reticulumBleBondDesync.test.ts` around lines 14 - 24, Update the test “notifies subscribers when the flag changes” to trigger an additional Reticulum bond desync state transition after calling unsub(), then assert that seen remains [true, false]. Keep the existing pre-unsubscribe notification assertions unchanged so the test verifies both subscription behavior and the unsubscribe contract.Sources: Coding guidelines, Path instructions
src/renderer/components/flasher/RNodeFlasherSection.tsx-571-573 (1)
571-573: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a renderer test for the clear paired devices flow.
Cover opening the confirmation modal, canceling without calling
clearBluetoothBonds(), confirming once withrunWithRNode, and reporting theflasher.clearPairedDevicesSuccessstatus. Include an axe assertion for the modal’s interactive role, because this new direct action changes UI behavior; the existing component-level test does not cover this workflow.🤖 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/flasher/RNodeFlasherSection.tsx` around lines 571 - 573, Add a renderer test for the clear paired devices flow in RNodeFlasherSection that exercises the onClearPairedDevices path: verify it opens the confirmation modal, canceling does not call clearBluetoothBonds(), confirming calls clearBluetoothBonds() once through runWithRNode, and the success path reports flasher.clearPairedDevicesSuccess. Include an axe assertion for the modal’s interactive role, and extend the existing component-level coverage rather than changing the implementation.Sources: Coding guidelines, Path instructions
src/renderer/components/reticulum/ReticulumInterfacesPanel.tsx-2138-2142 (1)
2138-2142: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd behavioral coverage for the BLE transport hint.
ReticulumInterfacesPanel.test.tsxdoes not assert thernodeTransportBleHintmessage behavior. Add a passing test that renders the hint for BLE transport and omitted it for serial and Wi-Fi transports before considering this change complete.🤖 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 2138 - 2142, Add behavioral coverage in ReticulumInterfacesPanel.test.tsx for the rnodeTransport conditional: assert rnodeTransportBleHint is rendered when the transport is BLE and absent for both serial and Wi-Fi transports. Use the existing panel render helpers and translation/query conventions.Sources: Coding guidelines, Path instructions
scripts/check-i18n-quality.mjs-446-447 (1)
446-447: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the bond false-friend expression Unicode-safe.
FLASHER_BT_BOND_FINANCIAL_FALSE_FRIENDSuses\b, which does not create boundaries for Cyrillic or CJK terms. Translate values such asоблигаци,債券, and债券as plain strings outside the ASCII\bgroups so these non-Latin financial bonds are caught.Proposed fix
export const FLASHER_BT_BOND_FINANCIAL_FALSE_FRIENDS = - /\b(Bonos|obligaciones|obligaties|Obligationen|облигаци|債券|债券|tahvil|obligacje|cautionnement|títulos de rádio|radiobonos)\b/i; + /(?:\b(?:Bonos|obligaciones|obligaties|Obligationen|tahvil|obligacje|cautionnement|títulos de rádio|radiobonos)\b|облигаци|債券|债券)/iu;🤖 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 `@scripts/check-i18n-quality.mjs` around lines 446 - 447, Update FLASHER_BT_BOND_FINANCIAL_FALSE_FRIENDS so non-Latin terms such as облигаци, 債券, and 债券 are matched as standalone plain-string alternatives rather than relying on ASCII \b boundaries; retain the existing boundary-wrapped handling for Latin terms and preserve case-insensitive matching.src/renderer/components/ConnectionPanel.tsx-351-352 (1)
351-352: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a
ConnectionPanelregression test for Bluetooth administration routing.Render the Reticulum connection path, raise a stale BLE-bond alert, click the Open Admin Bluetooth action, and assert that
onOpenAdminBluetoothruns. The current component tests validate the alert block in isolation. They do not validate this new prop path.As per coding guidelines, ship a passing test for behavioral changes.
Also applies to: 373-373, 3094-3094
🤖 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 351 - 352, Add a ConnectionPanel regression test covering the Reticulum connection path: render it with onOpenAdminBluetooth mocked, trigger the stale BLE-bond alert, click the Open Admin Bluetooth action, and assert the callback executes. Keep the existing isolated alert tests unchanged while validating the prop path through the rendered component.Source: Coding guidelines
src/main/reticulum-sidecar-manager.test.ts-674-681 (1)
674-681: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert prepare-stop before
SIGTERM.This test finds the POST and
SIGTERMindependently. It still passes if a future change sendsSIGTERMbefore the request.Record the call sequence and assert that the prepare-stop request occurs before
proc.kill('SIGTERM').As per path instructions, prefer behavioral assertions in
.test.tsfiles.🤖 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/reticulum-sidecar-manager.test.ts` around lines 674 - 681, Update the stop-order test around manager.stop() to record the invocation sequence for the prepare-stop fetch request and proc.kill, then assert that the POST to /api/v1/stack/prepare-stop occurs before proc.kill('SIGTERM'). Keep the existing method and call assertions while making ordering part of the behavioral verification.Sources: Coding guidelines, Path instructions
src/main/index.ts-6302-6314 (1)
6302-6314: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd regression coverage for oversized Meshtastic TCP chunks.
The
meshtastic:tcp-datadatahandler drops an oversized chunk and destroys the socket, but no main-process behavior test exercises that path. Add a test that emits a chunk larger thanMESHTASTIC_TCP_DATA_MAX_BYTESand asserts destruction, plus nomeshtastic:tcp-datapayload.🤖 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 6302 - 6314, Add regression coverage for the meshtastic:tcp-data data handler’s oversized-chunk branch: emit a chunk exceeding MESHTASTIC_TCP_DATA_MAX_BYTES, assert the associated socket is destroyed, and verify no meshtastic:tcp-data payload is emitted. Reuse the existing main-process IPC test setup and handler symbols rather than adding unrelated coverage.Sources: Coding guidelines, Path instructions
🧹 Nitpick comments (1)
src/renderer/lib/meshcore/meshcoreTcpInitBurst.ts (1)
132-144: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftFragile fixed-tick coupling between
yieldToMeshcoreTcpUserTxSendsandensureTcpLiveForUserTx.
yieldToMeshcoreTcpUserTxSendswaits exactly twoPromise.resolve()ticks before snapshottinginFlightUserTxSends. This count only works becauseuseMeshcoreRuntime.ts'sensureTcpLiveForUserTxresolves the waiter through exactly two async hops beforetrackMeshcoreTcpUserTxSendruns (waiter resolve →ensureTcpLiveForUserTxreturn →useSendMessage.tsresume → track). Add or remove a singleawaitanywhere in that chain and the snapshot silently misses the in-flight send, reintroducing the SoftAP FIN race this PR is meant to close. No test currently exercises the fullensureTcpLiveForUserTx→useSendMessage.tspath together withyieldToMeshcoreTcpUserTxSends(existing tests calltrackMeshcoreTcpUserTxSenddirectly).Add a code comment documenting the exact tick-count dependency, or replace the fixed-tick wait with an explicit signal (for example, have
ensureTcpLiveForUserTxitself register the send before resolving) so future changes cannot silently break this ordering.🤖 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/meshcore/meshcoreTcpInitBurst.ts` around lines 132 - 144, Make the ordering between yieldToMeshcoreTcpUserTxSends and ensureTcpLiveForUserTx explicit rather than relying on an undocumented two-tick delay: preferably register the user send before ensureTcpLiveForUserTx resolves its waiter, or otherwise add a precise comment documenting the required async-hop dependency. Ensure the snapshot still includes sends initiated through the full ensureTcpLiveForUserTx → useSendMessage flow.
🤖 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/reticulum-sidecar-manager.ts`:
- Around line 461-467: Update the watchdog restart path to use the tracked stop
operation exposed by stop() instead of calling stopProc() directly, ensuring
stopPromise remains set while shutdown is in progress and concurrent start()
calls cannot bypass the stop guard.
In `@src/renderer/components/ReticulumSidecarIssueAlertsBlock.tsx`:
- Around line 109-136: Update the onStopStack click handler in
ReticulumSidecarIssueAlertsBlock to handle rejected promises instead of
discarding them with void. Await or chain the action and log any failure with an
appropriate console debug, warn, or error call while preserving the existing
button behavior.
In `@src/renderer/lib/flasher/rnode.test.ts`:
- Around line 31-38: Replace the source-text assertions in the RNode Bluetooth
bond test with a behavioral test: create an RNode using createMockSerialPort(),
call clearBluetoothBonds(), and assert the serial port received the framed KISS
command containing RNode.CMD_BT_UNPAIR and 0x01.
In `@src/renderer/runtime/useReticulumRuntime.reconnect-hardening.test.ts`:
- Line 125: Replace the source-text regex assertions in the reconnect-hardening
tests with behavioral coverage of the mounted runtime. Mock Electron IPC,
exercise suspend and resume—including a forced fresh start—and assert stop, bond
release, status/IPC ordering, and message rekey effects. Update the related
assertions around the other referenced test block while preserving the existing
runtime setup.
In `@src/renderer/runtime/useReticulumRuntime.ts`:
- Around line 1524-1534: Update the coalescing branch in connect so a caller
requesting reuseIfRunning: false does not return after awaiting
connectInFlightDoneRef.current; once the pending flight settles, rerun
connect(opts) to start the required fresh connection, while preserving the
existing coalesced return for reusable calls. Add a regression test covering
suspend and resume during an in-flight connection.
---
Outside diff comments:
In `@reticulum-sidecar/src/stack/lxmf_delivery.rs`:
- Around line 284-323: Bound admission in spawn_lxmf_outbound_backchannel
instead of using an unbounded outer_tx, so producer traffic is dropped once the
configured capacity is reached rather than accumulating in outer_rx. Preserve
the existing drop-newest behavior and saturation warning, and add a test that
blocks delivery, floods the producer, and verifies the ingress queue remains
bounded.
In `@src/renderer/lib/reticulum/useReticulumNobleBleYieldWatcher.ts`:
- Around line 100-106: Prevent stale Noble-yield synchronizations from
reacquiring the Reticulum lease after bond-desync cleanup. In
src/renderer/lib/reticulum/useReticulumNobleBleYieldWatcher.ts:100-106, create
an AbortController per active effect, abort it during cleanup, and pass its
signal to syncReticulumNobleBleYield. In
src/renderer/lib/reticulum/reticulumNobleBleYield.ts:46-64, serialize competing
synchronizations or check cancellation after every awaited lease operation
before changing yieldActive or acquiring a lease. In
src/renderer/lib/reticulum/reticulumNobleBleYield.test.ts:266-301, add a
deferred-promise regression test where a normal tick starts before bond desync
and resolves afterward, verifying the stale tick cannot reacquire the lease.
In `@src/renderer/runtime/useMeshcoreRuntime.ts`:
- Around line 2868-2876: Move the resets for meshcoreTcpBridgeDeadRef.current
and setMeshcoreTcpSoftApDeadAccepted(false) out of the type === 'tcp' guard in
prepareRfConnect so they execute for every transport. Keep the TCP-only burst,
contacts, and deferred reconnect resets inside the TCP guard, preserving the
existing preserveReconnectState behavior.
---
Other comments:
In `@scripts/check-i18n-quality.mjs`:
- Around line 446-447: Update FLASHER_BT_BOND_FINANCIAL_FALSE_FRIENDS so
non-Latin terms such as облигаци, 債券, and 债券 are matched as standalone
plain-string alternatives rather than relying on ASCII \b boundaries; retain the
existing boundary-wrapped handling for Latin terms and preserve case-insensitive
matching.
In `@src/main/index.ts`:
- Around line 6302-6314: Add regression coverage for the meshtastic:tcp-data
data handler’s oversized-chunk branch: emit a chunk exceeding
MESHTASTIC_TCP_DATA_MAX_BYTES, assert the associated socket is destroyed, and
verify no meshtastic:tcp-data payload is emitted. Reuse the existing
main-process IPC test setup and handler symbols rather than adding unrelated
coverage.
In `@src/main/reticulum-sidecar-manager.test.ts`:
- Around line 674-681: Update the stop-order test around manager.stop() to
record the invocation sequence for the prepare-stop fetch request and proc.kill,
then assert that the POST to /api/v1/stack/prepare-stop occurs before
proc.kill('SIGTERM'). Keep the existing method and call assertions while making
ordering part of the behavioral verification.
In `@src/renderer/components/ConnectionPanel.tsx`:
- Around line 351-352: Add a ConnectionPanel regression test covering the
Reticulum connection path: render it with onOpenAdminBluetooth mocked, trigger
the stale BLE-bond alert, click the Open Admin Bluetooth action, and assert the
callback executes. Keep the existing isolated alert tests unchanged while
validating the prop path through the rendered component.
In `@src/renderer/components/flasher/RNodeFlasherSection.tsx`:
- Around line 571-573: Add a renderer test for the clear paired devices flow in
RNodeFlasherSection that exercises the onClearPairedDevices path: verify it
opens the confirmation modal, canceling does not call clearBluetoothBonds(),
confirming calls clearBluetoothBonds() once through runWithRNode, and the
success path reports flasher.clearPairedDevicesSuccess. Include an axe assertion
for the modal’s interactive role, and extend the existing component-level
coverage rather than changing the implementation.
In `@src/renderer/components/reticulum/ReticulumInterfacesPanel.tsx`:
- Around line 2138-2142: Add behavioral coverage in
ReticulumInterfacesPanel.test.tsx for the rnodeTransport conditional: assert
rnodeTransportBleHint is rendered when the transport is BLE and absent for both
serial and Wi-Fi transports. Use the existing panel render helpers and
translation/query conventions.
In `@src/renderer/lib/reticulum/reticulumBleBondDesync.test.ts`:
- Around line 14-24: Update the test “notifies subscribers when the flag
changes” to trigger an additional Reticulum bond desync state transition after
calling unsub(), then assert that seen remains [true, false]. Keep the existing
pre-unsubscribe notification assertions unchanged so the test verifies both
subscription behavior and the unsubscribe contract.
---
Nitpick comments:
In `@src/renderer/lib/meshcore/meshcoreTcpInitBurst.ts`:
- Around line 132-144: Make the ordering between yieldToMeshcoreTcpUserTxSends
and ensureTcpLiveForUserTx explicit rather than relying on an undocumented
two-tick delay: preferably register the user send before ensureTcpLiveForUserTx
resolves its waiter, or otherwise add a precise comment documenting the required
async-hop dependency. Ensure the snapshot still includes sends initiated through
the full ensureTcpLiveForUserTx → useSendMessage flow.
🪄 Autofix
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: 6cba939f-984f-480e-9013-14a5fb4cf7a3
⛔ Files ignored due to path filters (21)
patches/@liamcottle__meshcore.js@1.13.0.patchis excluded by!patches/**patches/README.mdis excluded by!patches/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yamlreticulum-sidecar/patches/README.mdis excluded by!reticulum-sidecar/patches/**reticulum-sidecar/patches/rsReticulum-ble-rnode-bond-desync.patchis excluded by!reticulum-sidecar/patches/**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 (66)
AGENTS.mdARCHITECTURE.mdCONTRIBUTING.mdREADME.mddocs/accessibility-checklist.mddocs/diagnostics.mddocs/meshcore-meshtastic-parity.mddocs/reticulum-games-parity.mddocs/reticulum-sidecar-ipc.mddocs/reticulum.mddocs/troubleshooting.mdreticulum-sidecar/src/api/mod.rsreticulum-sidecar/src/api/system.rsreticulum-sidecar/src/stack/games_outbound_store.rsreticulum-sidecar/src/stack/live.rsreticulum-sidecar/src/stack/lxmf_delivery.rsreticulum-sidecar/src/stack/mod.rsscripts/apply-rsReticulum-ble-rnode-bond-desync.shscripts/check-i18n-quality.mjsscripts/lib/ratspeak-overlay-apply-list.shscripts/update.shsrc/main/index.tssrc/main/ipc/reticulum-db-handlers.test.tssrc/main/ipc/reticulum-db-handlers.tssrc/main/reticulum-sidecar-manager.test.tssrc/main/reticulum-sidecar-manager.tssrc/main/reticulumSidecarIssueTracker.test.tssrc/main/reticulumSidecarIssueTracker.tssrc/renderer/App.tsxsrc/renderer/components/ConnectionPanel.tsxsrc/renderer/components/ReticulumSidecarIssueAlertsBlock.test.tsxsrc/renderer/components/ReticulumSidecarIssueAlertsBlock.tsxsrc/renderer/components/ReticulumStackPanel.test.tsxsrc/renderer/components/ReticulumStackPanel.tsxsrc/renderer/components/flasher/BluetoothConfig.tsxsrc/renderer/components/flasher/RNodeFlasherSection.tsxsrc/renderer/components/reticulum/ReticulumInterfacesPanel.tsxsrc/renderer/hooks/meshcore/meshcoreConnSideEffects.test.tssrc/renderer/hooks/meshcore/meshcoreConnSideEffects.tssrc/renderer/hooks/useSendMessage.test.tssrc/renderer/hooks/useSendMessage.tssrc/renderer/lib/diagnostics/ReticulumDiagnosticEngine.test.tssrc/renderer/lib/diagnostics/ReticulumDiagnosticEngine.tssrc/renderer/lib/diagnostics/RoutingDiagnosticEngine.test.tssrc/renderer/lib/diagnostics/RoutingDiagnosticEngine.tssrc/renderer/lib/flasher/rnode.test.tssrc/renderer/lib/flasher/rnode.tssrc/renderer/lib/ingest/reticulumIngest.test.tssrc/renderer/lib/meshcore/meshcoreTcpInitBurst.test.tssrc/renderer/lib/meshcore/meshcoreTcpInitBurst.tssrc/renderer/lib/radio/BaseRadioProvider.tssrc/renderer/lib/radio/protocol-capabilities.test.tssrc/renderer/lib/reticulum/reticulumBleBondDesync.test.tssrc/renderer/lib/reticulum/reticulumBleBondDesync.tssrc/renderer/lib/reticulum/reticulumLocalInterfaceHealth.test.tssrc/renderer/lib/reticulum/reticulumLocalInterfaceHealth.tssrc/renderer/lib/reticulum/reticulumLxmfOutboundBackchannel.contract.test.tssrc/renderer/lib/reticulum/reticulumNobleBleYield.test.tssrc/renderer/lib/reticulum/reticulumNobleBleYield.tssrc/renderer/lib/reticulum/useReticulumNobleBleYieldWatcher.tssrc/renderer/lib/sessions/meshcoreSession.tssrc/renderer/runtime/useMeshcoreRuntime.reconnect.test.tssrc/renderer/runtime/useMeshcoreRuntime.tssrc/renderer/runtime/useMeshtasticRuntime.tssrc/renderer/runtime/useReticulumRuntime.reconnect-hardening.test.tssrc/renderer/runtime/useReticulumRuntime.ts
Apply still-valid review fixes: watchdog stop guard, connect fresh-start coalesce, SoftAP latch reset, Noble yield abort, and focused regression tests.
1 participant
Summary
Multi-fix branch covering MeshCore SoftAP dead-bridge chat TX, MeshCore hop-goblin false positives (#804), Reticulum BLE RNode bond desync / TX-drop guidance, and meshcore.js patch cleanup after upstream closures.
MeshCore SoftAP / OpenHop dead-bridge TX (
fec0c830)getContacts, leaving a configured session with a dead socket — chat TX then fails while reconnect thrashing tries to “fix” it.meshcoreTcpInitBurstcoordination: keep SoftAP/OpenHop configured after contacts FIN, reopen a live TCP window for user sends, and letinitConnrelease SoftAP user-TX waiters aftergetSelfInfoso sends complete before the contacts dump/FIN race.useSendMessage+useMeshcoreRuntime/ conn side effects wired to wait on that live window instead of treating the dead bridge as a full reconnect loop.MeshCore hop goblins (
227242c3) — Fixes #804hasDistanceBasedHopAnomalies(Meshtastictrue; MeshCore/Reticulumfalse).RoutingDiagnosticEngineskipshop_goblinand close-inbad_routewhen the capability is false.Reticulum BLE RNode bond desync / TX-drop guidance (
5f618c38)rsReticulum-ble-rnode-bond-desync.patch+ apply script / update.sh watch entry.ReticulumSidecarIssueAlertsBlockbond-removed actions (Stop stack / Open Admin Bluetooth), local interface health helpers, diagnostic engine coverage, i18n across locales.meshcore.js patch hygiene
ee53b8e9— drop redundant empty-password hunk (upstream Add Node connection health to Node Info #29 closed;writeString("")already emits zero password bytes).256baf9f— drop unused LoginFail hunk (upstream NPM packages are out of date #32 closed; stock firmware does not push0x86on bad credentials).Commits (vs
main)ee53b8e9— chore: drop redundant meshcore.js empty-password patch hunk256baf9f— chore: drop unused meshcore.js LoginFail patch hunk227242c3— fix: suppress MeshCore hop goblin diagnostics (Fixes #804)5f618c38— fix(reticulum): harden BLE RNode bond desync and TX-drop guidancefec0c830— fix(meshcore): recover SoftAP dead-bridge chat TX without reconnect thrashScope: ~87 files, +2430 / −431 (includes locales, docs, sidecar overlay, tests).
Test plan
getContacts) still lands; no stuck “sending” when bridge dies after contacts dumppnpm run check:pr(or at least lint + typecheck +test:runfor touched areas); sidecarpnpm run check:reticulum-sidecarif cargo availableNotes
main(feat: TCP/HTTP RF auto-connect and MeshCore SoftAP reconnect hardening #806 / fix: MeshCore TCP SoftAP/OpenHop, Rooms/contacts, Reticulum Games/LXMF/RNode, hang detection #803).Summary by CodeRabbit