Skip to content

feat(push): hold a notification tap that arrives before setup() - #792

Merged
turnipdabeets merged 7 commits into
mainfrom
fix/push-open-prewarm
Sep 9, 2026
Merged

feat(push): hold a notification tap that arrives before setup()#792
turnipdabeets merged 7 commits into
mainfrom
fix/push-open-prewarm

Conversation

@turnipdabeets

@turnipdabeets turnipdabeets commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

🔗 Related PRs

One fix, five PRs — three SDKs plus the docs. Each Flutter PR is gated on the native release it depends on.

PR What it does
#792 native iOS — hold a tap delivered before setup() — ← this PR
PostHog/posthog-android#753 native Android — read a launch intent the SDK installed too late to see
PostHog/posthog-flutter#556 Flutter iOS cold start · needs posthog-ios 3.72.0 · fixes PostHog/posthog-flutter#555
PostHog/posthog-flutter#557 Flutter Android cold + warm start · needs posthog-android 3.62.0 · fixes PostHog/posthog-flutter#558
PostHog/posthog.com#19905 docs for all of the above

Order: #792 and PostHog/posthog-android#753 merge and release first → PostHog/posthog-flutter#556 and PostHog/posthog-flutter#557 leave draft and go green on their own once the floors publish → PostHog/posthog.com#19905 last, since posthog.com deploys on merge.

💡 Motivation and Context

Fixes the native half of PostHog/posthog-flutter#555$push_notification_opened is never captured in a stock Flutter app on iOS. Two independent causes, both reproduced on a simulator:

  1. Nobody is listening. A stock Flutter app sets no UNUserNotificationCenter delegate, so iOS reports the tap to nobody and our swizzling has nothing to attach to. The integration logs installed and then stays silent forever, which is near-impossible to diagnose from outside.
  2. We start too late. On a cold launch from a tap, didReceive arrives ~150 ms in — about 90 ms before a Dart/JS host can reach setup(). The swizzles aren't in place yet.

Native iOS apps that call setup() from didFinishLaunchingWithOptions are unaffected by (2) and need none of this.

💚 How did you test it?

  • make test — 907 tests pass.
  • make testOniOSSimulator** TEST SUCCEEDED **, 190 cases, 0 failures.
  • 9 new unit tests covering the buffer, prewarm idempotency, prewarm-with-a-live-subscriber, and the setup() discard gate. The opted-out one is a real regression test — reverting the fix makes it fail.
  • On device (posthog-flutter example app, iPhone 17 Pro sim / iOS 26.4, built against this branch): 4/4 notification taps captured, one event each — 1 warm start and 3 cold starts, each read out of the /batch payload the SDK sent. Before the fix, cold starts captured 0.

Testing

Beyond the unit suites, verified on an iPhone 17 Pro simulator (iOS 26.4) through two hosts, with each event read out of the SDK's own /batch payload:

posthog-ios's own PostHogExample (setup() in didFinishLaunchingWithOptions, delegate wired) — the configuration this change does not need to alter:

Scenario Result
Cold launch from a tap captured
Tap while the app is running captured

A Flutter host, where setup() runs from Dart ~90ms after the tap is delivered: cold start went from 0 captures to 1, repeatedly, across four rounds.

Also checked: with the app's UNUserNotificationCenter delegate removed, nothing is captured in any app state and the new warning fires; with it present, no warning. capturePushNotificationOpened: false installs no integration and captures nothing.

Re-verified 2026-09-08, after rebasing on main (posthog-ios is still 3.71.6, so this minor lands on 3.72.0 and the Flutter floor is unchanged): 907 swift-testing cases and 190 XCTest cases green, and the cold/warm matrix re-run on an iPhone 17 simulator through both hosts — one event each, four for four.

Method note: the example apps point at a local HTTP server standing in for the ingestion host, so every assertion above is made against the exact /batch body the SDK sent, not a dashboard query.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm changeset to generate a changeset file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Built with Claude Code (session), driven by @turnipdabeets. Investigation started from posthog-flutter#555 and reproduced both causes on a simulator before any code was written.

