Remove Mobile Push#592
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@copilot review but do not make fixes |
|
Warning Review limit reached
More reviews will be available in 21 minutes and 32 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR removes the entire Apple Push Notification Service (APNs) and mobile push notification stack across the monorepo. It deletes all shared types, desktop notification services ( ChangesAPNs/Mobile Push Removal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
arul28
force-pushed
the
ade/remove-mobile-push-704e26d6
branch
from
df47401 to
debf872
Compare
|
@codex review |
arul28
force-pushed
the
ade/remove-mobile-push-704e26d6
branch
from
debf872 to
bdcb188
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdcb188ebb
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/components/app/SettingsPage.tsx (1)
33-35: Clean up unusedsettingsSections.tsdefinitions or consolidate with active tab resolver.
settingsSections.tscontains aTAB_ALIASESmapping that conflicts with the one inSettingsPage.tsx(sync/devices/multi-device → "workspace" vs. "general"), but the functionresolveSettingsSectionFromTaband this module are not imported or used anywhere in the renderer. OnlySettingsPage.tsx's local tab resolution logic is active. Remove the unusedTAB_ALIASESandresolveSettingsSectionFromTabfromsettingsSections.ts, or consolidate tab resolution logic to a single canonical location to avoid confusion and future maintenance issues.🤖 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 `@apps/desktop/src/renderer/components/app/SettingsPage.tsx` around lines 33 - 35, Remove the unused TAB_ALIASES mapping and resolveSettingsSectionFromTab function from settingsSections.ts, as they conflict with and are superseded by the active tab resolution logic in SettingsPage.tsx (which maps sync/devices/multi-device to "general"). Since only SettingsPage.tsx's local tab resolution is actively used in the renderer, eliminate the duplicate and conflicting definitions in settingsSections.ts to maintain a single canonical source of truth for tab resolution.
🤖 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 `@apps/ios/ADE/App/ADEApp.swift`:
- Around line 88-90: Deep-link commands are incorrectly being routed through the
PR action path. Add an `openDeeplink` case to the `ADEIntentCommandKind` enum to
provide a dedicated deep-link command type. Then update the switch statement
that maps `ADEIntentCommandKind` cases (around line 88 where `.openPr` is
mapped) to handle the new `openDeeplink` case by mapping it to
`RemoteCommandKind.openDeeplink`. Finally, update the deep-link AppIntent caller
to dispatch the `openDeeplink` case instead of `openPr` when handling deep-link
intents with URL payloads.
---
Nitpick comments:
In `@apps/desktop/src/renderer/components/app/SettingsPage.tsx`:
- Around line 33-35: Remove the unused TAB_ALIASES mapping and
resolveSettingsSectionFromTab function from settingsSections.ts, as they
conflict with and are superseded by the active tab resolution logic in
SettingsPage.tsx (which maps sync/devices/multi-device to "general"). Since only
SettingsPage.tsx's local tab resolution is actively used in the renderer,
eliminate the duplicate and conflicting definitions in settingsSections.ts to
maintain a single canonical source of truth for tab resolution.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8c8d96a2-1723-43c3-98f8-b04e51777165
⛔ Files ignored due to path filters (13)
.ade/ade.yamlis excluded by!.ade/**CHANGELOG.mdis excluded by!*.mdapps/ios/ADE.xcodeproj/project.pbxprojis excluded by!**/*.xcodeproj/project.pbxprojarchitecture.mdxis excluded by!*.mdxchangelog/v1.0.19.mdxis excluded by!changelog/**changelog/v1.1.5.mdxis excluded by!changelog/**docs/ARCHITECTURE.mdis excluded by!docs/**docs/features/onboarding-and-settings/README.mdis excluded by!docs/**docs/features/sync-and-multi-device/README.mdis excluded by!docs/**docs/features/sync-and-multi-device/ios-companion.mdis excluded by!docs/**docs/perf/ios-mobile-action-inventory.mdis excluded by!docs/**index.mdxis excluded by!*.mdxkey-concepts.mdxis excluded by!*.mdx
📒 Files selected for processing (66)
apps/ade-cli/src/bootstrap.tsapps/ade-cli/src/services/sync/deviceRegistryService.tsapps/ade-cli/src/services/sync/syncHostService.tsapps/ade-cli/src/services/sync/syncService.tsapps/desktop/apps/ios/TestFixtures/README.mdapps/desktop/apps/ios/TestFixtures/chat-awaiting-input.jsonapps/desktop/apps/ios/TestFixtures/chat-failed.jsonapps/desktop/apps/ios/TestFixtures/pr-ci-failing.jsonapps/desktop/apps/ios/TestFixtures/pr-merge-ready.jsonapps/desktop/apps/ios/TestFixtures/pr-review-requested.jsonapps/desktop/src/main/main.tsapps/desktop/src/main/services/adeActions/registry.test.tsapps/desktop/src/main/services/adeActions/registry.tsapps/desktop/src/main/services/config/projectConfigService.test.tsapps/desktop/src/main/services/config/projectConfigService.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/notifications/apnsBridgeService.test.tsapps/desktop/src/main/services/notifications/apnsBridgeService.tsapps/desktop/src/main/services/notifications/apnsService.test.tsapps/desktop/src/main/services/notifications/apnsService.tsapps/desktop/src/main/services/notifications/notificationEventBus.test.tsapps/desktop/src/main/services/notifications/notificationEventBus.tsapps/desktop/src/main/services/notifications/notificationMapper.test.tsapps/desktop/src/main/services/notifications/notificationMapper.tsapps/desktop/src/main/services/prs/prPollingService.tsapps/desktop/src/main/services/sync/deviceRegistryService.test.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.test.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/browserMock.tsapps/desktop/src/renderer/components/app/SettingsPage.tsxapps/desktop/src/renderer/components/app/settingsSections.tsapps/desktop/src/renderer/components/settings/MobilePushPanel.tsxapps/desktop/src/renderer/onboarding/tours/settingsHighlightsTour.tsapps/desktop/src/shared/ipc.tsapps/desktop/src/shared/types/config.tsapps/desktop/src/shared/types/sync.tsapps/ios/ADE/ADE.entitlementsapps/ios/ADE/App/ADEApp.swiftapps/ios/ADE/App/AppDelegate.swiftapps/ios/ADE/App/NotificationCategories.swiftapps/ios/ADE/Info.plistapps/ios/ADE/Models/NotificationPreferences.swiftapps/ios/ADE/Services/LiveActivityCoordinator.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADE/Shared/ADESharedContainer.swiftapps/ios/ADE/Shared/ADESharedModels.swiftapps/ios/ADE/Shared/ADESharedTheme.swiftapps/ios/ADE/Shared/LiveActivityIntentsForward.swiftapps/ios/ADE/Shared/WidgetAppIntents.swiftapps/ios/ADE/Views/AttentionDrawer/AttentionDrawerModel.swiftapps/ios/ADE/Views/Settings/ConnectionSettingsView.swiftapps/ios/ADE/Views/Settings/NotificationsCenterView.swiftapps/ios/ADE/Views/Settings/PerSessionOverrideView.swiftapps/ios/ADE/Views/Settings/QuietHoursEditorView.swiftapps/ios/ADE/Views/Settings/SettingsNotificationsSection.swiftapps/ios/ADE/Views/Work/WorkNewChatScreen.swiftapps/ios/ADENotificationService/ADENotificationService.entitlementsapps/ios/ADENotificationService/Info.plistapps/ios/ADENotificationService/NotificationService.swiftapps/ios/ADETests/ADETests.swiftapps/ios/ADEWidgets/ADEControlWidget.swiftapps/ios/ADEWidgets/ADEWidgetBundle.swiftapps/ios/ExportOptions.plistapps/web/src/app/pages/PrivacyPage.tsxplans/tui-parity-roadmap.md
💤 Files with no reviewable changes (48)
- apps/ios/ADE/ADE.entitlements
- apps/desktop/apps/ios/TestFixtures/pr-review-requested.json
- apps/desktop/apps/ios/TestFixtures/README.md
- apps/ios/ADENotificationService/ADENotificationService.entitlements
- apps/ios/ADE/Info.plist
- apps/desktop/apps/ios/TestFixtures/chat-failed.json
- apps/desktop/apps/ios/TestFixtures/chat-awaiting-input.json
- apps/desktop/apps/ios/TestFixtures/pr-merge-ready.json
- apps/ios/ExportOptions.plist
- apps/desktop/src/renderer/components/settings/MobilePushPanel.tsx
- apps/ios/ADENotificationService/Info.plist
- apps/ios/ADEWidgets/ADEWidgetBundle.swift
- apps/ios/ADE/App/NotificationCategories.swift
- apps/desktop/src/main/services/notifications/notificationMapper.test.ts
- apps/ios/ADE/Views/Settings/QuietHoursEditorView.swift
- apps/ios/ADENotificationService/NotificationService.swift
- apps/desktop/src/main/services/notifications/apnsBridgeService.ts
- apps/desktop/src/main/services/notifications/notificationEventBus.test.ts
- apps/desktop/src/main/services/notifications/notificationEventBus.ts
- apps/desktop/apps/ios/TestFixtures/pr-ci-failing.json
- apps/ios/ADE/Models/NotificationPreferences.swift
- apps/ios/ADE/Views/Settings/SettingsNotificationsSection.swift
- apps/desktop/src/main/services/notifications/apnsService.test.ts
- apps/desktop/src/main/services/notifications/apnsBridgeService.test.ts
- apps/ios/ADE/Views/Settings/ConnectionSettingsView.swift
- apps/ios/ADE/Views/Settings/NotificationsCenterView.swift
- apps/desktop/src/renderer/browserMock.ts
- apps/desktop/src/main/services/config/projectConfigService.test.ts
- apps/ios/ADE/Views/Settings/PerSessionOverrideView.swift
- apps/desktop/src/main/services/notifications/notificationMapper.ts
- apps/desktop/src/main/services/prs/prPollingService.ts
- apps/desktop/src/main/services/notifications/apnsService.ts
- apps/ade-cli/src/services/sync/deviceRegistryService.ts
- apps/ios/ADE/App/AppDelegate.swift
- apps/desktop/src/main/services/adeActions/registry.test.ts
- apps/desktop/src/shared/ipc.ts
- apps/desktop/src/preload/global.d.ts
- apps/desktop/src/shared/types/config.ts
- apps/desktop/src/main/services/sync/deviceRegistryService.test.ts
- apps/desktop/src/preload/preload.ts
- apps/ade-cli/src/bootstrap.ts
- apps/ios/ADEWidgets/ADEControlWidget.swift
- apps/desktop/src/main/services/ipc/registerIpc.ts
- apps/ade-cli/src/services/sync/syncService.ts
- apps/desktop/src/main/services/adeActions/registry.ts
- apps/desktop/src/preload/preload.test.ts
- apps/ade-cli/src/services/sync/syncHostService.ts
- apps/desktop/src/shared/types/sync.ts
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4eeb7e0689
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4568478eb
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01f829a2c5
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 817da405d5
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
| @MainActor | ||
| public func perform() async throws -> some IntentResult { | ||
| let trimmedPrId = prId.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| guard prNumber > 0, !trimmedPrId.isEmpty else { return .result() } | ||
| await ADEIntentCommandRegistry.dispatch( | ||
| .openPr, | ||
| payload: ["prNumber": prNumber, "prId": trimmedPrId] | ||
| ) | ||
| return .result() | ||
| } |
There was a problem hiding this comment.
The updated PR intent returns before dispatching unless both prNumber and prId are present. AttentionItem.prId is optional, and the app still supports number-only navigation through PrNavigationRequest(prNumber:) / requestPrNavigationFromIntentPayload, so an attention payload with a GitHub PR number but no internal ADE PR id opens the app without selecting the PR. Dispatch whenever prNumber is valid, and include prId only when it is present.
Artifacts
Repro: focused executable harness for number-only OpenADEPrIntent dispatch
- Contains supporting evidence from the run (text/x-python; charset=utf-8).
Repro: failing harness output showing early return and no dispatch
- Keeps the command output available without making the summary code-heavy.
Repro: generated Swift XCTest for the number-only intent path
- Contains supporting evidence from the run (text/x-swift; charset=utf-8).
Repro: Swift XCTest execution attempt blocked by missing Swift toolchain
- Keeps the command output available without making the summary code-heavy.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/ios/ADEWidgets/ADELiveActivityPrimitives.swift
Line: 509-518
Comment:
**Allow number-only opens**
The updated PR intent returns before dispatching unless both `prNumber` and `prId` are present. `AttentionItem.prId` is optional, and the app still supports number-only navigation through `PrNavigationRequest(prNumber:)` / `requestPrNavigationFromIntentPayload`, so an attention payload with a GitHub PR number but no internal ADE PR id opens the app without selecting the PR. Dispatch whenever `prNumber` is valid, and include `prId` only when it is present.
How can I resolve this? If you propose a fix, please make it concise.
1 participant
Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Summary by CodeRabbit
Greptile Summary
Confidence Score: 3/5
Merge should wait for the Live Activity PR intent navigation regression to be fixed.
The removed push-notification paths are broad but mostly mechanical; the remaining concern is a concrete iOS widget/action flow where valid number-only PR payloads no longer navigate to the selected PR.
apps/ios/ADEWidgets/ADELiveActivityPrimitives.swift
What T-Rex did
Comments Outside Diff (1)
apps/ios/ADE/Shared/LiveActivityIntentsForward.swift, line 511-513 (link)OpenADEDeepLinkIntentsends aurlpayload, but it dispatches.openPr. The bridge maps.openPrtoRemoteCommandKind.openPr, and that path only forwardsprNumbertoprs.getDetail, so Live Activity buttons like “Open agent” can return success while sending an empty PR-detail command instead of opening theade://session/...,ade://pr/..., or workspace URL on the paired Mac. Route these URL intents through the existing.openDeeplink/deeplinks.opencommand path instead.Artifacts
Repro: focused command-routing harness
Repro: failing harness output showing openPr to prs.getDetail routing
Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (4): Last reviewed commit: "ship: iteration 6 - navigate PR intents ..." | Re-trigger Greptile