fix: BLE/DB/log hygiene, MeshCore parity, RNCP, ESLint gates, drop Sonar - #734
Merged
Conversation
Stop Meshtastic BLE reconnect flaps from overlapping opens, run retention for all protocols regardless of last-active tab, default sidecar RUST_LOG to warn with stdout filtering, and repair all NULL message statuses.
Keep ASCII/Unicode box borders aligned by using white-space: pre/pre-wrap on Nomad pages, and mirror aria-labels as title tooltips on browser chrome buttons.
… gaps Move advertised position into Position/GPS, wire Send Position and reboot, show Diagnostics foreign-LoRa on the MeshCore tab, and refresh the parity matrix.
Persist save-folder prefs, apply receive-dest shares without pending gate, show transfer progress in Chat DM, and only save addresses when Remember is checked.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR updates protocol-scoped diagnostics, database repair and pruning, Reticulum sidecar logging, RNCP destination sharing and transfers, Meshtastic reconnect handling, MeshCore controls, runtime validation, developer checks, localization, tests, and documentation. ChangesProtocol diagnostics and maintenance
Runtime and UI behavior
Developer workflow and validation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
Enable prod no-unsafe-* and shared/lib no-unnecessary-condition with a full cleanup, path-gate strict-shared typecheck, run full-feature sidecar Clippy/tests on staged sidecar changes, and add check:pr plus branch-scoped pre-push Vitest.
Drop Autoscan property files that drove PR cleanup loops, and add a quiet CodeRabbit config with path filters and auto-pause to conserve Free rate limits.
rinchen
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/components/DiagnosticsPanel.tsx (1)
233-267: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winScope the repeater-conflict banner to the active listener.
meshcoreRepeaterConflictstill matchesmeshtasticListenerNodeId, while these tables now useforeignLoraListenerNodeId. On the MeshCore tab this can hide a matching warning or show one from the Meshtastic listener. Match the diagnostic row againstforeignLoraListenerNodeIdinstead.🤖 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/DiagnosticsPanel.tsx` around lines 233 - 267, Update the diagnosticRows check in meshcoreRepeaterConflict to compare each RF row’s nodeId with foreignLoraListenerNodeId instead of meshtasticListenerNodeId, keeping the existing condition and conflict matching unchanged.
🧹 Nitpick comments (7)
src/main/db-schema-sync.ts (1)
828-834: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueName no longer matches behavior.
repairMeshtasticInboundNullStatusnow repairs everystatus IS NULLrow, including outbound rows that never got a device ack, and stamps them'acked'. Consider renaming (e.g.repairMeshtasticNullMessageStatus) and adding a short comment on why blanket'acked'is safe for outbound rows.🤖 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/db-schema-sync.ts` around lines 828 - 834, The function name repairMeshtasticInboundNullStatus no longer reflects its blanket repair of all messages with null status. Rename it to a name such as repairMeshtasticNullMessageStatus, update every call site, and add a brief comment explaining why assigning 'acked' is safe for outbound rows.src/renderer/lib/startupDbPrune.test.ts (1)
160-168: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStubs assigned onto the shared
window.electronAPI.dbare never restored. These direct property assignments persist for the rest of the file/run; considervi.spyOn/vi.restoreAllMocks()(or reassigning inafterEach) so ordering changes don't silently break other tests.🤖 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/startupDbPrune.test.ts` around lines 160 - 168, The test setup directly assigns stubs onto the shared window.electronAPI.db object without restoring them. Update the mock setup around deleteNodesByAge, prunePositionHistory, and the related database methods to use restorable spies or reset the assignments in afterEach, ensuring each test starts with the original database API behavior.src/renderer/runtime/useMeshtasticRuntime.reconnect-hardening.test.ts (1)
112-148: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSource-text assertions won't catch the single-flight regressions they target. These checks pass as long as the strings exist; e.g. the BLE-only deferred-reconnect flush flagged in
useMeshtasticRuntime.ts(Line 2189-2202) satisfies every assertion here. A behavioral test that driveshandleConnectionLostduring an in-flight open (BLE and serial) and asserts exactly one open plus one follow-up cycle would give real coverage.As per coding guidelines, "Behavioral changes must include a passing test."
🤖 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/useMeshtasticRuntime.reconnect-hardening.test.ts` around lines 112 - 148, The current source-text assertions in the reconnect hardening test do not verify single-flight behavior. Replace or supplement them with behavioral tests that drive handleConnectionLost while attemptReconnect has an open in flight for both BLE and serial, then assert exactly one open occurs followed by one deferred reconnect cycle; retain coverage for the existing reconnect-generation and timeout behavior.Source: Coding guidelines
src/renderer/lib/startupDbPrune.ts (1)
105-112: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAll-protocol retention now fans out many concurrent DB IPCs per prune. Every startup and every session prune fires up to ~15 write-heavy operations in parallel against one SQLite file, including the unconditional
migrateRfStubNodes/deleteNodesNeverHearddeletes that are no longer gated by the active protocol. Consider batching sequentially (or in small groups) to bound write contention, and confirmdeleteNodesNeverHeardrunning on repeated session prunes is intended.Also applies to: 274-276
🤖 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/startupDbPrune.ts` around lines 105 - 112, Bound concurrency in the startup/session prune flow instead of pushing all database operations into one concurrent Promise batch. Update the operations around migrateRfStubNodes and deleteNodesNeverHeard to run sequentially or through a small concurrency limit, and verify that deleteNodesNeverHeard is intentionally executed on every repeated session prune; gate or remove it if not required for the active protocol.src/renderer/lib/reticulum/reticulumDestinationInput.test.ts (1)
42-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover malformed RNCP shares too.
The implementation adds a deliberate
return nullpath for inputs containing the RNCP sentinel but no valid hash, yet these tests cover only valid shares. Add malformed single-line and multiline cases to prevent regression to generic bare-hex extraction.🤖 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/reticulumDestinationInput.test.ts` around lines 42 - 50, Extend the tests in the RNCP parsing case around parseReticulumDestinationInput to cover malformed single-line and multiline inputs containing the RNCP sentinel but no valid hash, asserting both return null. Ensure these cases verify malformed RNCP shares do not fall back to generic bare-hex extraction.src/renderer/components/ChatPanel.tsx (1)
2230-2249: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMemoize
rncpShareCandidatesto avoid re-filtering the DM history on every render.Unlike
viewMessages/filteredMessages/unreadCountsetc. in this same component,rncpShareCandidatesrecomputes on every render while a reticulum DM is open (including on scroll-driven state updates), doing an O(n) filter+map over the DM's message history each time. Hoist it into a top-leveluseMemo(can't calluseMemoinside this conditional IIFE per Rules of Hooks).♻️ Proposed fix
Add near the other
useMemos (e.g. afterreticulumDmPathProbe):const rncpShareCandidates = useMemo( () => protocol === 'reticulum' && isDmMode ? viewMessages .filter((m) => !isOwnNode(m.sender_id)) .map((m) => ({ payload: m.payload, senderHash: m.reticulum_sender_hash ?? null, senderName: m.sender_name ?? null, timestamp: m.timestamp, })) : [], [protocol, isDmMode, viewMessages, isOwnNode], );- const rncpShareCandidates = - protocol === 'reticulum' && isDmMode - ? viewMessages - .filter((m) => !isOwnNode(m.sender_id)) - .map((m) => ({ - payload: m.payload, - senderHash: m.reticulum_sender_hash ?? null, - senderName: m.sender_name ?? null, - timestamp: m.timestamp, - })) - : []; const rncpControl =As per coding guidelines, "Avoid hot-path O(n) work and perform lazy cleanup when collections become large."
🤖 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/ChatPanel.tsx` around lines 2230 - 2249, Move the rncpShareCandidates computation into a top-level useMemo near the other memoized values, keyed by protocol, isDmMode, viewMessages, and isOwnNode. Remove the inline filter/map from the conditional IIFE and continue passing the memoized candidates to ChatDmRncpControl.Source: Coding guidelines
src/renderer/components/remote/ChatDmRncpControl.tsx (1)
245-304: 📐 Maintainability & Code Quality | 🔵 TrivialConfirm test coverage for the new handlers.
handleForgetSaved,handleCancel,handleUseFromChat, and the transfer progress/badge rendering are new behavior, but the diff only adds one new test (hydration on mount). Please confirm coverage exists for forget/cancel/use-from-chat/badge-count paths, or let me know if you'd like help drafting them.As per coding guidelines, "Behavioral changes must include a passing test."
🤖 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/remote/ChatDmRncpControl.tsx` around lines 245 - 304, Expand the test coverage for ChatDmRncpControl to exercise the new handleForgetSaved, handleCancel, and handleUseFromChat callbacks, including their success and relevant no-op/error paths, and verify transfer progress/badge-count rendering. Add passing behavioral tests alongside the existing hydration-on-mount test, using the component’s existing mocks and interaction patterns.Source: Coding guidelines
🤖 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.test.ts`:
- Around line 244-245: Update the test around the default RUST_LOG assertion to
isolate process environment state by clearing or stubbing both
MESH_CLIENT_RUST_LOG and RUST_LOG before invoking sidecarChildEnv(), then
restore their original values afterward. Keep the assertion expecting the
default “warn” value.
In `@src/main/reticulum-sidecar-manager.ts`:
- Around line 340-345: Update the proc.stdout handling around
recordSidecarOutputLine to buffer arbitrary chunks and replay complete
newline-delimited lines individually before calling
shouldForwardReticulumSidecarStdout, while retaining incomplete trailing text
for the next chunk and flushing it when the stream ends. Add a manager-level
test covering a mixed chunk containing both suppressible and forwardable lines.
In `@src/main/reticulumSidecarStderrLog.ts`:
- Around line 12-18: Update shouldForwardReticulumSidecarStdout to match WARN or
ERROR only in the formatter’s severity field, rather than anywhere in the
message text. Preserve forwarding for actual WARN/ERROR lines, reject INFO lines
whose payload mentions WARN or ERROR, and add a regression test covering that
case.
In `@src/renderer/App.tsx`:
- Around line 3401-3406: Add focused behavioral tests for the
onSendPositionToDevice selection in the renderer component, covering Meshtastic
capabilities, MeshCore capabilities, and unsupported capabilities yielding
undefined. Include equivalent coverage for the second occurrence of this handler
selection, and verify each selected callback is the expected action.
In `@src/renderer/components/DiagnosticsPanel.tsx`:
- Around line 264-267: Update the showForeignLoraTables feature gate to use the
existing ProtocolCapabilities value, obtained through
useRadioProvider(protocol), instead of comparing protocol directly to
'meshtastic' or 'meshcore'. Preserve the foreignLoraListenerNodeId and
isConnected requirements while relying on the established LoRa diagnostics
capability.
In `@src/renderer/components/NomadNetworkPanel.test.tsx`:
- Around line 732-740: Add assertions to the tooltip-title test in
NomadNetworkPanel.test.tsx for the newly added send-message, forward, home,
source, and close controls, using their accessible roles or existing test
selectors and expected title translation keys. Keep the existing openWidth,
back, and reloadPage assertions unchanged.
In `@src/renderer/components/RadioPanel.tsx`:
- Around line 919-944: Add regression tests for the advert-coordinate
synchronization effect in RadioPanel, covering initial hydration from advert
coordinates, preserving dirty user-entered values when adverts update, and
resuming synchronization after a successful send resets the form-dirty state.
Reuse the existing RadioPanel test setup and exercise the meshcoreSelfInfo
advert updates and send-success flow without changing unrelated behavior.
In `@src/renderer/components/remote/ChatDmRncpControl.tsx`:
- Around line 129-145: Compute activeTransferCount from all relevant transfers
before peerTransfers applies the top-five slice, while preserving peerTransfers
as the newest five display items. Reuse the same local ID and destination
matching criteria from the peerTransfers useMemo so active transfers outside the
displayed list are included.
In `@src/renderer/components/remote/RemoteSettingsSection.tsx`:
- Around line 210-222: Split the synchronization effect in the
RemoteSettingsSection component into separate effects for saveDir, fetchJail,
allowFetch, and overwrite. Each effect should depend only on its corresponding
settings property and update only that field’s local state, preventing a
persisted change to one field from resetting unsaved edits in the others.
In `@src/renderer/components/remote/RncpEnableRequestModal.tsx`:
- Around line 86-103: Prevent duplicate RNCP share messages in the auto-sharing
effect near shareRncpReceiveDestWithPeer: after a successful auto-share for
peerHash, dismiss the request modal so the enable-button path cannot send again,
or otherwise ensure the post-enable handler skips sending when autoSharedPeerRef
already matches that peer. Preserve sharing for peers that have not yet been
auto-shared.
In `@src/renderer/lib/startupDbPrune.ts`:
- Around line 104-152: Update the error messages in executeDbPrune’s mesh-node
and MeshCore retention handlers to interpolate the received label instead of
hardcoding “[App] startup”. Apply this consistently to migrateRfStubNodes,
deleteNodesNeverHeard, deleteNodesByAge, pruneNodesByCount,
deleteNodesWithoutLongname, both prunePositionHistory calls, and all
deleteMeshcore/pruneMeshcore handlers, preserving each operation’s existing
error details.
In `@src/renderer/locales/de/translation.json`:
- Line 599: Translate the German locale’s destinationHelp value into German,
replacing the current English text while preserving its guidance and
terminology; use the corresponding translated values in the other locales as
context and have the wording verified by a native speaker.
In `@src/renderer/locales/en/translation.json`:
- Around line 1341-1342: Update the foreignLoraDescription translation to use
protocol-neutral wording that refers to separately listed MeshCore detections
without stating that Meshtastic transmitters appear in the section above; leave
foreignLoraHeading unchanged.
In `@src/renderer/locales/it/translation.json`:
- Line 602: Update the transfersTitle translations to use file-transfer
terminology: in src/renderer/locales/it/translation.json lines 602-602 replace
“Bonifici” with an Italian file-transfer term; in
src/renderer/locales/ja/translation.json lines 603-603 replace “送金” with
Japanese file-transfer wording; in src/renderer/locales/ko/translation.json
lines 603-603 replace “픽업 서비스” with Korean file-transfer wording; and in
src/renderer/locales/nl/translation.json lines 601-601 replace
“Overschrijvingen” with Dutch file-transfer wording.
In `@src/renderer/locales/ja/translation.json`:
- Line 600: Translate the destinationHelp entry from the English source text
into Japanese in src/renderer/locales/ja/translation.json at lines 600-600 and
into Korean in src/renderer/locales/ko/translation.json at lines 600-600,
preserving the guidance and placeholders’ meaning in both locales.
In `@src/renderer/locales/pl/translation.json`:
- Line 599: Translate the destinationHelp string into the target locale while
preserving its RNCP instructions and UI meaning. Update
src/renderer/locales/pl/translation.json lines 599-599,
src/renderer/locales/pt-BR/translation.json lines 600-600,
src/renderer/locales/ru/translation.json lines 600-600,
src/renderer/locales/tr/translation.json lines 600-600, and
src/renderer/locales/zh/translation.json lines 600-600; do not alter the English
source-of-truth locale.
- Around line 600-601: Add locale-specific few and many plural entries for
activeTransfersBadgeAria in src/renderer/locales/pl/translation.json (lines
600-601), src/renderer/locales/ru/translation.json (lines 601-602), and
src/renderer/locales/uk/translation.json (lines 601-602), using grammatically
correct translations for each locale while retaining the existing one and other
forms.
In `@src/renderer/locales/tr/translation.json`:
- Around line 611-613: Update the Turkish translation entries forgetAddress and
forgotAddressToast: complete forgetAddress as an imperative action label, and
rewrite forgotAddressToast to state that the saved destination was forgotten
rather than using first-person wording. Keep the existing forgetAddressAria
entry unchanged.
In `@src/renderer/locales/zh/translation.json`:
- Around line 603-608: Update the Chinese locale entries transfersTitle and
transferProgressAria to use the intended Chinese file-transfer terminology,
replacing the incorrect job-reassignment wording and English “PERCENT” text
while preserving the existing interpolation placeholders.
In `@src/renderer/runtime/useMeshtasticRuntime.ts`:
- Around line 2179-2202: Flush meshtasticDeferredReconnectRef in the reconnect
attempt’s finally block for every transport, not only when isBleReconnect is
true. Preserve BLE-specific cleanup of bleConnectInProgressRef, but move
deferred-flag consumption and handleConnectionLostRef scheduling outside that
condition so non-BLE reconnects restart when an in-flight attempt becomes stale.
Add or update a test covering deferred reconnect recovery for a non-BLE
transport.
---
Outside diff comments:
In `@src/renderer/components/DiagnosticsPanel.tsx`:
- Around line 233-267: Update the diagnosticRows check in
meshcoreRepeaterConflict to compare each RF row’s nodeId with
foreignLoraListenerNodeId instead of meshtasticListenerNodeId, keeping the
existing condition and conflict matching unchanged.
---
Nitpick comments:
In `@src/main/db-schema-sync.ts`:
- Around line 828-834: The function name repairMeshtasticInboundNullStatus no
longer reflects its blanket repair of all messages with null status. Rename it
to a name such as repairMeshtasticNullMessageStatus, update every call site, and
add a brief comment explaining why assigning 'acked' is safe for outbound rows.
In `@src/renderer/components/ChatPanel.tsx`:
- Around line 2230-2249: Move the rncpShareCandidates computation into a
top-level useMemo near the other memoized values, keyed by protocol, isDmMode,
viewMessages, and isOwnNode. Remove the inline filter/map from the conditional
IIFE and continue passing the memoized candidates to ChatDmRncpControl.
In `@src/renderer/components/remote/ChatDmRncpControl.tsx`:
- Around line 245-304: Expand the test coverage for ChatDmRncpControl to
exercise the new handleForgetSaved, handleCancel, and handleUseFromChat
callbacks, including their success and relevant no-op/error paths, and verify
transfer progress/badge-count rendering. Add passing behavioral tests alongside
the existing hydration-on-mount test, using the component’s existing mocks and
interaction patterns.
In `@src/renderer/lib/reticulum/reticulumDestinationInput.test.ts`:
- Around line 42-50: Extend the tests in the RNCP parsing case around
parseReticulumDestinationInput to cover malformed single-line and multiline
inputs containing the RNCP sentinel but no valid hash, asserting both return
null. Ensure these cases verify malformed RNCP shares do not fall back to
generic bare-hex extraction.
In `@src/renderer/lib/startupDbPrune.test.ts`:
- Around line 160-168: The test setup directly assigns stubs onto the shared
window.electronAPI.db object without restoring them. Update the mock setup
around deleteNodesByAge, prunePositionHistory, and the related database methods
to use restorable spies or reset the assignments in afterEach, ensuring each
test starts with the original database API behavior.
In `@src/renderer/lib/startupDbPrune.ts`:
- Around line 105-112: Bound concurrency in the startup/session prune flow
instead of pushing all database operations into one concurrent Promise batch.
Update the operations around migrateRfStubNodes and deleteNodesNeverHeard to run
sequentially or through a small concurrency limit, and verify that
deleteNodesNeverHeard is intentionally executed on every repeated session prune;
gate or remove it if not required for the active protocol.
In `@src/renderer/runtime/useMeshtasticRuntime.reconnect-hardening.test.ts`:
- Around line 112-148: The current source-text assertions in the reconnect
hardening test do not verify single-flight behavior. Replace or supplement them
with behavioral tests that drive handleConnectionLost while attemptReconnect has
an open in flight for both BLE and serial, then assert exactly one open occurs
followed by one deferred reconnect cycle; retain coverage for the existing
reconnect-generation and timeout behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cb440683-670c-4aac-993e-13bc3011d031
📒 Files selected for processing (60)
AGENTS.mdARCHITECTURE.mddocs/diagnostics.mddocs/meshcore-meshtastic-parity.mddocs/troubleshooting.mdsrc/main/db-schema-sync.test.tssrc/main/db-schema-sync.tssrc/main/reticulum-sidecar-manager.test.tssrc/main/reticulum-sidecar-manager.tssrc/main/reticulumSidecarStderrLog.test.tssrc/main/reticulumSidecarStderrLog.tssrc/main/verify-backup-repairs.test.tssrc/renderer/App.tsxsrc/renderer/components/ChatPanel.tsxsrc/renderer/components/DiagnosticsPanel.test.tsxsrc/renderer/components/DiagnosticsPanel.tsxsrc/renderer/components/NomadMicronPageView.test.tsxsrc/renderer/components/NomadNetworkPanel.test.tsxsrc/renderer/components/NomadNetworkPanel.tsxsrc/renderer/components/RadioPanel.tsxsrc/renderer/components/remote/ChatDmRncpControl.test.tsxsrc/renderer/components/remote/ChatDmRncpControl.tsxsrc/renderer/components/remote/RemoteSettingsSection.tsxsrc/renderer/components/remote/RncpEnableRequestModal.test.tsxsrc/renderer/components/remote/RncpEnableRequestModal.tsxsrc/renderer/lib/applyRncpReceiveDestShareFromChatHistory.test.tssrc/renderer/lib/applyRncpReceiveDestShareFromChatHistory.tssrc/renderer/lib/connection.tssrc/renderer/lib/meshcoreUtils.test.tssrc/renderer/lib/meshcoreUtils.tssrc/renderer/lib/meshtastic/meshtasticRuntimeWireEffects.tssrc/renderer/lib/meshtastic/meshtasticSdkRoutingErrorConsoleHook.tssrc/renderer/lib/meshtastic/meshtasticSdkRoutingErrorLog.test.tssrc/renderer/lib/meshtasticBacklogUtils.tssrc/renderer/lib/nomad/micronParser.test.tssrc/renderer/lib/reticulum/reticulumDestinationInput.test.tssrc/renderer/lib/reticulum/reticulumDestinationInput.tssrc/renderer/lib/startupDbPrune.test.tssrc/renderer/lib/startupDbPrune.tssrc/renderer/locales/cs/translation.jsonsrc/renderer/locales/de/translation.jsonsrc/renderer/locales/en/translation.jsonsrc/renderer/locales/es/translation.jsonsrc/renderer/locales/fr/translation.jsonsrc/renderer/locales/id/translation.jsonsrc/renderer/locales/it/translation.jsonsrc/renderer/locales/ja/translation.jsonsrc/renderer/locales/ko/translation.jsonsrc/renderer/locales/nl/translation.jsonsrc/renderer/locales/pl/translation.jsonsrc/renderer/locales/pt-BR/translation.jsonsrc/renderer/locales/ru/translation.jsonsrc/renderer/locales/tr/translation.jsonsrc/renderer/locales/uk/translation.jsonsrc/renderer/locales/zh/translation.jsonsrc/renderer/runtime/useMeshtasticRuntime.reconnect-hardening.test.tssrc/renderer/runtime/useMeshtasticRuntime.tssrc/renderer/runtime/useReticulumRuntime.tssrc/renderer/styles.cssvitest.config.ts
Comment on lines
+244
to
+245
| const spawnEnv = spawnMock.mock.calls[0]?.[2]?.env as NodeJS.ProcessEnv | undefined; | ||
| expect(spawnEnv?.RUST_LOG).toBe('warn'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the implementation and the test case.
rg -n "sidecarChildEnv|RUST_LOG|MESH_CLIENT_RUST_LOG" src/main/reticulum-sidecar-manager.ts src/main/reticulum-sidecar-manager.test.ts
# Read the relevant sections with line numbers.
sed -n '1,220p' src/main/reticulum-sidecar-manager.ts
printf '\n--- TEST ---\n'
sed -n '200,280p' src/main/reticulum-sidecar-manager.test.tsRepository: Colorado-Mesh/mesh-client
Length of output: 10947
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Rust log resolution helper.
rg -n "function resolveSidecarRustLog|export function resolveSidecarRustLog|MESH_CLIENT_RUST_LOG|RUST_LOG" src/main/reticulumSidecarStderrLog.ts src/main/reticulum-sidecar-manager.test.ts
printf '\n--- HELPER ---\n'
sed -n '1,220p' src/main/reticulumSidecarStderrLog.ts
printf '\n--- TEST HEADER ---\n'
sed -n '1,120p' src/main/reticulum-sidecar-manager.test.tsRepository: Colorado-Mesh/mesh-client
Length of output: 7561
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect setup/teardown for environment manipulation in the test file.
rg -n "beforeEach|afterEach|process\.env|MESH_CLIENT_RUST_LOG|RUST_LOG|mockRestore|mockReturnValue" src/main/reticulum-sidecar-manager.test.ts
printf '\n--- TEST MIDDLE ---\n'
sed -n '120,220p' src/main/reticulum-sidecar-manager.test.ts
printf '\n--- TEST TAIL ---\n'
sed -n '220,340p' src/main/reticulum-sidecar-manager.test.tsRepository: Colorado-Mesh/mesh-client
Length of output: 9153
Make the default RUST_LOG assertion hermetic.
sidecarChildEnv() honors MESH_CLIENT_RUST_LOG first and then RUST_LOG, so this case can drift into an override path instead of exercising the default. Clear or stub both env vars here and restore them 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/main/reticulum-sidecar-manager.test.ts` around lines 244 - 245, Update
the test around the default RUST_LOG assertion to isolate process environment
state by clearing or stubbing both MESH_CLIENT_RUST_LOG and RUST_LOG before
invoking sidecarChildEnv(), then restore their original values afterward. Keep
the assertion expecting the default “warn” value.
Comment on lines
+3401
to
+3406
| onSendPositionToDevice={ | ||
| capabilities.hasFullPositionConfig | ||
| ? meshtasticPanelActions.sendPositionToDevice | ||
| : undefined | ||
| : capabilities.hasCompanionContactManagementConfig | ||
| ? meshcorePanelActions.sendPositionToDevice | ||
| : undefined |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add passing coverage for both capability-selected handlers.
These behavioral branches need focused tests covering Meshtastic, MeshCore, and unsupported-capability fallbacks. As per coding guidelines, behavioral changes must include a passing test.
Also applies to: 3685-3690
🤖 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/App.tsx` around lines 3401 - 3406, Add focused behavioral tests
for the onSendPositionToDevice selection in the renderer component, covering
Meshtastic capabilities, MeshCore capabilities, and unsupported capabilities
yielding undefined. Include equivalent coverage for the second occurrence of
this handler selection, and verify each selected callback is the expected
action.
Source: Coding guidelines
Comment on lines
+600
to
+601
| "activeTransfersBadgeAria_one": "{{count}} aktywny transfer plików z tym partnerem", | ||
| "activeTransfersBadgeAria_other": "{{count}} aktywne transfery plików z tym partnerem", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Search activeTransfersBadgeAria ==\n'
rg -n '"activeTransfersBadgeAria_(one|other|few|many|zero|two)"|activeTransfersBadgeAria' src/renderer/locales src | sed -n '1,200p'
printf '\n== Search i18n/plural resolver setup ==\n'
rg -n 'plural|pluralResolver|i18next|i18n\.init|Intl\.PluralRules|returnObjects|compatibilityJSON' src | sed -n '1,220p'
printf '\n== List likely i18n files ==\n'
git ls-files | rg 'i18n|locale|locales|translation\.json|plural' | sed -n '1,200p'Repository: Colorado-Mesh/mesh-client
Length of output: 31173
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'activeTransfersBadgeAria|pluralResolver|Intl\.PluralRules|i18next|translation\.json' src | sed -n '1,240p'Repository: Colorado-Mesh/mesh-client
Length of output: 24585
🌐 Web query:
i18next pluralResolver Polish Russian Ukrainian plural categories few many other documentation
💡 Result:
In modern versions of i18next (v24+), plural resolution relies on the standard Intl.PluralRules API [1][2]. This API uses CLDR (Common Locale Data Repository) plural categories to determine the correct suffix for translation keys based on a provided count [3][4]. For languages like Polish, Russian, and Ukrainian, which possess more than the simple singular/plural forms found in English, you must provide translations for all relevant CLDR categories to ensure correct resolution [3][4]. Plural Categories The Intl.PluralRules API utilizes the following standard categories [3][4]: - one: Used for singular forms. - few: Used for specific small quantities (e.g., in Polish for counts ending in 2, 3, 4, excluding 12, 13, 14). - many: Used for specific large quantities (e.g., in Polish for counts ending in 0, 1, 5-9, 11-19). - other: The mandatory fallback category, used when no other rules apply (e.g., in Polish for fractional values). - zero: Used by some languages for exactly 0. Language-Specific Requirements - Polish (pl): Uses 'one', 'few', 'many', and 'other' [3][4]. - Russian (ru) and Ukrainian (uk): Similarly use 'one', 'few', 'many', and 'other' [5][3]. Crucial Implementation Notes - Mandatory Intl.PluralRules: i18next v24 no longer includes a fallback for manual plural rules; it requires the Intl.PluralRules API [1][2]. In environments lacking this (e.g., some React Native/Hermes setups), you must install an Intl polyfill (such as intl-pluralrules) to prevent the library from degrading to simple English-style (one/other) resolution [1][2]. - Consistent Keying: You must pass the 'count' option in your translation function call: i18next.t('key', { count: n }) [1][2]. If 'count' is omitted, i18next will not attempt to resolve a plural suffix, which often results in missing keys if your JSON contains only suffixed keys [3][4]. - Fallback Safety: Always include an 'other' suffix in your JSON for every plural key set [4]. It serves as the required fallback for all counts that do not explicitly match the other categories [3][4]. To verify the categories for any specific language, you can run the following in your browser console: new Intl.PluralRules('pl').resolvedOptions.pluralCategories // Output: ["one", "few", "many", "other"] [3]
Citations:
- 1: https://www.i18next.com/translation-function/plurals
- 2: https://www.i18next.com/how-to/faq.md
- 3: plural keys don't resolve consistently i18next/i18next#2423
- 4: https://localepack.app/guides/i18next-json-format-namespaces-plurals
- 5: i18next is not detecting the correct plural forms for Ukrainian language i18next/i18next#1995
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== src/renderer/lib/i18n.ts ==\n'
cat -n src/renderer/lib/i18n.ts | sed -n '1,220p'
printf '\n== src/renderer/lib/i18n.test.ts (plural-related sections) ==\n'
rg -n 'plural|Plural|dmNodeHops|activeTransfersBadgeAria|resolvedOptions|Intl\.PluralRules|i18next' src/renderer/lib/i18n.test.ts
cat -n src/renderer/lib/i18n.test.ts | sed -n '1,220p'
printf '\n== src/renderer/vitest.setup.ts relevant section ==\n'
cat -n src/renderer/vitest.setup.ts | sed -n '1,120p'Repository: Colorado-Mesh/mesh-client
Length of output: 8219
Add few/many plural forms for activeTransfersBadgeAria.
Polish, Russian, and Ukrainian need locale-specific plural keys here; the current _other strings are wrong for counts like 2 and 5.
src/renderer/locales/pl/translation.jsonsrc/renderer/locales/ru/translation.jsonsrc/renderer/locales/uk/translation.json
📍 Affects 3 files
src/renderer/locales/pl/translation.json#L600-L601(this comment)src/renderer/locales/ru/translation.json#L601-L602src/renderer/locales/uk/translation.json#L601-L602
🤖 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/locales/pl/translation.json` around lines 600 - 601, Add
locale-specific few and many plural entries for activeTransfersBadgeAria in
src/renderer/locales/pl/translation.json (lines 600-601),
src/renderer/locales/ru/translation.json (lines 601-602), and
src/renderer/locales/uk/translation.json (lines 601-602), using grammatically
correct translations for each locale while retaining the existing one and other
forms.
Comment on lines
+611
to
+613
| "forgetAddress": "Kaydedilen varış noktasını", | ||
| "forgetAddressAria": "Bu eş için kaydedilen rncp alma hedefini kaldır", | ||
| "forgotAddressToast": "Bu eş için kaydedilen rncp hedefini unuttum.", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Complete the Turkish forget-destination copy.
forgetAddress ends mid-phrase ("Kaydedilen varış noktasını"), so the action label is incomplete. Use an imperative such as “Kaydedilen varış noktasını unut” and make the toast describe the completed action rather than using first-person "unuttum".
🤖 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/locales/tr/translation.json` around lines 611 - 613, Update the
Turkish translation entries forgetAddress and forgotAddressToast: complete
forgetAddress as an imperative action label, and rewrite forgotAddressToast to
state that the saved destination was forgotten rather than using first-person
wording. Keep the existing forgetAddressAria entry unchanged.
There was a problem hiding this comment.
Actionable comments posted: 27
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 (4)
src/renderer/lib/webbluetooth-ble-manager.ts (1)
456-471: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKeep BLE catch paths defensive.
err as DOMExceptionis only a compile-time cast. A null, primitive, or non-DOM exception can makedomErr.name/domErr.messagethrow while handling the original failure, and can produce anundefineduser-facing message. Separately, a disconnect/cleanup race can leaveprimarynull beforeprimary.properties.readexecutes.Use guarded error-name/message extraction, reuse
errLikeToLogString(err)for the message, and change the fallback check toprimary?.properties.read.As per coding guidelines,
**/*.{ts,tsx}requires strict TypeScript and prefersunknownwith guards.Proposed direction
-const domErr = err as DOMException; +const errorMessage = errLikeToLogString(err); +const errorName = + typeof err === 'object' && + err !== null && + 'name' in err && + typeof (err as { name?: unknown }).name === 'string' + ? (err as { name: string }).name + : 'UnknownError'; ... - domErr.name, - domErr.message, + errorName, + errorMessage, ... - `Bluetooth connection failed${isPairing ? ' (pairing issue)' : ''}: ${domErr.message}`, + `Bluetooth connection failed${isPairing ? ' (pairing issue)' : ''}: ${errorMessage}`, ... - if (primary.properties.read) { + if (primary?.properties.read) { ... - `Failed to start Bluetooth notifications${isPairing ? ' (pairing issue)' : ''}: ${domErr.message}`, + `Failed to start Bluetooth notifications${isPairing ? ' (pairing issue)' : ''}: ${errorMessage}`,Also applies to: 532-547, 575-612
🤖 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/webbluetooth-ble-manager.ts` around lines 456 - 471, Make the BLE catch paths around the gatt connection and the additional indicated handlers defensive by treating caught errors as unknown, safely extracting the error name, and using errLikeToLogString(err) for the user-facing message and logging fallback instead of directly accessing DOMException fields. Preserve pairing classification and wrapped-error behavior, and change the primary characteristic read guard to primary?.properties.read to tolerate cleanup races.Source: Coding guidelines
src/main/noble-ble-manager.ts (1)
1158-1166: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRemove the
scanStopwait here.doStopScanning()already clearsscanningActiveand returns immediately; ifscanStopnever fires,connect()never reachesfinallyandconnectQueuestays blocked.await this.doStopScanning();is enough.🤖 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 1158 - 1166, The connection flow’s scan-stop handling can block indefinitely by waiting for the scanStop event. In the scanningActive branch of connect, remove the onScanStop listener and Promise wait, and directly await this.doStopScanning() so cleanup and connectQueue progression do not depend on scanStop firing.src/renderer/lib/protocols/MeshCoreProtocol.ts (1)
442-450: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRestore the runtime public-key type guard.
rawis externalunknown; a malformed advert withoutpublicKeynow throws atd.publicKey.lengthand can break the event listener. MatchdecodeContactby checkingd.publicKey instanceof Uint8Arrayfirst.Proposed fix
- if (d.publicKey.length !== 32) return []; + if (!(d.publicKey instanceof Uint8Array) || d.publicKey.length !== 32) return [];🤖 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/protocols/MeshCoreProtocol.ts` around lines 442 - 450, Update the raw advert validation before accessing its length: in the surrounding protocol handler, require d.publicKey to be an instance of Uint8Array and return [] when it is not, then retain the existing 32-byte length check and pubkeyToNodeId flow for valid keys.src/renderer/components/ChatPanel.tsx (1)
2245-2264: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd coverage for RNCP share candidates.
Test that inbound Reticulum DM rows are forwarded, while self-authored rows are excluded.
🤖 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/ChatPanel.tsx` around lines 2245 - 2264, Add test coverage for the RNCP candidate-building flow around ChatPanel’s rncpShareCandidates and ChatDmRncpControl props: verify inbound Reticulum DM messages are forwarded with their payload, sender metadata, and timestamp, while messages identified by isOwnNode(m.sender_id) are excluded.Source: Coding guidelines
🟡 Other comments (2)
src/renderer/components/RadioPanel.tsx-85-87 (1)
85-87: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject arrays as config objects.
Line 86 accepts
[], so malformed imports are reported as successful no-ops. Require a non-array object.🤖 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/RadioPanel.tsx` around lines 85 - 87, Update isStringKeyedRecord to return true only for non-null objects that are not arrays, rejecting array values as configuration objects while preserving support for ordinary records.src/renderer/lib/startupDbPrune.ts-26-27 (1)
26-27: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard the preload API before dereferencing it.
When
windowexists but preload is absent,window.electronAPI.dbthrows before this function can safely skip vacuum scheduling. Preserve the former optional API guard.🤖 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/startupDbPrune.ts` around lines 26 - 27, Update the startup database-pruning guard to safely check that the preload API and its db object exist before dereferencing vacuumReticulumTables. Preserve the existing early return when the vacuum API is unavailable, including environments where window exists but electronAPI or db is absent.
🧹 Nitpick comments (1)
src/renderer/runtime/useReticulumRuntime.ts (1)
135-138: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid normalizing RMAP rows twice.
useReticulumDiscoveryMapStore.setDiscovered()already callsnormalizeRmapDiscoveryRows(rows), so this handler now performs the same scan, sort, and truncation twice for every discovery event. Keep normalization at one boundary, preferably in the store.As per coding guidelines,
src/renderer/**/*.{ts,tsx}says to “Avoid hot-path O(n) work.”Also applies to: 704-706
🤖 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/useReticulumRuntime.ts` around lines 135 - 138, Remove the normalizeRmapDiscoveryRows import and the redundant normalization in the discovery handler, passing raw rows directly to useReticulumDiscoveryMapStore.setDiscovered(). Keep normalization centralized in the store boundary and preserve the handler’s existing event flow.Source: Coding guidelines
🤖 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 `@scripts/check-pr.mjs`:
- Around line 109-115: Update the mergeBase fallback in the PR-check flow around
resolveOriginMainMergeBase so an unavailable origin/main does not return success
without validation. Run check:reticulum-sidecar unconditionally in this path, or
explicitly fail and require fetching origin/main, while preserving the existing
merge-base behavior when it is available.
In `@src/main/db-compat.ts`:
- Around line 27-39: Wrap the `require('node:sqlite')` operation in a
`try`/`finally` block and restore the original `process.emitWarning` via
`warnSave` in the `finally` path, so restoration occurs whether loading succeeds
or throws. Preserve the existing warning suppression and `DatabaseSync`
assignment behavior, and document the failure point, fallback, and relevant
logging as required for this main-process path.
In `@src/renderer/components/RadioPanel.tsx`:
- Around line 89-91: Update numericArray to accept only values that are finite
integers between 0 and 255 inclusive before they reach Uint8Array.from in the
key conversion flow around lines 1218-1225. Preserve the existing null result
for invalid input and ensure valid byte arrays continue through unchanged.
- Around line 1162-1168: Remove the complete-config console.debug call in the
import flow after JSON parsing, or replace it with non-sensitive metadata that
cannot include private_key or other identity material; keep the existing
current-device-state diagnostic log unchanged.
In `@src/renderer/lib/chatScrollUtils.ts`:
- Around line 134-137: Update the size calculation in the ResizeObserver
measurement logic to safely access the first borderBoxSize entry using an
existence guard, while preserving the existing htmlEl[sizeProp] fallback when no
box is available. Keep the horizontal/block dimension selection and rounding
behavior unchanged.
In `@src/renderer/lib/connection.ts`:
- Around line 412-413: Update the MeshDevice construction in the surrounding
connection logic to pass the validated transport without the `any` cast,
preserving the existing runtime assertion and using the SDK constructor’s
transport type.
In `@src/renderer/lib/connectionPanelErrorHumanize.ts`:
- Around line 39-40: Update the platform detection guard in
connectionPanelErrorHumanize to verify window.electronAPI exists before
accessing getPlatform, while preserving the existing user-agent fallback for
browser or non-Electron renderers.
In `@src/renderer/lib/identityByProtocol.ts`:
- Line 66: Update the active-identity check in identityByProtocol to guard
against a missing active record before reading active.protocol.type, allowing
stale activeIdentityId values to fall back to the first matching protocol
identity. Preserve the existing valid-active-identity behavior and add a
regression test covering a non-null active ID absent from identities.
In `@src/renderer/lib/meshcore/meshcoreAdvertEventApply.ts`:
- Line 191: Update the last_heard assignment in meshcoreAdvertEventApply to use
a numeric fallback when existing.last_heard is undefined before applying
Math.max, preserving the current timestamp behavior for defined values. Add or
update a regression test covering an existing node without last_heard and verify
the resulting node record contains a valid numeric timestamp rather than NaN.
In `@src/renderer/lib/meshcore/meshcoreRfRxRuntime.ts`:
- Line 165: Restore the numeric fallback when calculating node last-heard
timestamps: update the nextLastHeard calculations in
src/renderer/lib/meshcore/meshcoreRfRxRuntime.ts at lines 165-165 and 302-302 to
use existing.last_heard ?? 0 before Math.max, preserving nowSec as the current
timestamp.
- Line 464: Update the name access in the foreign-LoRa handling flow to
optional-chain the name value before calling trim, preserving the existing
fallback when name is absent. Change the expression near selfInfoRef to use
name?.trim() so partial self information cannot throw.
In `@src/renderer/lib/meshcoreChannelText.ts`:
- Around line 161-164: Update the display-name resolution in the MeshCore node
resolver to null-safely trim both node.long_name and node.short_name before
applying the Node-${senderId} fallback. Preserve the existing preference order
and behavior for valid names, ensuring the shared resolver remains safe for live
and queued ingest.
In `@src/renderer/lib/meshcoreContactPathDiagnostics.ts`:
- Around line 26-34: Update the precondition in the meshcore contact diagnostics
flow to independently validate or safely default
window.electronAPI.db.getAllMeshcorePathHistory alongside getMeshcoreContacts,
ensuring a missing or rejected history call does not discard valid contacts.
Preserve the existing Promise.all behavior for available methods and add
coverage for a partial IPC surface.
In `@src/renderer/lib/meshcoreDualNobleBleInit.ts`:
- Around line 14-16: Update the platform detection logic around
window.electronAPI so it safely handles an absent bridge before accessing
getPlatform. Use an optional/function check, preserve the existing non-Linux
result when getPlatform is available, and retain the fallback behavior
otherwise; add a regression covering a missing electronAPI bridge.
In `@src/renderer/lib/meshcoreMqttTopicPrefix.ts`:
- Line 30: Normalize persisted MQTT settings before the helper logic in
tryAutoLaunchMqtt, ensuring missing server and topicPrefix values receive their
established defaults before trim, includes, or connection-payload construction.
Update the related handling around server and topicPrefix so both IATA and
non-IATA paths operate on normalized values without throwing or returning
undefined.
In `@src/renderer/lib/meshcorePathChainDisplay.ts`:
- Around line 206-209: Update the loop constructing the path-chain display
entries to guard mismatched `snrs` and `segments` lengths, preventing access to
`seg.resolvedLabel` or `seg.hex` when no segment exists. Bound iteration by both
array lengths or skip missing segments, and add a regression test covering more
SNR values than path segments.
In `@src/renderer/lib/meshcoreRoomLoginPathSync.ts`:
- Around line 59-65: Update the contact argument construction in
addOrUpdateContact so every required metadata field has a defined fallback when
raw or synthetic contacts omit it, including the corresponding fields at the
alternate call site. Preserve existing contact values when present and use the
established defaults for missing flags, path length, advertising name,
last-advertisement timestamp, latitude, and longitude.
In `@src/renderer/lib/meshcoreStoreDedup.ts`:
- Around line 79-87: Update the fallback key in the deduplication logic
surrounding meshcoreChannelMessageStoreId to normalize missing msg.channel
values to the stable -1 sentinel. Preserve the existing handling for valid
channels and ensure omitted, null, and channel: -1 DM messages produce the same
dedupe/store key.
In `@src/renderer/lib/meshcoreWaitingMessageItem.ts`:
- Around line 39-40: Update the first assignment in the surrounding
Array.isArray boundary logic to explicitly declare first as unknown when reading
value[0]. Remove the eslint-disable-next-line suppression, preserving the
existing validation flow.
In `@src/renderer/lib/mqttAutoLaunch.ts`:
- Line 33: Update the password checks in the relevant auto-launch guard and
validateLetsMeshManualCredentials to handle missing persisted passwords before
calling trim. Preserve the existing Boolean validation behavior for present
values while treating an omitted password as invalid without throwing.
In `@src/renderer/lib/nodeHealthScore.ts`:
- Around line 15-16: Update the SNR calculation in nodeHealthScore to handle
absent or non-finite node.snr values before applying Math.min, Math.max, and
Math.round. Restore the prior fallback of -20 (or equivalent validation) so the
computed signal and resulting health score remain finite for incomplete runtime
nodes.
In `@src/renderer/lib/nodeLongNameOrHex.ts`:
- Line 29: The node name fallbacks must remain safe for partial runtime records.
In src/renderer/lib/nodeLongNameOrHex.ts lines 29-29, update the name access in
nodeLongNameOrHex to optional-chain long_name before trim; in
src/renderer/lib/buildMeshPeerTopologyGraph.ts lines 69-72, optional-chain both
short_name and long_name before their trim calls, preserving the existing hex
fallback.
In `@src/renderer/lib/reticulum/reticulumSidecarReads.ts`:
- Around line 331-334: Validate all nullish IPC/proxy responses before property
access. In src/renderer/lib/reticulum/reticulumSidecarReads.ts at lines 331-334,
346-349, and 359-360, guard the switch, create, and delete responses before
reading ok, error, or id while preserving existing failure behavior. In
src/renderer/lib/reticulum/reticulumDefaultHubPresets.ts at lines 314-318,
335-341, and 361-367, handle nullish disable, repair, and add responses safely
so default-hub synchronization continues without throwing.
In `@src/renderer/lib/reticulum/reticulumStartupAutostartGate.ts`:
- Around line 78-80: Update the guard in the reticulum startup autostart flow to
safely check each Electron API boundary before accessing getState, including
window.electronAPI and bleCoexistence. Preserve the early return when the API or
function is unavailable, and ensure the check remains safe during renderer
teardown or partial preload initialization.
In `@src/renderer/lib/rrcMessagePersist.ts`:
- Around line 15-16: Update the validation flow around storageRoomKey and the
persistence guard to reject missing or invalid msg.room and msg.id values before
calling storageRoomKey(msg.room) or msg.id.trim(). Preserve the existing
early-return behavior for invalid hub, room, id, or body inputs, and only derive
the room key after the raw values are safely validated.
In `@src/renderer/lib/writeClipboardText.ts`:
- Around line 9-13: Restore optional chaining guards in writeClipboardText
around window.electronAPI?.clipboard?.writeText and
navigator.clipboard?.writeText so fallback behavior and the explicit unavailable
error remain reachable. In src/renderer/lib/writeClipboardText.ts lines 9-13,
update both bridge checks; in src/renderer/lib/connection.ts line 97, guard the
optional Electron logging bridge so logging cannot abort connection attempts
outside a preloaded Electron renderer.
In `@src/renderer/runtime/useReticulumRuntime.reconnect-hardening.test.ts`:
- Around line 162-164: Replace the source-text regex assertion in the reconnect
hardening test with behavioral coverage for RMAP discovery handling. Exercise
normalizeRmapDiscoveryRows() or the store setter using stale and valid rows,
then assert stale rows are filtered, remaining rows are sorted correctly, and
the configured maximum-row limit is enforced.
---
Outside diff comments:
In `@src/main/noble-ble-manager.ts`:
- Around line 1158-1166: The connection flow’s scan-stop handling can block
indefinitely by waiting for the scanStop event. In the scanningActive branch of
connect, remove the onScanStop listener and Promise wait, and directly await
this.doStopScanning() so cleanup and connectQueue progression do not depend on
scanStop firing.
In `@src/renderer/components/ChatPanel.tsx`:
- Around line 2245-2264: Add test coverage for the RNCP candidate-building flow
around ChatPanel’s rncpShareCandidates and ChatDmRncpControl props: verify
inbound Reticulum DM messages are forwarded with their payload, sender metadata,
and timestamp, while messages identified by isOwnNode(m.sender_id) are excluded.
In `@src/renderer/lib/protocols/MeshCoreProtocol.ts`:
- Around line 442-450: Update the raw advert validation before accessing its
length: in the surrounding protocol handler, require d.publicKey to be an
instance of Uint8Array and return [] when it is not, then retain the existing
32-byte length check and pubkeyToNodeId flow for valid keys.
In `@src/renderer/lib/webbluetooth-ble-manager.ts`:
- Around line 456-471: Make the BLE catch paths around the gatt connection and
the additional indicated handlers defensive by treating caught errors as
unknown, safely extracting the error name, and using errLikeToLogString(err) for
the user-facing message and logging fallback instead of directly accessing
DOMException fields. Preserve pairing classification and wrapped-error behavior,
and change the primary characteristic read guard to primary?.properties.read to
tolerate cleanup races.
---
Other comments:
In `@src/renderer/components/RadioPanel.tsx`:
- Around line 85-87: Update isStringKeyedRecord to return true only for non-null
objects that are not arrays, rejecting array values as configuration objects
while preserving support for ordinary records.
In `@src/renderer/lib/startupDbPrune.ts`:
- Around line 26-27: Update the startup database-pruning guard to safely check
that the preload API and its db object exist before dereferencing
vacuumReticulumTables. Preserve the existing early return when the vacuum API is
unavailable, including environments where window exists but electronAPI or db is
absent.
---
Nitpick comments:
In `@src/renderer/runtime/useReticulumRuntime.ts`:
- Around line 135-138: Remove the normalizeRmapDiscoveryRows import and the
redundant normalization in the discovery handler, passing raw rows directly to
useReticulumDiscoveryMapStore.setDiscovered(). Keep normalization centralized in
the store boundary and preserve the handler’s existing event flow.
🪄 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: 917ec775-a49d-4ff1-bc04-cce837640a07
📒 Files selected for processing (179)
.coderabbit.yaml.githooks/pre-commit.githooks/pre-push.sonarcloud.propertiesAGENTS.mddocs/ci-cd.mddocs/development-environment.mdeslint.config.mjspackage.jsonscripts/check-pr.mjsscripts/check-pr.test.mjsscripts/check-reticulum-sidecar.shsonar-project.propertiessrc/main/db-compat.tssrc/main/index.contract.test.tssrc/main/index.tssrc/main/ipc/reticulum-handlers.tssrc/main/ipc/tak-handlers.tssrc/main/mqtt-manager.tssrc/main/noble-ble-manager.tssrc/main/updater.tssrc/renderer/components/ChatComposer.tsxsrc/renderer/components/ChatPanel.tsxsrc/renderer/components/RadioPanel.tsxsrc/renderer/lib/bleReconnectHelper.tssrc/renderer/lib/buildMeshPeerTopologyGraph.tssrc/renderer/lib/chatComposerLimits.tssrc/renderer/lib/chatMentionSegments.tssrc/renderer/lib/chatOutboxDrain.tssrc/renderer/lib/chatScrollUtils.tssrc/renderer/lib/connection.tssrc/renderer/lib/connectionPanelErrorHumanize.test.tssrc/renderer/lib/connectionPanelErrorHumanize.tssrc/renderer/lib/connectionWebStreams.test.tssrc/renderer/lib/database-migrations.test.tssrc/renderer/lib/debugSnapshot.tssrc/renderer/lib/decodeQrFromImageSource.tssrc/renderer/lib/diagnostics/RFDiagnosticEngine.tssrc/renderer/lib/diagnostics/RemediationEngine.tssrc/renderer/lib/diagnostics/ReticulumDiagnosticEngine.tssrc/renderer/lib/diagnostics/RoutingDiagnosticEngine.tssrc/renderer/lib/drivers/ConnectionDriver.test.tssrc/renderer/lib/drivers/PacketRouter.tssrc/renderer/lib/drivers/attachTypedPacketListener.test.tssrc/renderer/lib/flasher/rnode.tssrc/renderer/lib/gpsSource.tssrc/renderer/lib/hydrateIdentityStoresFromDb.test.tssrc/renderer/lib/hydrateIdentityStoresFromDb.tssrc/renderer/lib/identityByProtocol.tssrc/renderer/lib/identityStoreReads.tssrc/renderer/lib/ingest/meshcoreIngest.test.tssrc/renderer/lib/ingest/meshcoreIngest.tssrc/renderer/lib/ingest/meshcoreSenderRepair.test.tssrc/renderer/lib/ingest/meshcoreSenderRepair.tssrc/renderer/lib/ingest/meshtasticIngest.test.tssrc/renderer/lib/ingest/meshtasticIngest.tssrc/renderer/lib/ingest/reticulumIngest.tssrc/renderer/lib/lastConnectionStorage.tssrc/renderer/lib/letsMeshConnectionGuards.tssrc/renderer/lib/logAnalyzerI18n.tssrc/renderer/lib/mergeOfflineIdentityStore.test.tssrc/renderer/lib/mergeOfflineIdentityStore.tssrc/renderer/lib/meshcore/meshcoreAdvertEventApply.tssrc/renderer/lib/meshcore/meshcoreChatSenderNode.tssrc/renderer/lib/meshcore/meshcoreDmAckRuntime.test.tssrc/renderer/lib/meshcore/meshcoreLiveContactPersist.tssrc/renderer/lib/meshcore/meshcoreMessageI18n.tssrc/renderer/lib/meshcore/meshcoreRfRxRuntime.test.tssrc/renderer/lib/meshcore/meshcoreRfRxRuntime.tssrc/renderer/lib/meshcore/meshcoreSerialTransportLoss.tssrc/renderer/lib/meshcoreChannelText.tssrc/renderer/lib/meshcoreContactPathDiagnostics.tssrc/renderer/lib/meshcoreDbCacheHydration.test.tssrc/renderer/lib/meshcoreDirectMessageDecode.test.tssrc/renderer/lib/meshcoreDualNobleBleInit.tssrc/renderer/lib/meshcoreMqttTopicPrefix.tssrc/renderer/lib/meshcorePathChainDisplay.tssrc/renderer/lib/meshcoreProcessWaitingMessageItem.tssrc/renderer/lib/meshcoreRawPacketSender.tssrc/renderer/lib/meshcoreRepeaterTracePath.tssrc/renderer/lib/meshcoreRoomLoginPathSync.tssrc/renderer/lib/meshcoreStoreDedup.test.tssrc/renderer/lib/meshcoreStoreDedup.tssrc/renderer/lib/meshcoreUtils.tssrc/renderer/lib/meshcoreWaitingMessageItem.tssrc/renderer/lib/meshcoreWaitingMessagesDrain.tssrc/renderer/lib/meshtastic/meshtasticApplyErrorMessage.tssrc/renderer/lib/meshtastic/meshtasticChatSenderNode.tssrc/renderer/lib/meshtastic/meshtasticMqttClientProxy.test.tssrc/renderer/lib/meshtastic/meshtasticMqttClientProxy.tssrc/renderer/lib/meshtastic/meshtasticNodeSideEffects.tssrc/renderer/lib/meshtastic/meshtasticRawPacketExpand.tssrc/renderer/lib/meshtastic/meshtasticRuntimeWireEffects.tssrc/renderer/lib/meshtastic/meshtasticTransportLossDetection.tssrc/renderer/lib/meshtastic/meshtasticTransportSideEffects.tssrc/renderer/lib/meshtastic/meshtasticXmodemTransfer.tssrc/renderer/lib/meshtasticBacklogUtils.test.tssrc/renderer/lib/meshtasticBacklogUtils.tssrc/renderer/lib/meshtasticLastHeard.tssrc/renderer/lib/meshtasticMqttPublish.tssrc/renderer/lib/meshtasticMqttTlsMigration.tssrc/renderer/lib/meshtasticRemoteAdmin.tssrc/renderer/lib/meshtasticTraceRouteLookupKeys.tssrc/renderer/lib/mqttAutoLaunch.tssrc/renderer/lib/networkDiscovery.tssrc/renderer/lib/nodeHealthScore.tssrc/renderer/lib/nodeLongNameOrHex.tssrc/renderer/lib/nomad/micronParser.test.tssrc/renderer/lib/nomad/micronParser.tssrc/renderer/lib/offlineProtocolIdentities.tssrc/renderer/lib/protocols/MeshCoreProtocol.test.tssrc/renderer/lib/protocols/MeshCoreProtocol.tssrc/renderer/lib/protocols/MeshtasticProtocol.test.tssrc/renderer/lib/protocols/MeshtasticProtocol.tssrc/renderer/lib/protocols/meshcore/MeshCoreTransport.tssrc/renderer/lib/pushRncpListenerPolicy.tssrc/renderer/lib/rawPacketLogSort.tssrc/renderer/lib/replyPreview.tssrc/renderer/lib/reticulum/applyReticulumOutboundDeliveryStatus.test.tssrc/renderer/lib/reticulum/applyReticulumOutboundDeliveryStatus.tssrc/renderer/lib/reticulum/buildReticulumTopologyLayout.tssrc/renderer/lib/reticulum/markStaleReticulumOutbound.test.tssrc/renderer/lib/reticulum/markStaleReticulumOutbound.tssrc/renderer/lib/reticulum/reticulumConfigAudit.test.tssrc/renderer/lib/reticulum/reticulumDefaultHubPresets.test.tssrc/renderer/lib/reticulum/reticulumDefaultHubPresets.tssrc/renderer/lib/reticulum/reticulumDiscoveryMapLayout.tssrc/renderer/lib/reticulum/reticulumIconAppearance.tssrc/renderer/lib/reticulum/reticulumIngestMerge.tssrc/renderer/lib/reticulum/reticulumOutboundFailureBridge.test.tssrc/renderer/lib/reticulum/reticulumOutboundFailureBridge.tssrc/renderer/lib/reticulum/reticulumOutboundRetryStatus.test.tssrc/renderer/lib/reticulum/reticulumPeerListRows.tssrc/renderer/lib/reticulum/reticulumSidecarReads.tssrc/renderer/lib/reticulum/reticulumStartupAutostartGate.tssrc/renderer/lib/reticulum/sendReticulumChatMessage.tssrc/renderer/lib/reticulum/useReticulumNobleBleYieldWatcher.test.tssrc/renderer/lib/reticulum/useReticulumNobleBleYieldWatcher.tssrc/renderer/lib/reticulum/useReticulumSidecarApi.tssrc/renderer/lib/rfReconnectHelper.tssrc/renderer/lib/rncpInboundPolicyLists.tssrc/renderer/lib/rrcInactiveNotifications.tssrc/renderer/lib/rrcMention.tssrc/renderer/lib/rrcMessagePersist.tssrc/renderer/lib/rrcNoticeParsers.tssrc/renderer/lib/rrcRoomMembers.tssrc/renderer/lib/runtimeSideEffectAttach.contract.test.tssrc/renderer/lib/sanitize-log-message.test.tssrc/renderer/lib/sendRncpRequestEnable.tssrc/renderer/lib/serialPortAutoRediscovery.tssrc/renderer/lib/serialPortSignature.tssrc/renderer/lib/startupDbPrune.tssrc/renderer/lib/storeRecordAdapters.tssrc/renderer/lib/types.tssrc/renderer/lib/webbluetooth-ble-manager.tssrc/renderer/lib/writeClipboardText.tssrc/renderer/runtime/useMeshtasticRuntime.tssrc/renderer/runtime/useReticulumRuntime.reconnect-hardening.test.tssrc/renderer/runtime/useReticulumRuntime.tssrc/renderer/stores/mapViewportStore.tssrc/renderer/vitest.setup.tssrc/renderer/workers/messageEncoder.worker.tssrc/shared/connectHost.tssrc/shared/docLinks.tssrc/shared/meshClientDeepLink.tssrc/shared/meshcoreMqttEnvelope.tssrc/shared/meshcorePathHash.test.tssrc/shared/meshcorePathHash.tssrc/shared/meshcoreRfPacketParse.test.tssrc/shared/meshcoreRfPacketParse.tssrc/shared/meshcoreRfPath.test.tssrc/shared/meshcoreRfPath.tssrc/shared/meshtasticChannelPskLine.tssrc/shared/meshtasticDefaultPublicPsk.tssrc/shared/meshtasticUrlEncoder.test.tssrc/shared/meshtasticUrlEncoder.tssrc/shared/networkTransientErrors.tssrc/shared/randomPrefixedId.test.tssrc/shared/reticulumPropagationAutoSync.ts
💤 Files with no reviewable changes (3)
- sonar-project.properties
- .sonarcloud.properties
- src/renderer/lib/logAnalyzerI18n.ts
Comment on lines
+109
to
+115
| const mergeBase = resolveOriginMainMergeBase(); | ||
| if (!mergeBase) { | ||
| console.error( | ||
| 'check:pr: skip sidecar path check (origin/main unavailable); run pnpm run check:reticulum-sidecar manually if needed', | ||
| ); | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Run the sidecar check conservatively when origin/main is unavailable.
This returns success before checking a sidecar-changing branch, contradicting the PR-parity gate’s stated behavior. Run check:reticulum-sidecar unconditionally in this fallback (or fail and require fetching origin/main) rather than silently passing.
Proposed fix
if (!mergeBase) {
console.error(
- 'check:pr: skip sidecar path check (origin/main unavailable); run pnpm run check:reticulum-sidecar manually if needed',
+ 'check:pr: origin/main unavailable; running check:reticulum-sidecar conservatively',
);
- return 0;
+ return run('pnpm', ['run', 'check:reticulum-sidecar']);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const mergeBase = resolveOriginMainMergeBase(); | |
| if (!mergeBase) { | |
| console.error( | |
| 'check:pr: skip sidecar path check (origin/main unavailable); run pnpm run check:reticulum-sidecar manually if needed', | |
| ); | |
| return 0; | |
| } | |
| if (!mergeBase) { | |
| console.error( | |
| 'check:pr: origin/main unavailable; running check:reticulum-sidecar conservatively', | |
| ); | |
| return run('pnpm', ['run', 'check:reticulum-sidecar']); | |
| } |
🤖 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-pr.mjs` around lines 109 - 115, Update the mergeBase fallback
in the PR-check flow around resolveOriginMainMergeBase so an unavailable
origin/main does not return success without validation. Run
check:reticulum-sidecar unconditionally in this path, or explicitly fail and
require fetching origin/main, while preserving the existing merge-base behavior
when it is available.
Comment on lines
+27
to
+39
| type EmitWarningCompat = (warning: string | Error, ...args: unknown[]) => void; | ||
| const warnSave = process.emitWarning.bind(process) as EmitWarningCompat; | ||
|
|
||
| (process as any).emitWarning = (warning: string | Error, ...args: unknown[]) => { | ||
| process.emitWarning = (warning: string | Error, ...args: unknown[]) => { | ||
| const msg = typeof warning === 'string' ? warning : (warning.message ?? ''); | ||
| if (msg.includes('SQLite is an experimental feature')) return; | ||
|
|
||
| return (_warnSave as any).call(process, warning, ...args); | ||
| warnSave(warning, ...args); | ||
| }; | ||
| // eslint-disable-next-line @typescript-eslint/no-require-imports | ||
| const { DatabaseSync } = require('node:sqlite') as { DatabaseSync: typeof DatabaseSyncType }; | ||
|
|
||
| process.emitWarning = _warnSave; // restore after sqlite is loaded | ||
| process.emitWarning = warnSave; // restore after sqlite is loaded |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Restore process.emitWarning with finally.
If require('node:sqlite') throws, line 39 is never reached and the process-wide warning handler remains replaced, suppressing matching warnings for all later main-process code. Capture the original function and restore it in a finally block.
As per path instructions, src/main/**/*.ts must preserve state and I/O integrity on failure, documenting the failure point, fallback, and relevant logging.
Proposed fix
-const warnSave = process.emitWarning.bind(process) as EmitWarningCompat;
+const originalEmitWarning = process.emitWarning;
+const warnSave = originalEmitWarning.bind(process) as EmitWarningCompat;
-process.emitWarning = (warning: string | Error, ...args: unknown[]) => {
- const msg = typeof warning === 'string' ? warning : (warning.message ?? '');
- if (msg.includes('SQLite is an experimental feature')) return;
- warnSave(warning, ...args);
-};
+let sqliteModule: { DatabaseSync: typeof DatabaseSyncType };
+try {
+ process.emitWarning = (warning: string | Error, ...args: unknown[]) => {
+ const msg = typeof warning === 'string' ? warning : (warning.message ?? '');
+ if (msg.includes('SQLite is an experimental feature')) return;
+ warnSave(warning, ...args);
+ };
+
+ // eslint-disable-next-line `@typescript-eslint/no-require-imports`
+ sqliteModule = require('node:sqlite');
+} finally {
+ process.emitWarning = originalEmitWarning;
+}
+
+const { DatabaseSync } = sqliteModule;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| type EmitWarningCompat = (warning: string | Error, ...args: unknown[]) => void; | |
| const warnSave = process.emitWarning.bind(process) as EmitWarningCompat; | |
| (process as any).emitWarning = (warning: string | Error, ...args: unknown[]) => { | |
| process.emitWarning = (warning: string | Error, ...args: unknown[]) => { | |
| const msg = typeof warning === 'string' ? warning : (warning.message ?? ''); | |
| if (msg.includes('SQLite is an experimental feature')) return; | |
| return (_warnSave as any).call(process, warning, ...args); | |
| warnSave(warning, ...args); | |
| }; | |
| // eslint-disable-next-line @typescript-eslint/no-require-imports | |
| const { DatabaseSync } = require('node:sqlite') as { DatabaseSync: typeof DatabaseSyncType }; | |
| process.emitWarning = _warnSave; // restore after sqlite is loaded | |
| process.emitWarning = warnSave; // restore after sqlite is loaded | |
| type EmitWarningCompat = (warning: string | Error, ...args: unknown[]) => void; | |
| const originalEmitWarning = process.emitWarning; | |
| const warnSave = originalEmitWarning.bind(process) as EmitWarningCompat; | |
| let sqliteModule: { DatabaseSync: typeof DatabaseSyncType }; | |
| try { | |
| process.emitWarning = (warning: string | Error, ...args: unknown[]) => { | |
| const msg = typeof warning === 'string' ? warning : (warning.message ?? ''); | |
| if (msg.includes('SQLite is an experimental feature')) return; | |
| warnSave(warning, ...args); | |
| }; | |
| // eslint-disable-next-line `@typescript-eslint/no-require-imports` | |
| sqliteModule = require('node:sqlite') as { DatabaseSync: typeof DatabaseSyncType }; | |
| } finally { | |
| process.emitWarning = originalEmitWarning; | |
| } | |
| const { DatabaseSync } = sqliteModule; |
🤖 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/db-compat.ts` around lines 27 - 39, Wrap the
`require('node:sqlite')` operation in a `try`/`finally` block and restore the
original `process.emitWarning` via `warnSave` in the `finally` path, so
restoration occurs whether loading succeeds or throws. Preserve the existing
warning suppression and `DatabaseSync` assignment behavior, and document the
failure point, fallback, and relevant logging as required for this main-process
path.
Source: Path instructions
Comment on lines
+89
to
+91
| function numericArray(value: unknown): number[] | null { | ||
| if (!Array.isArray(value) || !value.every((entry) => typeof entry === 'number')) return null; | ||
| return value; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate key bytes before Uint8Array conversion.
Uint8Array.from() coerces NaN, infinities, negatives, fractions, and values above 255. Lines 1218-1225 can therefore persist a corrupted identity as a seemingly valid 32-byte key. Accept only finite integer bytes in [0, 255].
🤖 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/RadioPanel.tsx` around lines 89 - 91, Update
numericArray to accept only values that are finite integers between 0 and 255
inclusive before they reach Uint8Array.from in the key conversion flow around
lines 1218-1225. Preserve the existing null result for invalid input and ensure
valid byte arrays continue through unchanged.
Comment on lines
+1162
to
1168
| const parsed: unknown = JSON.parse(await file.text()); | ||
| if (!isStringKeyedRecord(parsed)) throw new Error('Invalid config JSON'); | ||
| const cfg = parsed; | ||
| console.debug('[RadioPanel] parsed config JSON:', cfg); | ||
| console.debug( | ||
| `[RadioPanel] current device state before import: radioFreqHz=${radioFreqHz} bandwidth=${bandwidth}`, | ||
| ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not log imported identity material.
Line 1165 logs the complete user-provided config, including private_key. Remove this log or emit only non-sensitive metadata; browser debug logs can be inspected or collected.
Proposed fix
- console.debug('[RadioPanel] parsed config JSON:', cfg);
+ console.debug('[RadioPanel] parsed config JSON keys:', Object.keys(cfg));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const parsed: unknown = JSON.parse(await file.text()); | |
| if (!isStringKeyedRecord(parsed)) throw new Error('Invalid config JSON'); | |
| const cfg = parsed; | |
| console.debug('[RadioPanel] parsed config JSON:', cfg); | |
| console.debug( | |
| `[RadioPanel] current device state before import: radioFreqHz=${radioFreqHz} bandwidth=${bandwidth}`, | |
| ); | |
| const parsed: unknown = JSON.parse(await file.text()); | |
| if (!isStringKeyedRecord(parsed)) throw new Error('Invalid config JSON'); | |
| const cfg = parsed; | |
| console.debug('[RadioPanel] parsed config JSON keys:', Object.keys(cfg)); | |
| console.debug( | |
| `[RadioPanel] current device state before import: radioFreqHz=${radioFreqHz} bandwidth=${bandwidth}`, | |
| ); |
🤖 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/RadioPanel.tsx` around lines 1162 - 1168, Remove the
complete-config console.debug call in the import flow after JSON parsing, or
replace it with non-sensitive metadata that cannot include private_key or other
identity material; keep the existing current-device-state diagnostic log
unchanged.
Sources: Coding guidelines, Path instructions
Comment on lines
+134
to
137
| const box = entry?.borderBoxSize[0]; | ||
| const domSize = box | ||
| ? Math.round(box[instance.options.horizontal ? 'inlineSize' : 'blockSize']) | ||
| : htmlEl[sizeProp]; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Retain the borderBoxSize existence guard.
Line 134 throws when a ResizeObserver implementation supplies an entry without borderBoxSize, preventing chat measurement. Restore entry?.borderBoxSize?.[0] and retain the offset-size fallback.
🤖 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/chatScrollUtils.ts` around lines 134 - 137, Update the size
calculation in the ResizeObserver measurement logic to safely access the first
borderBoxSize entry using an existence guard, while preserving the existing
htmlEl[sizeProp] fallback when no box is available. Keep the horizontal/block
dimension selection and rounding behavior unchanged.
Comment on lines
+331
to
+334
| if (res.ok === false) { | ||
| throw new Error(res.error ?? 'identity switch failed'); | ||
| } | ||
| return Boolean(res?.ok); | ||
| return Boolean(res.ok); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate proxy responses before direct property access.
These six sites removed null-safe access from externally returned IPC/proxy values that are only type-asserted. A nullish response can now throw and either abort identity operations or stop default-hub synchronization.
src/renderer/lib/reticulum/reticulumSidecarReads.ts#L331-L334: guard the switch response before readingok/error.src/renderer/lib/reticulum/reticulumSidecarReads.ts#L346-L349: validate the create response before readingok/id.src/renderer/lib/reticulum/reticulumSidecarReads.ts#L359-L360: guard the delete response before readingok.src/renderer/lib/reticulum/reticulumDefaultHubPresets.ts#L314-L318: handle a nullish disable response and continue safely.src/renderer/lib/reticulum/reticulumDefaultHubPresets.ts#L335-L341: handle a nullish repair response and continue safely.src/renderer/lib/reticulum/reticulumDefaultHubPresets.ts#L361-L367: handle a nullish add response and continue safely.
📍 Affects 2 files
src/renderer/lib/reticulum/reticulumSidecarReads.ts#L331-L334(this comment)src/renderer/lib/reticulum/reticulumSidecarReads.ts#L346-L349src/renderer/lib/reticulum/reticulumSidecarReads.ts#L359-L360src/renderer/lib/reticulum/reticulumDefaultHubPresets.ts#L314-L318src/renderer/lib/reticulum/reticulumDefaultHubPresets.ts#L335-L341src/renderer/lib/reticulum/reticulumDefaultHubPresets.ts#L361-L367
🤖 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/reticulumSidecarReads.ts` around lines 331 - 334,
Validate all nullish IPC/proxy responses before property access. In
src/renderer/lib/reticulum/reticulumSidecarReads.ts at lines 331-334, 346-349,
and 359-360, guard the switch, create, and delete responses before reading ok,
error, or id while preserving existing failure behavior. In
src/renderer/lib/reticulum/reticulumDefaultHubPresets.ts at lines 314-318,
335-341, and 361-367, handle nullish disable, repair, and add responses safely
so default-hub synchronization continues without throwing.
Comment on lines
+78
to
80
| // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- Runtime guard protects external or callback-mutated state. | ||
| if (!window.electronAPI.bleCoexistence.getState) { | ||
| return; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Restore the teardown-safe Electron API guard.
Line [79] now dereferences window.electronAPI.bleCoexistence before the try block. During renderer teardown or partial preload initialization, this throws instead of returning as intended. Restore optional chaining or guard each object/function boundary.
🤖 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/reticulumStartupAutostartGate.ts` around lines 78
- 80, Update the guard in the reticulum startup autostart flow to safely check
each Electron API boundary before accessing getState, including
window.electronAPI and bleCoexistence. Preserve the early return when the API or
function is unavailable, and ensure the check remains safe during renderer
teardown or partial preload initialization.
Comment on lines
+15
to
+16
| const room = storageRoomKey(msg.room); | ||
| if (!hub || !room || !msg.id.trim() || !msg.body) return; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate before calling string methods.
storageRoomKey() trims its input, and msg.id.trim() can also throw. Missing room/id values now crash persistence instead of being rejected by the guard.
Proposed fix
- const room = storageRoomKey(msg.room);
- if (!hub || !room || !msg.id.trim() || !msg.body) return;
+ const room = storageRoomKey(msg.room ?? '');
+ if (!hub || !room || !msg.id?.trim() || !msg.body) return;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const room = storageRoomKey(msg.room); | |
| if (!hub || !room || !msg.id.trim() || !msg.body) return; | |
| const room = storageRoomKey(msg.room ?? ''); | |
| if (!hub || !room || !msg.id?.trim() || !msg.body) return; |
🤖 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/rrcMessagePersist.ts` around lines 15 - 16, Update the
validation flow around storageRoomKey and the persistence guard to reject
missing or invalid msg.room and msg.id values before calling
storageRoomKey(msg.room) or msg.id.trim(). Preserve the existing early-return
behavior for invalid hub, room, id, or body inputs, and only derive the room key
after the raw values are safely validated.
Comment on lines
+9
to
+13
| if (typeof window.electronAPI.clipboard.writeText === 'function') { | ||
| await window.electronAPI.clipboard.writeText(text); | ||
| return; | ||
| } | ||
| if (typeof navigator.clipboard?.writeText === 'function') { | ||
| if (typeof navigator.clipboard.writeText === 'function') { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore runtime guards for optional renderer bridges.
These direct dereferences throw before the intended fallback/error handling when preload APIs or navigator.clipboard are unavailable.
src/renderer/lib/writeClipboardText.ts#L9-L13: guardwindow.electronAPI?.clipboard?.writeTextandnavigator.clipboard?.writeTextso browser fallback and the explicit “Clipboard API unavailable” error remain reachable.src/renderer/lib/connection.ts#L97-L97: guard the optional Electron logging bridge so telemetry logging cannot abort a connection attempt outside a preloaded Electron renderer.
Proposed fix
- const fn = window.electronAPI.log.logDeviceConnection;
+ // eslint-disable-next-line `@typescript-eslint/no-unnecessary-condition` -- Browser/test renderers may not have the Electron preload bridge.
+ const fn = window.electronAPI?.log?.logDeviceConnection;- if (typeof window.electronAPI.clipboard.writeText === 'function') {
+ // eslint-disable-next-line `@typescript-eslint/no-unnecessary-condition` -- Browser/test renderers may not have the Electron preload bridge.
+ if (typeof window.electronAPI?.clipboard?.writeText === 'function') {
await window.electronAPI.clipboard.writeText(text);
return;
}
- if (typeof navigator.clipboard.writeText === 'function') {
+ // eslint-disable-next-line `@typescript-eslint/no-unnecessary-condition` -- Clipboard is unavailable in some browser contexts.
+ if (typeof navigator.clipboard?.writeText === 'function') {
await navigator.clipboard.writeText(text);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (typeof window.electronAPI.clipboard.writeText === 'function') { | |
| await window.electronAPI.clipboard.writeText(text); | |
| return; | |
| } | |
| if (typeof navigator.clipboard?.writeText === 'function') { | |
| if (typeof navigator.clipboard.writeText === 'function') { | |
| // eslint-disable-next-line `@typescript-eslint/no-unnecessary-condition` -- Browser/test renderers may not have the Electron preload bridge. | |
| const fn = window.electronAPI?.log?.logDeviceConnection; |
Suggested change
| if (typeof window.electronAPI.clipboard.writeText === 'function') { | |
| await window.electronAPI.clipboard.writeText(text); | |
| return; | |
| } | |
| if (typeof navigator.clipboard?.writeText === 'function') { | |
| if (typeof navigator.clipboard.writeText === 'function') { | |
| // eslint-disable-next-line `@typescript-eslint/no-unnecessary-condition` -- Browser/test renderers may not have the Electron preload bridge. | |
| if (typeof window.electronAPI?.clipboard?.writeText === 'function') { | |
| await window.electronAPI.clipboard.writeText(text); | |
| return; | |
| } | |
| // eslint-disable-next-line `@typescript-eslint/no-unnecessary-condition` -- Clipboard is unavailable in some browser contexts. | |
| if (typeof navigator.clipboard?.writeText === 'function') { | |
| await navigator.clipboard.writeText(text); |
📍 Affects 2 files
src/renderer/lib/writeClipboardText.ts#L9-L13(this comment)src/renderer/lib/connection.ts#L97-L97
🤖 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/writeClipboardText.ts` around lines 9 - 13, Restore optional
chaining guards in writeClipboardText around
window.electronAPI?.clipboard?.writeText and navigator.clipboard?.writeText so
fallback behavior and the explicit unavailable error remain reachable. In
src/renderer/lib/writeClipboardText.ts lines 9-13, update both bridge checks; in
src/renderer/lib/connection.ts line 97, guard the optional Electron logging
bridge so logging cannot abort connection attempts outside a preloaded Electron
renderer.
Comment on lines
+162
to
+164
| expect(SOURCE).toMatch( | ||
| /evt\.type === 'rmap\.discovery'[\s\S]*?setDiscovered\(normalizeRmapDiscoveryRows\(p\.discovered\)\)/, | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Test RMAP behavior instead of source text.
This regex only proves that the call appears in useReticulumRuntime.ts; it does not verify stale-row filtering, sorting, or the maximum-row limit. Add behavioral assertions around normalizeRmapDiscoveryRows() or the store setter.
As per path instructions, **/*.test.ts says: “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/runtime/useReticulumRuntime.reconnect-hardening.test.ts` around
lines 162 - 164, Replace the source-text regex assertion in the reconnect
hardening test with behavioral coverage for RMAP discovery handling. Exercise
normalizeRmapDiscoveryRows() or the store setter using stale and valid rows,
then assert stale rows are filtered, remaining rows are sorted correctly, and
the configured maximum-row limit is enforced.
Source: Path instructions
Flush deferred Meshtastic reconnect for all transports, gate Diagnostics foreign-LoRa via capabilities, fix RNCP badge/settings/modal races, restore Linux noble behavior-test connect without native BLE, and correct RNCP i18n.
This was referenced Jul 31, 2026
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Unifies work Cursor had split across
fix/db-log-ble-hygiene,chore/eslint-quality-gates, andchore/remove-sonar-tune-coderabbitonto one branch/PR.Reliability, parity, and tooling: Meshtastic BLE reconnect / SQLite retention & NULL-status repair / Reticulum sidecar log noise, MeshCore Radio GPS + Admin reboot + Diagnostics foreign-LoRa, Nomad Micron layout, Chat DM RNCP file-send UX, stricter ESLint + pre-commit/pre-push quality gates, and SonarCloud removal with a quiet CodeRabbit Free config.
rsReticulum pin bump and keepalive sunset (
5657c27f)RS_RETICULUM_REFto9928abe(includes upstreamRNodeIdleProbeTCP idle probes).rsReticulum-rnode-tcp-activity-keepaliveoverlay (On macos quit, doesn't quit #15 closed without merge; still tracked inRATSPEAK_PATCH_ENTRIESfor the closed warning).Box::pinStackHandle::bootstrapfor clippylarge_futuresafter the pin bump.pnpm run update: noble/postcss bumps + sidecarCargo.lockrns 1.1.0; full-feature sidecar build OK.BLE reconnect, cross-protocol prune, and log noise (
67655f5f)messages.status IS NULLrows on schema upgrade.RUST_LOGtowarn; filter INFO/DEBUG stdout spam frommesh-client.log.Nomad Micron box padding and browser tooltips (
92cf9e8b)white-space: pre/pre-wrap.aria-labels astitletooltips on Nomad browser chrome.MeshCore Radio GPS, Admin reboot, foreign-LoRa parity (
7b5b4663)Chat RNCP file send autofill, progress, and prefs (
5e7b1361)ESLint scopes and PR/pre-push quality gates (
901c8da9)no-unsafe-*; shared/libno-unnecessary-condition.typecheck:strict-shared; full-feature sidecar Clippy/tests when sidecar staged.pnpm run check:pr+.githooks/pre-pushbranch-scoped Vitest--changed.Remove SonarCloud and tune CodeRabbit for Free (
585ed932).coderabbit.yaml(path filters, auto-pause).docs/ci-cd.md.Verification
pnpm run test:run(full Vitest): 743 files / 6553 tests passed (~66s) onfix/db-log-ble-hygiene@585ed932.Out of scope
Test plan
messages.statusincluding nullpacket_idrows.MESH_CLIENT_RUST_LOGoverride works.test:staged; pre-push--changed;pnpm run check:pr..coderabbit.yamlpresent.Summary by CodeRabbit
messages.statuswhen it is missing.