Three review/triage rounds ran over the branch, and each caught something material:

  • Buffering was initially unconditional, so after setup()close() a tap was retained in the process-wide publisher and replayed into the next setup() within 30s — a silent behaviour change for all posthog-ios users, with a consent dimension. Now the buffer only lives inside the prewarm window.
  • The discard was first placed at the end of installIntegrations(), which setup() only calls when not opted out — so it never ran for anyone with a persisted opt-out. Moved into setup() with the gate widened.
  • The branch did not compile in the test-ios-simulator lane (a new test called the iOS-14-only API unguarded). swift test on macOS hides this because Swift raises the arm64 floor to 11.0.

Two alternatives were considered and rejected: a hasEverHadSubscriber latch to close the window permanently (it would silently kill cold-start capture forever for a Flutter add-to-app host that prewarms after close()), and replaying launchOptions[.remoteNotification] (verified on iOS 26 — that key is not populated for a user tap).

Not done here, deliberately: docs. The posthog_flutter side is PostHog/posthog-flutter#556 and needs a release of this one first.

🤖 Generated with Claude Code

https://claude.ai/code/session_012txiHBCZRkShMdE7V25Jrd

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

posthog-ios Compliance Report

Date: 2026-09-09 01:19:40 UTC
Duration: 223659ms

✅ All Tests Passed!

45/45 tests passed


Capture Tests

29/29 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields 3446ms
Format Validation.Event Has Uuid 2892ms
Format Validation.Event Has Lib Properties 2954ms
Format Validation.Distinct Id Is String 2945ms
Format Validation.Token Is Present 2854ms
Format Validation.Custom Properties Preserved 2899ms
Format Validation.Event Has Timestamp 2845ms
Retry Behavior.Retries On 503 11676ms
Retry Behavior.Does Not Retry On 400 4805ms
Retry Behavior.Does Not Retry On 401 4830ms
Retry Behavior.Respects Retry After Header 5583ms
Retry Behavior.Implements Backoff 21828ms
Retry Behavior.Retries On 500 5321ms
Retry Behavior.Retries On 502 9207ms
Retry Behavior.Retries On 504 9395ms
Retry Behavior.Max Retries Respected 21745ms
Deduplication.Generates Unique Uuids 2881ms
Deduplication.Preserves Uuid On Retry 5355ms
Deduplication.Preserves Uuid And Timestamp On Retry 16285ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 7841ms
Deduplication.No Duplicate Events In Batch 2864ms
Deduplication.Different Events Have Different Uuids 2881ms
Compression.Sends Gzip When Enabled 2831ms
Batch Format.Uses Proper Batch Structure 2765ms
Batch Format.Flush With No Events Sends Nothing 303ms
Batch Format.Multiple Events Batched Together 2921ms
Error Handling.Does Not Retry On 403 4774ms
Error Handling.Does Not Retry On 413 4808ms
Error Handling.Retries On 408 5407ms

Feature_Flags Tests

16/16 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 2822ms
Request Payload.Flags Request Uses V2 Query Param 2813ms
Request Payload.Flags Request Hits Flags Path Not Decide 2852ms
Request Payload.Flags Request Omits Authorization Header 2835ms
Request Payload.Token In Flags Body Matches Init 2867ms
Request Payload.Groups Round Trip 2764ms
Request Payload.Groups Default To Empty Object 2823ms
Request Payload.Person Properties Distinct Id Auto Populated When Caller Omits It 2713ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False 2807ms
Request Payload.Disable Geoip Omitted Defaults To False 2790ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key 2767ms
Request Lifecycle.No Flags Request On Init Alone 75ms
Request Lifecycle.No Flags Request On Normal Capture 2657ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests 5724ms
Request Lifecycle.Mock Response Value Is Returned To Caller 2860ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event 2961ms

turnipdabeets and others added 3 commits September 7, 2026 20:07
A cold launch from a notification tap delivers the response ~150ms in,
before a Flutter or React Native host can reach setup() from its own
runtime, so the swizzles were not yet installed and the open was lost.

Adds prewarmPushNotificationOpenCapture() to install the notification
delegate swizzles early and hold one response until the integration
subscribes, and warns when no UNUserNotificationCenter delegate exists
at all — the case where taps reach nobody and nothing is capturable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012txiHBCZRkShMdE7V25Jrd
hedgeLogEnabled defaults to false, so the warning only prints when
config.debug is on — a release build sees nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012txiHBCZRkShMdE7V25Jrd
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012txiHBCZRkShMdE7V25Jrd
…requisite

