Skip to content

feat: add push notification support for PostHog Workflows - #667

Merged
ioannisj merged 15 commits into
mainfrom
feat/push-notifications
Jul 31, 2026
Merged

feat: add push notification support for PostHog Workflows#667
ioannisj merged 15 commits into
mainfrom
feat/push-notifications

Conversation

@ioannisj

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

Ships push notification support for PostHog Workflows: device token registration (auto via Firebase Cloud Messaging when firebase-messaging is on the classpath, manual via registerPushNotificationToken(...)), $push_notification_opened capture, identity-verified subscription requests, and durable register/unregister with passive retry.

The pieces of this branch were already reviewed as they landed (#642 push notification support, #656 identity token attachment), but the branch has diverged a bit since: durable unregister intents, passive flush/identify/relaunch-driven retry with a backoff ladder that persists across triggers (no timers), retryable-by-default error classification, and a fix for a race where a queued logout DELETE could cancel a re-registered subscription for the same identity. I think it's worth a fresh full review before release.

💚 How did you test it?

Unit tests for the push subscription manager (47 tests: offline deferral, backoff/halt, identity change, durable unregister, the DELETE/POST supersede), plus spotlessCheck and apiCheck.

📝 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

DRI: @ioannisj
Autonomy: Human-driven (agent-assisted)

ioannisj added 11 commits July 24, 2026 10:52
* feat: add push notification token registration and open capture

* feat(android): auto-register push tokens and capture notification opens

* fix: stop push token registration after opt-out and guard cold-start crash

* feat: unregister push token on reset and add unregisterPushNotificationToken

Claude-Session: https://claude.ai/code/session_013yuqz2SN7ZfLr3kwQN2evU

* refactor: align push notification API with iOS and gate on isOptedOut

Claude-Session: https://claude.ai/code/session_01DLdpQV47JafPqUqGMzcQBk

* fix: halt in-session push subscription retries after a non-retryable failure

* docs: clarify reset moves push token independently of capture flag

* fix: skip push notification open capture on activity restore

* fix: bail push retry when the pending record was cleared mid-unregister
)

* feat: attach optional identity token to push subscription requests

* chore: drop changeset until release

* fix: do not cache identity token minted after opt-out

* fix: harden async push identity token mint against races, hangs, and leaks

* fix: clear didAuthRetry on opt-out and order cache clear on executor
…at/push-notifications

# Conflicts:
#	posthog-android/build.gradle.kts
@ioannisj ioannisj self-assigned this Jul 30, 2026
@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

posthog-android Compliance Report

Date: 2026-07-30 21:16:14 UTC
Duration: 118474ms

✅ All Tests Passed!

46/46 tests passed


Capture Tests

29/29 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields 411ms
Format Validation.Event Has Uuid 32ms
Format Validation.Event Has Lib Properties 35ms
Format Validation.Distinct Id Is String 33ms
Format Validation.Token Is Present 34ms
Format Validation.Custom Properties Preserved 30ms
Format Validation.Event Has Timestamp 32ms
Retry Behavior.Retries On 503 7025ms
Retry Behavior.Does Not Retry On 400 4026ms
Retry Behavior.Does Not Retry On 401 4025ms
Retry Behavior.Respects Retry After Header 7026ms
Retry Behavior.Implements Backoff 17035ms
Retry Behavior.Retries On 500 7019ms
Retry Behavior.Retries On 502 7019ms
Retry Behavior.Retries On 504 7020ms
Retry Behavior.Max Retries Respected 17033ms
Deduplication.Generates Unique Uuids 39ms
Deduplication.Preserves Uuid On Retry 7014ms
Deduplication.Preserves Uuid And Timestamp On Retry 12024ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 7016ms
Deduplication.No Duplicate Events In Batch 37ms
Deduplication.Different Events Have Different Uuids 23ms
Compression.Sends Gzip When Enabled 19ms
Batch Format.Uses Proper Batch Structure 20ms
Batch Format.Flush With No Events Sends Nothing 11ms
Batch Format.Multiple Events Batched Together 32ms
Error Handling.Does Not Retry On 403 4020ms
Error Handling.Does Not Retry On 413 4022ms
Error Handling.Retries On 408 5027ms

