Repository navigation
feat: [SDK-5397] send a heartbeat every 30 minutes of foreground time - #1785
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Multi-model review (Opus 5.5, GPT 5.6 Sol, Grok 4.7)
Intent: enqueue a duration-only session update every 30 minutes of foreground time so sessions-API sessions stay open and a foreground kill loses little duration.
Opus and Grok independently flagged overlapping issues. GPT 5.6 Sol reported no findings. No criticals; the generation counter and executor coalescing look sound on the normal tracker resume path.
Act on
- Heartbeat vs create ordering.
startNewSessionswaps the record, unlocks, thenrestartHeartbeatbumpsheartbeatGenerationand may runcheckHeartbeatsynchronously. A due check can callsessionHeartbeatbeforeenqueueSessionCreate.admitUpdatethen drops that update (“no create or server session ID”) afterlastHeartbeatDurationhas already advanced, so the next send waits a full interval. Bump generation in the same lock as the record swap, and do not send a heartbeat until the create is queued. - No
focusedAt→ no heartbeat. Heartbeats requirefocusedAt != nil, which onlyonFocus()sets. Consent-gatedOneSignalTracker.onFocusreturns beforeOSSessionService.onFocus, and init while already active may never get another become-active. That first long foreground visit then gets no heartbeats — the case this feature is meant to protect. Seed focus when a session starts and the app is already active (same idea asStartFeatureFlagsRefresh).
Consider
- The interval is foreground time, not wall-clock. Short interruptions stretch time between updates; confirm that still satisfies the backend keep-alive (including repo poll + retry).
endTimetakesmax(..., lastHeartbeatTime). A forward clock jump stored on a heartbeat can outrank a later unfocus and send a futureend_time.- A missed unfocus (e.g. consent revoke) leaves the chain running and can persist background time into
reportedDuration/endTime. resetAndClearStoredRecorddoes not invalidate an in-flight instance; a global-queuepersist()can rewrite the old record after an app-id change.
Noted
- Protocol says
sessionHeartbeatis off-main; the firstcheckHeartbeatis synchronous on the caller (often main). - Stale
asyncAfterblocks still callpinnableIds()before the generation guard. - Tests cover the virtual-clock happy path; they miss overdue-on-focus, heartbeat-before-create, and concurrent unfocus.
Sent by Cursor Automation: PR Reviews
Start a new session's heartbeat only after its create is queued, stop it on reset, and drop the heartbeat time from the end once the app leaves the foreground after it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
abdulraqeeb33
left a comment
There was a problem hiding this comment.
Follow-up pass on the heartbeat, reviewed with a three model fan-out (Opus, Grok, GPT), with each finding checked against the code before writing it up.
Since the last round: the create-ordering race is genuinely fixed, and clearing lastHeartbeatTime on unfocus closes the wall-clock path raised on endTime. The generation counter holds up. I traced every bump against the stale-callback windows, and the one interleave that can reorder sends is absorbed by admitUpdate's max-fold.
Two things worth fixing before this lands, and both are consequences of where the last round's fixes landed rather than new regressions:
- Clearing
lastHeartbeatTimeon unfocus fixed the unfocus path, but the kill path it deliberately carved out still carries an unclamped wall-clock sample. - Moving the arming into the User module left
startHeartbeat()as a single production line that no test covers.
Everything else is a nit or a residual risk, noted inline.
- Time a heartbeat from the visit's focus on the monotonic clock, so a wall-clock jump can't move a killed session's end. - startNewSession takes the enqueue as a closure and starts the heartbeat after it, so there's no second call to forget. - Log when a heartbeat starts, with the session ID. - Cover a stale check after unfocus, a late check, and a kill after a clock jump. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>


Description
One Line Summary
When the sessions API flag is on, enqueue a duration-only session update every 30 minutes of foreground time.
Details
Motivation
Part of the iOS SDK Sessions stack (SDK-5397). Keeps long foreground sessions open on the server and limits lost duration if the app is killed in the foreground. Replaces #1773, which was closed when its base merged. Android counterpart: OneSignal/OneSignal-Android-SDK#2782.
Scope
OSSessionServiceruns the heartbeat while a sessions API session is in the foreground. It starts when a session starts (afterstartNewSession'senqueueclosure queues the create, since an update before its create is discarded) or when a focus continues the current session, and stops on unfocus. A focus that will start a new session leaves the heartbeat to that session. Flag off: no timer.OSSessionUserProvider.sessionHeartbeat(_:), which enqueues an update delta withoutend_time. The executor's existing coalescing folds unsent heartbeats and discards them once the end is queued. No executor changes.OSSessionRecord.lastHeartbeatDuration/lastHeartbeatTime. An app killed in the foreground never adds its last visit toactiveDuration, so updates now sendreportedDuration(at least the last heartbeat's duration), andendTimealso considers the last heartbeat time. That time is the visit's focus time plus its monotonic duration, so a wall-clock jump can't move a killed session's end.HEARTBEAT_INTERVAL_MS, and the two should change together.Testing
Unit testing
Added
OSSessionHeartbeatTestson a virtual clock: one heartbeat after 30 minutes with its time recorded, two after 60, a short background pauses the interval, a new session restarts it, a focus that starts a new session doesn't heartbeat the old one, a new session's heartbeat waits for its create, a pending check after unfocus does nothing, a late check reports the full duration, no timer with the flag off, and a kill after a heartbeat (including after a clock jump) ends with the heartbeat's duration and time.SessionRecordUserTestschecks that the heartbeat and end deltas carry at least the last heartbeat's duration.OneSignalOSCore tests pass locally. In OneSignalUserTests,
ExecutorRetryBackoffTests.testACreateUserRejectedAfterANewSessionStartedIsResentRightAwayfails before and after this round of changes, andUserConcurrencyTests.testSubscriptionExecutorConcurrencyis intermittent. SwiftLint: no new violations beyondOSSessionService.swiftgrowing past thefile_lengthwarning it was already near.Manual testing
Not tested on device yet; 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