capturePushNotificationOpened's doc never mentioned that iOS reports a tap
only through UNUserNotificationCenter.current().delegate — the one surface a
native developer reads, and the last one still silent about it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012txiHBCZRkShMdE7V25Jrd
@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
### Issue 1
PostHog/PushNotifications/PushNotificationPublisher.swift:139-145
**Prewarm race retains notifications**

The public APIs in this SDK must be thread-safe, but the subscriber-count check and the `isPrewarmed` update happen separately. If the first subscription is added between them, its callback clears `isPrewarmed` before this code sets it back to `true`. Closing that SDK then removes the subscriber without clearing the reopened flag, so a later notification can be buffered without a new prewarm and captured by another `setup()` within 30 seconds.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "docs(push): cut comments that restate th..." | Re-trigger Greptile

Comment thread PostHog/PushNotifications/PushNotificationPublisher.swift
The subscriber-count read in prewarmNotificationResponseCapture() sits
outside stateLock — it has to, since the count is behind the multicast's
own lock and nesting them the other way inverts the ordering the
subscriber-count callback already uses. A subscribe landing between that
read and the isPrewarmed write therefore left the flag set with a live
subscriber, and nothing cleared it when that subscriber went away, so a
later response could be buffered with no live prewarm and replayed into
the next setup() inside the TTL.

Clearing the flag alongside the un-swizzle converges both racers on "not
prewarmed" without nesting the locks.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012txiHBCZRkShMdE7V25Jrd

@dustinbyrne dustinbyrne left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor finding in PushNotificationPublisher.swift:196–197: the zero-subscriber check and delegate-setter teardown aren’t coordinated with a new subscription.

With multiple SDK instances, A’s setup with push capture disabled could observe zero subscribers, B could then finish optIn() and install interception, and A could subsequently remove it. If the host then assigns a previously unwrapped notification delegate, B would miss automatic push-open captures.

This requires a narrow cross-instance interleaving and doesn’t affect normal single-instance setup. I haven’t reproduced it at runtime. Worth coordinating subscriber ownership with the interception transition and adding a deterministic regression test.

discardPrewarmedNotificationResponseCapture() reads the subscriber count
and clears the prewarm flag, then tears the swizzles down — but
prewarmPushNotificationOpenCapture() is a public static that does not run
under setupLock, so it can set the flag and install between those steps
and have the teardown undo it. That leaves a live prewarm with an
un-swizzled delegate setter, so a delegate assigned afterwards is never
wrapped.

Re-checking both signals after the teardown converges on "installed"
whichever way the two interleave.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012txiHBCZRkShMdE7V25Jrd
@turnipdabeets

turnipdabeets commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@dustinbyrne

  • The two-instance interleaving isn't reachable. PostHogSDK.init is private and shared is the only exposure, and the single subscribe site plus the discard both run inside setupLock, so A-discards-while-B-optIns can't happen.
  • Device-checked the reachable version on an iPhone 17 sim: setup with capture disabled (discard un-swizzles the setter) → assign a brand-new delegate class while torn downclose() + setup with capture enabled → push → tap. Captured. installNotificationDelegateSwizzles() re-wraps whatever delegate is currently assigned when it re-arms, so the teardown recovers.
  • Your structural point holds on a different path, though: prewarmPushNotificationOpenCapture() is a public static that doesn't take setupLock, so it can race the discard and have its install undone — a live prewarm with an un-swizzled setter, no second instance needed. Fixed in efde953 by re-checking both signals after the teardown so they converge on "installed". Test in af022f6

Both guards exist for races whose only route into the bad state is the
race itself, so neither had a regression test. Adds a TESTING-gated hook
at the two points where a concurrent caller can overtake — after the
subscriber-count read in prewarm and in discard — plus counters for the
swizzle transitions, which stay observable where the swizzle state does
not (a test runner is never an app, so the bundle guard no-ops it).

Each test fails only with its own fix reverted: the prewarm one buffers a
response that should have been dropped, the discard one stops at one
install where it should re-arm.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012txiHBCZRkShMdE7V25Jrd
@turnipdabeets
turnipdabeets merged commit 8566372 into main Sep 9, 2026
47 checks passed
@turnipdabeets
turnipdabeets deleted the fix/push-open-prewarm branch September 9, 2026 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants