fix(notifications): resolve delivery failures and complete deep-link flow#331
Conversation
- Added new intent filters in AndroidManifest.xml for handling deep links to "/incident" and "/verify". - Updated MainActivity to forward deep links when the activity is already running. - Removed the obsolete useAppLinkHandler hook and integrated deep link handling directly in AppNavigator. - Enhanced Home screen to handle deep links for incident and alert navigation. - Configured iOS AppDelegate for custom URL schemes and universal links. - Updated Info.plist to define the custom URL scheme for deep linking.
When the app is launched from a terminated state by tapping a notification, the OneSignal click event fires before NavigationContainer is mounted. Previously the deep link was silently dropped. Now handleNotificationOpen queues the event when navigationRef is not ready, and NavigationContainer's onReady callback flushes it once navigation is fully mounted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…notification handling - Replace placeholder URLs in deep-linking config with production domains - Add missing `flushPendingNotification` call in Home screen to handle cold-start notifications - Update deep-linking verification plan with concrete testing steps and checklist - Ensure notifications work across foreground, background, and cold-start
…flow Critical bug fixes: - AlertMethodMethod was imported as type-only, causing a runtime ReferenceError that silently skipped every device notification. - SendNotifications Promise.all iterated the unfiltered notifications list, sending pushes to disabled AlertMethods. - Logger 'debug' level was being routed to Logtail as INFO; now dev-only. Deep-link completion: - Device notifications now use firealert:// scheme, avoiding iOS Safari intercept (Universal Links not yet configured). - Added OneSignal_suppress_launch_urls to Info.plist, paired with the existing Android suppressLaunchURLs. - Home's deep-link effect now zooms to the alert via calculateAlertCamera, skips the default user-location zoom when params carry alertId/incidentId, and uses a __ts nonce so re-tapping the same notification re-fires. Debug instrumentation added across the notification pipeline for server and client diagnostics (dev only). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughImplements app deep-linking end-to-end: React Navigation linking config and navigation ref; platform intent/URL handlers (iOS AppDelegate, Android manifest/MainActivity); OneSignal click routing with queuing/flush via Changes
Sequence Diagram(s)sequenceDiagram
participant OS as OneSignal
participant App as Native App
participant OSC as OneSignalContext
participant HNO as handleNotificationOpen
participant Nav as NavigationContainer
participant Home as Home Screen
OS->>App: user clicks notification
App->>OSC: emit 'click' event
OSC->>HNO: handleNotificationOpen(event)
alt nav.ready
HNO->>HNO: extract launchURL / additionalData
HNO->>HNO: derive path and call linking.getStateFromPath
HNO->>Nav: dispatch(CommonActions.reset(state))
Nav->>Home: navigate with alertId/incidentId params
Home->>Home: open detail sheet & focus map
else not ready
HNO->>HNO: store in _pendingEvent
Note right of HNO: wait for NavigationContainer.onReady
end
Nav->>HNO: onReady -> flushPendingNotification()
HNO->>Nav: replay event -> dispatch(CommonActions.reset(state))
Nav->>Home: navigate with params
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/server/src/Services/Notifications/SendNotifications.ts (1)
330-347:⚠️ Potential issue | 🟡 MinorFilter the "mark as skipped" set from
filteredNotifications, notnotifications.
notificationsat line 330 still contains the entries whosealertMethodwas disabled — those were already updated withisSkipped: trueon lines 96-99 and never pushed intofilteredNotifications. Using the unfiltered list here causes two issues:
- The subsequent
prisma.notification.updateManyrewrites the same already-skipped rows again with{isSkipped: true, isDelivered: false, sentAt: null}— idempotent but an unnecessary DB write per batch.- The debug log on line 339 reports those rows as freshly "marked as skipped", which is misleading when diagnosing delivery failures vs. disabled-method skips.
Since the whole point of the PR's "filtered iteration" fix was to keep disabled alert methods out of the send pipeline, this companion filter should also operate on
filteredNotifications.🐛 Suggested fix
- const unsuccessfulNotifications = notifications.filter( + const unsuccessfulNotifications = filteredNotifications.filter( ({id}) => !successfulNotificationIds.includes(id), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/Services/Notifications/SendNotifications.ts` around lines 330 - 347, The current logic builds unsuccessfulNotifications from notifications (which still contains already-disabled alertMethod entries) causing redundant DB updates and misleading logs; change the filter to use filteredNotifications instead of notifications when computing unsuccessfulNotifications (i.e., compute unsuccessfulNotifications = filteredNotifications.filter(({id}) => !successfulNotificationIds.includes(id))), then derive unsuccessfulNotificationIds from that set and pass those ids to prisma.notification.updateMany and the logger (keep references to successfulNotificationIds, batchCount, prisma.notification.updateMany, logger, and filteredNotifications to locate the change).AGENTS copy.md (1)
1-26:⚠️ Potential issue | 🟡 MinorLikely accidental duplicate of
AGENTS.md— remove or rename.The filename
AGENTS copy.md(with an embedded space andcopysuffix) is the telltale pattern of a macOS Finder duplicate that was committed by mistake. Two near-identical instruction files will drift out of sync and confuse tooling that expects a singleAGENTS.md.Please either delete this file or, if it is intentional, rename it to something descriptive (kebab-case, per the very guideline it documents) and explain how it differs from
AGENTS.md.#!/bin/bash # Confirm whether this file is a duplicate of AGENTS.md (or where AGENTS.md lives). fd -H -t f 'AGENTS' --max-depth 3 echo '---' # If both exist at repo root, diff them to show drift. if [ -f AGENTS.md ] && [ -f "AGENTS copy.md" ]; then diff -u AGENTS.md "AGENTS copy.md" || true fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS` copy.md around lines 1 - 26, The file "AGENTS copy.md" appears to be an accidental duplicate of "AGENTS.md"; either delete "AGENTS copy.md" or rename it to a descriptive kebab-case name (e.g. agents-<purpose>.md) and update any references; before committing, run the provided verification script to confirm whether both files exist and diff them, and if you keep a renamed file include a short note at the top explaining how it differs from AGENTS.md.apps/nativeapp/ios/FireAlert/AppDelegate.swift (1)
1-4:⚠️ Potential issue | 🔴 CriticalAdd
RCTLinkingManagerimport to a bridging header.The code uses
RCTLinkingManagerat lines 41 and 50, butRCTLinkingManageris not exposed byimport Reactin React Native 0.81.4. A bridging header must explicitly import<React/RCTLinkingManager.h>for Swift to access it.Create or update a bridging header file (e.g.,
FireAlert-Bridging-Header.h) with:`#import` <React/RCTLinkingManager.h>Then ensure the bridging header is configured in your Xcode project's Build Settings under "Bridging Header" (SWIFT_OBJC_BRIDGING_HEADER).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nativeapp/ios/FireAlert/AppDelegate.swift` around lines 1 - 4, Add a Swift bridging header that imports RCTLinkingManager so Swift can access it: create or update a bridging header (e.g., FireAlert-Bridging-Header.h) and add an import for <React/RCTLinkingManager.h>, then set the project's Swift bridging header path in Build Settings (SWIFT_OBJC_BRIDGING_HEADER); this will expose RCTLinkingManager used by AppDelegate.swift (references to RCTLinkingManager in methods handling URLs) to the Swift code.
🧹 Nitpick comments (9)
.yarnrc (1)
3-3: This change is safe to merge; consider its scope relative to PR objectives.The
--install.frozen-lockfile truesetting enforces lockfile integrity and is a best practice for ensuring reproducible builds. Verification confirmsyarn.lockis currently in sync, so adding this flag will not cause immediate CI failures.However, this configuration change is unrelated to the PR's core objectives (notification delivery and deep-linking fixes). Consider whether moving this to a dedicated infrastructure/tooling PR would provide clearer change tracking and separation of concerns.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.yarnrc at line 3, This change adds the Yarn config flag "--install.frozen-lockfile true" in .yarnrc which is unrelated to the notification delivery/deep-linking work; either remove the added flag from this PR or move the .yarnrc change into a separate infrastructure/tooling PR — update the .yarnrc commit to revert/remove the "--install.frozen-lockfile true" line here (or extract it into a standalone commit/branch and open a separate PR) so the functional PR remains focused on Notification and deep-linking fixes.apps/nativeapp/app/contexts/OneSignalContext.tsx (1)
151-159: Consider gating the OneSignal click log behind__DEV__.This is one of the
console.logcalls the PR description flags as a follow-up to "strip or guard". Since OneSignal clicks fire on every notification tap in production, the unconditional log will run on users' devices. Low-risk, but wrapping inif (__DEV__)(or routing through your existing dev logger) avoids shipping diagnostic noise — especially sinceevent.notification?.notificationIdis user-activity data.♻️ Suggested guard
const openedHandler = (event: NotificationClickEvent) => { - console.log('[deepLink] OneSignal click event received', { - hasHandler: !!handlersRef.current?.onOpened, - notificationId: event.notification?.notificationId, - }); + if (__DEV__) { + console.log('[deepLink] OneSignal click event received', { + hasHandler: !!handlersRef.current?.onOpened, + notificationId: event.notification?.notificationId, + }); + } if (handlersRef.current?.onOpened) { handlersRef.current.onOpened(event); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nativeapp/app/contexts/OneSignalContext.tsx` around lines 151 - 159, The console.log in the openedHandler (NotificationClickEvent handler) should be gated so it only runs in development; wrap the logging call (or route it through the app's dev logger) with a __DEV__ check to avoid emitting user activity and noisy logs in production — update the openedHandler function to only call console.log or the dev logger when __DEV__ is true, leaving the existing handlersRef.current?.onOpened(event) behavior unchanged.apps/server/src/Services/Notifier/Notifier/DeviceNotifier.ts (1)
56-57: Avoid double-emitting the same message viaconsole.*+logger.Both the missing-OneSignal-config path (Lines 56-57) and the OneSignal rejection path (Lines 97-101) now log via both
console.warn/console.errorandlogger(..., 'warn'|'error'). In production,loggeralready routes to Logtail and also to stdout on failure, so every real incident will be recorded twice (with slightly different text), which inflates Logtail volume and makes alert de-dup harder. This matches the PR description's own follow-up to "consider removing duplicate server console errors/warns".Recommend picking one channel per severity — typically
loggerfor operational events, and reservingconsole.*for local-only diagnostics gated byNODE_ENV. The same pattern appears inSendNotifications.tsline 309 and should be addressed together.♻️ Suggested consolidation
if (!env.ONESIGNAL_APP_ID || !env.ONESIGNAL_REST_API_KEY) { - console.warn(`[device-notifier] OneSignal not configured — ONESIGNAL_APP_ID=${!!env.ONESIGNAL_APP_ID} ONESIGNAL_REST_API_KEY=${!!env.ONESIGNAL_REST_API_KEY}`); - logger(`Push notifications are disabled: OneSignal is not configured`, 'warn'); + logger( + `Push notifications disabled: OneSignal not configured (APP_ID set=${!!env.ONESIGNAL_APP_ID}, REST_API_KEY set=${!!env.ONESIGNAL_REST_API_KEY})`, + 'warn', + ); return Promise.resolve(false); } @@ if (!response.ok) { - console.error(`[device-notifier] OneSignal rejected: status=${response.status} statusText="${response.statusText}" notificationId=${parameters.id} player_id=${destination} body=${JSON.stringify(responseBody)}`); logger( - `Failed to send device notification. Error: ${response.statusText} for ${parameters.id}`, + `Failed to send device notification: status=${response.status} statusText="${response.statusText}" notificationId=${parameters.id} player_id=${destination} body=${JSON.stringify(responseBody)}`, 'error', );Also applies to: 97-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/Services/Notifier/Notifier/DeviceNotifier.ts` around lines 56 - 57, The code double-emits missing-OneSignal and OneSignal rejection messages via console.* and logger; update DeviceNotifier (around the missing-config check and the OneSignal rejection handling in DeviceNotifier.notify/sendNotification or similar functions) to use only logger(...) for operational events and remove the console.warn/console.error calls, or alternatively keep console.* only when NODE_ENV==='development' and otherwise use logger; apply the same change to the corresponding duplicate at SendNotifications (around the handler at ~line 309) so each event is emitted once through the selected channel.apps/server/src/Services/Notifications/CreateNotifications.ts (1)
202-209: Reuse the computedeligiblecomponents instead of re-invokingisSiteAlertMethod.
isSiteAlertMethod(alertMethod.method)is evaluated once for theeligibleexpression on lines 202-205 and again inside the log template on line 207. It's cheap today but (a) duplicates logic that could drift, and (b) embeds a function call in a template literal that fires for every alertMethod of every SiteAlert in the batch. Inlining the already-computed boolean is clearer and avoids the double call.♻️ Suggested tweak
alertMethods.forEach(alertMethod => { - const eligible = - alertMethod.isVerified && - alertMethod.isEnabled && - isSiteAlertMethod(alertMethod.method); + const isSite = isSiteAlertMethod(alertMethod.method); + const eligible = + alertMethod.isVerified && alertMethod.isEnabled && isSite; logger( - ` alertMethod ${alertMethod.method}: isVerified=${alertMethod.isVerified}, isEnabled=${alertMethod.isEnabled}, isSiteAlertMethod=${isSiteAlertMethod(alertMethod.method)} → eligible=${eligible}`, + ` alertMethod ${alertMethod.method}: isVerified=${alertMethod.isVerified}, isEnabled=${alertMethod.isEnabled}, isSiteAlertMethod=${isSite} → eligible=${eligible}`, 'debug', );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/Services/Notifications/CreateNotifications.ts` around lines 202 - 209, The log currently calls isSiteAlertMethod(alertMethod.method) twice; compute the boolean once and reuse it: extract a local const (e.g., isSite = isSiteAlertMethod(alertMethod.method)), use that in the eligible expression (eligible = alertMethod.isVerified && alertMethod.isEnabled && isSite) and then reference isSite in the logger template instead of re-invoking isSiteAlertMethod; update references to alertMethod.method, eligible, isSite and the logger call accordingly.apps/nativeapp/app/screens/Home/Home.tsx (1)
754-760: Strip or guard the[deepLink]console.logs before release.Already listed in PR follow-ups; calling it out here since this file is where most of the client-side noise lives. In production these will run on every deep-link tap and on every re-render that reaches the effect, and will appear in release Android/iOS logs. A tiny
if (__DEV__)guard or a sharedlogger.debug()helper removes the concern without losing the instrumentation during development.Also applies to: 767-767, 773-773, 786-786, 795-795
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nativeapp/app/screens/Home/Home.tsx` around lines 754 - 760, The console.log calls in Home.tsx's deep-link useEffect (e.g., the console.log starting "[deepLink] Home useEffect fired" and the other logs at the referenced lines) must be removed or guarded for production; wrap these debug logs in a development-only guard (e.g., if (__DEV__) { ... }) or replace them with a debug-level logger helper (e.g., logger.debug(...)) so they do not run in release builds while preserving dev instrumentation. Ensure you update every occurrence in that effect (lines near the useEffect, including the logs at the other noted line numbers) rather than leaving any stray console.log calls.apps/nativeapp/android/app/src/main/AndroidManifest.xml (1)
28-41: Looks good — configuration is consistent withlinkingConfig.tsand the custom-scheme filter correctly omitsandroid:host.The HTTPS
autoVerifydependency on the deployedassetlinks.jsonis covered in the root-cause comment onapps/nativeapp/deeplinking/assetlinks.json; no action needed here.One minor note:
android:pathPrefix="/verify"is a prefix match, so/verifying,/verify-email, etc. will also be claimed by the app. If that's intentional (todaylinkingConfig.tsonly mapsverify/:verificationType), great; otherwise considerandroid:pathPattern="/verify/.*"for a tighter match.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nativeapp/android/app/src/main/AndroidManifest.xml` around lines 28 - 41, The intent-filter using android:pathPrefix="/verify" will match any path starting with /verify (e.g., /verifying or /verify-email); if you want only routes like verify/:verificationType, update the intent-filter in the <intent-filter android:autoVerify="true"> to use android:pathPattern="/verify/.*" instead of android:pathPrefix, and verify that linkingConfig.ts's route mapping for "verify/:verificationType" remains consistent with this tighter pattern; leave the custom-scheme intent-filter (android:scheme="firealert") and autoVerify host entries unchanged.apps/nativeapp/app/utils/linking/handleNotificationOpen.ts (3)
4-4:DEEP_LINK_HOSTimport is unused.Only
DEEP_LINK_SCHEMEandlinkingare referenced in this module; dropDEEP_LINK_HOSTfrom the import list to avoid a stray symbol.-import {DEEP_LINK_HOST, DEEP_LINK_SCHEME, linking} from './linkingConfig'; +import {DEEP_LINK_SCHEME, linking} from './linkingConfig';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nativeapp/app/utils/linking/handleNotificationOpen.ts` at line 4, Remove the unused import symbol DEEP_LINK_HOST from the import statement that currently reads "import {DEEP_LINK_HOST, DEEP_LINK_SCHEME, linking} from './linkingConfig';" so only DEEP_LINK_SCHEME and linking are imported; update the import to reference only DEEP_LINK_SCHEME and linking to eliminate the stray DEEP_LINK_HOST symbol.
7-7: Single-slot pending queue: last event wins if multiple arrive before nav is ready.
_pendingEventstores at most one click event. For the typical cold-start path (a singlegetInitialNotification()replay) this is fine, but if two notification taps land beforenavigationRef.isReady()— e.g., rapid taps during a slow JS-mount — the earlier one is silently dropped. If that ordering is acceptable, consider a short comment documenting the "latest tap wins" policy; otherwise switch to a small FIFO and drain it influshPendingNotification.♻️ Optional: FIFO drain
-let _pendingEvent: NotificationClickEvent | null = null; +const _pendingEvents: NotificationClickEvent[] = []; @@ - if (!navigationRef.isReady() || !linking.getStateFromPath) { - console.log('[deepLink] navigation not ready, queuing event'); - _pendingEvent = event; - return; - } + if (!navigationRef.isReady() || !linking.getStateFromPath) { + console.log('[deepLink] navigation not ready, queuing event'); + _pendingEvents.push(event); + return; + } @@ -export const flushPendingNotification = () => { - if (_pendingEvent) { - const event = _pendingEvent; - _pendingEvent = null; - handleNotificationOpen(event); - } -}; +export const flushPendingNotification = () => { + while (_pendingEvents.length > 0) { + const event = _pendingEvents.shift()!; + handleNotificationOpen(event); + } +};Also applies to: 81-85, 101-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nativeapp/app/utils/linking/handleNotificationOpen.ts` at line 7, _pendingEvent currently holds a single NotificationClickEvent so concurrent taps before nav is ready drop earlier events; replace it with a small FIFO (e.g., _pendingEvents: NotificationClickEvent[]) and push incoming events instead of overwriting, then update the logic in the functions that enqueue events (the handler that previously assigned _pendingEvent) and in flushPendingNotification to dequeue and process events in order while navigationRef.isReady(), draining the queue; alternatively, if you intend "latest wins", add a clear comment next to _pendingEvent and any enqueue logic documenting that policy.
60-65: Remove the unsafeunknowncast;launchURLis properly typed in react-native-onesignal v5.2.13+.The project uses
react-native-onesignal@^5.2.13(resolved to v5.2.14), which properly exposeslaunchURL?: stringin theOSNotificationTypeScript definitions. The double-castas unknown as {launchURL?: string}is unnecessary and should be replaced with direct property access:const launchUrl = notification.launchURL;This removes an unsafe escape hatch and improves type safety.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/nativeapp/app/utils/linking/handleNotificationOpen.ts` around lines 60 - 65, In handleNotificationOpen replace the unsafe double-cast used to access launchURL with direct property access on the notification object: remove "(notification as unknown as {launchURL?: string}).launchURL" and read notification.launchURL instead (the OSNotification type from react-native-onesignal v5.2.13+ already defines launchURL?: string), keeping existing variable name launchUrl and preserving the surrounding logic that uses additionalData.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/plans/deep-linking-verification-fix.md:
- Around line 30-38: The review points out that adding JavaScript-style `//
TODO` comments inside strict JSON files apple-app-site-association and
assetlinks.json will break parsing; update the guidance so the files use
sentinel placeholder strings (e.g. "REPLACE_TEAM_ID" in
apple-app-site-association and "REPLACE_WITH_RELEASE_SHA256_FINGERPRINT" in
assetlinks.json) instead of `// TODO` lines, and move any human TODO details to
a sibling README.md in apps/nativeapp/deeplinking/ or an external ticket
referenced from that README so the Web team can find and replace the
placeholders before deployment.
In `@apps/nativeapp/app/screens/Home/Home.tsx`:
- Around line 778-803: The effect currently always dispatches
dispatch(openAlertDetails({alertId})) but only sets deepLinkHandledRef.current =
key when alertData exists, causing unbounded re-dispatches; fix by marking the
deep link handled immediately after dispatch (move or add
deepLinkHandledRef.current = key right after
dispatch(openAlertDetails({alertId}))) or alternatively implement a small retry
budget using a deepLinkRetryCountRef keyed by alertId and set
deepLinkHandledRef.current = key after N attempts/timeout to stop repeated
dispatches; reference alertId, alerts?.json?.data, dispatch(openAlertDetails),
deepLinkHandledRef.current and key when making the change.
In `@apps/nativeapp/deeplinking/apple-app-site-association`:
- Line 6: The apple-app-site-association file currently contains the placeholder
appID "REPLACE_TEAM_ID.eco.pp.firealert" which breaks iOS app-ID validation;
replace the REPLACE_TEAM_ID prefix with your 10-character Apple Team ID (e.g.,
ABCDE12345) so the "appIDs" array contains the real team-scoped ID(s) (and add
any other build-specific bundle IDs like the notification service extension if
required), or rename the file to apple-app-site-association.template and
document the substitution to avoid accidental deployment.
In `@apps/nativeapp/deeplinking/assetlinks.json`:
- Around line 7-9: The assetlinks.json currently contains a placeholder
fingerprint string; replace the "REPLACE_WITH_RELEASE_SHA256_FINGERPRINT" entry
inside the sha256_cert_fingerprints array in assetlinks.json with the actual
release signing certificate SHA-256 fingerprint (colon-separated, uppercase
hex), and if you use Google Play App Signing add both the upload-key and Play
app-signing-key fingerprints as separate entries in that same
sha256_cert_fingerprints array; after updating, ensure the file is served at
https://firealert.plant-for-the-planet.org/.well-known/assetlinks.json with
Content-Type: application/json and no redirects so Android's verifier (used by
AndroidManifest autoVerify) can validate app links.
---
Outside diff comments:
In `@AGENTS` copy.md:
- Around line 1-26: The file "AGENTS copy.md" appears to be an accidental
duplicate of "AGENTS.md"; either delete "AGENTS copy.md" or rename it to a
descriptive kebab-case name (e.g. agents-<purpose>.md) and update any
references; before committing, run the provided verification script to confirm
whether both files exist and diff them, and if you keep a renamed file include a
short note at the top explaining how it differs from AGENTS.md.
In `@apps/nativeapp/ios/FireAlert/AppDelegate.swift`:
- Around line 1-4: Add a Swift bridging header that imports RCTLinkingManager so
Swift can access it: create or update a bridging header (e.g.,
FireAlert-Bridging-Header.h) and add an import for <React/RCTLinkingManager.h>,
then set the project's Swift bridging header path in Build Settings
(SWIFT_OBJC_BRIDGING_HEADER); this will expose RCTLinkingManager used by
AppDelegate.swift (references to RCTLinkingManager in methods handling URLs) to
the Swift code.
In `@apps/server/src/Services/Notifications/SendNotifications.ts`:
- Around line 330-347: The current logic builds unsuccessfulNotifications from
notifications (which still contains already-disabled alertMethod entries)
causing redundant DB updates and misleading logs; change the filter to use
filteredNotifications instead of notifications when computing
unsuccessfulNotifications (i.e., compute unsuccessfulNotifications =
filteredNotifications.filter(({id}) =>
!successfulNotificationIds.includes(id))), then derive
unsuccessfulNotificationIds from that set and pass those ids to
prisma.notification.updateMany and the logger (keep references to
successfulNotificationIds, batchCount, prisma.notification.updateMany, logger,
and filteredNotifications to locate the change).
---
Nitpick comments:
In @.yarnrc:
- Line 3: This change adds the Yarn config flag "--install.frozen-lockfile true"
in .yarnrc which is unrelated to the notification delivery/deep-linking work;
either remove the added flag from this PR or move the .yarnrc change into a
separate infrastructure/tooling PR — update the .yarnrc commit to revert/remove
the "--install.frozen-lockfile true" line here (or extract it into a standalone
commit/branch and open a separate PR) so the functional PR remains focused on
Notification and deep-linking fixes.
In `@apps/nativeapp/android/app/src/main/AndroidManifest.xml`:
- Around line 28-41: The intent-filter using android:pathPrefix="/verify" will
match any path starting with /verify (e.g., /verifying or /verify-email); if you
want only routes like verify/:verificationType, update the intent-filter in the
<intent-filter android:autoVerify="true"> to use
android:pathPattern="/verify/.*" instead of android:pathPrefix, and verify that
linkingConfig.ts's route mapping for "verify/:verificationType" remains
consistent with this tighter pattern; leave the custom-scheme intent-filter
(android:scheme="firealert") and autoVerify host entries unchanged.
In `@apps/nativeapp/app/contexts/OneSignalContext.tsx`:
- Around line 151-159: The console.log in the openedHandler
(NotificationClickEvent handler) should be gated so it only runs in development;
wrap the logging call (or route it through the app's dev logger) with a __DEV__
check to avoid emitting user activity and noisy logs in production — update the
openedHandler function to only call console.log or the dev logger when __DEV__
is true, leaving the existing handlersRef.current?.onOpened(event) behavior
unchanged.
In `@apps/nativeapp/app/screens/Home/Home.tsx`:
- Around line 754-760: The console.log calls in Home.tsx's deep-link useEffect
(e.g., the console.log starting "[deepLink] Home useEffect fired" and the other
logs at the referenced lines) must be removed or guarded for production; wrap
these debug logs in a development-only guard (e.g., if (__DEV__) { ... }) or
replace them with a debug-level logger helper (e.g., logger.debug(...)) so they
do not run in release builds while preserving dev instrumentation. Ensure you
update every occurrence in that effect (lines near the useEffect, including the
logs at the other noted line numbers) rather than leaving any stray console.log
calls.
In `@apps/nativeapp/app/utils/linking/handleNotificationOpen.ts`:
- Line 4: Remove the unused import symbol DEEP_LINK_HOST from the import
statement that currently reads "import {DEEP_LINK_HOST, DEEP_LINK_SCHEME,
linking} from './linkingConfig';" so only DEEP_LINK_SCHEME and linking are
imported; update the import to reference only DEEP_LINK_SCHEME and linking to
eliminate the stray DEEP_LINK_HOST symbol.
- Line 7: _pendingEvent currently holds a single NotificationClickEvent so
concurrent taps before nav is ready drop earlier events; replace it with a small
FIFO (e.g., _pendingEvents: NotificationClickEvent[]) and push incoming events
instead of overwriting, then update the logic in the functions that enqueue
events (the handler that previously assigned _pendingEvent) and in
flushPendingNotification to dequeue and process events in order while
navigationRef.isReady(), draining the queue; alternatively, if you intend
"latest wins", add a clear comment next to _pendingEvent and any enqueue logic
documenting that policy.
- Around line 60-65: In handleNotificationOpen replace the unsafe double-cast
used to access launchURL with direct property access on the notification object:
remove "(notification as unknown as {launchURL?: string}).launchURL" and read
notification.launchURL instead (the OSNotification type from
react-native-onesignal v5.2.13+ already defines launchURL?: string), keeping
existing variable name launchUrl and preserving the surrounding logic that uses
additionalData.
In `@apps/server/src/Services/Notifications/CreateNotifications.ts`:
- Around line 202-209: The log currently calls
isSiteAlertMethod(alertMethod.method) twice; compute the boolean once and reuse
it: extract a local const (e.g., isSite =
isSiteAlertMethod(alertMethod.method)), use that in the eligible expression
(eligible = alertMethod.isVerified && alertMethod.isEnabled && isSite) and then
reference isSite in the logger template instead of re-invoking
isSiteAlertMethod; update references to alertMethod.method, eligible, isSite and
the logger call accordingly.
In `@apps/server/src/Services/Notifier/Notifier/DeviceNotifier.ts`:
- Around line 56-57: The code double-emits missing-OneSignal and OneSignal
rejection messages via console.* and logger; update DeviceNotifier (around the
missing-config check and the OneSignal rejection handling in
DeviceNotifier.notify/sendNotification or similar functions) to use only
logger(...) for operational events and remove the console.warn/console.error
calls, or alternatively keep console.* only when NODE_ENV==='development' and
otherwise use logger; apply the same change to the corresponding duplicate at
SendNotifications (around the handler at ~line 309) so each event is emitted
once through the selected channel.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9fc93482-7ba4-450b-8f88-7eb74b788685
⛔ Files ignored due to path filters (1)
apps/nativeapp/ios/Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
.claude/plans/deep-link-testing-plan.md.claude/plans/deep-linking-verification-fix.md.yarnrcAGENTS copy.mdapps/nativeapp/android/app/src/main/AndroidManifest.xmlapps/nativeapp/android/app/src/main/java/eco/pp/firealert/MainActivity.ktapps/nativeapp/app/contexts/OneSignalContext.tsxapps/nativeapp/app/hooks/notification/useAppLinkHandler.tsapps/nativeapp/app/routes/AppNavigator.tsxapps/nativeapp/app/screens/Home/Home.tsxapps/nativeapp/app/utils/linking/handleNotificationOpen.tsapps/nativeapp/app/utils/linking/index.tsapps/nativeapp/app/utils/linking/linkingConfig.tsapps/nativeapp/app/utils/linking/navigationRef.tsapps/nativeapp/deeplinking/apple-app-site-associationapps/nativeapp/deeplinking/assetlinks.jsonapps/nativeapp/ios/FireAlert/AppDelegate.swiftapps/nativeapp/ios/FireAlert/Info.plistapps/server/src/Services/Notifications/CreateNotifications.tsapps/server/src/Services/Notifications/SendNotifications.tsapps/server/src/Services/Notifier/Notifier/DeviceNotifier.tsapps/server/src/server/logger.ts
💤 Files with no reviewable changes (1)
- apps/nativeapp/app/hooks/notification/useAppLinkHandler.ts
| #### 1. `apple-app-site-association` has placeholder Team ID | ||
| **File:** `apps/nativeapp/deeplinking/apple-app-site-association:6` | ||
| **Owner:** Web team — needs Apple Developer Team ID + server deployment to `/.well-known/`. | ||
| Add a `// TODO` comment and leave for web team to fill in. | ||
|
|
||
| #### 2. `assetlinks.json` has placeholder SHA256 fingerprint | ||
| **File:** `apps/nativeapp/deeplinking/assetlinks.json:8` | ||
| **Owner:** Web team — needs release keystore SHA256 + server deployment to `/.well-known/`. | ||
| Add a `// TODO` comment and leave for web team to fill in. |
There was a problem hiding this comment.
Nit: // TODO comments inside apple-app-site-association / assetlinks.json will break JSON parsing.
Both files are strict JSON (no JSONC). Any // line added by the web team would cause the OS/verifier to reject the association file. Consider rewording the guidance to track the TODO outside the file — e.g., via a ticket, a sibling README.md in apps/nativeapp/deeplinking/, or a sentinel placeholder string (REPLACE_TEAM_ID, REPLACE_WITH_RELEASE_SHA256_FINGERPRINT) that is easy to grep for and fail-fast on deploy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/plans/deep-linking-verification-fix.md around lines 30 - 38, The
review points out that adding JavaScript-style `// TODO` comments inside strict
JSON files apple-app-site-association and assetlinks.json will break parsing;
update the guidance so the files use sentinel placeholder strings (e.g.
"REPLACE_TEAM_ID" in apple-app-site-association and
"REPLACE_WITH_RELEASE_SHA256_FINGERPRINT" in assetlinks.json) instead of `//
TODO` lines, and move any human TODO details to a sibling README.md in
apps/nativeapp/deeplinking/ or an external ticket referenced from that README so
the Web team can find and replace the placeholders before deployment.
| if (alertId) { | ||
| const alertData = alerts?.json?.data?.find( | ||
| (a: any) => a.id === alertId, | ||
| ); | ||
| // Always open the details sheet (idempotent Redux dispatch). | ||
| dispatch(openAlertDetails({alertId})); | ||
| if (!alertData) { | ||
| // Alerts not loaded yet — don't mark handled so we retry on data load. | ||
| console.log('[deepLink] Home: alert data not loaded yet for', alertId); | ||
| return; | ||
| } | ||
| deepLinkHandledRef.current = key; | ||
| const cameraSettings = calculateAlertCamera( | ||
| alertData, | ||
| alertData.detectedBy, | ||
| alertData.confidence, | ||
| ); | ||
| console.log('[deepLink] Home: zooming to alert', {alertId, cameraSettings}); | ||
| if (camera?.current?.setCamera && cameraSettings) { | ||
| camera.current.setCamera({ | ||
| centerCoordinate: cameraSettings.centerCoordinate, | ||
| zoomLevel: cameraSettings.zoomLevel, | ||
| animationDuration: 500, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Unbounded re-dispatch if alertId never resolves in alerts.json.data.
When the deep-linked alertId is not present in the current alerts list (expired, filtered out by duration, different account, server lag, typo), this branch:
- Dispatches
openAlertDetails({alertId})unconditionally. - Returns without setting
deepLinkHandledRef.current.
Every subsequent refresh of alerts.json.data (refetch, cache update, duration filter change) will re-enter the effect and re-dispatch the same openAlertDetails action. It's idempotent at the Redux level, but it means the detail sheet stays stuck in a "loading/not found" state indefinitely without any escape for the user, and the effect keeps firing.
Consider:
- A short retry budget (e.g., mark handled after N data loads or T ms), or
- Marking handled once you've dispatched
openAlertDetailsand letting the detail sheet own the "alert not found" UI.
💡 One possible shape
if (alertId) {
const alertData = alerts?.json?.data?.find(
(a: any) => a.id === alertId,
);
- // Always open the details sheet (idempotent Redux dispatch).
- dispatch(openAlertDetails({alertId}));
- if (!alertData) {
- // Alerts not loaded yet — don't mark handled so we retry on data load.
- console.log('[deepLink] Home: alert data not loaded yet for', alertId);
- return;
- }
- deepLinkHandledRef.current = key;
+ // Open the details sheet immediately; sheet handles its own loading/not-found state.
+ dispatch(openAlertDetails({alertId}));
+ if (!alerts?.json?.data) {
+ // Alerts query hasn't resolved at all yet — retry when it does.
+ return;
+ }
+ // Mark handled whether or not we found the alert; sheet owns missing-alert UX.
+ deepLinkHandledRef.current = key;
+ if (!alertData) {
+ return;
+ }
const cameraSettings = calculateAlertCamera(📝 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 (alertId) { | |
| const alertData = alerts?.json?.data?.find( | |
| (a: any) => a.id === alertId, | |
| ); | |
| // Always open the details sheet (idempotent Redux dispatch). | |
| dispatch(openAlertDetails({alertId})); | |
| if (!alertData) { | |
| // Alerts not loaded yet — don't mark handled so we retry on data load. | |
| console.log('[deepLink] Home: alert data not loaded yet for', alertId); | |
| return; | |
| } | |
| deepLinkHandledRef.current = key; | |
| const cameraSettings = calculateAlertCamera( | |
| alertData, | |
| alertData.detectedBy, | |
| alertData.confidence, | |
| ); | |
| console.log('[deepLink] Home: zooming to alert', {alertId, cameraSettings}); | |
| if (camera?.current?.setCamera && cameraSettings) { | |
| camera.current.setCamera({ | |
| centerCoordinate: cameraSettings.centerCoordinate, | |
| zoomLevel: cameraSettings.zoomLevel, | |
| animationDuration: 500, | |
| }); | |
| } | |
| } | |
| if (alertId) { | |
| const alertData = alerts?.json?.data?.find( | |
| (a: any) => a.id === alertId, | |
| ); | |
| // Open the details sheet immediately; sheet handles its own loading/not-found state. | |
| dispatch(openAlertDetails({alertId})); | |
| if (!alerts?.json?.data) { | |
| // Alerts query hasn't resolved at all yet — retry when it does. | |
| return; | |
| } | |
| // Mark handled whether or not we found the alert; sheet owns missing-alert UX. | |
| deepLinkHandledRef.current = key; | |
| if (!alertData) { | |
| return; | |
| } | |
| const cameraSettings = calculateAlertCamera( | |
| alertData, | |
| alertData.detectedBy, | |
| alertData.confidence, | |
| ); | |
| console.log('[deepLink] Home: zooming to alert', {alertId, cameraSettings}); | |
| if (camera?.current?.setCamera && cameraSettings) { | |
| camera.current.setCamera({ | |
| centerCoordinate: cameraSettings.centerCoordinate, | |
| zoomLevel: cameraSettings.zoomLevel, | |
| animationDuration: 500, | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/nativeapp/app/screens/Home/Home.tsx` around lines 778 - 803, The effect
currently always dispatches dispatch(openAlertDetails({alertId})) but only sets
deepLinkHandledRef.current = key when alertData exists, causing unbounded
re-dispatches; fix by marking the deep link handled immediately after dispatch
(move or add deepLinkHandledRef.current = key right after
dispatch(openAlertDetails({alertId}))) or alternatively implement a small retry
budget using a deepLinkRetryCountRef keyed by alertId and set
deepLinkHandledRef.current = key after N attempts/timeout to stop repeated
dispatches; reference alertId, alerts?.json?.data, dispatch(openAlertDetails),
deepLinkHandledRef.current and key when making the change.
| "sha256_cert_fingerprints": [ | ||
| "REPLACE_WITH_RELEASE_SHA256_FINGERPRINT" | ||
| ] |
There was a problem hiding this comment.
Placeholder fingerprint will break Android App Links verification on release.
Two things to resolve before this ships:
- Replace
REPLACE_WITH_RELEASE_SHA256_FINGERPRINTwith the actual release signing certificate SHA-256 (colon-separated, uppercase hex). Shipping this file as-is will causeautoVerify="true"inAndroidManifest.xmlto fail silently on all devices — HTTPS deep links will continue to open in the browser instead of the app. - If the app is distributed via Google Play App Signing, you typically need both the upload-key fingerprint and the Play-generated app-signing-key fingerprint in the
sha256_cert_fingerprintsarray. Otherwise users installing from Play will get broken app links even though local release installs work.
Also make sure this file is served at https://firealert.plant-for-the-planet.org/.well-known/assetlinks.json with Content-Type: application/json and no redirects — Android's verifier is strict.
📘 Suggested shape (once you have the real fingerprints)
"sha256_cert_fingerprints": [
- "REPLACE_WITH_RELEASE_SHA256_FINGERPRINT"
+ "AA:BB:CC:...:upload-key-sha256...",
+ "11:22:33:...:play-signing-key-sha256..."
]Want me to draft a short verification checklist (hosting, content-type, adb shell pm verify-app-links) for the release runbook?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/nativeapp/deeplinking/assetlinks.json` around lines 7 - 9, The
assetlinks.json currently contains a placeholder fingerprint string; replace the
"REPLACE_WITH_RELEASE_SHA256_FINGERPRINT" entry inside the
sha256_cert_fingerprints array in assetlinks.json with the actual release
signing certificate SHA-256 fingerprint (colon-separated, uppercase hex), and if
you use Google Play App Signing add both the upload-key and Play app-signing-key
fingerprints as separate entries in that same sha256_cert_fingerprints array;
after updating, ensure the file is served at
https://firealert.plant-for-the-planet.org/.well-known/assetlinks.json with
Content-Type: application/json and no redirects so Android's verifier (used by
AndroidManifest autoVerify) can validate app links.
- Update Xcode project format to object version 70 - Add required shared app group entitlement for iOS - Configure correct headers for Apple App Site Association endpoint - Move AASA file to server public .well-known directory - Clean up iOS project Cocoapods build phase entries Enables reliable deep linking behaviour when opening push notifications.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/server/next.config.mjs (1)
41-51: Consider a longer Cache-Control max-age for AASA.
max-age=300is fairly short for an Apple App Site Association file — Apple'sswcdaggressively caches AASA contents (often for ~24h regardless), and a 5-minute origin TTL mostly affects intermediaries/CDN re-fetches, not iOS client behavior. Once your Team ID and paths are stabilized, a longer TTL (e.g.,max-age=3600or higher) reduces unnecessary origin hits. Keep the short value if you're still iterating on the file.Headers themselves are correct:
application/json(no charset) is what Apple expects, and Next.js custom headers do apply to files served frompublic/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/next.config.mjs` around lines 41 - 51, Update the Cache-Control TTL for the AASA header in the async headers() config: replace the current max-age=300 value for the source '/.well-known/apple-app-site-association' with a longer duration (e.g., max-age=3600 or higher) to reduce origin/CDN fetches once Team ID/paths are stable; keep the shorter value only while actively iterating on the AASA file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/nativeapp/ios/FireAlert.xcodeproj/project.pbxproj`:
- Line 6: The Gemfile currently pins xcodeproj to an incompatible older version;
update the gem constraint for xcodeproj (the entry referencing gem 'xcodeproj')
to require at least 1.26.0 (for objectVersion = 70) — e.g., change it to ">=
1.26.0" and cap to "< 2.0" (or match Gemfile.lock 1.27.0) so bundle/pod install
succeeds, then run bundle install and update Gemfile.lock; also update CI images
and contributor docs to note Xcode 16+ is required due to
PBXFileSystemSynchronizedRootGroup support.
In `@apps/server/next.config.mjs`:
- Around line 41-51: The apple-app-site-association blob served by the headers()
route contains the placeholder hostname "REPLACE_TEAM_ID.eco.pp.firealert" which
will break Universal Links once cached by iOS; replace that placeholder with the
actual Team ID-prefixed string for your app (the real "TEAMID.<bundle-id>"
entry) inside the apple-app-site-association file served at source
'/.well-known/apple-app-site-association' so the AASA JSON is correct before
enabling deep links, then redeploy so the new AASA is served with the existing
Content-Type and Cache-Control headers.
---
Nitpick comments:
In `@apps/server/next.config.mjs`:
- Around line 41-51: Update the Cache-Control TTL for the AASA header in the
async headers() config: replace the current max-age=300 value for the source
'/.well-known/apple-app-site-association' with a longer duration (e.g.,
max-age=3600 or higher) to reduce origin/CDN fetches once Team ID/paths are
stable; keep the shorter value only while actively iterating on the AASA file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a313e3bb-6121-4a67-8b54-751cc8f9e170
📒 Files selected for processing (5)
apps/nativeapp/ios/FireAlert.xcodeproj/project.pbxprojapps/nativeapp/ios/FireAlert/FireAlert.entitlementsapps/server/next.config.mjsapps/server/public/.well-known/apple-app-site-associationapps/server/public/.well-known/assetlinks.json
✅ Files skipped from review due to trivial changes (2)
- apps/server/public/.well-known/apple-app-site-association
- apps/nativeapp/ios/FireAlert/FireAlert.entitlements
| classes = { | ||
| }; | ||
| objectVersion = 55; | ||
| objectVersion = 70; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for pinned Xcode versions in CI / tooling configs that may need updating.
fd -HI -t f -e yml -e yaml -e rb -e json -e toml | xargs rg -nP -i 'xcode[_-]?version|XCODE_VERSION|xcode\s*:\s*[\"'\'']?\d|developer_dir|DEVELOPER_DIR' 2>/dev/null
fd -HI 'Gemfile|fastlane|Fastfile|Appfile' -t f | xargs rg -nP -C2 'xcode' 2>/dev/nullRepository: Plant-for-the-Planet-org/FireAlert
Length of output: 2114
🏁 Script executed:
#!/bin/bash
# Search for CI config files that might pin Xcode versions
fd -HI -t f '.github' | head -20
fd -HI -t f -name '*.yml' -o -name '*.yaml' | grep -i -E '(github|gitlab|circleci|bitrise|fastlane)' | head -20
fd -HI -t f '.github/workflows' 2>/dev/null | head -20
find . -maxdepth 3 -type f \( -name '.github' -o -name 'workflows' \) 2>/dev/null
git ls-files '.github/workflows/*.yml' '.github/workflows/*.yaml' 2>/dev/null | head -20Repository: Plant-for-the-Planet-org/FireAlert
Length of output: 405
🏁 Script executed:
#!/bin/bash
# Get more details about the iOS Gemfile and Fastfile
cat -n apps/nativeapp/ios/Gemfile 2>/dev/null || echo "File not found"
fd -HI 'Fastfile|fastlane' -t f | head -10Repository: Plant-for-the-Planet-org/FireAlert
Length of output: 240
🌐 Web query:
xcodeproj gem version 1.25.1 objectVersion 70 Xcode 16 compatibility
💡 Result:
The xcodeproj gem version 1.25.1 does not fully support Xcode 16 projects with objectVersion 70. ObjectVersion 70 corresponds to Xcode 16.0 (as per current master branch constants.rb), but this mapping was added in a later version after 1.25.1 (released October 3, 2024). In 1.25.1, projects with objectVersion 70 trigger an ArgumentError: "[Xcodeproj] Unable to find compatibility version string for object version 70". This commonly occurs when using Xcode 16 features like "New Folder" (blue icon folders, PBXFileSystemSynchronizedRootGroup) instead of groups, which bumps the objectVersion to 70 without full project format upgrade to 77 (Xcode 16.0 compatible). Workarounds include: convert folders to groups in Xcode, or manually edit project.pbxproj to set objectVersion to 60 (Xcode 15.0) or change project format to Xcode 16. Full support requires updating to xcodeproj >=1.26.0 or later (e.g., 1.27.0).
Citations:
- 1: Feature: Xcode 16.0 support CocoaPods/Xcodeproj#942
- 2: ArgumentError - [Xcodeproj] Unable to find compatibility version string for object version
70. CocoaPods/Xcodeproj#996 - 3: https://github.com/CocoaPods/Xcodeproj/releases/tag/1.25.0
- 4: https://stackoverflow.com/questions/79203974/cocoapods-error-unable-to-find-compatibility-version-string-for-object-version
- 5: Cocoapods not working in Xcode 16 macOS 15.0.1 CocoaPods/CocoaPods#12625
- 6: https://rubygems.org/gems/xcodeproj/versions/1.25.1
- 7: CocoaPods/Xcodeproj@8dedfc8
- 8: https://github.com/CocoaPods/Xcodeproj/blob/master/lib/xcodeproj/constants.rb
🏁 Script executed:
#!/bin/bash
# Check CI workflows for Xcode version
cat apps/nativeapp/.github/workflows/codeql-analysis.yml 2>/dev/null || echo "Not found in nativeapp"
cat .github/workflows/codeql-analysis.yml 2>/dev/null || echo "Not found in root"Repository: Plant-for-the-Planet-org/FireAlert
Length of output: 2507
🏁 Script executed:
#!/bin/bash
# Check the current state of Gemfile.lock for xcodeproj version
grep -A2 "xcodeproj" apps/nativeapp/ios/Gemfile.lock | head -10Repository: Plant-for-the-Planet-org/FireAlert
Length of output: 289
🏁 Script executed:
#!/bin/bash
# Check if Gemfile was updated
cat apps/nativeapp/GemfileRepository: Plant-for-the-Planet-org/FireAlert
Length of output: 460
Update xcodeproj gem constraint to support objectVersion 70.
objectVersion = 70 requires xcodeproj >= 1.26.0 to avoid ArgumentError: Unable to find compatibility version string for object version 70. However, apps/nativeapp/Gemfile pins xcodeproj to '< 1.26.0', which is incompatible.
Update the Gemfile constraint from gem 'xcodeproj', '< 1.26.0' to gem 'xcodeproj', '>= 1.26.0', '< 2.0' (or equivalent to match Gemfile.lock's 1.27.0) before this PR lands. Without this, pod install will fail immediately for all developers.
Additionally, this change requires Xcode 16+ due to PBXFileSystemSynchronizedRootGroup support. Ensure CI images and contributor docs are updated accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/nativeapp/ios/FireAlert.xcodeproj/project.pbxproj` at line 6, The
Gemfile currently pins xcodeproj to an incompatible older version; update the
gem constraint for xcodeproj (the entry referencing gem 'xcodeproj') to require
at least 1.26.0 (for objectVersion = 70) — e.g., change it to ">= 1.26.0" and
cap to "< 2.0" (or match Gemfile.lock 1.27.0) so bundle/pod install succeeds,
then run bundle install and update Gemfile.lock; also update CI images and
contributor docs to note Xcode 16+ is required due to
PBXFileSystemSynchronizedRootGroup support.
2 participants
Critical bug fixes (server)
AlertMethodMethodimported as type-only — the enum was imported withimport {type AlertMethodMethod}, which TypeScript erases at compile time. Every runtime check likealertMethod === AlertMethodMethod.emailthrewReferenceErrorand was silently caught, skipping every device notification. Changed to value import.Promise.alliterated unfiltered notifications — after filtering out disabled AlertMethods intofilteredNotifications, the sender still iterated the originalnotificationsarray, delivering pushes to disabled methods. Fixed to iteratefilteredNotifications.debuglevel routed to Logtail as INFO — debug entries were polluting Logtail. Now gated onNODE_ENV === 'development'and emitted only to the console.Deep-link completion (client + server)
firealert://alert/${alertId}instead of the HTTPS URL, so the OS opens the app via the registered scheme. Universal Links are not yet configured (placeholder Team ID / SHA256 still indeeplinking/), so this is the reliable path.OneSignal_suppress_launch_urls = truetoios/FireAlert/Info.plist, paired with the existingcom.onesignal.suppressLaunchURLsinAndroidManifest.xml. Prevents OneSignal from auto-opening the URL before the app's click handler runs.useEffectnow locates the alert in the fetched data and callscamera.setCamerausingcalculateAlertCamera(same helper used byhandleAlertTapFromIncident). The default user-location zoom is skipped whenalertId/incidentIdis present in route params.__tsnonce to deep-link params inlinkingConfig.homeStateso every tap produces a unique params object, forcing the useEffect to re-fire. Dedup key uses${target}:${__ts}so the effect runs once per tap.Debug instrumentation
Debug logs added across the notification pipeline (
CreateNotifications,SendNotifications,DeviceNotifier) and the client deep-link path (OneSignalContext,Home,handleNotificationOpen) for local diagnosis. Server debug logs are dev-only via the logger guard; client[deepLink]logs are plainconsole.logand can be stripped before release.Verification
/api/cron/notification-sender— notifications deliver end-to-end to the device; OneSignal returnsstatus=200.__tseach call).Follow-ups
__DEV__-guard the client[deepLink]console.logs before shipping.console.error/console.warncalls added for diagnostic visibility — the logger's dev fallback already prints[ERROR]/[WARN]to console.apps/server/src/Services/Notifications/*has no test harness yet. Worth considering for the regression-prone fixes above.@
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation