feat: ✨ ios notifications#554
Open
ethancha0 wants to merge 8 commits into
Open
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
3 issues found across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
9 issues found across 17 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/db/migrations/0019_native_push_tokens.sql">
<violation number="1" location="src/db/migrations/0019_native_push_tokens.sql:1">
P3: This migration duplicates the same `native_push_tokens` DDL already present in `0020_nice_synch.sql`, which makes the migration history harder to reason about and easy to drift.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| @@ -0,0 +1,18 @@ | |||
| CREATE TABLE IF NOT EXISTS "native_push_tokens" ( | |||
Contributor
There was a problem hiding this comment.
P3: This migration duplicates the same native_push_tokens DDL already present in 0020_nice_synch.sql, which makes the migration history harder to reason about and easy to drift.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/db/migrations/0019_native_push_tokens.sql, line 1:
<comment>This migration duplicates the same `native_push_tokens` DDL already present in `0020_nice_synch.sql`, which makes the migration history harder to reason about and easy to drift.</comment>
<file context>
@@ -0,0 +1,18 @@
+CREATE TABLE IF NOT EXISTS "native_push_tokens" (
+ "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL,
+ "user_id" text NOT NULL,
</file context>
Contributor
There was a problem hiding this comment.
1 issue found across 17 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/db/migrations/0019_native_push_tokens.sql">
<violation number="1" location="src/db/migrations/0019_native_push_tokens.sql:1">
P3: This migration duplicates the same `native_push_tokens` DDL already present in `0020_nice_synch.sql`, which makes the migration history harder to reason about and easy to drift.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
4 issues found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/db/migrations/0019_native_push_tokens.sql">
<violation number="1" location="src/db/migrations/0019_native_push_tokens.sql:1">
P3: This migration duplicates the same `native_push_tokens` DDL already present in `0020_nice_synch.sql`, which makes the migration history harder to reason about and easy to drift.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
5 issues found across 16 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/app/api/push-tokens/route.ts">
<violation number="1" location="src/app/api/push-tokens/route.ts:108">
P1: The DELETE route performs an external SNS endpoint deletion before verifying token ownership, allowing authenticated users to delete endpoints they do not own if they know an ARN.</violation>
</file>
<file name="sst.config.ts">
<violation number="1" location="sst.config.ts:39">
P1: Scope SNS IAM resources more narrowly than `*` to avoid granting account-wide publish/endpoint management permissions.</violation>
</file>
<file name="src/lib/push/sns-register.ts">
<violation number="1" location="src/lib/push/sns-register.ts:28">
P1: `CreatePlatformEndpoint` errors are treated as fatal, but SNS can throw `InvalidParameter` for an existing token with different attributes; handle that case by recovering the existing endpoint ARN instead of returning `null`.</violation>
</file>
<file name="src/lib/push/send-push.ts">
<violation number="1" location="src/lib/push/send-push.ts:65">
P1: `InvalidParameterException` is treated as a stale endpoint unconditionally, which can cause valid push endpoints to be deleted on transient/message-format errors.</violation>
</file>
<file name="ios/src/ZotMeet/AppDelegate.swift">
<violation number="1" location="ios/src/ZotMeet/AppDelegate.swift:39">
P3: Use the existing `apnsTokenHexString(from:)` helper instead of duplicating APNs token formatting logic.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| return NextResponse.json({ error: "Invalid token" }, { status: 400 }); | ||
| } | ||
|
|
||
| if (isSnsEndpointArn(token)) { |
Contributor
There was a problem hiding this comment.
P1: The DELETE route performs an external SNS endpoint deletion before verifying token ownership, allowing authenticated users to delete endpoints they do not own if they know an ARN.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/app/api/push-tokens/route.ts, line 108:
<comment>The DELETE route performs an external SNS endpoint deletion before verifying token ownership, allowing authenticated users to delete endpoints they do not own if they know an ARN.</comment>
<file context>
@@ -88,17 +100,21 @@ export async function DELETE(request: Request) {
+ return NextResponse.json({ error: "Invalid token" }, { status: 400 });
+ }
+
+ if (isSnsEndpointArn(token)) {
+ await deleteIosPushEndpoint(token);
}
</file context>
| "sns:GetEndpointAttributes", | ||
| "sns:DeleteEndpoint", | ||
| ], | ||
| resources: ["*"], |
Contributor
There was a problem hiding this comment.
P1: Scope SNS IAM resources more narrowly than * to avoid granting account-wide publish/endpoint management permissions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At sst.config.ts, line 39:
<comment>Scope SNS IAM resources more narrowly than `*` to avoid granting account-wide publish/endpoint management permissions.</comment>
<file context>
@@ -27,12 +27,28 @@ export default $config({
+ "sns:GetEndpointAttributes",
+ "sns:DeleteEndpoint",
+ ],
+ resources: ["*"],
+ },
+ ],
</file context>
| ); | ||
| warnedMissingPlatformArn = true; | ||
| } | ||
| return null; |
Contributor
There was a problem hiding this comment.
P1: CreatePlatformEndpoint errors are treated as fatal, but SNS can throw InvalidParameter for an existing token with different attributes; handle that case by recovering the existing endpoint ARN instead of returning null.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/push/sns-register.ts, line 28:
<comment>`CreatePlatformEndpoint` errors are treated as fatal, but SNS can throw `InvalidParameter` for an existing token with different attributes; handle that case by recovering the existing endpoint ARN instead of returning `null`.</comment>
<file context>
@@ -0,0 +1,82 @@
+ );
+ warnedMissingPlatformArn = true;
+ }
+ return null;
+ }
+
</file context>
| const message = "message" in error ? String(error.message) : ""; | ||
| return ( | ||
| name === "EndpointDisabledException" || | ||
| name === "InvalidParameterException" || |
Contributor
There was a problem hiding this comment.
P1: InvalidParameterException is treated as a stale endpoint unconditionally, which can cause valid push endpoints to be deleted on transient/message-format errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/push/send-push.ts, line 65:
<comment>`InvalidParameterException` is treated as a stale endpoint unconditionally, which can cause valid push endpoints to be deleted on transient/message-format errors.</comment>
<file context>
@@ -88,18 +30,47 @@ function chunkArray<T>(items: T[], size: number) {
- code === "messaging/invalid-registration-token" ||
- code === "messaging/registration-token-not-registered"
+ name === "EndpointDisabledException" ||
+ name === "InvalidParameterException" ||
+ message.includes("Endpoint is disabled") ||
+ message.includes("Invalid parameter: TargetArn")
</file context>
| // // Messaging.messaging().apnsToken = deviceToken | ||
| // } | ||
| func application(_ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data) { | ||
| let token = deviceToken.map { String(format: "%02.2hhx", $0) }.joined() |
Contributor
There was a problem hiding this comment.
P3: Use the existing apnsTokenHexString(from:) helper instead of duplicating APNs token formatting logic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ios/src/ZotMeet/AppDelegate.swift, line 39:
<comment>Use the existing `apnsTokenHexString(from:)` helper instead of duplicating APNs token formatting logic.</comment>
<file context>
@@ -30,127 +18,45 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
-// // Messaging.messaging().apnsToken = deviceToken
-// }
+ func application(_ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data) {
+ let token = deviceToken.map { String(format: "%02.2hhx", $0) }.joined()
+ setApnsDeviceTokenHex(token)
+ sendApnsTokenToWebView(token: token)
</file context>
Suggested change
| let token = deviceToken.map { String(format: "%02.2hhx", $0) }.joined() | |
| let token = apnsTokenHexString(from: deviceToken) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Hook current notifications to push IOS mobile notifications
Test Plan
This feature is only testable through the mobile ios Testflight, meaning it's not possible to test before merging
Issues