Feature_Flags Tests

17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 46ms
Request Payload.Flags Request Uses V2 Query Param 24ms
Request Payload.Flags Request Hits Flags Path Not Decide 22ms
Request Payload.Flags Request Omits Authorization Header 28ms
Request Payload.Token In Flags Body Matches Init 23ms
Request Payload.Groups Round Trip 19ms
Request Payload.Groups Default To Empty Object 23ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False 22ms
Request Payload.Disable Geoip Omitted Defaults To False 20ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key 23ms
Request Lifecycle.No Flags Request On Init Alone 13ms
Request Lifecycle.No Flags Request On Normal Capture 33ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests 33ms
Request Lifecycle.Mock Response Value Is Returned To Caller 23ms
Retry Behavior.Retries Flags On 502 327ms
Retry Behavior.Retries Flags On 504 322ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event 24ms

@ioannisj
ioannisj marked this pull request as ready for review July 30, 2026 14:02
@ioannisj
ioannisj requested a review from a team as a code owner July 30, 2026 14:02
@greptile-apps

greptile-apps Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Security Review

Two identity-lifecycle races can leave a device subscribed to previous identities: a stale asynchronous token mint can register the pre-change identity, and the single persisted unregister slot can discard an earlier identity's pending deletion.

Prompt To Fix All With AI
### Issue 1
posthog/src/main/java/com/posthog/internal/PostHogPushSubscriptionManager.kt:397-401
**Stale identity mint passes guard**

When an asynchronous identity-token mint remains pending while `reset()` or `identify()` changes the distinct ID, this guard accepts the replacement record because it compares only the token and app ID. The stale completion then registers the device for the previous identity, leaving a logged-out or previously identified user subscribed. **How this was verified:** The captured distinct ID survives the asynchronous mint while reset replaces the record with the same token and app ID.

```suggestion
            val current = currentRecord()
            if (current == null ||
                current.deviceToken != record.deviceToken ||
                current.appId != record.appId ||
                distinctIdProvider() != distinctId
            ) {
                config.logger.log("Push subscription send skipped: record or identity changed during identity token mint.")
                return
            }
```

### Issue 2
posthog/src/main/java/com/posthog/internal/PostHogPushSubscriptionManager.kt:619-623
**Unregister intents overwrite each other**

When two resets or unregister operations for different identities occur while offline or before the first deletion can be retried, this single slot replaces the earlier in-memory and persisted intent. The earlier identity's DELETE is permanently forgotten, leaving that logged-out user associated with the device token. **How this was verified:** Each offline unregister relies on this persisted slot, and every subsequent write unconditionally replaces both copies.

---

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

Reviews (1): Last reviewed commit: "chore: collapse push changesets into the..." | Re-trigger Greptile

@posthog

posthog Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

🦔 ReviewHog reviewed this pull request

Found 1 must fix, 12 should fix, 3 consider.

Published 16 findings (view the review).

@posthog

posthog Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

@posthog posthog Bot 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.

ReviewHog Report

Business logic

Issues: 7 issues

Files (1)
  • posthog/src/main/java/com/posthog/internal/PostHogPushSubscriptionManager.kt
