Real-time mobile push delivery (high priority + time-sensitive)#92
Conversation
…ive) Set priority:high (APNs priority 10, bypasses power-considerate batching), sound:default, and interruptionLevel:time-sensitive on the Expo payload so agent alerts arrive immediately instead of in ~10-minute coalesced bursts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Action performedReview finished.
|
|
Warning Review limit reached
More reviews will be available in 9 minutes and 59 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request adds three Expo push notification configuration parameters ( ChangesExpo Push Notification Enhancement
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
relay/src/security-delivery.ts (2)
109-128: Consider high-priority delivery for security push notifications.The
sendSecurityPushfunction does not include the newpriority,sound, andinterruptionLevelfields. While agent notifications benefit from immediate delivery (as addressed in this PR), security alerts (login attempts, device changes) may also warrant high-priority, time-sensitive delivery to ensure users are promptly notified of potential security events.🤖 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 `@relay/src/security-delivery.ts` around lines 109 - 128, sendSecurityPush currently crafts Expo message objects without priority/sound/interruptionLevel, so update the message construction in sendSecurityPush to include high-priority delivery fields (e.g., priority: "high", sound: "default" or a defined alert sound, and interruptionLevel: "time-sensitive") inside each mapped message's top-level properties and/or data payload as required by sendExpoPush/Expo format; modify the mapping in sendSecurityPush (the .map callback creating to/title/body/data) to add these fields so sendExpoPush receives messages marked for immediate/time-sensitive delivery.
47-49: ⚡ Quick winConsider typing the Expo message interface.
The message objects are currently typed as
unknown[](Line 62), which bypasses compile-time validation of field names and value types. Consider defining a typed interface for Expo push messages to catch typos and ensure required fields are present.📝 Example typed interface
interface ExpoPushMessage { to: string; title: string; body: string; sound?: string; priority?: "default" | "normal" | "high"; interruptionLevel?: "passive" | "active" | "time-sensitive" | "critical"; data?: Record<string, unknown>; }Then update the function signature:
-async function sendExpoPush(messages: unknown[]): Promise<void> { +async function sendExpoPush(messages: ExpoPushMessage[]): Promise<void> {🤖 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 `@relay/src/security-delivery.ts` around lines 47 - 49, Define a proper ExpoPushMessage interface (fields: to, title, body, optional sound, priority, interruptionLevel, data) and replace the loose unknown[] type used for messages (e.g., the messages parameter or variable named messages in sendPushNotifications/sendExpoNotifications) with ExpoPushMessage[]; update any construction sites to match the interface and fix any typos/missing required fields (e.g., ensure sound, priority, interruptionLevel values match the allowed union types) so the compiler enforces correct shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@relay/src/security-delivery.ts`:
- Around line 47-49: The notification payload uses interruptionLevel:
"time-sensitive" (kebab-case) and unguarded iOS-only fields; change
interruptionLevel to the Expo-expected camelCase "timeSensitive", and ensure
iOS-only fields (sound and interruptionLevel) are only applied on iOS (e.g.,
gate them with a platform/OS check or place them under an ios-specific
sub-object used for iOS notifications) while keeping priority: "high"
cross-platform; update the corresponding tests in security-delivery.test.ts to
expect "timeSensitive" and the platform gating behavior for
sound/interruptionLevel.
---
Nitpick comments:
In `@relay/src/security-delivery.ts`:
- Around line 109-128: sendSecurityPush currently crafts Expo message objects
without priority/sound/interruptionLevel, so update the message construction in
sendSecurityPush to include high-priority delivery fields (e.g., priority:
"high", sound: "default" or a defined alert sound, and interruptionLevel:
"time-sensitive") inside each mapped message's top-level properties and/or data
payload as required by sendExpoPush/Expo format; modify the mapping in
sendSecurityPush (the .map callback creating to/title/body/data) to add these
fields so sendExpoPush receives messages marked for immediate/time-sensitive
delivery.
- Around line 47-49: Define a proper ExpoPushMessage interface (fields: to,
title, body, optional sound, priority, interruptionLevel, data) and replace the
loose unknown[] type used for messages (e.g., the messages parameter or variable
named messages in sendPushNotifications/sendExpoNotifications) with
ExpoPushMessage[]; update any construction sites to match the interface and fix
any typos/missing required fields (e.g., ensure sound, priority,
interruptionLevel values match the allowed union types) so the compiler enforces
correct shape.
🪄 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: f33593f2-58f6-4549-ad98-65bdc2a5b882
📒 Files selected for processing (2)
relay/src/security-delivery.test.tsrelay/src/security-delivery.ts
Expo expects interruptionLevel "timeSensitive" (kebab-case is silently ignored). Gate sound + interruptionLevel to iOS tokens; priority stays cross-platform. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
Problem
Agent push notifications arrived in ~10-minute coalesced bursts instead of in real time. The Expo payload set no
priority, so APNs treated them as priority 5 ("power-considerate") and held them for the next radio wake.Fix
deliverNotificationPushnow sets on each Expo message:priority: "high"— APNs priority 10, immediate delivery, bypassing batching.sound: "default"— audible, less deferrable.interruptionLevel: "time-sensitive"— breaks through Focus/summary (the device's "Time Sensitive Notifications" toggle gates this; entitlement already present).Relay-only change — no native build needed; ships on the next
wrangler deploy.Tests
security-delivery.test.tsasserts the new fields on the outgoing payload. Suite green.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests