Skip to content

feat: [SDK-5400] attribute a session started by a notification open - #1788

Open
fadi-george wants to merge 7 commits into
new-session-eventsfrom
fadi/sdk-5400
Open

fadi-george wants to merge 7 commits into
new-session-eventsfrom
fadi/sdk-5400

Conversation

@fadi-george

@fadi-george fadi-george commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Description

One Line Summary

Send direct_attribution_id on the session create when a notification open starts a new session.

Details

Motivation

Part of the iOS SDK Sessions stack (SDK-5400). The sessions API attributes a session to the notification that started it, but the open only reached the legacy influence manager. Android counterpart: OneSignal/OneSignal-Android-SDK#2783.

Scope

  • OSNotificationsManager forwards opens from the background (the same !isActive path that feeds OSSessionManager) to OSSessionService.onNotificationOpened.
  • Unlike Android, init's session start is dispatched to the main queue before iOS delivers a cold-start open, so the session usually starts first. Both orders are handled:
    • Open before the session starts (resume past the threshold): the ID is held and startNewSession returns it on OSSessionStart.directAttributionId, which goes on the create delta. A focus that continues the current session, or any unfocus, discards it, so an open during an active session never changes it or carries over.
    • Open after the session starts (cold start): if the current session started in this process, was never focused, and is within the 30 second threshold, the open is the one bringing it to the foreground. OSSessionUserProvider.sessionAttributed enqueues a new OS_ATTRIBUTE_SESSION_DELTA, which the executor folds into that session's queued create.
  • The fold only applies while the create has never been prepared for sending. That state is persisted, so it holds across a kill. After a send attempt the attribution is dropped, since a retry replays the same idempotency_key and must send the same body.
  • The attribute delta is exempt from identity verification like the other session deltas.
  • Normal opens send no direct_attribution_id. Legacy outcome attribution is unchanged, and nothing is sent when the session doesn't use the sessions API.
  • Opens delivered while the app is already active are not attributed, matching the legacy influence path.
  • Only opens delivered while the app is inactive, which is how a tap that brings the app forward arrives, are attributed. A background action button (only possible with an app-defined category, since OneSignal's own buttons always use UNNotificationActionOptionForeground) runs with the app in the background and would otherwise credit a later normal open or a background-launch session. This mirrors Android's shouldInitDirectSessionFromNotificationOpen, which only forwards opens that launch the app. Legacy influence still gets every !isActive open.
  • Opens are only attributed when direct influence is enabled in the remote params (OSInfluenceDataRepository.isDirectInfluenceEnabled, off by default), matching Android's influenceParams.isDirectEnabled check.

Testing

Unit testing

  • OSSessionServiceTests: open before start attributes the new session; normal open omits it; open within the threshold leaves the session alone and doesn't attribute the next one; an open followed by unfocus isn't credited later; cold-start open after and before the session starts; open past the threshold of a background session, and for a session from an earlier process, attributes the next session.
  • OSSessionOperationExecutorTests: attribution folded into a waiting create and in the same flush as the create; dropped after the create was sent and when there is no create; survives a kill; a sent create is not changed by an attribution replayed after a kill.
  • SessionRecordUserTests: end-to-end for both orders, and no attribution on a normal start.
  • The inactive-state and direct influence checks in OSNotificationsManager have no unit test: the test host is always active, and the legacy ObjC open tests are disabled, so reaching it would mean swizzling UIApplication.
  • Full UnitTestApp_TestPlan_Reduced passes locally (302 tests, iOS 26.5 simulator).

Manual testing

Not tested on device yet; the cold-start ordering is from main-queue dispatch order, not device logs. End-to-end verification is planned in the iOS rollout and QA ticket.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

🤖 Generated with Claude Code

Send direct_attribution_id on the session create when a notification
open from the background starts the session. On a cold start the
launch's session usually starts before iOS delivers the open, so the
ID is folded into that session's still-queued create.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Multi-model review (Claude Opus 5.5, GPT 5.6, Grok 4.7)

The two-ordering design is coherent and the tests cover the main happy paths. One stated invariant does not hold across a process kill.

Act on

  • OSRequestCreateSession.prepared is in-memory only. After a kill, a create that was already sent decodes as unprepared. An OS_ATTRIBUTE_SESSION_DELTA persisted immediately by OSOperationRepo.enqueueDelta can then change direct_attribution_id, and the retry rebuilds a different body with the same idempotency_key. All three reviewers flagged this. Persist the freeze. testAttributionAfterTheCreateWasSentIsDropped only covers an in-process 500.

Consider

  • After the first flush (~5s) the create is frozen. A later open that still matches opensUnfocusedRecord (never-focused, <30s) is sent as a fold, dropped, and not held for the next session. A process already running in the background, then opened by a tap, loses attribution.
  • startNewSession publishes the record before enqueue(start). A concurrent open can enqueue the attribute delta first; attribute(_:) drops it if the create is not in requestQueue yet.

Noted

  • New required method on public OSSessionUserProvider (in-repo conformers updated).
  • pendingAttributionId has no TTL; a background action that never focuses can credit a later session.
  • Open and OSSessionStartGate sample the 30s window from different clocks.
Open in Web View Automation 

Sent by Cursor Automation: PR Reviews

fadi-george and others added 2 commits October 30, 2026 11:07
… forward

A background action button runs with the app in the background, so it
never starts the session it would otherwise be credited with.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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

The attribution fold and the prepared-body freeze look right. One nit on the pending id.

let attributed: (record: OSSessionRecord, provider: OSSessionUserProvider)? = stateLock.withLock {
loadIfNeeded()
guard let record, let userProvider, opensUnfocusedRecord(at: wallNow()) else {
pendingAttributionId = notificationId

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.

pendingAttributionId is cleared by onUnfocus, and by a focus that continues the current record. The tracker runs onUnfocus once per willResignActive, and didEnterBackground does not run it again. A custom action without UNNotificationActionOptionForeground can be delivered while the app is still Inactive, after that unfocus, and the next startNewSession past 30 seconds will send it. Stamp the time when the id is stored, and drop it when the gap is at least OSSessionStartGate.threshold.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. Fixed in 1998a74: the held open is stamped and dropped if the next session starts 30s or more after it.

That alone misses closing Notification Center between 30s after the unfocus and 30s after the action, so 1b48ff2 also only forwards opens that bring the app forward: a tap on the notification or one of its OneSignal buttons (always UNNotificationActionOptionForeground). Actions from app-defined categories are no longer attributed, which matches Android only attributing opens that launch the app.

fadi-george and others added 4 commits November 2, 2026 10:48
…ct influence is enabled

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eshold

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…forward

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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

Can you test this on device with a direct session, but also influenced sessions?

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

The expiry and the forward-only open check look right. The new influence gate drops attribution while the flag is still missing.

// Only an open that brings the app forward starts or continues a session because of it.
if (messageId.length > 0 && applicationState == UIApplicationStateInactive
&& [self openBringsAppForward:notification actionID:actionID]
&& OSInfluenceDataRepository.sharedInfluenceDataRepository.isDirectInfluenceEnabled)

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.

isDirectInfluenceEnabled defaults to false and is only saved from outcomes.direct.enabled. The sessions create is already gated by the sessions cutover flag. An open before that save, and any open while the key is missing, never calls onNotificationOpened, so direct_attribution_id is omitted and not retried. Drop this check, or treat a missing key as enabled, and log the skip.

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