Repository navigation
feat: [SDK-5400] attribute a session started by a notification open - #1788
fadi-george wants to merge 7 commits into
Conversation
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>
There was a problem hiding this comment.
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.preparedis in-memory only. After a kill, a create that was already sent decodes as unprepared. AnOS_ATTRIBUTE_SESSION_DELTApersisted immediately byOSOperationRepo.enqueueDeltacan then changedirect_attribution_id, and the retry rebuilds a different body with the sameidempotency_key. All three reviewers flagged this. Persist the freeze.testAttributionAfterTheCreateWasSentIsDroppedonly 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. startNewSessionpublishes the record beforeenqueue(start). A concurrent open can enqueue the attribute delta first;attribute(_:)drops it if the create is not inrequestQueueyet.
Noted
- New required method on public
OSSessionUserProvider(in-repo conformers updated). pendingAttributionIdhas no TTL; a background action that never focuses can credit a later session.- Open and
OSSessionStartGatesample the 30s window from different clocks.
Sent by Cursor Automation: PR Reviews
… 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
left a comment
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
…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
left a comment
There was a problem hiding this comment.
Can you test this on device with a direct session, but also influenced sessions?
abdulraqeeb33
left a comment
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.


Description
One Line Summary
Send
direct_attribution_idon 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
OSNotificationsManagerforwards opens from the background (the same!isActivepath that feedsOSSessionManager) toOSSessionService.onNotificationOpened.startNewSessionreturns it onOSSessionStart.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.OSSessionUserProvider.sessionAttributedenqueues a newOS_ATTRIBUTE_SESSION_DELTA, which the executor folds into that session's queued create.idempotency_keyand must send the same body.direct_attribution_id. Legacy outcome attribution is unchanged, and nothing is sent when the session doesn't use the sessions API.UNNotificationActionOptionForeground) runs with the app in the background and would otherwise credit a later normal open or a background-launch session. This mirrors Android'sshouldInitDirectSessionFromNotificationOpen, which only forwards opens that launch the app. Legacy influence still gets every!isActiveopen.OSInfluenceDataRepository.isDirectInfluenceEnabled, off by default), matching Android'sinfluenceParams.isDirectEnabledcheck.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.OSNotificationsManagerhave no unit test: the test host is always active, and the legacy ObjC open tests are disabled, so reaching it would mean swizzlingUIApplication.UnitTestApp_TestPlan_Reducedpasses 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
Checklist
Overview
Testing
Final pass
🤖 Generated with Claude Code