APK build fix #80
APK build fix #80
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe workflow stops running EAS init. Android config adds new permissions and an Expo plugin for a foreground service. A new Notifee-based plugin modifies the Android manifest. The audio streamer hook gains foreground service management, heartbeat, reconnection, and notification-based UX. Dependencies updated to include @notifee/react-native. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Hook as useAudioStreamer
participant Notifee
participant Net as NetInfo
participant WS as WebSocket Server
Note over Hook,Notifee: Start streaming
User->>Hook: startStreaming()
Hook->>Notifee: ensure permission + register channel
Hook->>Notifee: start foreground notification
Hook->>WS: connect()
WS-->>Hook: open
Hook->>Hook: reset backoff, set isStreaming=true, isConnecting=false
Hook->>WS: send audio-start
Hook->>Hook: start heartbeat (interval)
Note over Net,Hook: Connectivity events
Net-->>Hook: becomes online
alt WS not OPEN and not manually stopped
Hook->>WS: reconnect (exponential backoff)
end
Note over Hook,WS: Heartbeat
loop every N seconds
Hook->>WS: ping/heartbeat
WS-->>Hook: pong/ack (implicit)
end
Note over Hook,Notifee: Stop streaming
User->>Hook: stopStreaming()
Hook->>WS: send audio-stop
Hook->>WS: close(code=1000,"manual-stop")
Hook->>Hook: clear heartbeat/retry timers
Hook->>Notifee: stop foreground notification
Hook->>Hook: set isStreaming=false
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/app.json (1)
18-21: Fix iOS build: declare encryption usagePipeline failure indicates missing
ITSAppUsesNonExemptEncryption. Add it toios.infoPlist."ios": { "supportsTablet": true, - "bundleIdentifier": "com.cupbearer5517.friendlite" + "bundleIdentifier": "com.cupbearer5517.friendlite", + "infoPlist": { + "ITSAppUsesNonExemptEncryption": false + } },
🧹 Nitpick comments (4)
.github/workflows/android-apk-build.yml (1)
65-93: Optional: switch to maintained release action
actions/create-release@v1andactions/upload-release-asset@v1are archived. Considersoftprops/action-gh-releasefor fewer quirks and better maintenance.app/plugins/with-ws-fgs.js (1)
47-51: Service types OK; ensure option wiring matches app.json
microphoneoption is supported here, but app.json passes{ android: { permissions: [...] } }to this plugin (not consumed). Either read that shape or update app.json to{ "microphone": true }when needed.app/app.json (1)
66-69: Deduplicate usesCleartextTraffic configurationYou set
android.usesCleartextTraffic: trueat the top level and again viaexpo-build-properties. Keep one (prefer the top-level Expo key) to avoid drift.[ "expo-build-properties", { "android": { - "usesCleartextTraffic": true + // usesCleartextTraffic configured at top-level android } } ]app/app/hooks/useAudioStreamer.ts (1)
99-101: Use platform-safe timer types in RN
NodeJS.Timeoutisn’t accurate in RN. Use ReturnType to satisfy both web and native.- const reconnectTimeoutRef = useRef<NodeJS.Timeout | null>(null); - const heartbeatRef = useRef<NodeJS.Timeout | null>(null); + type TimeoutHandle = ReturnType<typeof setTimeout>; + type IntervalHandle = ReturnType<typeof setInterval>; + const reconnectTimeoutRef = useRef<TimeoutHandle | null>(null); + const heartbeatRef = useRef<IntervalHandle | null>(null);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.github/workflows/android-apk-build.yml(1 hunks)app/app.json(3 hunks)app/app/hooks/useAudioStreamer.ts(3 hunks)app/package.json(1 hunks)app/plugins/with-ws-fgs.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
app/**/*.{ts,tsx,js,jsx}: Follow standard React Native conventions for TypeScript/JavaScript in the mobile app
Mobile app WebSocket streaming must implement the Wyoming protocol: send JSONL header (audio-start/chunk/stop) followed by binary payloads
Files:
app/plugins/with-ws-fgs.jsapp/app/hooks/useAudioStreamer.ts
🧠 Learnings (1)
📚 Learning: 2025-09-05T06:06:37.640Z
Learnt from: CR
PR: AnkushMalaker/friend-lite#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T06:06:37.640Z
Learning: Applies to app/**/*.{ts,tsx,js,jsx} : Mobile app WebSocket streaming must implement the Wyoming protocol: send JSONL header (audio-start/chunk/stop) followed by binary payloads
Applied to files:
app/app/hooks/useAudioStreamer.ts
🪛 GitHub Actions: iOS IPA Build
app/app.json
[error] 1-1: app.json is missing ios.infoPlist.ITSAppUsesNonExemptEncryption boolean. Manual configuration is required in App Store Connect before the app can be tested. Command: eas build --platform ios --profile local --local --non-interactive --output /Users/runner/work/friend-lite/friend-lite/app-release.ipa
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
.github/workflows/android-apk-build.yml (1)
52-53: Removal ofeas initis safe.
app/eas.jsonalready defines thelocalAndroid profile, and theexpo/expo-github-actionstep logs in withsecrets.EXPO_TOKEN, soeas build --platform android --profile local --localwill automatically fetch and apply the keystore credentials from Expo servers.app/package.json (2)
17-17: Locking expo-build-properties with ~ is goodUsing
~0.14.8reduces churn while still getting patch fixes. LGTM.
12-12: No changes needed for @notifee/react-native version
Notifee 9.1.x (9.1.2 and above, including 9.1.8) officially supports React Native 0.79 / Expo SDK 53.app/app.json (1)
32-36: Android permissions: LGTMAdding
ACCESS_NETWORK_STATE,FOREGROUND_SERVICE,FOREGROUND_SERVICE_DATA_SYNC, andPOST_NOTIFICATIONSaligns with FGS + NetInfo usage.app/app/hooks/useAudioStreamer.ts (2)
142-148: Protocol detail: confirm version stringYou set
event.version = '1.0.0'. Some Wyoming impls use'1.0'. Confirm expected version for your server.
342-359: Connectivity listener: LGTMReconnecting on network restoration with backoff is solid.
| [ | ||
| "./plugins/with-ws-fgs", | ||
| { | ||
| "android": { | ||
| "permissions": [ | ||
| "FOREGROUND_SERVICE", | ||
| "FOREGROUND_SERVICE_DATA_SYNC", | ||
| "POST_NOTIFICATIONS" | ||
| ] | ||
| } | ||
| } | ||
| ], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Plugin options don’t match implementation; pass microphone instead
The plugin expects { microphone?: boolean }, but you pass { android: { permissions: [...] } } which is ignored. Adjust to avoid confusion.
- [
- "./plugins/with-ws-fgs",
- {
- "android": {
- "permissions": [
- "FOREGROUND_SERVICE",
- "FOREGROUND_SERVICE_DATA_SYNC",
- "POST_NOTIFICATIONS"
- ]
- }
- }
- ],
+ [
+ "./plugins/with-ws-fgs",
+ { "microphone": false }
+ ],📝 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.
| [ | |
| "./plugins/with-ws-fgs", | |
| { | |
| "android": { | |
| "permissions": [ | |
| "FOREGROUND_SERVICE", | |
| "FOREGROUND_SERVICE_DATA_SYNC", | |
| "POST_NOTIFICATIONS" | |
| ] | |
| } | |
| } | |
| ], | |
| [ | |
| "./plugins/with-ws-fgs", | |
| { "microphone": false } | |
| ], |
🤖 Prompt for AI Agents
In app/app.json around lines 51 to 62, the with-ws-fgs plugin is being passed an
Android permissions object which the plugin ignores; replace the object with the
expected option shape by removing the nested "android" block and instead pass {
"microphone": true } (or false as appropriate) so the plugin receives its {
microphone?: boolean } option; also remove or relocate the explicit permission
list into the appropriate Android config (e.g., android.permissions) if those
permissions still need to be declared elsewhere.
| const startStreaming = useCallback(async (url: string): Promise<void> => { | ||
| if (!url || url.trim() === '') { | ||
| Alert.alert('WebSocket URL Missing', 'Please provide a valid WebSocket URL.'); | ||
| const trimmed = (url || '').trim(); | ||
| if (!trimmed) { | ||
| const errorMsg = 'WebSocket URL is required.'; | ||
| setError(errorMsg); | ||
| setStateSafe(setError, errorMsg); | ||
| return Promise.reject(new Error(errorMsg)); | ||
| } | ||
|
|
||
| // Store the URL for reconnection attempts | ||
| currentUrlRef.current = url.trim(); | ||
|
|
||
| // Reset the manually stopped flag when starting a new connection | ||
| currentUrlRef.current = trimmed; | ||
| manuallyStoppedRef.current = false; | ||
|
|
||
|
|
||
| // Network gate | ||
| const netState = await NetInfo.fetch(); | ||
| if (!netState.isConnected || !netState.isInternetReachable) { | ||
| Alert.alert("No Internet", "Please check your internet connection to stream audio."); | ||
| const errorMsg = 'No internet connection.'; | ||
| setError(errorMsg); | ||
| setStateSafe(setError, errorMsg); | ||
| return Promise.reject(new Error(errorMsg)); | ||
| } |
There was a problem hiding this comment.
Network check can false-negative when isInternetReachable is null
On Android, isInternetReachable may be null. Treat only false as offline.
- const netState = await NetInfo.fetch();
- if (!netState.isConnected || !netState.isInternetReachable) {
+ const netState = await NetInfo.fetch();
+ const offline = netState.isConnected !== true || netState.isInternetReachable === false;
+ if (offline) {
const errorMsg = 'No internet connection.';
setStateSafe(setError, errorMsg);
return Promise.reject(new Error(errorMsg));
}📝 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.
| const startStreaming = useCallback(async (url: string): Promise<void> => { | |
| if (!url || url.trim() === '') { | |
| Alert.alert('WebSocket URL Missing', 'Please provide a valid WebSocket URL.'); | |
| const trimmed = (url || '').trim(); | |
| if (!trimmed) { | |
| const errorMsg = 'WebSocket URL is required.'; | |
| setError(errorMsg); | |
| setStateSafe(setError, errorMsg); | |
| return Promise.reject(new Error(errorMsg)); | |
| } | |
| // Store the URL for reconnection attempts | |
| currentUrlRef.current = url.trim(); | |
| // Reset the manually stopped flag when starting a new connection | |
| currentUrlRef.current = trimmed; | |
| manuallyStoppedRef.current = false; | |
| // Network gate | |
| const netState = await NetInfo.fetch(); | |
| if (!netState.isConnected || !netState.isInternetReachable) { | |
| Alert.alert("No Internet", "Please check your internet connection to stream audio."); | |
| const errorMsg = 'No internet connection.'; | |
| setError(errorMsg); | |
| setStateSafe(setError, errorMsg); | |
| return Promise.reject(new Error(errorMsg)); | |
| } | |
| const startStreaming = useCallback(async (url: string): Promise<void> => { | |
| const trimmed = (url || '').trim(); | |
| if (!trimmed) { | |
| const errorMsg = 'WebSocket URL is required.'; | |
| setStateSafe(setError, errorMsg); | |
| return Promise.reject(new Error(errorMsg)); | |
| } | |
| currentUrlRef.current = trimmed; | |
| manuallyStoppedRef.current = false; | |
| // Network gate | |
| - const netState = await NetInfo.fetch(); | |
| const netState = await NetInfo.fetch(); | |
| const offline = netState.isConnected !== true || netState.isInternetReachable === false; | |
| if (offline) { | |
| const errorMsg = 'No internet connection.'; | |
| setStateSafe(setError, errorMsg); | |
| return Promise.reject(new Error(errorMsg)); | |
| } |
🤖 Prompt for AI Agents
In app/app/hooks/useAudioStreamer.ts around lines 223 to 240, the network check
treats any falsy isInternetReachable (including null on Android) as offline;
change the condition so only an explicit false marks offline. Replace the
current check (!netState.isConnected || !netState.isInternetReachable) with a
check that treats null/undefined as unknown (online) and only errors when
isConnected is false or isInternetReachable === false, preserving the existing
error handling and Promise.reject behavior.
| if (websocketRef.current) await stopStreaming(); // close any existing | ||
|
|
There was a problem hiding this comment.
Calling stopStreaming here sets manual-stop and disables auto-reconnect
stopStreaming() flips manuallyStoppedRef to true. Subsequent closes won’t reconnect. Close the existing socket without toggling manual state or stopping FGS.
- if (websocketRef.current) await stopStreaming(); // close any existing
+ // Close any existing socket without marking manual-stop
+ if (websocketRef.current) {
+ try { websocketRef.current.close(1001, 'restart'); } catch {}
+ websocketRef.current = null;
+ }📝 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.
| if (websocketRef.current) await stopStreaming(); // close any existing | |
| // Close any existing socket without marking manual-stop | |
| if (websocketRef.current) { | |
| try { | |
| websocketRef.current.close(1001, 'restart'); | |
| } catch {} | |
| websocketRef.current = null; | |
| } |
🤖 Prompt for AI Agents
In app/app/hooks/useAudioStreamer.ts around lines 246-247, calling
stopStreaming() here sets manuallyStoppedRef to true which prevents
auto-reconnect; instead close the existing websocket directly without toggling
manual state or stopping FGS: replace the stopStreaming() invocation with logic
that calls websocketRef.current.close() (await/handle the close promise if
applicable), clear websocketRef.current and any event listeners, but do NOT set
manuallyStoppedRef or call the FGS stop path so auto-reconnect behavior remains
intact.
| // Start heartbeat | ||
| if (heartbeatRef.current) clearInterval(heartbeatRef.current); | ||
| heartbeatRef.current = setInterval(() => { | ||
| try { | ||
| if (websocketRef.current?.readyState === WebSocket.OPEN) { | ||
| websocketRef.current.send(JSON.stringify({ type: 'ping', t: Date.now() })); | ||
| } | ||
| } catch {} | ||
| }, HEARTBEAT_MS); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Heartbeat sends non-Wyoming frames; risks corrupting the JSONL stream
Per repo guidelines, Wyoming requires JSONL headers (audio-start/chunk/stop) followed by binary payloads only. Sending ad-hoc ping JSON (no newline) can merge with the next header and break parsing.
- // Start heartbeat
- if (heartbeatRef.current) clearInterval(heartbeatRef.current);
- heartbeatRef.current = setInterval(() => {
- try {
- if (websocketRef.current?.readyState === WebSocket.OPEN) {
- websocketRef.current.send(JSON.stringify({ type: 'ping', t: Date.now() }));
- }
- } catch {}
- }, HEARTBEAT_MS);
+ // Removed heartbeat; rely on FGS + TCP keepalives. If a keepalive is required,
+ // implement a Wyoming-compliant no-op agreed by server.
+ if (heartbeatRef.current) { clearInterval(heartbeatRef.current); heartbeatRef.current = null; }🌐 Web query:
Does the Wyoming WS streaming protocol allow custom 'ping' messages, or should only Wyoming JSONL headers be sent?
💡 Result:
Send only Wyoming JSONL messages on the application channel. Wyoming expects newline-delimited JSON headers (optionally followed by data/payload); do not inject custom non‑JSON application messages. If you need a heartbeat, use the Wyoming Ping/Pong event types (defined in Wyoming) as JSON messages, or rely on standard WebSocket control ping/pong frames (protocol-level) — but do not send arbitrary custom payloads outside the Wyoming JSONL format. [1][2]
Sources:
[1] Wyoming protocol format and message types (JSONL + optional binary payload).
[2] WebSocket control frames (Ping/Pong) are separate protocol-level frames (not application JSON).
Remove non-Wyoming heartbeat JSON
The interval block at app/app/hooks/useAudioStreamer.ts:263–272 sends arbitrary { type: 'ping' } JSON outside the Wyoming JSONL spec, which will corrupt the audio stream. Replace with either WebSocket control-ping frames or Wyoming’s defined Ping/Pong JSONL headers.
| // Ensure uses-permission array exists | ||
| manifest.manifest['uses-permission'] ||= []; | ||
| const addPerm = (name) => { | ||
| const exists = manifest.manifest['uses-permission'].some( | ||
| (p) => p.$['android:name'] === name | ||
| ); | ||
| if (!exists) { | ||
| manifest.manifest['uses-permission'].push({ $: { 'android:name': name } }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden permission node handling to avoid non-array shapes
uses-permission can be an object or array depending on parse state. Normalize before .some() to avoid runtime errors.
Apply:
- manifest.manifest['uses-permission'] ||= [];
+ const upKey = 'uses-permission';
+ const existing = manifest.manifest[upKey];
+ if (!Array.isArray(existing)) {
+ manifest.manifest[upKey] = existing ? [existing] : [];
+ }📝 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.
| // Ensure uses-permission array exists | |
| manifest.manifest['uses-permission'] ||= []; | |
| const addPerm = (name) => { | |
| const exists = manifest.manifest['uses-permission'].some( | |
| (p) => p.$['android:name'] === name | |
| ); | |
| if (!exists) { | |
| manifest.manifest['uses-permission'].push({ $: { 'android:name': name } }); | |
| } | |
| }; | |
| // Ensure uses-permission array exists | |
| const upKey = 'uses-permission'; | |
| const existing = manifest.manifest[upKey]; | |
| if (!Array.isArray(existing)) { | |
| manifest.manifest[upKey] = existing ? [existing] : []; | |
| } | |
| const addPerm = (name) => { | |
| const exists = manifest.manifest['uses-permission'].some( | |
| (p) => p.$['android:name'] === name | |
| ); | |
| if (!exists) { | |
| manifest.manifest['uses-permission'].push({ $: { 'android:name': name } }); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In app/plugins/with-ws-fgs.js around lines 16 to 25, the code assumes
manifest.manifest['uses-permission'] is always an array which can cause runtime
errors if it's undefined or an object; normalize the value to an array first (if
falsy set to [], if an object wrap it in an array) before calling .some() or
.push(), and then use Array.isArray checks to safely add the permission object
only when it doesn't already exist.
| const app = AndroidConfig.Manifest.getMainApplicationOrThrow(manifest); | ||
| app.service ||= []; | ||
| const svcName = 'app.notifee.core.ForegroundService'; | ||
|
|
||
| let svc = app.service.find((s) => s.$['android:name'] === svcName); | ||
| if (!svc) { | ||
| svc = { $: { 'android:name': svcName, 'android:exported': 'false' } }; | ||
| app.service.push(svc); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Normalize application.service to an array before .find
app.service can be a single object. Normalize to prevent .find from throwing.
- app.service ||= [];
+ if (!Array.isArray(app.service)) {
+ app.service = app.service ? [app.service] : [];
+ }📝 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.
| const app = AndroidConfig.Manifest.getMainApplicationOrThrow(manifest); | |
| app.service ||= []; | |
| const svcName = 'app.notifee.core.ForegroundService'; | |
| let svc = app.service.find((s) => s.$['android:name'] === svcName); | |
| if (!svc) { | |
| svc = { $: { 'android:name': svcName, 'android:exported': 'false' } }; | |
| app.service.push(svc); | |
| } | |
| const app = AndroidConfig.Manifest.getMainApplicationOrThrow(manifest); | |
| if (!Array.isArray(app.service)) { | |
| app.service = app.service ? [app.service] : []; | |
| } | |
| const svcName = 'app.notifee.core.ForegroundService'; | |
| let svc = app.service.find((s) => s.$['android:name'] === svcName); | |
| if (!svc) { | |
| svc = { $: { 'android:name': svcName, 'android:exported': 'false' } }; | |
| app.service.push(svc); | |
| } |
🤖 Prompt for AI Agents
In app/plugins/with-ws-fgs.js around lines 37 to 45, normalize app.service to an
array before calling .find to avoid crashes when it's a single object: detect if
app.service exists and is not an array (use Array.isArray), convert a single
object into an array containing that object (or set to an empty array if
undefined), then proceed to find and push the service as currently implemented.
Summary by CodeRabbit
New Features
Improvements
Permissions