Conversation
Replace the sidecar voice stub with TelephonyService, Peers/Chat Call UI, and renderer Web Audio PCM; raise pnpm floors for undici, postcss, and ip-address.
Extract pure path-slot helpers so pathMedium can import the store statically without a cycle, silencing the Vite INEFFECTIVE_DYNAMIC_IMPORT warning.
|
Important Review skippedToo many files! This PR contains 119 files, which is 19 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: QUIET Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (18)
📒 Files selected for processing (119)
You can disable this status message by setting the 📝 WalkthroughWalkthroughThe PR integrates LXST voice calls into the Reticulum stack. It adds sidecar telephony APIs, renderer IPC and audio handling, call state management, peer and chat controls, an active-call overlay, capability gating, tests, and rsLXST build support. ChangesLXST voice integration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (7)
src/renderer/components/reticulum/ReticulumVoiceOverlay.tsx-62-67 (1)
62-67: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle IPC rejection before discarding the Answer promise.
reticulumVoiceAnswer()awaits the IPC operation without a catch. If the IPC invoke rejects,void reticulumVoiceAnswer()produces an unhandled rejection and gives no failure feedback. Catch and log the error in the session helper, then show the existing call-failed feedback.As per coding guidelines, “Catches must log, rethrow, or include a
// catch-no-log-ok <reason>justification” and “Stateful or I/O code must preserve integrity on failure and document failure points, fallbacks, and relevant logging.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/components/reticulum/ReticulumVoiceOverlay.tsx` around lines 62 - 67, Update the reticulumVoiceAnswer session helper used by the answer button to catch IPC invocation failures, log the caught error, and trigger the existing call-failed feedback path. Ensure the button’s void-discarded promise no longer produces an unhandled rejection, while preserving successful answer behavior.Source: Coding guidelines
src/renderer/components/ReticulumPeerListPanel.tsx-585-587 (1)
585-587: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDisable Call when the Reticulum stack is unavailable.
The button remains enabled when
isConnectedis false. The call then invokes the voice IPC path while the sidecar is unavailable. Match the ChatPanel behavior.Proposed fix
- <ReticulumVoiceCallButton lxmfPeerHash={peer.destination_hash} disabled={busy} /> + <ReticulumVoiceCallButton + lxmfPeerHash={peer.destination_hash} + disabled={busy || !isConnected} + />🤖 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/ReticulumPeerListPanel.tsx` around lines 585 - 587, Update the ReticulumVoiceCallButton usage in ReticulumPeerListPanel to disable the button when either busy is true or the Reticulum connection state is unavailable, matching ChatPanel’s isConnected-based behavior. Preserve the existing hasLxstVoice conditional and pass the connection state through the button’s disabled prop.src/renderer/lib/reticulumVoiceSession.ts-176-183 (1)
176-183: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSurface a capture failure to the user.
When
startCaptureAndTxrejects, for example becausegetUserMediafails, this catch writes one log line. Playback stays active, so the call continues with receive-only audio and the user sees no indication that the microphone is not working.startCaptureAndTxtoasts only in the denied-permission branch.Push a toast here so the failure is visible.
🐛 Proposed fix
} catch (e) { console.warn('[reticulumVoice] mic capture failed', e); + pushAppToast(i18n.t('reticulumVoice.errors.micFailed'), 'error'); }Register
reticulumVoice.errors.micFailedin the English i18n keys. As per path instructions, "This repo follows AGENTS.md multi-protocol and i18n rules."🤖 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/reticulumVoiceSession.ts` around lines 176 - 183, Update startReticulumVoiceMediaForActiveCall so its catch block keeps the existing warning and also displays the reticulumVoice.errors.micFailed toast when startCaptureAndTx rejects. Register the matching reticulumVoice.errors.micFailed key in the English i18n resources, following the repository’s i18n conventions.Source: Path instructions
src/shared/voice-types.ts-57-65 (1)
57-65: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate
roleandstatusagainst their declared literals.
isVoiceActiveCallaccepts arbitrary strings for both fields. The type predicate then stores invalid call states asVoiceActiveCall.Check
roleandstatusagainst the supported union values before returningtrue.As per path instructions, “validate external IPC/API input.”
🤖 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/shared/voice-types.ts` around lines 57 - 65, Update isVoiceActiveCall to validate role and status against their declared supported union literals, rather than accepting arbitrary strings. Preserve the existing checks for link_id and remote_identity, and return true only when all fields match valid VoiceActiveCall values.Source: Path instructions
src/renderer/runtime/useReticulumRuntime.ts-1186-1192 (1)
1186-1192: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRequire a finite integer channel count.
The current condition accepts
1.5andInfinity. These values are forwarded to audio listeners as channel counts.Use
Number.isInteger(p.channels) && p.channels > 0before emitting audio. Default invalid values to one channel or reject the event.As per path instructions, “validate external IPC/API input.”
🤖 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 1186 - 1192, Update the channel validation in the voice.audio handling block before useReticulumVoiceStore.getState().emitAudio so only finite positive integer values are accepted via Number.isInteger(p.channels) && p.channels > 0; default invalid or missing values to one channel, while preserving the existing sample decoding and emission flow.Source: Path instructions
scripts/clone-ratspeak-stack.test.mjs-35-41 (1)
35-41: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTest the rsLXST checkout contract, not only source strings.
These assertions pass when the strings appear anywhere in
clone-ratspeak-stack.sh. They do not prove thatensure_reporeceives${LXST_DIR}and${RS_LXST_REF}, that pinning checks out the requested revision, or that the reported SHA belongs to the checkout.Add a temporary local-Git fixture for default and pinned refs. At minimum, assert the complete
ensure_repocall and thelxst_shaassignment.As per coding guidelines: “Ship a passing test for behavioral changes.”
🤖 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/clone-ratspeak-stack.test.mjs` around lines 35 - 41, Replace the source-string-only assertions in the rsLXST test with behavioral tests using a temporary local Git repository fixture covering both default and pinned refs. Execute the clone script and verify the complete ensure_repo invocation uses ${LXST_DIR} and ${RS_LXST_REF}, pinning checks out the requested revision, and the lxst_sha assignment reports the SHA of the resulting checkout.Source: Coding guidelines
scripts/clone-ratspeak-stack.sh-114-117 (1)
114-117: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReport the branch that was actually selected.
ensure_repofalls back toorigin/masterwhenorigin/mainis unavailable, butlxst_modeis always set tofloated origin/main. The final status can therefore report the wrong checkout mode for rsLXST.Derive the mode from the selected reference or return the selected reference from
ensure_repo.Suggested fix
-lxst_mode='floated origin/main' +if git -C "${LXST_DIR}" rev-parse --verify --quiet 'origin/main' >/dev/null; then + lxst_mode='floated origin/main' +else + lxst_mode='floated origin/master' +fi🤖 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/clone-ratspeak-stack.sh` around lines 114 - 117, Update the rsLXST handling around ensure_repo and lxst_mode so the reported mode reflects the reference actually selected, including the origin/master fallback when origin/main is unavailable. Derive lxst_mode from ensure_repo’s selected reference or have ensure_repo return that reference, while preserving the existing status reporting for the other repositories.
🧹 Nitpick comments (3)
src/renderer/lib/reticulumVoiceAudio.test.ts (1)
26-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for resampling and hash normalization.
The packing test only exercises the identity case, where the input rate equals
LXST_QUALITY_HIGH_SAMPLE_RATE_HZand the input length equals one frame. The resample arithmetic inpackQualityHighFrameis therefore untested. The identity tests also skip normalization and rejection.Add cases for a non-48 kHz input rate, a multichannel mix-down, an uppercase identity hash, and an invalid-length identity hash.
💚 Proposed additional cases
+ it('upsamples 24k mono to a QualityHigh frame', () => { + const input = new Float32Array(LXST_QUALITY_HIGH_FRAME_SAMPLES / 2).fill(0.25); + const packed = packQualityHighFrame(input, 24_000, 1); + expect(packed?.length).toBe(LXST_QUALITY_HIGH_FRAME_SAMPLES); + expect(packed?.[0]).toBeCloseTo(0.25); + expect(packed?.[LXST_QUALITY_HIGH_FRAME_SAMPLES - 1]).toBeCloseTo(0.25); + }); + + it('mixes stereo down to mono', () => { + const input = new Float32Array(LXST_QUALITY_HIGH_FRAME_SAMPLES * 2); + for (let i = 0; i < LXST_QUALITY_HIGH_FRAME_SAMPLES; i += 1) { + input[i * 2] = 1; + input[i * 2 + 1] = -1; + } + expect(packQualityHighFrame(input, 48_000, 2)?.[0]).toBeCloseTo(0); + }); + + it('normalizes case and rejects malformed identity hashes', () => { + const upper = 'A'.repeat(32); + expect(resolveVoiceDialIdentityHash({ identityHash: upper })).toEqual({ + identityHash: 'a'.repeat(32), + }); + expect(resolveVoiceDialIdentityHash({ identityHash: 'a'.repeat(31) })).toEqual({ + errorKey: 'reticulumVoice.errors.noIdentity', + }); + });🤖 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/reticulumVoiceAudio.test.ts` around lines 26 - 55, Extend the tests for packQualityHighFrame and resolveVoiceDialIdentityHash to cover non-48 kHz resampling, multichannel mix-down, uppercase identity-hash normalization, and rejection of identity hashes with invalid lengths. Assert the expected packed output shape or behavior and normalized lowercase hash result while preserving the existing identity and fallback cases.src/renderer/lib/reticulumVoiceAudio.ts (1)
12-45: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReplace per-byte string building in the base64 codec.
encodeF32LeBase64anddecodeF32LeBase64build an intermediate binary string one code point at a time. Each QualityHigh frame is 2,880 samples, so each call runs 11,520 string concatenations.startCaptureAndTxinsrc/renderer/lib/reticulumVoiceSession.tsencodes about 33 frames per second, and the runtime decodes inbound frames at the same rate. Use a typed-array view plus chunked conversion instead.Note that
String.fromCharCode(...bytes)in one call would exceed the argument limit at this size, so keep the chunk loop.♻️ Proposed chunked conversion
export function encodeF32LeBase64(samples: Float32Array | number[]): string { - const bytes = new Uint8Array(samples.length * 4); - const view = new DataView(bytes.buffer); - for (let i = 0; i < samples.length; i += 1) { - view.setFloat32(i * 4, samples[i] ?? 0, true); - } - let binary = ''; - for (const b of bytes) { - binary += String.fromCodePoint(b); - } - return btoa(binary); + const floats = samples instanceof Float32Array ? samples : Float32Array.from(samples); + const bytes = new Uint8Array(floats.buffer, floats.byteOffset, floats.byteLength); + const parts: string[] = []; + const CHUNK = 8192; + for (let i = 0; i < bytes.length; i += CHUNK) { + parts.push(String.fromCharCode(...bytes.subarray(i, i + CHUNK))); + } + return btoa(parts.join('')); }The
Float32Arrayview is only valid on little-endian hosts, which is every platform this app targets. If you prefer to keep the explicitDataViewwrites, keep them and change only the string-building loop.🤖 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/reticulumVoiceAudio.ts` around lines 12 - 45, Replace the per-byte string construction in encodeF32LeBase64 and decodeF32LeBase64 with chunked typed-array-to-string conversion, using String.fromCharCode on bounded byte slices rather than one call over the full buffer. Preserve the existing little-endian float encoding/decoding and malformed-input behavior, and keep the chunk loop to avoid argument-limit failures.src/shared/voice-types.test.ts (1)
18-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd negative cases for wrong field types and unexpected enum values.
The rejection cases only cover
nulland missing fields. Two gaps remain:
- No case passes a wrong-typed field, for example
{ available: 'yes', enabled: true }. That is the shape a schema drift in the sidecar would produce.- No case passes an unexpected
roleorstatus.ReticulumVoiceOverlay.tsxbranches onstatus === 'ringing'andstatus === 'established', so whetherisVoiceActiveCallrestricts those values to an allowed set is behavior worth pinning.💚 Proposed additional cases
it('rejects garbage status', () => { expect(isVoiceStatusResponse(null)).toBe(false); expect(isVoiceStatusResponse({ available: true })).toBe(false); + expect(isVoiceStatusResponse({ available: 'yes', enabled: true })).toBe(false); + expect(isVoiceStatusResponse([])).toBe(false); }); @@ it('rejects incomplete active_call', () => { expect(isVoiceActiveCall({ link_id: 'x' })).toBe(false); + expect( + isVoiceActiveCall({ + link_id: 'a'.repeat(32), + remote_identity: 'b'.repeat(32), + role: 'sideways', + status: 'ringing', + }), + ).toBe(false); });If
isVoiceActiveCalldoes not currently constrainroleandstatus, tighten the guard rather than the 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/shared/voice-types.test.ts` around lines 18 - 36, Expand the rejection tests in voice-types.test.ts to cover wrong-typed fields such as available being a string, plus unsupported role and status values in isVoiceActiveCall. If the guard currently accepts arbitrary enum strings, tighten isVoiceActiveCall to allow only the defined role and status values used by ReticulumVoiceOverlay, while preserving valid active_call acceptance.
🤖 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 `@reticulum-sidecar/Cargo.toml`:
- Around line 98-106: Update the Cargo dependency setup for lxst-core and
lxst-telephony so stub builds resolve without requiring the ../../rsLXST
checkout, using an available stub crate or generated manifest. Add build
validation for both environments: a stub configuration with the sibling checkout
absent and a full-stack configuration with it present, while preserving the
existing optional feature behavior.
In `@reticulum-sidecar/src/stack/voice_session.rs`:
- Around line 213-224: Update remember_identity_for_dest to use a bounded-cache
insertion strategy matching insert_display_name_bounded and the
display_name_cache configuration, evicting an existing entry when the
destination-to-identity map reaches its configured capacity. Preserve the
current normalization and 32-character validation, and keep the cache bounded
for every announce or path-response insertion.
- Around line 345-363: Update the OpusFramesReceived handling in voice session
event processing so voice.audio no longer uses shared.event_tx or the broadcast
bus backing /ws. Route encoded audio through a dedicated high-throughput channel
or endpoint, following the existing high-rate packet tap stream pattern, while
preserving the current per-frame payload fields and encoding.
In `@src/renderer/components/reticulum/ReticulumVoiceOverlay.tsx`:
- Around line 34-44: Update the incoming-call state flow used by applyIncoming
and subsequent activeCall status updates so incomingCall is cleared once the
call progresses beyond incoming ringing, allowing the active-call bar to
display. Preserve showIncoming for genuinely ringing or available incoming
calls, and add a regression test covering answer followed by connecting or
established status that verifies the dialog closes.
In `@src/renderer/lib/reticulumVoiceSession.ts`:
- Around line 121-122: Update the capture setup around processor.connect so the
ScriptProcessorNode remains connected through a muted GainNode rather than
directly to ctx.destination, preventing microphone playback. Track the GainNode
with the other capture nodes and disconnect it in stopCapture() alongside the
existing capture cleanup.
- Around line 209-233: Update reticulumVoiceAnswer, reticulumVoiceReject,
reticulumVoiceHangup, and reticulumVoiceSetMuted to catch rejected IPC calls,
report failures through the existing voice error toast/i18n patterns, and
preserve local state integrity. Keep clearCall() unconditional after
reject/hangup attempts, and ensure setMicrophoneMuted(muted) runs even when
mute() fails; retain media teardown before reject/hangup and register the
reticulumVoice.errors.muteFailed English i18n key.
- Around line 149-174: Update startPlayback to maintain a monotonic playback
cursor and schedule each AudioBufferSourceNode at the cursor rather than
starting immediately at ctx.currentTime; initialize or clamp the cursor so late
arrivals do not create gaps or overlaps, then advance it by buffer.duration
after each frame. Reset playbackCursor in stopPlayback alongside the existing
playback cleanup.
- Around line 91-111: Guard startCaptureAndTx against overlapping invocations
with a capture generation token. Capture the generation before asynchronous
work, invalidate stale starts after each await and before assigning
captureStream, captureCtx, captureSource, or captureProcessor, and clean up any
stream or AudioContext acquired by a stale invocation. Increment the generation
in stopCapture so an explicit stop cancels in-flight startup.
- Around line 185-207: Update reticulumVoiceCallPeer to validate the raw result
from api.getStatus() with the exported isVoiceStatusResponse guard before
passing it to applyStatus; treat invalid status data as a failed call and show
the existing call-failed error toast. Wrap both api.getStatus() and api.call()
in rejection handling so IPC failures are converted to an error toast rather
than escaping as unhandled promise rejections, while preserving the existing
status, identity-resolution, and successful media-start flows.
In `@src/renderer/runtime/useReticulumRuntime.voice.test.ts`:
- Around line 11-24: Replace the source-text assertions in the voice routing
test with a behavioral test that registers the runtime’s onEvent callback
through the Electron mock, dispatches each voice.update, voice.incoming,
voice.terminated, voice.error, and voice.audio event, and verifies the
corresponding voice-store transitions plus decoded audio delivery through
emitAudio.
In `@src/renderer/stores/reticulumVoiceStore.ts`:
- Around line 55-57: Update applyStatus and snapshot handling to derive
incomingCall from the newly resolved activeCall, retaining it only when the call
is incoming with ringing or available status and clearing it for established,
active, or null calls. Add an assertion covering this transition, while
preserving existing activeCall and lastError behavior.
- Around line 104-118: Update applyTerminated to return the existing state
immediately when linkId is missing or empty, before clearing activeCall or
incomingCall; retain the existing stale-call comparison for valid IDs. Add a
test covering a termination event without a link ID and verify the current call
state remains unchanged.
---
Other comments:
In `@scripts/clone-ratspeak-stack.sh`:
- Around line 114-117: Update the rsLXST handling around ensure_repo and
lxst_mode so the reported mode reflects the reference actually selected,
including the origin/master fallback when origin/main is unavailable. Derive
lxst_mode from ensure_repo’s selected reference or have ensure_repo return that
reference, while preserving the existing status reporting for the other
repositories.
In `@scripts/clone-ratspeak-stack.test.mjs`:
- Around line 35-41: Replace the source-string-only assertions in the rsLXST
test with behavioral tests using a temporary local Git repository fixture
covering both default and pinned refs. Execute the clone script and verify the
complete ensure_repo invocation uses ${LXST_DIR} and ${RS_LXST_REF}, pinning
checks out the requested revision, and the lxst_sha assignment reports the SHA
of the resulting checkout.
In `@src/renderer/components/reticulum/ReticulumVoiceOverlay.tsx`:
- Around line 62-67: Update the reticulumVoiceAnswer session helper used by the
answer button to catch IPC invocation failures, log the caught error, and
trigger the existing call-failed feedback path. Ensure the button’s
void-discarded promise no longer produces an unhandled rejection, while
preserving successful answer behavior.
In `@src/renderer/components/ReticulumPeerListPanel.tsx`:
- Around line 585-587: Update the ReticulumVoiceCallButton usage in
ReticulumPeerListPanel to disable the button when either busy is true or the
Reticulum connection state is unavailable, matching ChatPanel’s
isConnected-based behavior. Preserve the existing hasLxstVoice conditional and
pass the connection state through the button’s disabled prop.
In `@src/renderer/lib/reticulumVoiceSession.ts`:
- Around line 176-183: Update startReticulumVoiceMediaForActiveCall so its catch
block keeps the existing warning and also displays the
reticulumVoice.errors.micFailed toast when startCaptureAndTx rejects. Register
the matching reticulumVoice.errors.micFailed key in the English i18n resources,
following the repository’s i18n conventions.
In `@src/renderer/runtime/useReticulumRuntime.ts`:
- Around line 1186-1192: Update the channel validation in the voice.audio
handling block before useReticulumVoiceStore.getState().emitAudio so only finite
positive integer values are accepted via Number.isInteger(p.channels) &&
p.channels > 0; default invalid or missing values to one channel, while
preserving the existing sample decoding and emission flow.
In `@src/shared/voice-types.ts`:
- Around line 57-65: Update isVoiceActiveCall to validate role and status
against their declared supported union literals, rather than accepting arbitrary
strings. Preserve the existing checks for link_id and remote_identity, and
return true only when all fields match valid VoiceActiveCall values.
---
Nitpick comments:
In `@src/renderer/lib/reticulumVoiceAudio.test.ts`:
- Around line 26-55: Extend the tests for packQualityHighFrame and
resolveVoiceDialIdentityHash to cover non-48 kHz resampling, multichannel
mix-down, uppercase identity-hash normalization, and rejection of identity
hashes with invalid lengths. Assert the expected packed output shape or behavior
and normalized lowercase hash result while preserving the existing identity and
fallback cases.
In `@src/renderer/lib/reticulumVoiceAudio.ts`:
- Around line 12-45: Replace the per-byte string construction in
encodeF32LeBase64 and decodeF32LeBase64 with chunked typed-array-to-string
conversion, using String.fromCharCode on bounded byte slices rather than one
call over the full buffer. Preserve the existing little-endian float
encoding/decoding and malformed-input behavior, and keep the chunk loop to avoid
argument-limit failures.
In `@src/shared/voice-types.test.ts`:
- Around line 18-36: Expand the rejection tests in voice-types.test.ts to cover
wrong-typed fields such as available being a string, plus unsupported role and
status values in isVoiceActiveCall. If the guard currently accepts arbitrary
enum strings, tighten isVoiceActiveCall to allow only the defined role and
status values used by ReticulumVoiceOverlay, while preserving valid active_call
acceptance.
🪄 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: 42b70207-61da-4c76-b70d-688ded354ebc
⛔ Files ignored due to path filters (18)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yamlreticulum-sidecar/Cargo.lockis excluded by!**/*.locksrc/renderer/locales/cs/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/de/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/en/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/es/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/fr/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/id/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/it/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ja/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ko/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/nl/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/pl/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/pt-BR/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ru/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/tr/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/uk/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/zh/translation.jsonis excluded by!src/renderer/locales/**
📒 Files selected for processing (45)
AGENTS.mddocs/reticulum-sidecar-ipc.mddocs/reticulum.mdelectron-builder.ymlpackage.jsonpnpm-workspace.yamlreticulum-sidecar/Cargo.tomlreticulum-sidecar/src/api/mod.rsreticulum-sidecar/src/api/system.rsreticulum-sidecar/src/api/voice.rsreticulum-sidecar/src/stack/announce_ws_coalesce.rsreticulum-sidecar/src/stack/live.rsreticulum-sidecar/src/stack/mod.rsreticulum-sidecar/src/stack/voice_session.rsscripts/clone-ratspeak-stack.shscripts/clone-ratspeak-stack.test.mjsscripts/update.shscripts/update.test.mjssrc/preload/index.tssrc/renderer/App.tsxsrc/renderer/components/ChatPanel.tsxsrc/renderer/components/ReticulumPeerDetailModal.tsxsrc/renderer/components/ReticulumPeerListPanel.test.tsxsrc/renderer/components/ReticulumPeerListPanel.tsxsrc/renderer/components/reticulum/ReticulumVoiceCallButton.test.tsxsrc/renderer/components/reticulum/ReticulumVoiceCallButton.tsxsrc/renderer/components/reticulum/ReticulumVoiceOverlay.test.tsxsrc/renderer/components/reticulum/ReticulumVoiceOverlay.tsxsrc/renderer/lib/radio/BaseRadioProvider.tssrc/renderer/lib/radio/protocol-capabilities.test.tssrc/renderer/lib/reticulum/clearReticulumSessionStores.tssrc/renderer/lib/reticulum/reticulumPathMedium.tssrc/renderer/lib/reticulum/reticulumPathSlots.tssrc/renderer/lib/reticulumVoiceAudio.test.tssrc/renderer/lib/reticulumVoiceAudio.tssrc/renderer/lib/reticulumVoiceSession.tssrc/renderer/runtime/useReticulumRuntime.tssrc/renderer/runtime/useReticulumRuntime.voice.test.tssrc/renderer/stores/reticulumPeerStore.tssrc/renderer/stores/reticulumVoiceStore.test.tssrc/renderer/stores/reticulumVoiceStore.tssrc/renderer/vitest.electronApiMock.tssrc/shared/electron-api.types.tssrc/shared/voice-types.test.tssrc/shared/voice-types.ts
💤 Files with no reviewable changes (1)
- reticulum-sidecar/src/api/system.rs
Comment on lines
+34
to
+44
| const showIncoming = | ||
| incoming?.role === 'incoming' && | ||
| (incoming.status === 'ringing' || incoming.status === 'available'); | ||
|
|
||
| const showInCall = | ||
| active != null && | ||
| !showIncoming && | ||
| (active.status === 'calling' || | ||
| active.status === 'connecting' || | ||
| active.status === 'established' || | ||
| active.status === 'ringing'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clear the incoming dialog when the call progresses.
applyIncoming() sets both store fields. Later status updates can change only activeCall to connecting or established while incomingCall remains ringing. This condition still shows the incoming dialog and hides the active-call bar after Answer.
Clear incomingCall when the active call leaves incoming ringing. Alternatively, derive showIncoming from active. Add a regression test that answers an incoming call, applies a connecting or established update, and verifies that the dialog closes.
Proposed local guard
- const showIncoming =
- incoming?.role === 'incoming' &&
- (incoming.status === 'ringing' || incoming.status === 'available');
+ const showIncoming =
+ active?.role === 'incoming' &&
+ (active.status === 'ringing' || active.status === 'available');📝 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 showIncoming = | |
| incoming?.role === 'incoming' && | |
| (incoming.status === 'ringing' || incoming.status === 'available'); | |
| const showInCall = | |
| active != null && | |
| !showIncoming && | |
| (active.status === 'calling' || | |
| active.status === 'connecting' || | |
| active.status === 'established' || | |
| active.status === 'ringing'); | |
| const showIncoming = | |
| active?.role === 'incoming' && | |
| (active.status === 'ringing' || active.status === 'available'); | |
| const showInCall = | |
| active != null && | |
| !showIncoming && | |
| (active.status === 'calling' || | |
| active.status === 'connecting' || | |
| active.status === 'established' || | |
| active.status === 'ringing'); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/components/reticulum/ReticulumVoiceOverlay.tsx` around lines 34
- 44, Update the incoming-call state flow used by applyIncoming and subsequent
activeCall status updates so incomingCall is cleared once the call progresses
beyond incoming ringing, allowing the active-call bar to display. Preserve
showIncoming for genuinely ringing or available incoming calls, and add a
regression test covering answer followed by connecting or established status
that verifies the dialog closes.
Comment on lines
+209
to
+233
| export async function reticulumVoiceAnswer(): Promise<void> { | ||
| const resp = await window.electronAPI.reticulum.voice.answer(); | ||
| if (!resp.ok) { | ||
| pushAppToast(resp.error || i18n.t('reticulumVoice.errors.callFailed'), 'error'); | ||
| return; | ||
| } | ||
| await startReticulumVoiceMediaForActiveCall(); | ||
| } | ||
|
|
||
| export async function reticulumVoiceReject(): Promise<void> { | ||
| stopReticulumVoiceMedia(); | ||
| await window.electronAPI.reticulum.voice.reject(); | ||
| useReticulumVoiceStore.getState().clearCall(); | ||
| } | ||
|
|
||
| export async function reticulumVoiceHangup(): Promise<void> { | ||
| stopReticulumVoiceMedia(); | ||
| await window.electronAPI.reticulum.voice.hangup(); | ||
| useReticulumVoiceStore.getState().clearCall(); | ||
| } | ||
|
|
||
| export async function reticulumVoiceSetMuted(muted: boolean): Promise<void> { | ||
| await window.electronAPI.reticulum.voice.mute({ muted }); | ||
| useReticulumVoiceStore.getState().setMicrophoneMuted(muted); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Preserve state integrity when the voice IPC calls fail.
None of these four functions handles a rejected IPC promise, and each one updates local state only after the call resolves. The failure modes differ:
reticulumVoiceAnswer(Line 210): a rejection escapes as an unhandled rejection. Callers usevoid reticulumVoiceAnswer(), so the user sees no error.reticulumVoiceRejectandreticulumVoiceHangup(Lines 220, 226): media stops first, which is correct, butclearCall()is skipped when the call rejects. The overlay then stays visible with stale call state and no dismiss path.reticulumVoiceSetMuted(Line 231): whenmute()rejects,setMicrophoneMutednever runs. The capture path instartCaptureAndTxreadsmicrophoneMutedfrom the store, so local capture gating diverges from the sidecar mute state.
Wrap each call and keep the local teardown unconditional.
🛡️ Proposed fix
export async function reticulumVoiceAnswer(): Promise<void> {
- const resp = await window.electronAPI.reticulum.voice.answer();
- if (!resp.ok) {
- pushAppToast(resp.error || i18n.t('reticulumVoice.errors.callFailed'), 'error');
- return;
- }
+ try {
+ const resp = await window.electronAPI.reticulum.voice.answer();
+ if (!resp.ok) {
+ pushAppToast(resp.error || i18n.t('reticulumVoice.errors.callFailed'), 'error');
+ return;
+ }
+ } catch (e) {
+ console.warn('[reticulumVoice] answer failed', e);
+ pushAppToast(i18n.t('reticulumVoice.errors.callFailed'), 'error');
+ return;
+ }
await startReticulumVoiceMediaForActiveCall();
}
export async function reticulumVoiceReject(): Promise<void> {
stopReticulumVoiceMedia();
- await window.electronAPI.reticulum.voice.reject();
- useReticulumVoiceStore.getState().clearCall();
+ try {
+ await window.electronAPI.reticulum.voice.reject();
+ } catch (e) {
+ console.warn('[reticulumVoice] reject failed', e);
+ } finally {
+ // Always drop local call state so the overlay cannot latch.
+ useReticulumVoiceStore.getState().clearCall();
+ }
}
export async function reticulumVoiceHangup(): Promise<void> {
stopReticulumVoiceMedia();
- await window.electronAPI.reticulum.voice.hangup();
- useReticulumVoiceStore.getState().clearCall();
+ try {
+ await window.electronAPI.reticulum.voice.hangup();
+ } catch (e) {
+ console.warn('[reticulumVoice] hangup failed', e);
+ } finally {
+ useReticulumVoiceStore.getState().clearCall();
+ }
}
export async function reticulumVoiceSetMuted(muted: boolean): Promise<void> {
- await window.electronAPI.reticulum.voice.mute({ muted });
- useReticulumVoiceStore.getState().setMicrophoneMuted(muted);
+ try {
+ const resp = await window.electronAPI.reticulum.voice.mute({ muted });
+ // Trust the sidecar's echoed state when present.
+ useReticulumVoiceStore.getState().setMicrophoneMuted(resp.microphone_muted ?? muted);
+ } catch (e) {
+ console.warn('[reticulumVoice] mute failed', e);
+ pushAppToast(i18n.t('reticulumVoice.errors.muteFailed'), 'error');
+ }
}Register reticulumVoice.errors.muteFailed in the English i18n keys. As per coding guidelines, "Stateful or I/O code must preserve integrity on failure and document failure points, fallbacks, and relevant logging."
🤖 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/reticulumVoiceSession.ts` around lines 209 - 233, Update
reticulumVoiceAnswer, reticulumVoiceReject, reticulumVoiceHangup, and
reticulumVoiceSetMuted to catch rejected IPC calls, report failures through the
existing voice error toast/i18n patterns, and preserve local state integrity.
Keep clearCall() unconditional after reject/hangup attempts, and ensure
setMicrophoneMuted(muted) runs even when mute() fails; retain media teardown
before reject/hangup and register the reticulumVoice.errors.muteFailed English
i18n key.
Source: Coding guidelines
rinchen
marked this pull request as draft
August 3, 2026 23:07
Keep hangup available during optimistic dial, improve identity resolution and LXST capability hints, add ringback/busy/fail tones with safety timeout, surface TX/RX stats, and log lifecycle-only call audits without per-packet noise.
Move PCM off the shared WS bus onto /ws/voice and reticulum:voiceAudio, bound dest→identity cache growth, and fix incoming-call UI plus session IPC/media races so answer, mute, and playback stay reliable.
Lock Probe-before-Path, shared cyan/slate class contracts, and Call/Send-file link shape so the Chat DM header cannot regress to mixed pills and bordered controls.
Flat cyan text-links did not read as clickable; shared action chrome now uses border/fill/hover so Peer details, Probe, Path, Call, and Send file match.
rinchen
changed the title
Persist Default, Unscoped, and named scopes independently by viewKey so mesh-wide Public and metro-style channels do not bleed when switching pills; document RegionMesh region/scope guidance.
Stop silent outgoing audio and false fail toasts, gate terminal/media teardown by link/generation, and keep mute/reject/hangup optimistic UI in sync with IPC. Cover the paths with tests and refresh voice/flood-scope i18n plus docs.
Keep Flatpak standalone pnpm URLs/sha256s aligned with package.json packageManager after the pnpm bump.
Member
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 58 minutes. |
Resolve remote_identity through the peer store so Calling/incoming UI shows a display name instead of a raw hash.
Member
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Move the in-call mini-panel from the bottom-right corner to the viewport center so dialing is easier to notice.
Prefer hash-backed senders when own IDs are unknown, filter self from DM tabs, and derive own node ids from identity lxmf_hash earlier so autofocus cannot land on yourself.
…r mic thrash Promote opportunistic LXMF and voice lifecycle lines to warn so developer bundles can classify inbound DM/call failures, and single-flight mic start per call generation on Answer.
Remember the last whisper peer so follow-up sends in the synthetic whispers room go out as direct NOTICE instead of the join-room guard. Fixes #788
…ress Cold-start MQTT connected before deviceStore channel configs arrived, so LongFast stayed on slot 0 (OnTrail). Re-push topic indices when resolved configs change, and pre-arm MeshCore BLE MAC suppress from sticky storage so Meshtastic NodeDB cannot revive the companion ghost before BLE attach.
Gate PCM TX until established and soft-drop early frames so lxst no longer fatal-errors mid-dial and sticks the line busy. Progress tones and dialer status follow connect → ring → in-call; toast only on connect failure and busy.
Switch ringback to a UK-style double ring with ~1s silence between pairs. Treat unanswered outbound terminate (null/terminated) as connect-failed so busy tone and toast always fire; harden safety hangup after silent clears.
Outbound calls play 2s dial, a peer-hash DTMF fingerprint, then UK ringback (3s cycle). Connect-fail uses reorder; no-answer/busy use short dual busy; toast on connect-fail, busy, no-answer, and unexpected drop.
Stop opening the mic on Answer before Established so Columba inbound calls cannot fatal-error rsLXST with pre-establish PCM; gate renderer TX the same way and cover Answer/TX/overlay/runtime paths with tests.
Derive the 4-digit burst from a full-hash XOR fold so peers no longer share prefix-identical melodies, lengthen digits slightly, and wait 250ms before UK ring.
When AutoInterface is down or Direct fails on Auto, prefer live private TCP/UDP hubs for LXMF recovery instead of hanging on a stuck Auto path.
…r pin Wire prefer_private into LXMF rediscovery, correlate voice.error by link/generation, pin RRC whisper reply targets against inbound overwrite, and backfill docs/i18n/tests.
Reconnect/connect contracts now assert commitConnectedMeshcoreBleSuppression and preserveOrClearMeshcoreBleSuppression after the sticky-MAC helper extract.
Persist last-known LXMF hash to seed own-node ids before sidecar identity, and gate inferred DM tabs until own is known.
Adopt the radio-reported path.hash.mode into app settings instead of pushing a stamped 1-byte default, and omit that key from AppPanel autosave unless the user changes the dropdown.
rinchen
changed the title
rinchen
changed the title
rinchen
marked this pull request as ready for review
August 4, 2026 23:00
Member
Author
|
@coderabbitai full review |
|
10 tasks
rinchen
added a commit
that referenced
this pull request
Aug 5, 2026
…ash allowlist (#793) * feat(reticulum): modem handshake then carrier during voice connect Replace post-DTMF auto-ringback with a short modem handshake and quiet carrier bed; cut to UK ringback as soon as the call reaches connecting. * fix(reticulum): allowlist reticulumLastSelfLxmfHash for appSettings:set SQLite mirror of the last LXMF self hash was rejected by the IPC allowlist since #785; localStorage still worked. Add the key and lock it in contracts. * feat(reticulum): add encrypted LXMF paper message create and ingest Wire sidecar paper APIs, deep-link/QR ingest, Chat share/scan UI, and Network Scan/import so encrypted paper messages can be exchanged without RF. * fix(reticulum): harden LXMF paper persist, ingest, and i18n Allow paper received_via through SQLite IPC, route create/ingest through shared LXMF helpers, map sidecar errors to stable codes, and fix locale scheme breakage check:i18n missed. * fix(reticulum): address paper PR review findings Gate paper UI on hasLxmfPaper, resolve paper-ingest sender names, restore modal focus, and harden i18n/tones/tooltip/tests from review.
9 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
Large
updatebranch covering:undici,postcss,ip-address), plus pnpm/Flatpak packaging bumps.Security
Raises transitive security floors in
pnpm-workspace.yamland bumps the package manager so CIpnpm audit/ Dependabot alerts clear after merge tomain.^7.29.0(was^7.28.0)type)^8.5.25(was^8.5.18; advisory needs ≥8.5.23).mapread whenfromunset)^10.3.1(new override; mqtt → socks)Also on this branch:
packageManager11.15.1 → 11.20.0 (chore: update deps and dedupe+ lockfile refresh).packageManager.Features
Reticulum LXST voice calls
TelephonyService: HTTP/api/v1/voice/*, WSvoice.*, dedicated/ws/voicePCM path, IPCreticulum:voice*/reticulum:voiceSendAudio/reticulum:onVoiceAudio.hasLxstVoice, with incoming/outgoing overlay, mute/reject/hangup, capability soft badge.established; sidecar soft-drops pre-establish PCM; Answer mic start coalesced bycallGeneration(stops Columba thrash / fatal mid-dial).rsLXSTviaclone-ratspeak-stack.sh; docs + i18n for voice strings.MeshCore Chat flood scope per channel
viewKeyso mesh-wide Public and metro-style channels do not bleed when switching pills.Reticulum Chat DM header polish
Internal cleanup
reticulumPeerStoredynamic imports (static path-slot helpers; silences ViteINEFFECTIVE_DYNAMIC_IMPORT).Bug fixes
Reticulum Chat / identity
lxmf_hashearlier.Reticulum LXMF / AutoInterface
prefer_privateinto LXMF rediscovery; promote opportunistic LXMF/voice triage lines towarnfor Columba/developer-bundle classification.Reticulum voice reliability (post-feature hardening)
voice.errorby link/generation; treat unanswered outbound terminate so busy tone + toast always fire./ws/voice; bound dest→identity cache growth; fix incoming-call UI + session IPC/media races.RRC
[whispers](remember last whisper peer so follow-ups go out as direct NOTICE); pin whisper reply targets against inbound overwrite.MeshCore
path.hash.modeinto app settings; AppPanel autosave no longer stamps default mode0just from visiting App (stops fighting MeshCore app Experimental 2-byte setting). Dropdown still writes the radio when the user changes it while connected.commitConnectedMeshcoreBleSuppression/preserveOrClearMeshcoreBleSuppression); reconnect contracts updated after extract.Meshtastic MQTT
Commits (
origin/main..HEAD)chore: update deps and dedupefeat(reticulum): integrate rsLXST voice calls(includes undici/postcss/ip-address floors)refactor(reticulum): drop ineffective peerStore dynamic importsfix(reticulum): voice call mini-panel, tones, and audit loggingfix(reticulum): harden LXST voice after review findingstest(reticulum): cover DM header action polish order and stylesfix(reticulum): make DM header actions outlined chip buttonsfeat(meshcore): remember Chat flood scope per channelfix(reticulum): harden LXST voice session lifecycle and docsfix(flatpak): bump vendored pnpm archives to 11.20.0merge: bring origin/main into updatefix(reticulum): show peer name in voice dialing overlayfix(reticulum): center voice calling overlay on screenfix(reticulum): stop opening a self DM on Chat launchfix(reticulum): surface Columba LXMF/voice triage logs and stop answer mic thrashfix(rrc): allow plain-text replies in [whispers]fix(rrc): pin chat to bottom with TanStack Virtualfix: re-sync MQTT channel map after RF configure and sticky Blue suppressfix(reticulum): dial tone then ring/busy for outbound LXST voicefix(reticulum): UK double-ring and busy on incomplete outbound voicefix(reticulum): dial–DTMF–ring sequence and short reorder/busy tonesfix(reticulum): defer LXST Answer media until establishedfix(reticulum): unique peer DTMF fold and gap before ringbackfix(reticulum): demote unhealthy Auto toward private LAN Direct pathsfix(reticulum): harden Auto private failover, voice races, and whisper pintest(meshcore): update BLE suppress source contracts for helper extractfix(reticulum): stop sticky self DM flash during launch hydratefix(meshcore): stop resetting companion path hash mode on connectTest plan
Security / packaging
pnpm auditclean (or only known accepted advisories) after install with new floorsmainpackageManager11.20.0Features
pnpm run check:pr(full lint/typecheck/test:run; sidecar path-aware)pnpm run reticulum:sidecar:test/ Clippy (siblingrsLXSTatorigin/main)Bug fixes
[whispers]plain follow-ups go to last whisper peer