feat(mobile): add remote push notifications - #386
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 754def28a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
754def2 to
f536137
Compare
Greptile SummaryThe PR adds opt-in remote push notifications for the mobile application, including token lifecycle management and notification-tap routing. One startup ordering issue can consume a notification before navigation is ready.
Confidence Score: 4/5This PR should not merge until notification responses are deferred until the root navigation tree is ready. A notification-launched app can hydrate its host registry while mobile configuration is still loading, causing the observer to navigate before RootNavigator mounts and then irreversibly clear the response. Files Needing Attention: apps/mobile/src/components/shell/notification-observer.ts, apps/mobile/src/app/_layout.tsx, apps/mobile/src/components/shell/root-navigator.tsx
|
| Filename | Overview |
|---|---|
| apps/mobile/src/components/shell/notification-observer.ts | Adds global token synchronization and response routing, but consumes startup responses without waiting for navigation readiness. |
| apps/mobile/src/runtime/notification-route.ts | Correctly validates the documented tunnelHostId/sessionId payload and maps tunnel identity to the persisted local host. |
| apps/mobile/src/runtime/cloud/account.ts | Makes push revocation best-effort during sign-out while retaining enrollment when both delivery-disabling paths fail. |
| apps/mobile/src/runtime/notifications.ts | Implements permission, channel, token registration, synchronization, and revocation lifecycle. |
| apps/mobile/src/runtime/notification-token-coordinator.ts | Serializes token registration and revocation and prevents stale in-flight acquisition from registering after disablement. |
| apps/mobile/src/components/settings/settings-screen.tsx | Adds an explicit cloud-only notification opt-in with permission-denial and update-error handling. |
| apps/mobile/src/stores/settings-store.ts | Adds the persisted notificationsEnabled preference consistently to state, schema, and partialization. |
Sequence Diagram
sequenceDiagram
participant OS as Notification response
participant Observer as NotificationObserver
participant Config as Mobile configuration
participant Navigator as RootNavigator
OS->>Observer: App launches from notification
Observer->>Observer: Host registry hydrates
Observer->>Navigator: router.push(destination)
Config-->>Navigator: Configuration not ready
Navigator-->>Observer: Navigation tree not mounted
Observer->>OS: Clear last response
Reviews (3): Last reviewed commit: "fix(mobile): retain enrollment on sign-o..." | Re-trigger Greptile
There was a problem hiding this comment.
Pull request overview
Adds an opt-in remote push notification flow to the mobile app (Expo Notifications), including token lifecycle management against the Cloud device API and notification-tap routing into the appropriate Thread (session) or Connect screen.
Changes:
- Introduces a persisted Notifications toggle and installs root-level notification / token listeners.
- Implements push token coordination (sync + revoke) and Cloud registration/revocation calls.
- Adds routing logic + tests for notification payloads, plus new Settings and error i18n strings (en / zh-CN).
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/presentation/i18n/src/locales/zh-cn.ts | Adds Settings + error copy for Notifications and sign-out failure (zh-CN). |
| packages/presentation/i18n/src/locales/en.ts | Adds Settings + error copy for Notifications and sign-out failure (en). |
| apps/mobile/src/stores/settings-store.ts | Persists notificationsEnabled and exposes a setter for the Settings toggle. |
| apps/mobile/src/runtime/notifications.ts | New notification permission/channel setup and push token sync/revoke wiring. |
| apps/mobile/src/runtime/notification-token-coordinator.ts | New serialized coordinator to avoid races between token acquisition, registration, and revocation. |
| apps/mobile/src/runtime/notification-route.ts | New payload validation + host resolution to route notification taps. |
| apps/mobile/src/runtime/cloud/devices.ts | Adds Cloud endpoints to register/revoke Expo push tokens. |
| apps/mobile/src/runtime/cloud/account.ts | Updates sign-out to disable notifications and optionally revoke token. |
| apps/mobile/src/runtime/tests/notification-token-coordinator.test.ts | Tests coordinator behavior around in-flight acquire/register vs revoke. |
| apps/mobile/src/runtime/tests/notification-route.test.ts | Tests notification payload routing behavior. |
| apps/mobile/src/components/shell/notification-observer.ts | New app-root observer for notification responses + token refresh hooks. |
| apps/mobile/src/components/settings/settings-screen.tsx | Adds Notifications section + opt-in permission flow UI. |
| apps/mobile/src/components/account/devices-section.tsx | Adjusts device-revoke flow to avoid redundant token revocation on sign-out. |
| apps/mobile/src/app/account.tsx | Adds UI error handling for sign-out failures. |
| apps/mobile/src/app/_layout.tsx | Mounts the NotificationObserver at the app root. |
Suppressed comments (1)
apps/mobile/src/stores/settings-store.ts:43
- The persisted settings store key is still
...:v2, but this PR changes the persisted shape by addingnotificationsEnabled. Repo conventions require bumping the:v<N>suffix on any persisted shape change so stale blobs are dropped instead of partially merging unexpected data.
{
name: 'linkcode.mobile.settings:v2',
schema: PersistedSettingsSchema,
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
apps/mobile/src/stores/settings-store.ts:43
- The persisted settings schema gained a new field (
notificationsEnabled) but the storage key remains:v2. Repo convention is to bump the persisted store version on any shape change so stale blobs are dropped and migrations don't happen implicitly (.claude/rules/frontend.md:37).
{
name: 'linkcode.mobile.settings:v2',
schema: PersistedSettingsSchema,
apps/mobile/src/components/shell/notification-observer.ts:35
expo-notificationsexposes async APIs for clearing the last notification response. Calling the non-async variant can be a no-op or undefined at runtime on SDK 57; useclearLastNotificationResponseAsync()instead.
This issue also appears on line 38 of the same file.
} finally {
Notifications.clearLastNotificationResponse();
}
apps/mobile/src/components/shell/notification-observer.ts:42
expo-notificationsreads the last notification response via async APIs on SDK 57 (getLastNotificationResponseAsync). Using a sync getter risks missing the initial tap routing on cold start.
useEffect(() => {
if (!hydrated) return;
const initial = Notifications.getLastNotificationResponse();
if (initial) openNotification(initial);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
apps/mobile/src/stores/settings-store.ts:42
PersistedSettingsSchemaaddsnotificationsEnabled, which changes the persisted settings shape, but the storage key remainslinkcode.mobile.settings:v2. Repo convention requires bumping the:v<N>suffix on any persisted shape change so stale blobs are dropped cleanly during rehydrate.
name: 'linkcode.mobile.settings:v2',
Summary
session-eventschannel before requesting permission or acquiring a token{ tunnelHostId, sessionId }notification data, resolve the persisted local tunnel host, and open the matching Thread or/connectWhy
A phone client needs to notify users when a remote agent completes a turn or requests approval while the app is backgrounded or terminated. The flow is opt-in and Cloud-only so Direct/LAN hosts do not promise delivery they cannot provide.
Linear: CODE-187
Validation
pnpm check:cipnpm test— 303 test files / 2436 tests passedpnpm -F @linkcode/mobile smoke:export— Android and iOS production Router exportsdevenv shell -- mobile— iOS dev build succeeded and launched on an iPhone 17 Pro simulatoraps-environmentRemaining before ready for review
/devices/push-tokenAPI from CODE-531