Skip to content

RU-T50 Voice fix for ios #258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ucswift merged 2 commits into from
Jul 28, 2026
Merged

RU-T50 Voice fix for ios #258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
11d3f72
RU-T50 Voice fix for ios
ucswift Jul 28, 2026
07903e5
RU-T50 PR#258 fixes
ucswift Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions scripts/extract-release-notes.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ extract_release_notes() {
notes="$cleaned_body"
fi

# Replace standalone "PR" with "Release" for release notes text
notes="$(printf '%s\n' "$notes" | sed -E 's/\bPR\b/Release/g')"
Comment on lines +49 to +50

@coderabbitai coderabbitai Bot Jul 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/sh
set -eu

input='PR PRs XPR PR.'
output="$(printf '%s\n' "$input" | sed -E 's/\bPR\b/Release/g')"
printf 'sed output: %s\n' "$output"

Repository: Resgrid/Unit

Length of output: 187

🏁 Script executed:

#!/bin/sh
set -eu

printf 'sed implementation/version:\n'
sed --version 2>&1 || true
sed -E 'x' /dev/null 2>&1 | sed -n '1p' || true

printf '\nCurrent sed behavior for word boundaries:\n'
input='PR PRs XPR PR'
output="$(printf '%s\n' "$input" | sed -E 's/\bPR\b/Release/g')"
printf '%s\n' "$output"

printf '\nGNU sed version string:\n'
sed --version 2>&1 | sed -n '1p'

Repository: Resgrid/Unit

Length of output: 903

🏁 Script executed:

#!/bin/sh
set -eu

echo "tracked workflow/tooling files:"
git ls-files | rg '(^\.github/workflows/|scripts/|README\.md|package\.json|justfile|\.circleci|\.github/)' | head -200

echo
echo "search for extract-release-notes usage:"
rg -n "extract-release-notes|extract-release-notes\.sh|RELEASE.*NOTES|release.*notes" .github scripts README.md package.json 2>/dev/null | head -200

echo
echo "script excerpt:"
cat -n scripts/extract-release-notes.sh | sed -n '1,120p'

Repository: Resgrid/Unit

Length of output: 8225

🏁 Script executed:

#!/bin/sh
set -eu

echo "Workflow file size:"
wc -l .github/workflows/react-native-cicd.yml

echo
echo "Relevant workflow slices around script invocations:"
sed -n '300,385p' .github/workflows/react-native-cicd.yml
echo "---"
sed -n '625,700p' .github/workflows/react-native-cicd.yml

echo
echo "Runner labels declared in repository:"
rg -n "runs-on:\s*\\$\\{\\{.*runner|runs-on:|ubuntu|macos|windows|self-hosted" .github/workflows/react-native-cicd.yml package.json README.md

Repository: Resgrid/Unit

Length of output: 7251

Use a portable word-boundary replacement.

This script runs on both Ubuntu and macOS runners, and the macOS runner uses BSD sed, where \b is not guaranteed to match “PR” at string boundaries. Replace the GNU-specific sed -E 's/\bPR\b/Release/g' with a portable boundary expression before publishing release notes.

Proposed portable replacement 
-  notes="$(printf '%s\n' "$notes" | sed -E 's/\bPR\b/Release/g')"
+  notes="$(printf '%s\n' "$notes" | sed -E 's/(^|[^[:alnum:]_])PR([^[:alnum:]_]|$)/\1Release\2/g')"
📝 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
# Replace standalone "PR" with "Release" for release notes text
notes="$(printf '%s\n' "$notes" | sed -E 's/\bPR\b/Release/g')"
# Replace standalone "PR" with "Release" for release notes text
notes="$(printf '%s\n' "$notes" | sed -E 's/(^|[^[:alnum:]_])PR([^[:alnum:]_]|$)/\1Release\2/g')"
🤖 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/extract-release-notes.sh` around lines 49 - 50, Update the notes
substitution in the release-notes script to replace standalone “PR” using a
BSD/GNU-compatible boundary expression rather than sed’s \b extension. Preserve
matches at the beginning and end of lines and avoid replacing “PR” within larger
words.


# Final trim
notes="$(printf '%s\n' "$notes" | sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//')"

Expand Down
44 changes: 14 additions & 30 deletions src/stores/app/__tests__/livekit-store.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ jest.mock('expo-audio', () => ({
jest.mock('../../../lib/logging', () => ({
logger: {
info: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
debug: jest.fn(),
},
Expand Down Expand Up @@ -259,58 +260,41 @@ describe('LiveKit Store - Permission Management', () => {
(Platform as any).OS = 'ios';
});

it('should successfully request permissions on iOS', async () => {
// Mock initial permission check - not granted
it('should return true on iOS without requesting when permission already granted', async () => {
mockGetRecordingPermissionsAsync.mockResolvedValueOnce({
granted: false,
canAskAgain: true,
expires: 'never',
status: 'undetermined',
} as any);

// Mock permission request - granted
mockRequestRecordingPermissionsAsync.mockResolvedValueOnce({
granted: true,
canAskAgain: true,
expires: 'never',
status: 'granted',
} as any);
Comment on lines 264 to 269

@coderabbitai coderabbitai Bot Jul 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -g '!node_modules/**' '"typescript"|typescript' package.json tsconfig*.json 2>/dev/null || true

Repository: Resgrid/Unit

Length of output: 388

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate file =="
fd -a 'livekit-store.test.ts' . || true

echo "== relevant lines =="
if [ -f src/stores/app/__tests__/livekit-store.test.ts ]; then
  nl -ba src/stores/app/__tests__/livekit-store.test.ts | sed -n '240,300p'
fi

echo "== imports around file =="
if [ -f src/stores/app/__tests__/livekit-store.test.ts ]; then
  nl -ba src/stores/app/__tests__/livekit-store.test.ts | sed -n '1,80p'
fi

echo "== usages of getRecordingPermissionsAsync in tests/source =="
rg -n "getRecordingPermissionsAsync|mockGetRecordingPermissionsAsync|MockedFn|As any|as any" src -g '!node_modules/**' || true

Repository: Resgrid/Unit

Length of output: 295

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="src/stores/app/__tests__/livekit-store.test.ts"

echo "== imports header =="
awk 'NR>=1 && NR<=90 {printf "%6d  %s\n", NR, $0}' "$file"

echo "== relevant lines =="
awk 'NR>=240 && NR<=300 {printf "%6d  %s\n", NR, $0}' "$file"

echo "== imports/permissions usages =="
rg -n "getRecordingPermissionsAsync|mockGetRecordingPermissionsAsync|mockGetCameraPermissionsAsync|as any" "$file" || true

Repository: Resgrid/Unit

Length of output: 7024

Replace the unsafe permission-response casts.

as any bypasses the mocked API contract in both permission tests. Use one typed response object, e.g. an isGranted helper, or infer the response shape without casting to any.

🤖 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/stores/app/__tests__/livekit-store.test.ts` around lines 263 - 268,
Replace the unsafe as any cast in the permission-response mocks used by the
tests around mockGetRecordingPermissionsAsync with a correctly typed shared
response object or typed helper such as isGranted. Reuse that typed shape in
both permission tests while preserving their existing granted-permission
behavior.

Source: Coding guidelines


const { requestPermissions } = useLiveKitStore.getState();
await requestPermissions();
const result = await requestPermissions();

expect(result).toBe(true);
expect(mockGetRecordingPermissionsAsync).toHaveBeenCalledTimes(1);
expect(mockRequestRecordingPermissionsAsync).toHaveBeenCalledTimes(1);
expect(mockLogger.info).toHaveBeenCalledWith({
message: 'Microphone permission granted successfully',
context: { platform: 'ios' },
});
expect(mockRequestRecordingPermissionsAsync).not.toHaveBeenCalled();
});

it('should handle iOS permission denial', async () => {
// Mock initial permission check - not granted
it('should never call session-activating request on iOS, even when not granted', async () => {
// expo-audio's requestRecordingPermissionsAsync activates AVAudioSession
// and deadlocks against expo-av — the store must only perform the
// non-activating check and let WebRTC prompt natively on publish.
mockGetRecordingPermissionsAsync.mockResolvedValueOnce({
granted: false,
canAskAgain: false,
expires: 'never',
status: 'denied',
} as any);

// Mock permission request - still denied
mockRequestRecordingPermissionsAsync.mockResolvedValueOnce({
granted: false,
canAskAgain: false,
expires: 'never',
status: 'denied',
} as any);

const { requestPermissions } = useLiveKitStore.getState();
await requestPermissions();
const result = await requestPermissions();

expect(result).toBe(false);
expect(mockGetRecordingPermissionsAsync).toHaveBeenCalledTimes(1);
expect(mockRequestRecordingPermissionsAsync).toHaveBeenCalledTimes(1);
expect(mockLogger.error).toHaveBeenCalledWith({
message: 'Microphone permission not granted',
expect(mockRequestRecordingPermissionsAsync).not.toHaveBeenCalled();
expect(mockLogger.warn).toHaveBeenCalledWith({
message: 'Microphone permission permanently denied',
context: { platform: 'ios' },
});
});
Expand Down
45 changes: 25 additions & 20 deletions src/stores/app/livekit-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { AudioSession } from '@livekit/react-native';
import { RTCAudioSession } from '@livekit/react-native-webrtc';
import notifee, { AndroidForegroundServiceType, AndroidImportance } from '@notifee/react-native';
import { getRecordingPermissionsAsync, requestRecordingPermissionsAsync } from 'expo-audio';
import { getRecordingPermissionsAsync } from 'expo-audio';
import { Audio, InterruptionModeIOS } from 'expo-av';
import * as Device from 'expo-device';
import { Room, RoomEvent } from 'livekit-client';
Expand Down Expand Up @@ -397,23 +397,27 @@ export const useLiveKitStore = create<LiveKitState>((set, get) => ({
});
return true;
} else if (Platform.OS === 'ios') {
// NOTE: expo-audio's requestRecordingPermissionsAsync activates
// AVAudioSession in record mode, which deadlocks against expo-av's
// active session and freezes the app. Only perform the
// non-activating status check here; WebRTC triggers the native mic
// prompt itself when a track is published.
const micPermission = await getRecordingPermissionsAsync();

if (!micPermission.granted) {
const result = await requestRecordingPermissionsAsync();
if (!result.granted) {
logger.error({
message: 'Microphone permission not granted',
if (!micPermission.canAskAgain) {
logger.warn({
message: 'Microphone permission permanently denied',
context: { platform: Platform.OS },
});
return false;
}
Comment on lines +408 to 414

@Resgrid-Bot Resgrid-Bot Jul 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Kody Rules low

Duplicated permission-permanently-denied check-and-log blocks at lines 408–414 and 974–980 in src/stores/app/livekit-store.ts force error-prone, lock-step maintenance for any changes to the message, log level, or return value. Extract a helper such as isMicPermissionPermanentlyDenied(permission) or handlePermanentlyDenied(platform) to encapsulate the canAskAgain check, warning log, and boolean return.

Kody rule violation: Extract duplicated logic into functions

Prompt for LLM 

File src/stores/app/livekit-store.ts:

Line 408 to 414:

Duplicated permission-permanently-denied check-and-log blocks at lines 408–414 and 974–980 in `src/stores/app/livekit-store.ts` force error-prone, lock-step maintenance for any changes to the message, log level, or return value. Extract a helper such as `isMicPermissionPermanentlyDenied(permission)` or `handlePermanentlyDenied(platform)` to encapsulate the `canAskAgain` check, warning log, and boolean return.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

​

​

logger.info({
message: 'Microphone permission not yet granted - WebRTC will prompt on publish',
context: { platform: Platform.OS },
});
}

logger.info({
message: 'Microphone permission granted successfully',
context: { platform: Platform.OS },
});
return true;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines 407 to 421

@Resgrid-Bot Resgrid-Bot Jul 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Bug high

Silent WebRTC publish failure occurs because requestPermissions returns true unconditionally on iOS (line 414), causing connectToRoom (line 489) to bypass the 'Open Settings' alert and abort logic (lines 495-506) when microphone permission is permanently denied (canAskAgain: false, status: 'denied'). Return false for permanently denied permissions to restore denied-permission guidance while preserving the deadlock fix that avoids requestRecordingPermissionsAsync.

const micPermission = await getRecordingPermissionsAsync();

if (!micPermission.granted) {
  if (micPermission.canAskAgain) {
    // Undetermined — WebRTC will trigger the native prompt on publish
    logger.info({
      message: 'Microphone permission not yet granted - WebRTC will prompt on publish',
      context: { platform: Platform.OS },
    });
    return true;
  }
  // Permanently denied — iOS will not re-prompt; direct user to Settings
  logger.error({
    message: 'Microphone permission denied',
    context: { platform: Platform.OS },
  });
  return false;
}

return true;
Prompt for LLM 

File src/stores/app/livekit-store.ts:

Line 407 to 414:

Silent WebRTC publish failure occurs because `requestPermissions` returns `true` unconditionally on iOS (line 414), causing `connectToRoom` (line 489) to bypass the 'Open Settings' alert and abort logic (lines 495-506) when microphone permission is permanently denied (`canAskAgain: false`, `status: 'denied'`). Return `false` for permanently denied permissions to restore denied-permission guidance while preserving the deadlock fix that avoids `requestRecordingPermissionsAsync`.

Suggested Code:

        const micPermission = await getRecordingPermissionsAsync();

        if (!micPermission.granted) {
          if (micPermission.canAskAgain) {
            // Undetermined — WebRTC will trigger the native prompt on publish
            logger.info({
              message: 'Microphone permission not yet granted - WebRTC will prompt on publish',
              context: { platform: Platform.OS },
            });
            return true;
          }
          // Permanently denied — iOS will not re-prompt; direct user to Settings
          logger.error({
            message: 'Microphone permission denied',
            context: { platform: Platform.OS },
          });
          return false;
        }

        return true;

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

​

​

}
return true; // Web/other platforms don't need runtime permissions
Expand Down Expand Up @@ -963,20 +967,21 @@ export const useLiveKitStore = create<LiveKitState>((set, get) => ({
}
return true;
} else if (Platform.OS === 'ios') {
// Non-activating check only — see requestPermissions for why
// requestRecordingPermissionsAsync must not be called on iOS.
const mic = await getRecordingPermissionsAsync();
if (mic.granted) return true;

logger.info({
message: 'Requesting microphone permission before opening voice UI',
context: { platform: Platform.OS },
});
const result = await requestRecordingPermissionsAsync();
if (!result.granted) {
logger.warn({
message: 'Microphone permission denied - voice UI will still open but joining will fail',
if (!mic.granted) {
if (!mic.canAskAgain) {
logger.warn({
message: 'Microphone permission permanently denied',
context: { platform: Platform.OS },
});
return false;
}
logger.info({
message: 'Microphone permission not yet granted - WebRTC will prompt on publish',
context: { platform: Platform.OS },
});
return false;
}
return true;
}
Expand Down
Loading