What were the main changes
  • New PostHogPushSubscriptionManager: persists a single latest-wins {deviceToken, appId, platform} registration and retries transient failures with an exponential backoff ladder (5s\u201330s) that persists across triggers
  • Passive retry model with no timers \u2014 recovery driven only by flush()/identify()/relaunch; halts for the session on non-retryable or exhausted-retry failures but keeps the record for one retry next launch
  • Identity token (pushIdentityProvider) resolution with in-memory caching, single 401 refresh-and-retry, and a 10s mint watchdog that falls back to a token-less send
  • Durable single-slot unregister intent persisted before DELETE for logout/reset, cleared only on 2xx or terminal 4xx, replayed by retryPending()
  • Race-condition fix so a fresh registration delivered to an identity supersedes a queued logout-DELETE for that same identity (log out of A, back into A)
  • Reviewer-flagged issues: stale identity-mint completion can bypass the distinct-id guard after reset()/identify(); single unregister slot can silently drop an earlier identity's DELETE when a second reset/unregister arrives

Changes

Issues: 4 issues

Files (7)
  • posthog/src/main/java/com/posthog/PostHog.kt
  • posthog/src/main/java/com/posthog/PostHogConfig.kt
  • posthog/src/main/java/com/posthog/PostHogInterface.kt
  • posthog/src/main/java/com/posthog/internal/PostHogApi.kt
  • posthog/src/main/java/com/posthog/internal/PostHogPushSubscriptionRequest.kt
  • posthog/src/testFixtures/java/com/posthog/PostHogFake.kt
  • posthog/api/posthog.api

Feature

Issues: 5 issues

Files (10)
  • posthog-android/src/main/java/com/posthog/android/PostHogAndroid.kt
  • posthog-android/src/main/java/com/posthog/android/PostHogAndroidConfig.kt
  • posthog-android/src/main/java/com/posthog/android/internal/PostHogActivityLifecycleCallbackIntegration.kt
  • posthog-android/src/main/java/com/posthog/android/internal/PostHogPushSubscriptionIntegration.kt
  • posthog-android/api/posthog-android.api
  • buildSrc/src/main/java/PosthogBuildConfig.kt
  • posthog-android/build.gradle.kts
  • posthog-android/consumer-rules.pro
  • posthog-android/gradle.lockfile
  • .changeset/quirky-melons-jog.md
What were the main changes
  • New PostHogPushSubscriptionIntegration auto-fetches the FCM token + Firebase project_id via reflection-gated FirebasePushTokenFetcher (compileOnly firebase-messaging dependency, no-op with debug log when absent) and registers it on startup
  • PostHogAndroidConfig adds capturePushNotificationSubscriptions and capturePushNotificationOpened flags (both default true)
  • PostHogAndroid wires the new integration and gates the activity-lifecycle integration on the new opened-capture flag
  • PostHogActivityLifecycleCallbackIntegration auto-captures $push_notification_opened on cold-start tray taps via the launch intent's google.message_id extra, deduped per process and gated on fresh (non-recreated) launches
  • Gradle/proguard/lockfile updates to add firebase-messaging as compileOnly + test dependency, plus consumer proguard rules for it
  • Changeset documenting the new push notification support for posthog and posthog-android

Comment thread .changeset/quirky-melons-jog.md Outdated
Comment thread posthog/src/main/java/com/posthog/PostHog.kt
Comment thread posthog/src/main/java/com/posthog/PostHog.kt
Comment thread posthog/src/main/java/com/posthog/internal/PostHogPushSubscriptionManager.kt Outdated

@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.

I've read through this and taken the time to understand it. I don't have any additional comments outside of what the 🤖🤖🤖 have found - overall this direction seems clear, and it's cleanly scoped to work with FCM. With the comments addressed, I'd stamp

Comment thread posthog/src/main/java/com/posthog/internal/PostHogPushSubscriptionManager.kt Outdated
@ioannisj
ioannisj requested a review from a team July 30, 2026 21:25
@ioannisj

Copy link
Copy Markdown
Contributor Author

I think everything above is addressed already. Thank you everyone

@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.

nice!

@ioannisj
ioannisj merged commit 08d7a83 into main Jul 31, 2026
16 checks passed
@ioannisj
ioannisj deleted the feat/push-notifications branch July 31, 2026 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants