Skip to content

RR-T40 PTT fixes for audio routing and chime sounds.#104

Merged
ucswift merged 4 commits intomasterfrom
develop
Feb 1, 2026
Merged

RR-T40 PTT fixes for audio routing and chime sounds.#104
ucswift merged 4 commits intomasterfrom
develop

Conversation

@ucswift
Copy link
Member

@ucswift ucswift commented Jan 31, 2026

Summary by CodeRabbit

  • New Features

    • Native in-call audio (improved Android playback) and a cross-platform in-call audio service.
    • System audio device option for routing virtual/system audio.
  • Improvements

    • Bluetooth selector: spinner, toast error feedback, and prevention of concurrent connections; ability to forget preferred device.
    • Better call audio routing, enhanced headset-button behavior (separate play/pause), CallKeep/foreground service handling, and native audio fallbacks.
  • Removed

    • In-app LiveKit call modal and related legacy call UI.
  • Documentation

    • Added mobile app coding guidelines.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 31, 2026

📝 Walkthrough

Walkthrough

Adds an Expo plugin and Android Kotlin module for in‑call audio, a cross‑platform InCallAudio service with native playback, major LiveKit/call routing and CallKeep/headset integrations, Bluetooth UX/store and service APIs, expanded Jest mocks and tests, removal of legacy LiveKit UI/store files, and coding rules documentation.

Changes

Cohort / File(s) Summary
Config & Plugin
app.config.ts, plugins/withInCallAudioModule.js
Registers new Expo plugin withInCallAudioModule that copies audio assets, generates Android Kotlin sources (InCallAudioModule/InCallAudioPackage), and patches MainApplication to register the package.
Native & Cross‑platform Audio
src/utils/InCallAudio.ts, src/services/audio.service.ts
Adds inCallAudio singleton; AudioService gains initializeNativeAudio, isNativeAudioInitialized, native init/cleanup and prefers native playback on Android with expo-av fallback.
LiveKit / Call Routing
src/stores/app/livekit-store.ts, src/features/livekit-call/... (removed)
Refactors LiveKit store for explicit audio routing, CallKeep debounce, foreground service and headset monitoring; removes LiveKitCallModal component and related store/tests.
Bluetooth UX & Store
src/components/settings/bluetooth-device-selection-bottom-sheet.tsx, src/stores/app/bluetooth-audio-store.ts, src/services/bluetooth-audio.service.ts
Improves device selection UX (toasts, connecting state, spinner, system-audio handling); introduces exported SYSTEM_AUDIO_DEVICE and BluetoothAudioService.forgetPreferredDevice API; class now exported.
CallKeep & Headset Button
src/services/callkeep.service.android.ts, src/services/callkeep.service.ios.ts, src/services/headset-button.service.ts
Implements full Android CallKeep with per-call UUIDs and event callbacks; iOS CallKeep adds end-call callback and mute gating; headset-button handling extended (new button types, mic sync, restore audio session).
Tests & Mocks
jest-setup.ts, many src/**/__tests__/*
Expands react‑native and @livekit/react-native mocks (InCallAudioModule, AudioSession, NativeEventEmitter); adds new bluetooth forget tests; many tests now include repeated mock resets.
UI / Minor
src/components/ui/bottom-sheet.tsx, src/components/livekit/livekit-bottom-sheet.tsx
Minor layout tweak (add flex-1) and removal of LiveKitCallModal and its tests.
Docs & Rules
.agent/rules/rules.md
Adds repository coding guidelines covering TypeScript, React Native, Expo, testing, and mobile best practices.
Misc
package.json, jest-setup.ts
Updates ios npm script to include --device; broadens Jest native mocks for new native modules.

Sequence Diagram

sequenceDiagram
    participant App as App Init
    participant AudioSvc as AudioService
    participant InCall as InCallAudioService
    participant Native as Android NativeModule
    participant Expo as Expo AV (iOS)
    participant Store as LiveKit Store
    participant CallKeep as CallKeep Service

    App->>AudioSvc: initializeAudio()
    AudioSvc->>InCall: initialize()
    alt Android
        InCall->>Native: initializeNativeAudio() / loadSound()
        Native-->>InCall: ready
    else iOS
        InCall->>Expo: configure audio mode
        Expo-->>InCall: ready
    end
    InCall-->>AudioSvc: initialized
    App->>Store: connectToRoom(token)
    Store->>CallKeep: startCall()
    Store->>InCall: playSound("connected")
    alt Android
        InCall->>Native: playSound(name)
    else iOS
        InCall->>Expo: play asset
    end
    User->>Store: toggleMicrophone()
    Store->>CallKeep: setMuted(...)
    Store->>InCall: playSound("transmit_start"/"transmit_stop")
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 I hopped through code to stitch each tone,

Kotlin helpers laid and asset seeds sown.
Android hums, iOS sings along,
Buttons, calls, and tests all join the song.
A rabbit's cheer — may every ringtone ring!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'RR-T40 PTT fixes for audio routing and chime sounds' accurately summarizes the main changes: implementing native in-call audio module, audio routing enhancements, microphone/headset state management, and related infrastructure across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch develop

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/utils/InCallAudio.ts`:
- Around line 33-72: The initializer race condition occurs because initialize()
is not async and does not await Audio.setAudioModeAsync() (and any async native
calls), yet playSound() calls initialize() without awaiting it; make
initialize() async, await any async native calls (e.g., await
Audio.setAudioModeAsync(...) in the iOS path and await
InCallAudioModule.initializeAudio() / loadSound promises if they exist on
Android), only set this.isInitialized = true after those awaits succeed, and
change playSound() to await this.initialize() (or check and await an in-progress
initialization Promise) before proceeding to play; also propagate and log errors
from awaited calls consistently.
- Around line 80-88: The playback status handler is registered after playback
starts (see Audio.Sound.createAsync / sound.playAsync and
sound.setOnPlaybackStatusUpdate in InCallAudio.ts), which can miss cleanup for
very short sounds; fix by registering the status update handler as the third
argument to Audio.Sound.createAsync and start playback via the initial status {
shouldPlay: true } (i.e., pass the handler and initial status into createAsync)
so the didJustFinish check/unloadAsync runs even for short sounds and you can
remove the separate call to sound.playAsync.
🧹 Nitpick comments (9)
src/components/settings/bluetooth-device-selection-bottom-sheet.tsx (2)

292-295: Inline function in onPress handler may cause unnecessary re-renders.

The anonymous arrow function () => !isDisabled && handleDeviceSelect(item) is created on every render. Since renderDeviceItem is already wrapped in useCallback, consider moving the conditional logic into handleDeviceSelect itself (which already has the guard) and use handleDeviceSelect directly.

♻️ Suggested simplification

Since handleDeviceSelect already has the connectingDeviceId guard at line 118, you can simplify:

       <Pressable
-        onPress={() => !isDisabled && handleDeviceSelect(item)}
-        disabled={isDisabled}
+        onPress={() => handleDeviceSelect(item)}
+        disabled={connectingDeviceId !== null}
         className={`mb-2 rounded-lg border p-4 ${isSelected ? 'border-primary-500 bg-primary-50 dark:bg-primary-950' : 'border-neutral-200 bg-white dark:border-neutral-700 dark:bg-neutral-800'} ${isDisabled && !isConnecting ? 'opacity-50' : ''}`}
       >

The disabled prop already prevents interaction, and handleDeviceSelect has its own early return guard.


300-306: Consider using Tailwind shorthand for sizing.

Static analysis suggests using size-4 instead of h-4 w-4.

♻️ Suggested shorthand
                {isConnecting ? (
-                  <Box className="mr-2 h-4 w-4">
+                  <Box className="mr-2 size-4">
                    <Spinner size="small" />
                  </Box>
plugins/withInCallAudioModule.js (2)

68-72: Incomplete fallback logic in loadSound.

The fallback block (lines 69-72) logs a warning but doesn't actually do anything. Either implement a meaningful fallback or remove the dead code to avoid confusion during debugging.

♻️ Suggested cleanup
        // Fallback: Try identifying without package name if first attempt fails (though context.packageName is usually correct)
        if (resId == 0) {
-             Log.w(TAG, "Resource $resourceName not found in \${context.packageName}, trying simplified lookup")
-             // Reflection-based lookup if needed, but getIdentifier is standard.
+             Log.w(TAG, "Resource $resourceName not found in \${context.packageName}")
+             return  // Exit early since we can't load the sound
        }

236-252: String-based MainApplication.kt modification is fragile but acceptable for config plugins.

The regex pattern and string replacements work for standard Expo-generated MainApplication files. However, if the file has been heavily customized, these patterns might not match. The defensive check at line 228 (!mainApplication.contents.includes('InCallAudioPackage')) helps avoid duplicate registrations.

Consider adding a warning if the pattern doesn't match:

♻️ Suggested defensive logging
      if (packagesMatch) {
        // ... existing replacement logic
        console.log('[withInCallAudioModule] Registered InCallAudioPackage in MainApplication.kt');
+     } else {
+       console.warn('[withInCallAudioModule] Could not find PackageList pattern in MainApplication.kt - manual registration may be required');
      }
src/services/audio.service.ts (2)

339-346: Empty catch block silently swallows cleanup errors.

While cleanup errors may not be critical, completely swallowing them can mask issues during debugging.

♻️ Suggested improvement
      if (Platform.OS === 'android' && this.isNativeAudioInitialized && InCallAudioModule) {
        try {
          InCallAudioModule.cleanup?.();
          this.isNativeAudioInitialized = false;
        } catch (e) {
-          /* ignore */
+          logger.debug({ message: 'Native audio cleanup error (non-critical)', context: { error: e } });
        }
      }

7-8: Consider adding TypeScript interface for InCallAudioModule.

The native module lacks type definitions, which could lead to runtime errors if the API changes.

♻️ Suggested type definition
interface InCallAudioModuleType {
  initializeAudio?: () => void;
  loadSound: (name: string, resourceName: string) => void;
  playSound: (name: string) => void;
  cleanup?: () => void;
}

const { InCallAudioModule } = NativeModules as { InCallAudioModule?: InCallAudioModuleType };
src/utils/InCallAudio.ts (3)

1-1: Rename this file to lowercase‑hyphenated.

src/utils/InCallAudio.ts doesn’t follow the src/** naming convention. Please rename to something like src/utils/in-call-audio.ts and update imports accordingly.

As per coding guidelines, src/**: Organize files by feature, grouping related components, hooks, and styles. Directory and file names should be lowercase and hyphenated (e.g., user-profile).


2-6: Type the native module instead of relying on any.

NativeModules access is untyped, which erodes TypeScript safety and hides return types (sync vs promise). Consider adding an explicit interface and casting.

♻️ Proposed change
-import { NativeModules, Platform } from 'react-native';
+import { NativeModules, Platform } from 'react-native';
 
-const { InCallAudioModule } = NativeModules;
+type InCallAudioModuleType = {
+  initializeAudio?: () => void | Promise<void>;
+  loadSound: (name: string, resourceName: string) => void | Promise<void>;
+  playSound: (name: string) => void | Promise<void>;
+};
+
+const InCallAudioModule =
+  NativeModules.InCallAudioModule as InCallAudioModuleType | undefined;

As per coding guidelines, Write concise, type-safe TypeScript code; Avoid using any; use precise types.


8-28: Use camelCase for the sound map constant.

SOUNDS isn’t camelCase. Rename it to sounds and update references.

♻️ Proposed change
-const SOUNDS = {
+const sounds = {
   connected: {
     android: 'space_notification1',
     ios: require('../../assets/audio/ui/space_notification1.mp3'),
   },
   disconnected: {
@@
-} as const;
+} as const;
 
-type SoundName = keyof typeof SOUNDS;
+type SoundName = keyof typeof sounds;
@@
-          Object.entries(SOUNDS).forEach(([name, config]) => {
+          Object.entries(sounds).forEach(([name, config]) => {
             InCallAudioModule.loadSound(name, config.android);
           });
@@
-        const source = SOUNDS[name].ios;
+        const source = sounds[name].ios;

As per coding guidelines, Use camelCase for variable and function names (e.g., isFetchingData, handleUserInput).

Also applies to: 43-45, 80-81


try {
// Generate a new UUID for this call
currentCallUUID = this.generateUUID();

Check failure

Code scanning / CodeQL

Insecure randomness High

This uses a cryptographically insecure random number generated at
Math.random()
in a security context.

Copilot Autofix

AI 3 days ago

In general, the fix is to stop using Math.random() in the generateUUID fallback and replace it with a cryptographically secure random source that is available in React Native on Android. The ideal is to use crypto.getRandomValues (standard Web Crypto API) when crypto.randomUUID is not available, and only if that is also unavailable, fall back to a less secure option with a clear warning.

Concretely, within src/services/callkeep.service.android.ts, we should modify generateUUID to:

  1. Prefer global.crypto.randomUUID (unchanged).
  2. Next, try global.crypto.getRandomValues to generate 16 random bytes and format them as a UUID v4, without using Math.random().
  3. Only as a last resort, use Math.random() but log a warning so it’s obvious to operators that their runtime lacks secure randomness. This keeps behavior compatible, but strongly favors secure randomness on supported platforms.

No other behavior of the service changes: generateUUID still returns a UUID-formatted string, and callers still use it exactly as before.

Suggested changeset 1
src/services/callkeep.service.android.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/services/callkeep.service.android.ts b/src/services/callkeep.service.android.ts
--- a/src/services/callkeep.service.android.ts
+++ b/src/services/callkeep.service.android.ts
@@ -293,10 +293,56 @@
    * Generate a UUID for CallKeep calls
    */
   private generateUUID(): string {
-    // RN 0.76 typically provides global crypto.randomUUID via Hermes/JSI
-    const rndUUID = (global as any)?.crypto?.randomUUID?.();
+    // Prefer crypto.randomUUID if available (RN 0.76+ / Hermes / JSI)
+    const cryptoObj = (global as any)?.crypto;
+    const rndUUID = cryptoObj?.randomUUID?.();
     if (typeof rndUUID === 'string') return rndUUID;
-    // Fallback
+
+    // Fallback: use crypto.getRandomValues to generate a RFC4122 v4 UUID
+    if (typeof cryptoObj?.getRandomValues === 'function') {
+      const bytes = new Uint8Array(16);
+      cryptoObj.getRandomValues(bytes);
+
+      // Per RFC4122 v4: set version and variant bits
+      bytes[6] = (bytes[6] & 0x0f) | 0x40; // version 4
+      bytes[8] = (bytes[8] & 0x3f) | 0x80; // variant 10xx
+
+      const byteToHex: string[] = [];
+      for (let i = 0; i < 256; i++) {
+        byteToHex.push((i + 0x100).toString(16).substring(1));
+      }
+
+      return (
+        byteToHex[bytes[0]] +
+        byteToHex[bytes[1]] +
+        byteToHex[bytes[2]] +
+        byteToHex[bytes[3]] +
+        '-' +
+        byteToHex[bytes[4]] +
+        byteToHex[bytes[5]] +
+        '-' +
+        byteToHex[bytes[6]] +
+        byteToHex[bytes[7]] +
+        '-' +
+        byteToHex[bytes[8]] +
+        byteToHex[bytes[9]] +
+        '-' +
+        byteToHex[bytes[10]] +
+        byteToHex[bytes[11]] +
+        byteToHex[bytes[12]] +
+        byteToHex[bytes[13]] +
+        byteToHex[bytes[14]] +
+        byteToHex[bytes[15]]
+      );
+    }
+
+    // Final fallback: non-cryptographic UUID-like value.
+    // This should rarely be used; log a warning for visibility.
+    logger.warn({
+      message:
+        'CallKeepService.generateUUID is falling back to Math.random(); crypto APIs are unavailable. UUIDs may be predictable.',
+    });
+
     return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, (c) => {
       const r = (Math.random() * 16) | 0;
       const v = c === 'x' ? r : (r & 0x3) | 0x8;
EOF
@@ -293,10 +293,56 @@
* Generate a UUID for CallKeep calls
*/
private generateUUID(): string {
// RN 0.76 typically provides global crypto.randomUUID via Hermes/JSI
const rndUUID = (global as any)?.crypto?.randomUUID?.();
// Prefer crypto.randomUUID if available (RN 0.76+ / Hermes / JSI)
const cryptoObj = (global as any)?.crypto;
const rndUUID = cryptoObj?.randomUUID?.();
if (typeof rndUUID === 'string') return rndUUID;
// Fallback

// Fallback: use crypto.getRandomValues to generate a RFC4122 v4 UUID
if (typeof cryptoObj?.getRandomValues === 'function') {
const bytes = new Uint8Array(16);
cryptoObj.getRandomValues(bytes);

// Per RFC4122 v4: set version and variant bits
bytes[6] = (bytes[6] & 0x0f) | 0x40; // version 4
bytes[8] = (bytes[8] & 0x3f) | 0x80; // variant 10xx

const byteToHex: string[] = [];
for (let i = 0; i < 256; i++) {
byteToHex.push((i + 0x100).toString(16).substring(1));
}

return (
byteToHex[bytes[0]] +
byteToHex[bytes[1]] +
byteToHex[bytes[2]] +
byteToHex[bytes[3]] +
'-' +
byteToHex[bytes[4]] +
byteToHex[bytes[5]] +
'-' +
byteToHex[bytes[6]] +
byteToHex[bytes[7]] +
'-' +
byteToHex[bytes[8]] +
byteToHex[bytes[9]] +
'-' +
byteToHex[bytes[10]] +
byteToHex[bytes[11]] +
byteToHex[bytes[12]] +
byteToHex[bytes[13]] +
byteToHex[bytes[14]] +
byteToHex[bytes[15]]
);
}

// Final fallback: non-cryptographic UUID-like value.
// This should rarely be used; log a warning for visibility.
logger.warn({
message:
'CallKeepService.generateUUID is falling back to Math.random(); crypto APIs are unavailable. UUIDs may be predictable.',
});

return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, (c) => {
const r = (Math.random() * 16) | 0;
const v = c === 'x' ? r : (r & 0x3) | 0x8;
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/services/__tests__/audio.service.test.ts (1)

130-138: ⚠️ Potential issue | 🟡 Minor

Use InterruptionModeIOS and InterruptionModeAndroid enum constants instead of string literals.

Both interruptionModeIOS (line 137) and interruptionModeAndroid (line 138) should use the enum constants to match the production code and maintain type safety.

Proposed change
      expect(mockAudioSetAudioModeAsync).toHaveBeenCalledWith({
        allowsRecordingIOS: true,
        staysActiveInBackground: true,
        playsInSilentModeIOS: true,
        shouldDuckAndroid: true,
        playThroughEarpieceAndroid: true,
-       interruptionModeIOS: 'mixWithOthers',
-       interruptionModeAndroid: 'duckOthers',
+       interruptionModeIOS: InterruptionModeIOS.MixWithOthers,
+       interruptionModeAndroid: InterruptionModeAndroid.DuckOthers,
      });
🤖 Fix all issues with AI agents
In @.agent/rules/rules.md:
- Around line 9-45: The rules document contains several wording and typo
issues—fix wording like "Directory and File names should be lowercase and
hyphenated" to "directory and file names should be lowercase and hyphenated",
correct "dictonary" to "dictionary", change "Create and use Jest to test to
validate" to "Create and run Jest tests to validate", and rephrase awkward
sentences such as "If there isn't a Gluestack component in the `components/ui`
directory for the component you are trying to use consistently style it either
through `StyleSheet.create()` or Styled Components" to a clearer instruction
referencing `gluestack-ui` and `components/ui`; also ensure consistent
capitalization for terms like `TypeScript`, `React.FC`, `tsconfig.json`,
`react-native-fast-image`, `getItemLayout`, and `t()` (from `react-i18next`)
throughout the file. Ensure the list items use consistent punctuation and
grammar and update "iOS and Android platforms and ensure that the app is
optimized" to a smoother phrasing mentioning both platforms once.

In `@package.json`:
- Line 12: The current npm script named "ios" forces a physical device by
including "--device", which breaks simulator and CI workflows; change the "ios"
script (the "ios" entry) to run "cross-env EXPO_NO_DOTENV=1 expo run:ios"
without "--device" so it defaults to the simulator, and add a new script (e.g.,
"ios:device") that runs "cross-env EXPO_NO_DOTENV=1 expo run:ios --device" for
developers who need a physical device; update any docs or references that call
the old "ios" script if necessary.

In `@src/services/bluetooth-audio.service.ts`:
- Around line 349-376: The forgetPreferredDevice flow should first read the
current store preference and only remove the persisted key if it matches the
device being forgotten; also await the async removeItem so failures are caught
by the try/catch. Concretely, in forgetPreferredDevice(): call
useBluetoothAudioStore.getState() early, check that store.preferredDevice?.id
=== deviceId before calling removeItem(PREFERRED_BLUETOOTH_DEVICE_KEY), await
that call, and then call store.setPreferredDevice(null); keep using the existing
symbols forgetPreferredDevice, PREFERRED_BLUETOOTH_DEVICE_KEY, removeItem,
useBluetoothAudioStore.getState, and store.setPreferredDevice.

In `@src/services/callkeep.service.ios.ts`:
- Line 2: Remove the unused import of config from dotenv in this file: delete
the line importing "config" (the unused symbol `config`) from 'dotenv' in
callkeep.service.ios.ts so the file no longer includes an unused dependency; if
dotenv is intended to be used globally, ensure it's initialized in a single
central entrypoint instead of here.

In `@src/services/headset-button.service.ts`:
- Around line 418-429: The new executeButtonAction branches for event.type ===
'play' and 'pause' are never reached because mapButtonType,
mapRemoteControlType, and mapMediaButtonType currently collapse play/pause into
'play_pause'; update those mapper functions so they preserve and return distinct
'play' and 'pause' when the raw input indicates one or the other (instead of
always returning 'play_pause'), so executeButtonAction's explicit play/pause
branches will be exercised; alternatively, if preserving the original raw event
is preferred, pass an indicator (e.g., originalType) through the mapping
pipeline and have executeButtonAction use that to decide between 'play' and
'pause'.
- Around line 145-156: setupIOSListeners and setupAndroidListeners only
subscribe to route/connection events and never wire up the headset button event
listener, so handleRemoteControlEvent is never invoked; add
DeviceEventEmitter.addListener calls in both setupIOSListeners and
setupAndroidListeners to listen for the native headset-button event (the same
event name emitted by the native side when StartHeadsetButtonMonitoring is sent)
and register handleRemoteControlEvent as the handler; ensure you push those
subscriptions onto this.subscriptions and remove them on teardown, and verify
the event name matches the native emitter so processButtonEvent is triggered via
handleRemoteControlEvent.

In `@src/stores/app/livekit-store.ts`:
- Line 7: Remove the invalid/unused import "set" from the import statement that
currently reads "import { set } from 'zod';" in livekit-store.ts; simply delete
that named import (or replace it with the correct zod symbol only if you
actually need it elsewhere) so the file no longer imports a non-existent symbol
from 'zod'.
🧹 Nitpick comments (41)
src/components/audio-stream/__tests__/audio-stream-bottom-sheet.test.tsx (1)

131-136: Collapse redundant mockReset calls into a single reset.

Multiple consecutive resets don’t add value beyond the first call and add noise to the setup.

Suggested diff
   beforeEach(() => {
     jest.clearAllMocks();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
     mockTrackEvent.mockReset();
   });
src/components/notes/__tests__/note-details-sheet-analytics.test.tsx (1)

46-51: Collapse redundant mockReset calls into a single reset.

Multiple consecutive resets don’t add value beyond the first call and add noise to the setup.

Suggested diff
   beforeEach(() => {
     jest.clearAllMocks();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
     mockTrackEvent.mockReset();
     // Reset mock store
     mockStore.notes = [];
src/components/units/__tests__/unit-details-sheet.test.tsx (1)

208-213: Collapse redundant mockReset calls into a single reset.

Multiple consecutive resets don’t add value beyond the first call and add noise to the setup.

Suggested diff
   beforeEach(() => {
     jest.clearAllMocks();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
     mockTrackEvent.mockReset();
     Object.assign(mockUnitsStore, {
src/components/calls/__tests__/call-notes-modal.test.tsx (1)

148-153: Collapse redundant mockReset calls into a single reset.

Multiple consecutive resets don’t add value beyond the first call and add noise to the setup.

Suggested diff
   beforeEach(() => {
     jest.clearAllMocks();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
     mockTrackEvent.mockReset();
 
     mockUseTranslation.mockReturnValue({
src/components/settings/__tests__/server-url-bottom-sheet.test.tsx (1)

169-174: Collapse redundant mockReset calls into a single reset.

Multiple consecutive resets don’t add value beyond the first call and add noise to the setup.

Suggested diff
   beforeEach(() => {
     jest.clearAllMocks();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
     mockTrackEvent.mockReset();
 
     // Default mock for analytics
src/components/messages/__tests__/message-details-sheet.test.tsx (1)

334-339: Collapse redundant mockReset calls into a single reset.

Multiple consecutive resets don’t add value beyond the first call and add noise to the setup.

Suggested diff
   beforeEach(() => {
     jest.clearAllMocks();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
     mockTrackEvent.mockReset();
 
     // Default mock for analytics
src/components/calls/__tests__/call-images-modal.test.tsx (1)

352-359: Collapse redundant mockReset calls into a single reset.

Multiple consecutive resets don’t add value beyond the first call and add noise to the setup.

Suggested diff
   beforeEach(() => {
     jest.clearAllMocks();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
     mockTrackEvent.mockReset();
     mockUseCallDetailStore.mockReturnValue(mockStore as any);
src/components/calls/__tests__/call-files-modal.test.tsx (1)

306-312: Consolidate redundant mock resets.

Five consecutive mockReset() calls are no-ops beyond the first and add noise. One reset is enough (or mockClear() if you only need call history).

Suggested change
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/shifts/__tests__/shift-details-sheet.test.tsx (1)

275-281: Collapse repeated mockReset() calls into a single reset.

Only the first call has an effect; the rest add noise.

Suggested change
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/calendar/__tests__/calendar-item-details-sheet-minimal.test.tsx (1)

133-139: Use a single mockReset() here.

Multiple consecutive resets don’t add value and reduce clarity.

Suggested change
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/bluetooth/__tests__/bluetooth-audio-modal.test.tsx (1)

189-195: Avoid redundant mockReset() calls.

Only the first reset is effective; the rest are no-ops.

Suggested change
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/notes/__tests__/note-details-sheet-integration.test.tsx (1)

47-53: Collapse repeated mockReset() calls.

One reset is sufficient; repeated calls are redundant.

Suggested change
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/calendar/__tests__/calendar-item-details-sheet.test.tsx (1)

242-248: Reduce repeated mockReset() calls to one.

Multiple consecutive resets don’t change behavior and add noise.

Suggested change
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/messages/__tests__/compose-message-sheet-simple.test.tsx (1)

195-201: Consolidate redundant mock resets.

Only one mockReset() is needed here.

Suggested change
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/settings/__tests__/login-info-bottom-sheet.test.tsx (1)

174-180: Remove redundant mock resets.

Five consecutive mockReset() calls add noise without changing behavior; a single reset is enough.

🧹 Suggested cleanup
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();

Also applies to: 255-262

src/components/contacts/__tests__/contact-details-sheet.test.tsx (1)

326-332: Remove redundant mock resets.

Multiple consecutive mockReset() calls are redundant; one reset is enough.

🧹 Suggested cleanup
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/units/__tests__/units-filter-sheet.test.tsx (1)

118-125: Remove redundant mock resets.

Five consecutive resets don’t add value; one reset is enough.

🧹 Suggested cleanup
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/staffing/__tests__/staffing-bottom-sheet.test.tsx (1)

254-261: Remove redundant mock resets.

Multiple consecutive mockReset() calls are unnecessary; keep one.

🧹 Suggested cleanup
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/messages/__tests__/compose-message-sheet.test.tsx (1)

370-377: Remove redundant mock resets.

Five consecutive resets add noise without changing behavior; a single reset is enough.

🧹 Suggested cleanup
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/calendar/__tests__/calendar-item-details-sheet-analytics.test.tsx (1)

207-213: Remove redundant mock resets.

Multiple consecutive mockReset() calls are unnecessary; keep one.

🧹 Suggested cleanup
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/personnel/__tests__/personnel-details-sheet.test.tsx (1)

161-167: Remove redundant mock resets.

Five consecutive mockReset() calls add noise; a single reset is enough.

🧹 Suggested cleanup
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/status/__tests__/personnel-status-integration.test.tsx (1)

231-237: Drop redundant mock resets in beforeEach.

jest.clearAllMocks() plus five consecutive mockReset() calls is unnecessary noise. One reset is enough (or omit entirely if clearAllMocks is sufficient).

♻️ Suggested cleanup
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/status/__tests__/personnel-status-bottom-sheet.test.tsx (1)

290-296: Remove repeated mockReset() calls.

One reset is enough; multiple consecutive resets add noise without changing behavior.

♻️ Suggested cleanup
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/status/__tests__/personnel-status-analytics.test.tsx (1)

49-55: Reduce redundant mock resets.

Multiple consecutive mockReset() calls add noise and slow test setup.

♻️ Suggested cleanup
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/components/notes/__tests__/note-details-sheet.test.tsx (1)

165-173: Consolidate redundant mock resets.

One reset is sufficient; multiple consecutive resets add noise.

♻️ Suggested cleanup
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/stores/app/bluetooth-audio-store.ts (1)

27-43: Consider aligning the name fields for consistency.

The outer name is 'System Audio' (line 29) while the nested device.name is 'System Audio / Airpods' (line 35). This inconsistency could cause confusion in the UI if both values are accessed in different contexts.

💡 Suggested alignment
 export const SYSTEM_AUDIO_DEVICE: BluetoothAudioDevice = {
   id: 'system-audio',
-  name: 'System Audio',
+  name: 'System Audio / Airpods',
   isConnected: false,
   hasAudioCapability: true,
   supportsMicrophoneControl: true,
   device: {
     id: 'system-audio',
     name: 'System Audio / Airpods',
src/app/__tests__/onboarding.test.tsx (1)

189-192: Remove redundant mockReset() calls.

Calling mockTrackEvent.mockReset() multiple times has no additional effect - a single call is sufficient. Additionally, jest.clearAllMocks() on line 188 already resets all mocks.

🧹 Suggested fix
   beforeEach(() => {
     jest.clearAllMocks();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
     (useRouter as jest.Mock).mockReturnValue(mockRouter);
src/app/login/__tests__/index.test.tsx (1)

46-50: Remove redundant mockReset() calls.

Same issue as in onboarding.test.tsx - calling mockTrackEvent.mockReset() multiple times is redundant. jest.clearAllMocks() on line 45 already handles the reset.

🧹 Suggested fix
   beforeEach(() => {
     jest.clearAllMocks();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
   });
src/components/calls/__tests__/call-detail-menu-integration.test.tsx (1)

99-105: Collapse redundant mock resets to a single call.

Multiple consecutive mockReset() calls don’t add value and slow/obscure setup; one reset is enough.

Proposed change
  beforeEach(() => {
    jest.clearAllMocks();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
+   mockTrackEvent.mockReset();
src/app/(app)/home/__tests__/calls.test.tsx (1)

165-172: Reduce repeated mock resets to a single call.

One mockReset() is enough; the extra five are redundant.

Proposed change
  beforeEach(() => {
    jest.clearAllMocks();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
+   mockTrackEvent.mockReset();
src/components/personnel/__tests__/personnel-filter-sheet.test.tsx (1)

98-105: Collapse redundant mock resets.

Repeated mockReset() calls are unnecessary; keep a single reset.

Proposed change
  beforeEach(() => {
    jest.clearAllMocks();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
+   mockTrackEvent.mockReset();
src/app/login/__tests__/login-form.test.tsx (1)

202-210: Use a single reset/clear for the analytics mock.

Multiple mockReset() calls plus mockClear() is redundant; keep one operation.

Proposed change
  beforeEach(() => {
    jest.clearAllMocks();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockClear();
+   mockTrackEvent.mockReset();
src/app/(app)/home/__tests__/index.test.tsx (1)

199-206: Collapse repeated mock resets to one.

Multiple consecutive mockReset() calls are redundant.

Proposed change
  beforeEach(() => {
    jest.clearAllMocks();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
+   mockTrackEvent.mockReset();
src/app/(app)/__tests__/notes.test.tsx (1)

33-39: Reduce redundant mock resets.

Only one mockReset() is needed in setup.

Proposed change
  beforeEach(() => {
    jest.clearAllMocks();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
-   mockTrackEvent.mockReset();
+   mockTrackEvent.mockReset();
src/app/(app)/__tests__/map.test.tsx (1)

316-322: Drop redundant mockReset() calls in beforeEach.

Five consecutive resets are no-ops after the first and add noise. One reset is enough.

Proposed cleanup
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
-    mockTrackEvent.mockReset();
+    mockTrackEvent.mockReset();
src/services/__tests__/bluetooth-audio-forget.test.ts (1)

49-71: Avoid any for the mocked store shape.

Use a typed slice of the store state so tests stay type-safe.

Example typing
+type MockBluetoothAudioStore = Pick<
+  ReturnType<typeof useBluetoothAudioStore.getState>,
+  | 'preferredDevice'
+  | 'connectedDevice'
+  | 'selectedAudioDevices'
+  | 'setPreferredDevice'
+  | 'setConnectedDevice'
+  | 'setSelectedMicrophone'
+  | 'setSelectedSpeaker'
+  | 'updateDevice'
+  | 'availableDevices'
+>;
+
 describe('BluetoothAudioService - forgetPreferredDevice', () => {
   let service: BluetoothAudioService;
-  let mockStore: any;
+  let mockStore: MockBluetoothAudioStore;

As per coding guidelines: Avoid using any; use precise types.

src/services/audio.service.ts (1)

76-102: Native audio initialization lacks async handling for native method calls.

The initializeNativeAudio() method is synchronous but invokes native methods (initializeAudio, loadSound) that may be asynchronous. If loadSound returns a Promise on Android, sounds may not be fully loaded when isNativeAudioInitialized is set to true, causing playSound() to fail silently.

Consider making this method async and awaiting the native calls, similar to the pattern used in InCallAudio.ts:

♻️ Proposed fix
-  private initializeNativeAudio(): void {
+  private async initializeNativeAudio(): Promise<void> {
     if (this.isNativeAudioInitialized) return;
 
     try {
       if (InCallAudioModule) {
-        InCallAudioModule.initializeAudio?.();
+        await InCallAudioModule.initializeAudio?.();
 
         // Map sound names to resource names (must match files in res/raw copied by plugin)
         // verify these match the filenames in plugins/withInCallAudioModule.js
-        InCallAudioModule.loadSound('startTransmitting', 'space_notification1');
-        InCallAudioModule.loadSound('stopTransmitting', 'space_notification2');
-        InCallAudioModule.loadSound('connectedDevice', 'positive_interface_beep');
-        InCallAudioModule.loadSound('connectedToAudioRoom', 'software_interface_start');
-        InCallAudioModule.loadSound('disconnectedFromAudioRoom', 'software_interface_back');
+        await Promise.all([
+          InCallAudioModule.loadSound('startTransmitting', 'space_notification1'),
+          InCallAudioModule.loadSound('stopTransmitting', 'space_notification2'),
+          InCallAudioModule.loadSound('connectedDevice', 'positive_interface_beep'),
+          InCallAudioModule.loadSound('connectedToAudioRoom', 'software_interface_start'),
+          InCallAudioModule.loadSound('disconnectedFromAudioRoom', 'software_interface_back'),
+        ]);
 
         this.isNativeAudioInitialized = true;

Then update the caller at line 54:

       // Initialize native audio module on Android
       if (Platform.OS === 'android') {
-        this.initializeNativeAudio();
+        await this.initializeNativeAudio();
       }
src/utils/InCallAudio.ts (1)

55-57: Avoid as any type assertion.

The SOUNDS object is typed as const, so TypeScript knows the structure. The as any cast is unnecessary and violates the coding guideline to avoid using any. You can type config properly using the typeof SOUNDS[SoundName] type.

♻️ Proposed fix
           const preloadPromises = Object.entries(SOUNDS).map(([name, config]) =>
-            InCallAudioModule.loadSound(name, (config as any).android)
+            InCallAudioModule.loadSound(name, config.android)
           );

The config is already typed as { android: string; ios: number } (where ios is the require result type), so the cast is not needed.

As per coding guidelines: "Avoid using any; use precise types".

src/stores/app/livekit-store.ts (2)

443-446: Consider documenting the magic delay value.

The 800ms delay is a pragmatic workaround for iOS CallKeep timing, but the reasoning isn't immediately clear. Consider extracting this to a named constant with a comment explaining why this specific duration was chosen.

♻️ Proposed improvement
+// Delay to allow disconnect sound to play before CallKeep tears down the audio session on iOS
+const IOS_DISCONNECT_SOUND_DELAY_MS = 800;
+
 // In disconnectFromRoom:
       // Small delay on iOS to allow sound to play before CallKeep potentially kills audio session
       if (Platform.OS === 'ios') {
-        await new Promise((resolve) => setTimeout(resolve, 800));
+        await new Promise((resolve) => setTimeout(resolve, IOS_DISCONNECT_SOUND_DELAY_MS));
       }

239-245: Empty configuration object can be simplified.

The publishDefaults object is empty with a comment placeholder. Either add the intended configuration or remove the unnecessary object literal.

♻️ Proposed fix
-      // Create a new room with default options to ensure no auto-publish if possible
-      const room = new Room({
-        // Prevent auto-publishing if that's a default behavior (though usually it isn't)
-        publishDefaults: {
-          // Additional config can go here
-        },
-      });
+      // Create a new room instance
+      const room = new Room();

Or if specific defaults are intended, add them explicitly.

src/services/callkeep.service.android.ts (1)

301-310: Placeholder methods should be documented or implemented.

removeMuteListener() and restoreMuteListener() are no-op placeholders. If they're part of a shared interface with the iOS implementation, consider adding a brief comment explaining when/why they would be implemented for Android, or mark them explicitly as not applicable.

📝 Proposed improvement
   /**
    * Remove the CallKeep mute listener (No-op on Android or handled differently)
    */
   removeMuteListener(): void {
-    // Android implementation if needed
+    // No-op: Android handles mute events differently via ConnectionService
   }
 
   /**
    * Restore the CallKeep mute listener (No-op on Android or handled differently)
    */
   restoreMuteListener(): void {
-    // Android implementation if needed
+    // No-op: Android handles mute events differently via ConnectionService
   }

Comment on lines +9 to +45
- This is a mobile application, so ensure all components are mobile friendly and responsive and support both iOS and Android platforms and ensure that the app is optimized for both platforms.

Naming Conventions:

- Use camelCase for variable and function names (e.g., `isFetchingData`, `handleUserInput`).
- Use PascalCase for component names (e.g., `UserProfile`, `ChatScreen`).
- Directory and File names should be lowercase and hyphenated (e.g., `user-profile`, `chat-screen`).

TypeScript Usage:

- Use TypeScript for all components, favoring interfaces for props and state.
- Enable strict typing in `tsconfig.json`.
- Avoid using `any`; strive for precise types.
- Utilize `React.FC` for defining functional components with props.

Performance Optimization:

- Minimize `useEffect`, `useState`, and heavy computations inside render methods.
- Use `React.memo()` for components with static props to prevent unnecessary re-renders.
- Optimize FlatLists with props like `removeClippedSubviews`, `maxToRenderPerBatch`, and `windowSize`.
- Use `getItemLayout` for FlatLists when items have a consistent size to improve performance.
- Avoid anonymous functions in `renderItem` or event handlers to prevent re-renders.

UI and Styling:

- Use consistent styling leveraging `gluestack-ui`. If there isn't a Gluestack component in the `components/ui` directory for the component you are trying to use consistently style it either through `StyleSheet.create()` or Styled Components.
- Ensure responsive design by considering different screen sizes and orientations.
- Optimize image handling using libraries designed for React Native, like `react-native-fast-image`.

Best Practices:

- Follow React Native's threading model to ensure smooth UI performance.
- Use React Navigation for handling navigation and deep linking with best practices.
- Create and use Jest to test to validate all generated components
- Generate tests for all components, services and logic generated. Ensure tests run without errors and fix any issues.
- The app is multi-lingual, so ensure all text is wrapped in `t()` from `react-i18next` for translations with the dictonary files stored in `src/translations`.
- Ensure support for dark mode and light mode.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix minor wording/typos in the new rules doc.

These are small but user-facing documentation issues.

✍️ Suggested edits
-- This is a mobile application, so ensure all components are mobile friendly and responsive and support both iOS and Android platforms and ensure that the app is optimized for both platforms.
+- This is a mobile application, so ensure all components are mobile-friendly and responsive, support both iOS and Android platforms, and are optimized for both platforms.

-- Create and use Jest to test to validate all generated components
+- Create and use Jest tests to validate all generated components.

-- The app is multi-lingual, so ensure all text is wrapped in `t()` from `react-i18next` for translations with the dictonary files stored in `src/translations`.
+- The app is multilingual, so ensure all text is wrapped in `t()` from `react-i18next` for translations with the dictionary files stored in `src/translations`.
🧰 Tools
🪛 LanguageTool

[grammar] ~9-~9: Use a hyphen to join words.
Context: ...ion, so ensure all components are mobile friendly and responsive and support both...

(QB_NEW_EN_HYPHEN)


[style] ~43-~43: Consider using a different verb for a more formal wording.
Context: ...ed. Ensure tests run without errors and fix any issues. - The app is multi-lingual,...

(FIX_RESOLVE)


[grammar] ~44-~44: Ensure spelling is correct
Context: ...eact-i18nextfor translations with the dictonary files stored insrc/translations`. - E...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🤖 Prompt for AI Agents
In @.agent/rules/rules.md around lines 9 - 45, The rules document contains
several wording and typo issues—fix wording like "Directory and File names
should be lowercase and hyphenated" to "directory and file names should be
lowercase and hyphenated", correct "dictonary" to "dictionary", change "Create
and use Jest to test to validate" to "Create and run Jest tests to validate",
and rephrase awkward sentences such as "If there isn't a Gluestack component in
the `components/ui` directory for the component you are trying to use
consistently style it either through `StyleSheet.create()` or Styled Components"
to a clearer instruction referencing `gluestack-ui` and `components/ui`; also
ensure consistent capitalization for terms like `TypeScript`, `React.FC`,
`tsconfig.json`, `react-native-fast-image`, `getItemLayout`, and `t()` (from
`react-i18next`) throughout the file. Ensure the list items use consistent
punctuation and grammar and update "iOS and Android platforms and ensure that
the app is optimized" to a smoother phrasing mentioning both platforms once.

"prebuild": "cross-env EXPO_NO_DOTENV=1 yarn expo prebuild",
"android": "cross-env EXPO_NO_DOTENV=1 expo run:android",
"ios": "cross-env EXPO_NO_DOTENV=1 expo run:ios",
"ios": "cross-env EXPO_NO_DOTENV=1 expo run:ios --device",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

Expo run:ios --device flag physical device requirement CLI documentation

💡 Result:

npx expo run:ios --device is intended for running on a connected physical Apple device (iPhone/iPad/Apple TV, depending on scheme). Expo’s CLI docs describe it as “Build directly on connected devices … using the --device flag” and note that to run on your device you “connect it” and run npx expo run:ios --device, then select the connected device. [1]

If you don’t have a physical device connected, use npx expo run:ios (no --device) to build/run on an iOS Simulator instead. [1]

Note: Even when targeting a physical device, Expo CLI may still query Simulator.app for available “devices” and can error if no simulators are installed (for example: No iOS devices available in Simulator.app). [2]

Sources: [1] Expo CLI docs, [2] expo/expo GitHub issue #27316


Avoid forcing physical device for the default iOS script.

The --device flag requires a connected physical device. Without it, expo run:ios runs on the iOS Simulator. This change breaks the simulator workflow for developers and CI environments. Keep the default simulator-friendly and add a separate device script instead.

Suggested script split
-    "ios": "cross-env EXPO_NO_DOTENV=1 expo run:ios --device",
+    "ios": "cross-env EXPO_NO_DOTENV=1 expo run:ios",
+    "ios:device": "cross-env EXPO_NO_DOTENV=1 expo run:ios --device",
📝 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
"ios": "cross-env EXPO_NO_DOTENV=1 expo run:ios --device",
"ios": "cross-env EXPO_NO_DOTENV=1 expo run:ios",
"ios:device": "cross-env EXPO_NO_DOTENV=1 expo run:ios --device",
🤖 Prompt for AI Agents
In `@package.json` at line 12, The current npm script named "ios" forces a
physical device by including "--device", which breaks simulator and CI
workflows; change the "ios" script (the "ios" entry) to run "cross-env
EXPO_NO_DOTENV=1 expo run:ios" without "--device" so it defaults to the
simulator, and add a new script (e.g., "ios:device") that runs "cross-env
EXPO_NO_DOTENV=1 expo run:ios --device" for developers who need a physical
device; update any docs or references that call the old "ios" script if
necessary.

@@ -1,6 +1,7 @@
import { RTCAudioSession } from '@livekit/react-native-webrtc';
import { config } from 'dotenv';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove unused dotenv import.

The config function from dotenv is imported but never used in this file. This creates unnecessary bundle size and potential confusion.

🧹 Suggested fix
 import { RTCAudioSession } from '@livekit/react-native-webrtc';
-import { config } from 'dotenv';
 import { Platform } from 'react-native';
📝 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
import { config } from 'dotenv';
import { RTCAudioSession } from '@livekit/react-native-webrtc';
import { Platform } from 'react-native';
🤖 Prompt for AI Agents
In `@src/services/callkeep.service.ios.ts` at line 2, Remove the unused import of
config from dotenv in this file: delete the line importing "config" (the unused
symbol `config`) from 'dotenv' in callkeep.service.ios.ts so the file no longer
includes an unused dependency; if dotenv is intended to be used globally, ensure
it's initialized in a single central entrypoint instead of here.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/stores/app/livekit-store.ts`:
- Around line 37-94: The iOS branches that call
AudioSession.setAppleAudioConfiguration (the bluetooth, speaker, and
earpiece/default branches) omit allowBluetooth and allowBluetoothA2DP in the
audioCategoryOptions for the speaker and earpiece cases, which strips Bluetooth
capability when the config is replaced; update the audioCategoryOptions arrays
in the non-bluetooth branches (the else-if handling speaker?.type === 'speaker'
and the final else) to include 'allowBluetooth' and 'allowBluetoothA2DP'
alongside their existing options so Bluetooth remains available across output
switches, or alternatively switch to LiveKit's useIOSAudioManagement to handle
these transitions automatically.
🧹 Nitpick comments (6)
src/services/headset-button.service.ts (2)

564-575: Remove dead code or add implementation for platform-specific MusicControl handling.

The Android and iOS branches contain only comments and no executable code. The if (Platform.OS === 'android') block is empty, and the iOS block only logs but doesn't execute any functionality. This creates confusing code that suggests future implementation but currently does nothing.

♻️ Suggested simplification
     try {
       this.isMonitoring = true;
       logger.info({ message: 'Started headset button monitoring for PTT' });

-      // Enable MusicControl session - ONLY for Android now, or if explicitly desired
-      // On iOS, we use CallKeep for lock screen controls. MusicControl conflicts with CallKeep.
-      if (Platform.OS === 'android') {
-        // Wait... Android uses MusicControl?
-        // Android PTT usually uses KeyEvents or dedicated Headset buttons.
-        // But for "Media Button" support on Android, MusicControl is fine.
-        // For iOS, if we use CallKeep, we MUST NOT use MusicControl.
-      } else if (Platform.OS === 'ios') {
-        // DISABLE MusicControl on iOS to prevent conflict with CallKit
-        // This prevents the "ticking" sound caused by rapid system sound feedback
-        // when both CallKit and MusicControl try to handle the same events.
-        // this.enableMusicControl();
-        logger.info({ message: 'MusicControl disabled on iOS (using CallKit native events)' });
-      }
+      // MusicControl is disabled on both platforms:
+      // - iOS: Conflicts with CallKit, causing "ticking" sounds
+      // - Android: Using native KeyEvents/headset button handling instead
+      if (Platform.OS === 'ios') {
+        logger.info({ message: 'MusicControl disabled on iOS (using CallKit native events)' });
+      }

       // Emit event for native modules to start capturing media buttons

666-674: Consider documenting or removing the no-op behavior of setMicrophoneState.

setMicrophoneState is a public method that only calls restoreAudioSession(). The comment says "MusicControl removed" but the method signature suggests it should do something with the enabled parameter. Either document why the parameter is intentionally unused, or consider if this method should be removed/renamed to clarify its purpose.

♻️ Suggested improvement for clarity
   /**
    * Update MusicControl state based on microphone state
-   * `@param` enabled - true if microphone is unmuted (Playing), false if muted (Paused)
+   * `@param` _enabled - Unused after MusicControl removal; kept for API compatibility
+   * `@remarks` MusicControl was removed; this now only restores the iOS audio session.
    */
-  setMicrophoneState(enabled: boolean): void {
-    // No-op: MusicControl removed.
-    // AudioSession restoration is handled by CallKeep/LiveKit interaction.
+  setMicrophoneState(_enabled: boolean): void {
+    // MusicControl removed - parameter kept for API compatibility.
+    // Only restore iOS audio session configuration.
     this.restoreAudioSession();
   }
src/stores/app/livekit-store.ts (1)

18-19: Module-level mutable state may cause testing/isolation issues.

Using a module-level let variable for debounce tracking works but can cause issues in tests where multiple test cases share the same module state. Consider encapsulating this within the store state or using a ref-like pattern.

src/services/bluetooth-audio.service.ts (1)

383-408: Unused variable selectionChanged can be removed.

The selectionChanged variable is set to true when microphone or speaker selections are reset, but it's never read or used afterward. Either remove it or use it for logging/return value.

♻️ Remove unused variable
       // 4. Reset audio selection if this device was selected
       const { selectedAudioDevices } = store;
-      let selectionChanged = false;

       // Check microphone
       if (selectedAudioDevices.microphone?.id === deviceId) {
         logger.info({ message: 'Resetting microphone selection as device is forgotten' });
         store.setSelectedMicrophone({
           id: 'default-mic',
           name: 'Default Microphone',
           type: 'default',
           isAvailable: true,
         });
-        selectionChanged = true;
       }

       // Check speaker
       if (selectedAudioDevices.speaker?.id === deviceId) {
         logger.info({ message: 'Resetting speaker selection as device is forgotten' });
         store.setSelectedSpeaker({
           id: 'default-speaker',
           name: 'Default Speaker',
           type: 'speaker',
           isAvailable: true,
         });
-        selectionChanged = true;
       }
src/services/__tests__/bluetooth-audio-forget.test.ts (2)

49-74: Consider adding singleton reset for test isolation.

BluetoothAudioService.getInstance() returns a singleton, which means state can leak between tests. Consider resetting or recreating the instance in beforeEach to ensure test isolation.

♻️ Add singleton reset for isolation
 describe('BluetoothAudioService - forgetPreferredDevice', () => {
   let service: BluetoothAudioService;
   let mockStore: any;

   beforeEach(() => {
     jest.clearAllMocks();
+    // Reset singleton for test isolation
+    // `@ts-expect-error` - accessing private static for testing
+    BluetoothAudioService.instance = undefined;
     service = BluetoothAudioService.getInstance();

108-116: Consider adding a test for speaker NOT matching the forgotten device.

There's a test for microphone not matching (it('should not reset microphone if it was NOT the forgotten device')), but no equivalent test for speaker. For completeness, consider adding a similar test.

🧪 Additional test case
  it('should not reset speaker if it was NOT the forgotten device', async () => {
    mockStore.selectedAudioDevices.speaker = { id: 'other-speaker', name: 'Other Speaker', type: 'wired' };
    await service.forgetPreferredDevice('test-device-id');
    expect(mockStore.setSelectedSpeaker).not.toHaveBeenCalled();
  });

Comment on lines +37 to +94
if (Platform.OS === 'android') {
let outputType = 'speaker'; // default

if (speaker?.type === 'bluetooth') {
outputType = 'bluetooth';
} else if (speaker?.type === 'wired') {
outputType = 'headset';
} else if (speaker?.type === 'speaker') {
outputType = 'speaker';
} else {
logger.debug({ message: 'Routing audio to Earpiece/Headset' });
audioModeConfig.playThroughEarpieceAndroid = true;
outputType = 'earpiece';
}

await Audio.setAudioModeAsync(audioModeConfig);
}
logger.debug({ message: `Routing audio to ${outputType} on Android` });

try {
// Ensure we are in a call-compatible mode
await Audio.setAudioModeAsync({
allowsRecordingIOS: true,
staysActiveInBackground: true,
playsInSilentModeIOS: true,
shouldDuckAndroid: true,
playThroughEarpieceAndroid: outputType !== 'speaker',
});

if (outputType === 'bluetooth') {
await AudioSession.startAudioSession();
}

// Handle LiveKit specific device switching if needed (mostly for web/desktop, but good to have)
if (speaker?.id && speaker.id !== 'default-speaker' && speaker.type === 'bluetooth') {
// logic for specific bluetooth device selection if feasible
await AudioSession.selectAudioOutput(outputType);
} catch (e) {
logger.warn({ message: 'Failed to select audio output via AudioSession', context: { error: e } });
}
} else if (Platform.OS === 'ios') {
// AudioSession.startAudioSession(); // Handled by CallKeep - DO NOT call this here for iOS

if (speaker?.type === 'bluetooth') {
// Bluetooth preferred
await AudioSession.setAppleAudioConfiguration({
audioCategory: 'playAndRecord',
audioCategoryOptions: ['allowBluetooth', 'allowBluetoothA2DP', 'mixWithOthers'],
audioMode: 'voiceChat',
});
} else if (speaker?.type === 'speaker') {
// Force speaker
await AudioSession.setAppleAudioConfiguration({
audioCategory: 'playAndRecord',
audioCategoryOptions: ['defaultToSpeaker', 'mixWithOthers'],
audioMode: 'videoChat',
});
} else {
// Earpiece / Default
await AudioSession.setAppleAudioConfiguration({
audioCategory: 'playAndRecord',
audioCategoryOptions: ['mixWithOthers'],
audioMode: 'voiceChat',
});
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

@livekit/react-native AudioSession setAppleAudioConfiguration audioCategoryOptions

💡 Result:

In @livekit/react-native, iOS audioCategoryOptions is an array of AVAudioSession “category options” you pass into AudioSession.setAppleAudioConfiguration(...) (or returned from useIOSAudioManagement). Examples seen in LiveKit RN usage include: mixWithOthers, allowBluetooth, allowBluetoothA2DP, allowAirPlay, defaultToSpeaker, interruptSpokenAudioAndMixWithOthers. [1][2][3]

import {
  AudioSession,
  AppleAudioCategory,
  AppleAudioMode,
  AppleAudioCategoryOption,
} from '@livekit/react-native';

await AudioSession.setAppleAudioConfiguration({
  audioCategory: AppleAudioCategory.playAndRecord,
  audioMode: AppleAudioMode.voiceChat,
  audioCategoryOptions: [
    AppleAudioCategoryOption.mixWithOthers,
    AppleAudioCategoryOption.allowBluetooth,
    AppleAudioCategoryOption.allowBluetoothA2DP,
    AppleAudioCategoryOption.defaultToSpeaker,
    // AppleAudioCategoryOption.allowAirPlay,
    // AppleAudioCategoryOption.interruptSpokenAudioAndMixWithOthers,
  ],
});

If your app’s “best” iOS config changes as tracks publish/unpublish, LiveKit recommends using useIOSAudioManagement(...) rather than setting a single static config once. [4]

Sources: [1] [2] [3] [4]


🌐 Web query:

LiveKit React Native iOS audio routing AudioSession.setAppleAudioConfiguration persistence

💡 Result:

AudioSession.setAppleAudioConfiguration(...) in @livekit/react-native should be treated as setting the SDK’s “desired/default” AVAudioSession config, not as a one-time, permanent change to iOS routing.

On iOS, LiveKit may reconfigure the underlying AVAudioSession over time (notably when local/remote audio tracks publish/unpublish, and when the session is restarted after interruptions / competing audio frameworks), which can make your earlier configuration appear to “not persist”. LiveKit explicitly recommends useIOSAudioManagement for iOS because “the most appropriate audio configuration may change over time” during a room session. [1] DeepWiki also describes useIOSAudioManagement as the hook that automatically configures and adjusts iOS audio session behavior. [2]

Practical implications / what to do:

  • If you want LiveKit to manage iOS audio, use useIOSAudioManagement and expect the SDK to adjust the session; if you need specific category options (e.g. mixWithOthers, Bluetooth options), be prepared to re-apply your setAppleAudioConfiguration(...) when you (re)start the audio session / reconnect, and after any competing library (like Expo audio/video) touches AVAudioSession. (This pattern is commonly suggested in community support threads.) [3]
  • If you need full control and true persistence, you generally must avoid competing audio-session managers (or disable/avoid LiveKit’s automatic iOS audio management layer) and own the AVAudioSession lifecycle yourself (conceptually similar to how LiveKit’s Swift SDK allows disabling automatic audio session configuration). [4]

Sources: LiveKit React Native README (iOS audio management guidance) [1], DeepWiki (hook behavior) [2], LiveKit community thread referencing setAppleAudioConfiguration usage and mixing options [3], LiveKit Swift SDK README (automatic AVAudioSession management + option to disable) [4].


🌐 Web query:

iOS AVAudioSession audioCategoryOptions allowBluetooth persistence between calls

💡 Result:

allowBluetooth is not something iOS “remembers between calls” by itself—it’s just an AVAudioSession category option that applies to the current audio-session configuration, and that configuration can be changed or replaced when a call ends/starts (especially with CallKit or other SDKs that manage the session). Apple explicitly models this as session configuration state (category, mode, categoryOptions) rather than a persistent setting. [1]

Key implications:

  • allowBluetooth is only valid with input-capable categories (.playAndRecord or .record). If your session ever gets set to something else (for example .playback), the option won’t apply. [2]
  • Many VoIP/call stacks (and CallKit flows) will deactivate and/or reconfigure the shared AVAudioSession at call boundaries; some SDKs even restore “original” category/options when the call ends, which can remove allowBluetooth for the next call unless you set it again. [3]
  • Going inactive has side effects and you generally should assume you may need to re-assert your preferred configuration when reactivating for a new call. [4]

Practical pattern (what typically fixes “works on first call, not on second”):

  1. On each call start / each CallKit activation, set the category with options you need, then activate:
    let s = AVAudioSession.sharedInstance()
    try s.setCategory(.playAndRecord, mode: .voiceChat, options: [.allowBluetooth]) // add others as needed
    try s.setActive(true)
  2. If you use CallKit, do this in/around provider(_:didActivate:) (or immediately before starting audio), because that’s when the system hands you an activated session to configure.
  3. Avoid later calling setCategory(...) again without options during the call lifecycle (that’s a common way options “disappear” between call phases, depending on your code/SDK).

If you share how your call lifecycle is wired (plain AVAudioSession vs CallKit, plus where you call setCategory / setActive), I can point to the exact spot where the option is getting dropped and suggest a minimal fix.

Sources: Apple AVAudioSession.CategoryOptions / categoryOptions docs [1]; Apple docs note .allowBluetooth category constraints [2]; example of call SDKs changing/restoring category/options across calls [3]; Apple QA on side effects of deactivation/inactivation [4].

[1] https://developer.apple.com/documentation/avfaudio/avaudiosession/categoryoptions-swift.struct
[2] https://stackoverflow.com/questions/52566011/avaudiosession-setcategory-return-alway-error
[3] https://developers.sinch.com/docs/in-app-calling/ios/miscellaneous
[4] https://developer.apple.com/library/archive/qa/qa1631/_index.html


Include allowBluetooth and allowBluetoothA2DP in all iOS audio configurations to enable seamless switching between Bluetooth and other output types.

The speaker and earpiece configurations (lines 82–93) don't include allowBluetooth and allowBluetoothA2DP options, whereas the Bluetooth configuration does. Since each call to setAppleAudioConfiguration replaces the prior configuration state on iOS, omitting these options in the speaker and earpiece branches removes Bluetooth capability from the session. Switching back to Bluetooth later would fail unless the configuration is re-applied with these options included. Add allowBluetooth and allowBluetoothA2DP to the audioCategoryOptions array in all three branches (lines 66–93) to maintain Bluetooth availability across output transitions. Alternatively, consider using LiveKit's useIOSAudioManagement hook, which handles these adjustments automatically.

🤖 Prompt for AI Agents
In `@src/stores/app/livekit-store.ts` around lines 37 - 94, The iOS branches that
call AudioSession.setAppleAudioConfiguration (the bluetooth, speaker, and
earpiece/default branches) omit allowBluetooth and allowBluetoothA2DP in the
audioCategoryOptions for the speaker and earpiece cases, which strips Bluetooth
capability when the config is replaced; update the audioCategoryOptions arrays
in the non-bluetooth branches (the else-if handling speaker?.type === 'speaker'
and the final else) to include 'allowBluetooth' and 'allowBluetoothA2DP'
alongside their existing options so Bluetooth remains available across output
switches, or alternatively switch to LiveKit's useIOSAudioManagement to handle
these transitions automatically.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/services/callkeep.service.android.ts`:
- Around line 113-116: The Android startCall implementation should fail-fast
like iOS: instead of just logging a warning when this.isSetup is false (and
leaving the misleading comment "attempting setup"), throw an Error immediately
to stop execution. Update the startCall method to check this.isSetup and throw a
descriptive error (referencing this.isSetup and logger.warn) when not
initialized, and remove the comment that suggests setup is attempted.
- Around line 295-304: In generateUUID (in
src/services/callkeep.service.android.ts and the identical function in
src/services/callkeep.service.ios.ts) remove the (global as any) cast and the
Math.random fallback; instead import/require the react-native-get-random-values
polyfill like src/lib/storage/secure-storage.ts does, assert that
global.crypto?.getRandomValues exists (throw an error if not), then use
global.crypto.randomUUID() when available or generate an RFC4122 v4 UUID using
crypto.getRandomValues to fill random bytes; ensure no any casts remain and that
failures throw an explicit error rather than silently falling back to
Math.random.
🧹 Nitpick comments (1)
src/services/callkeep.service.android.ts (1)

18-24: Harden CallKeep listener management (store handlers + gate by active call).

Listeners are added anonymously but removed by event name only; if RNCallKeep requires a handler reference, cleanup won’t detach them. Also, mute events should ignore calls that don’t match the active UUID to avoid stale state updates.

🧹 Suggested listener wiring
 export class CallKeepService {
   private static instance: CallKeepService | null = null;
   private isSetup = false;
   private isCallActive = false;
   private muteStateCallback: ((muted: boolean) => void) | null = null;
   private endCallCallback: (() => void) | null = null;
+  private endCallListener?: ({ callUUID }: { callUUID: string }) => void;
+  private muteListener?: ({ muted, callUUID }: { muted: boolean; callUUID: string }) => void;

   private setupEventListeners(): void {
     // Android specific events if any

     // Call ended from System UI
-    RNCallKeep.addEventListener('endCall', ({ callUUID }) => {
+    this.endCallListener = ({ callUUID }) => {
       logger.info({
         message: 'CallKeep call ended from system UI',
         context: { callUUID },
       });
@@
-    });
+    };
+    RNCallKeep.addEventListener('endCall', this.endCallListener);

     // Mute/unmute events
-    RNCallKeep.addEventListener('didPerformSetMutedCallAction', ({ muted, callUUID }) => {
+    this.muteListener = ({ muted, callUUID }) => {
       logger.debug({
         message: 'CallKeep mute state changed',
         context: { muted, callUUID },
       });
+      if (callUUID !== currentCallUUID) {
+        return;
+      }

       // Call the registered callback if available
       if (this.muteStateCallback) {
         try {
           this.muteStateCallback(muted);
@@
-    });
+    };
+    RNCallKeep.addEventListener('didPerformSetMutedCallAction', this.muteListener);
   }

   async cleanup(): Promise<void> {
@@
-      RNCallKeep.removeEventListener('endCall');
-      RNCallKeep.removeEventListener('didPerformSetMutedCallAction');
+      if (this.endCallListener) {
+        RNCallKeep.removeEventListener('endCall', this.endCallListener);
+      }
+      if (this.muteListener) {
+        RNCallKeep.removeEventListener('didPerformSetMutedCallAction', this.muteListener);
+      }

Also applies to: 243-289, 330-333

Comment on lines +113 to +116
if (!this.isSetup) {
// Auto-setup if not ready (fallback)
logger.warn({ message: 'CallKeep not setup before startCall, attempting setup' });
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cat -n src/services/callkeep.service.android.ts

Repository: Resgrid/Responder

Length of output: 12125


🏁 Script executed:

rg -r '$0' 'startCall|callKeepService\.setup' --type ts --type tsx -A 2 -B 2

Repository: Resgrid/Responder

Length of output: 88


🏁 Script executed:

rg 'startCall|callKeepService\.setup' --type ts -A 2 -B 2

Repository: Resgrid/Responder

Length of output: 6589


🏁 Script executed:

cat -n src/services/app-initialization.service.ts | grep -A 20 '_initializeCallKeep'

Repository: Resgrid/Responder

Length of output: 1639


🏁 Script executed:

cat -n src/services/callkeep.service.ios.ts | grep -A 20 'async startCall'

Repository: Resgrid/Responder

Length of output: 785


Make startCall fail-fast when setup has not been called, matching the iOS implementation.

The Android version currently logs a warning and continues, whereas the iOS version throws an error. This inconsistency creates unpredictable behavior. Additionally, the comment claims "attempting setup" but the code only logs—no actual setup is performed. Since app initialization can fail silently (errors are caught and suppressed), startCall may execute with an uninitialized service.

Suggested fix
-    if (!this.isSetup) {
-      // Auto-setup if not ready (fallback)
-      logger.warn({ message: 'CallKeep not setup before startCall, attempting setup' });
-    }
+    if (!this.isSetup) {
+      throw new Error('CallKeep not setup. Call setup() first.');
+    }
🤖 Prompt for AI Agents
In `@src/services/callkeep.service.android.ts` around lines 113 - 116, The Android
startCall implementation should fail-fast like iOS: instead of just logging a
warning when this.isSetup is false (and leaving the misleading comment
"attempting setup"), throw an Error immediately to stop execution. Update the
startCall method to check this.isSetup and throw a descriptive error
(referencing this.isSetup and logger.warn) when not initialized, and remove the
comment that suggests setup is attempted.

Comment on lines +295 to 304
private generateUUID(): string {
// RN 0.76 typically provides global crypto.randomUUID via Hermes/JSI
const rndUUID = (global as any)?.crypto?.randomUUID?.();
if (typeof rndUUID === 'string') return rndUUID;
// Fallback
return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, (c) => {
const r = (Math.random() * 16) | 0;
const v = c === 'x' ? r : (r & 0x3) | 0x8;
return v.toString(16);
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n src/services/callkeep.service.android.ts | sed -n '285,315p'

Repository: Resgrid/Responder

Length of output: 1128


🏁 Script executed:

fd package.json | head -5 | xargs cat

Repository: Resgrid/Responder

Length of output: 9888


🏁 Script executed:

rg "randomUUID|getRandomValues|crypto" --type ts --type tsx -A 3 -B 1

Repository: Resgrid/Responder

Length of output: 88


🏁 Script executed:

rg "crypto|getRandomValues|randomUUID" -A 3 -B 1 2>/dev/null | head -100

Repository: Resgrid/Responder

Length of output: 5985


🏁 Script executed:

rg "react-native-get-random-values" -A 5 -B 1

Repository: Resgrid/Responder

Length of output: 1842


🏁 Script executed:

grep -r "crypto" src/ --include="*.ts" --include="*.tsx" 2>/dev/null | head -20

Repository: Resgrid/Responder

Length of output: 1377


Remove any type and replace Math.random UUID fallback with crypto.getRandomValues on both Android and iOS services.

The (global as any) cast violates the coding guideline to avoid any types. Additionally, using Math.random for UUID generation is a security concern. Both src/services/callkeep.service.android.ts and src/services/callkeep.service.ios.ts have this issue with identical implementations.

The project already includes react-native-get-random-values as a dependency and has a working pattern in src/lib/storage/secure-storage.ts that demonstrates proper crypto initialization. Use that approach: require the polyfill, check for crypto.getRandomValues, and throw an error if unavailable rather than falling back to Math.random.

Proposed fix
   private generateUUID(): string {
-    // RN 0.76 typically provides global crypto.randomUUID via Hermes/JSI
-    const rndUUID = (global as any)?.crypto?.randomUUID?.();
-    if (typeof rndUUID === 'string') return rndUUID;
-    // Fallback
-    return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, (c) => {
-      const r = (Math.random() * 16) | 0;
-      const v = c === 'x' ? r : (r & 0x3) | 0x8;
-      return v.toString(16);
-    });
+    type CryptoLike = {
+      randomUUID?: () => string;
+      getRandomValues?: (array: Uint8Array) => Uint8Array;
+    };
+    require('react-native-get-random-values');
+    const cryptoObj = (globalThis as { crypto?: CryptoLike }).crypto;
+    if (cryptoObj?.randomUUID) return cryptoObj.randomUUID();
+    if (!cryptoObj?.getRandomValues) {
+      throw new Error('crypto.getRandomValues is unavailable');
+    }
+    const bytes = new Uint8Array(16);
+    cryptoObj.getRandomValues(bytes);
+    bytes[6] = (bytes[6] & 0x0f) | 0x40;
+    bytes[8] = (bytes[8] & 0x3f) | 0x80;
+    const hex = Array.from(bytes, (b) => b.toString(16).padStart(2, '0')).join('');
+    return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}`;
   }
📝 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
private generateUUID(): string {
// RN 0.76 typically provides global crypto.randomUUID via Hermes/JSI
const rndUUID = (global as any)?.crypto?.randomUUID?.();
if (typeof rndUUID === 'string') return rndUUID;
// Fallback
return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, (c) => {
const r = (Math.random() * 16) | 0;
const v = c === 'x' ? r : (r & 0x3) | 0x8;
return v.toString(16);
});
private generateUUID(): string {
type CryptoLike = {
randomUUID?: () => string;
getRandomValues?: (array: Uint8Array) => Uint8Array;
};
require('react-native-get-random-values');
const cryptoObj = (globalThis as { crypto?: CryptoLike }).crypto;
if (cryptoObj?.randomUUID) return cryptoObj.randomUUID();
if (!cryptoObj?.getRandomValues) {
throw new Error('crypto.getRandomValues is unavailable');
}
const bytes = new Uint8Array(16);
cryptoObj.getRandomValues(bytes);
bytes[6] = (bytes[6] & 0x0f) | 0x40;
bytes[8] = (bytes[8] & 0x3f) | 0x80;
const hex = Array.from(bytes, (b) => b.toString(16).padStart(2, '0')).join('');
return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}`;
}
🤖 Prompt for AI Agents
In `@src/services/callkeep.service.android.ts` around lines 295 - 304, In
generateUUID (in src/services/callkeep.service.android.ts and the identical
function in src/services/callkeep.service.ios.ts) remove the (global as any)
cast and the Math.random fallback; instead import/require the
react-native-get-random-values polyfill like src/lib/storage/secure-storage.ts
does, assert that global.crypto?.getRandomValues exists (throw an error if not),
then use global.crypto.randomUUID() when available or generate an RFC4122 v4
UUID using crypto.getRandomValues to fill random bytes; ensure no any casts
remain and that failures throw an explicit error rather than silently falling
back to Math.random.

@ucswift
Copy link
Member Author

ucswift commented Feb 1, 2026

Approve

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is approved.

@ucswift ucswift merged commit 7b4061f into master Feb 1, 2026
12 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant