[fadi-george]

Description

One Line Summary

Queue sessions API requests as persisted CreateSessionOperation / UpdateSessionOperations in OperationRepo, with idempotent retries, coalescing, and a queue cap.

Details

Motivation

Session requests need to survive process death, restarts, and offline periods, and be retried safely without double counting. Stacked on #2775 (sessions API client).

Scope

• CreateSessionOperation / UpdateSessionOperation, persisted through OperationModelStore. Each stores an idempotency key generated once and reused on every retry, including after a restart.
• SessionOperationExecutor catches BackendException and classifies with NetworkUtils.getResponseStatusType: RETRYABLE maps to FAIL_RETRY (passes Retry-After to the existing delayBeforeNextExecution), except 4xx other than 408/429 (e.g. 413, 422), which won't succeed on retry. Everything else is FAIL_NORETRY. A create success without a session ID retries.
• Attempt cap: OperationRepo retries at the head of the queue, so a session op is dropped after 5 failed attempts the backend answered (5xx, 408, 429, or a 2xx without a session ID). The count is persisted on the op and kept when updates are coalesced. Network errors (no response) don't count, so offline requests keep retrying until reconnect.
• Ordering: a create waits for non-local onesignal_id / subscription_id. Updates reference the session by SessionModel.localSessionId and wait until the create's ID translation gives them the backend session ID. - Session pinned IDs: OperationRepo writes each ID translation it applies into the current SessionModel (onesignalId, subscriptionId, and serverSessionId keyed by localSessionId), on SerialIO so it can't interleave with a new session starting, and joined so the IDs are saved before a successful create leaves the store. The subscription ID is only translated when the same response translates the session's OneSignal ID, since login reuses a local push subscription for the new user. This replaces PinnedLocalIdTranslator in SessionService from feat: [SDK-5369] session model and sessions v2 feature flag #2774, so the session model and its queued ops always agree on the owner, including after a restart.
• OperationRepo changes:
  • Coalescing at enqueue time: queued duration-only updates for a session fold into the incoming update, keeping the highest duration. An end replaces them the same way. An update that arrives after a queued end is discarded. The surviving op keeps its own idempotency key because its payload is different.
  • Cap: at most 100 session ops. Past that the oldest is dropped with a warning, and if it was a create, its updates go with it.
  • A dropped create also drops its queued updates, since they could never get a session ID.
  • ID translations are remembered for the process and applied to ops enqueued later. This closes a race where an update enqueued while its create was in flight kept the local ID forever. It applies to all ops, but only rewrites IDs that were already translated.
  • Ops with requiresJwt = false (the session ops) are no longer suppressed or purged as anonymous under identity verification. Instead, under required identity verification, session ops for a local OneSignal ID that no identified login will create are discarded at enqueue and dropped with the anonymous purge, since they could never be sent.
• Retry wake-ups: new IOperationRepo.retryNow() ends the failure backoff early but still waits out a backend Retry-After. forceExecuteOperations() behaves as before. OperationRetryTrigger calls retryNow() on app foreground and when a network becomes available while the app is foregrounded (registerNetworkCallback, API 21). The foreground check avoids attempts that Android 15+ blocks for cached background apps, which would only lengthen the backoff. It only acts while the repo is waiting to retry, so it never shortens a later backoff. This applies to all operations, not just session ones.
• Not in this PR: enqueueing from SessionService (SDK-5382), so nothing is enqueued yet and the flag-off path is unchanged.

Testing

Unit testing

• SessionOperationQueueTests: coalescing, end replaces update, update after end discarded, per-session isolation, cap plus orphan drop, create-before-update ordering with translation, late-enqueued update translated, failed create drops updates, retry keeps idempotency key, not purged under IV, anonymous-user session ops discarded at enqueue and dropped on purge under IV, translations written to session pins, earlier session's create doesn't set the current server session ID.
• OperationRepoTests: retryNow ends the backoff early, still waits out Retry-After, and is a no-op when not waiting.
• OperationRetryTriggerTests: retries on network available only while foregrounded and on foreground, and still starts if callback registration throws.
• SessionOperationExecutorTests: request mapping (ms to s), translation, retry/Retry-After for network errors, 408, 429, 5xx, drop for other 4xx, retry on missing session ID, drop after the attempt cap, network errors don't count toward it, canStartExecute and translation, persistence round-trip keeps keys.
• Full :onesignal:core:testReleaseUnitTest, spotlessCheck, and assembleRelease pass. detekt reports only the 3 findings already present in files this PR doesn't touch.

Manual testing

Not done. Nothing enqueues these ops until SDK-5382.

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

[github-actions]

📊 Diff Coverage Report

Diff Coverage Report (Changed Lines Only)

Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff).

Changed Files Coverage

• OperationModelStore.kt: 2/2 touched executable lines (100.0%) (5 touched lines in diff)
• OperationRepo.kt: 41/41 touched executable lines (100.0%) (83 touched lines in diff)
• OperationRepoSessionExtensions.kt: 68/69 touched executable lines (98.6%) (158 touched lines in diff)
• OperationRetryTrigger.kt: 22/23 touched executable lines (95.7%) (53 touched lines in diff)
• ⚠️ SessionModule.kt: Not in coverage report (may not be compiled/tested)
• SessionModel.kt: 6/6 touched executable lines (100.0%) (22 touched lines in diff)
• SessionService.kt: 73/83 touched executable lines (88.0%) (all lines — could not resolve diff)
• CreateSessionOperation.kt: 24/26 touched executable lines (92.3%) (57 touched lines in diff)
• SessionOperation.kt: 21/21 touched executable lines (100.0%) (58 touched lines in diff)
• UpdateSessionOperation.kt: 28/30 touched executable lines (93.3%) (66 touched lines in diff)
• SessionOperationExecutor.kt: 51/52 touched executable lines (98.1%) (122 touched lines in diff)

Overall (aggregate gate)

336/353 touched executable lines covered (95.2% — requires ≥ 80%)

📥 View workflow run

[cursor]

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

Intent: Persist sessions API create/update as OperationRepo ops with idempotent retries, enqueue coalescing, a 100-op cap, and in-process ID translation so requests survive restarts and offline periods. Nothing enqueues yet (SDK-5382).

Act on

1. (3/3) serverSessionId check-then-write is racy. createSession reads session.localSessionId then writes serverSessionId on the op-repo thread. SessionService.handleOnFocus assigns a new sessionId and later sets serverSessionId = null on serial IO. If the check still sees the old session, the write can land after rollover and persist the previous backend ID on the new session.
2. (3/3) Coalescing only sees queue. getNextOps has already removed the in-flight op; FAIL_RETRY requeues at index 0 without coalescing; loadSavedOperations(..., addToStore=false) skips it. A duration update can PATCH after an end, and a crash that persisted both ops reloads both.

Consider

• (2/3) appliedIdTranslations is process-local and SessionModel.onesignalId / subscriptionId are never rewritten. A session that starts on local IDs can enqueue later updates that stay canStartExecute == false after a restart.
• (1/3) dropOrphanedSessionUpdates only drops updates already queued when a create is dropped; later updates for that local session stall until the cap evicts them.
• (1/3) Cap uses sessionItems.first(); retry requeue at index 0 can make a newer op look oldest.
• (1/3) Coalesce persists the delete before the replacement is stored.
• (3/3) Absorbed enqueueAndWait waiters are woken true before the survivor runs (same as login dedupe).

Noted

• requiresJwt = false only on these two op types (default is true).
• Successful creates take the shared 5s delayForPostCreate path and can back up the whole repo.
• appliedIdTranslations applies to all new ops by design; watch LoginUserOperation.existingOnesignalId merge.

Dismissed

• IV purge/suppress changing non-session ops — only session ops override requiresJwt.

  

_{Sent by Cursor Automation: PR Reviews}

[abdulraqeeb33]

Queue policy looks right overall. Three things before this merges.

1. A 2xx create without a session_id returns FAIL_RETRY with no cap (inline). FAIL_RETRY re-adds at index 0 and the backoff is queue wide, so a captive portal or a sessions service outage now stalls user and subscription ops too. Please bound session op attempts and drop after N.
2. The pin translation into OperationRepo promised on #2774 is not here. PinnedLocalIdTranslator is still in SessionService.kt (84 to 122), and applied translations are in memory only, so an update rebuilt from pins after a restart can carry a local- id. If it moves to the sender PR, please say so in the description.
3. Under required identity verification, session ops are requiresJwt = false, so they skip the purge. A session pinned to an anonymous local onesignal_id never translates, canStartExecute stays false, and the ops sit persisted until the 100 cap evicts them. Drop them when the pinned id can no longer resolve.

[fadi-george]

Thanks @abdulraqeeb33, all three addressed in 6f3c6f0.

1. Capped as described inline.
2. Moved into OperationRepo: it writes each translation it applies into the session's pinned IDs and server session ID, on SerialIO so it can't race a new session. PinnedLocalIdTranslator is gone from SessionService.
3. Under required IV, session ops for a local onesignal_id that no identified login will create are discarded at enqueue and dropped with the anonymous purge.

[abdulraqeeb33]

The attempt cap holds for creates, and the identity-verification drop is right for LoginUserOperation. Two holes.

1. Coalescing an update keeps activeDuration and drops failedAttempts, so a session that keeps enqueueing updates never reaches 5. A 5xx update then stalls the queue again.
2. translateIds writes subscriptionId whenever that local id is in the map. Login's create-user response includes it, so a session still pinned to the previous user takes the new user's subscription. The old appliesTo check stopped this.

Smaller: the pin write at OperationRepo.kt:435 does not wait, and SUCCESS removes the create right after, so a kill in that gap loses serverSessionId. Join it, as the old code did. translateIds also reads localSessionId before the first session, and sessionId is unset until focus, so that throws and the error is only logged. Guard it. LoginUserFromSubscriptionOperation is not treated as a login that will create the user.

[abdulraqeeb33]

The attempt cap, the joined pin write, and the guard before the first session all look right.

One blocker: translateIds returns before writing subscriptionId unless the same map also translates the pinned onesignalId. A push subscription create returns only the subscription id, and that executor does update pushSubscriptionId on the config model. A session that already has a backend OneSignal id keeps the local subscription pin, and that pin is persisted. After a restart, ops built from it never pass canStartExecute. Translate subscriptionId when it is in the map, and skip that write only when the map translates a different user's OneSignal id.

Not blocking: under identity verification LoginUserFromSubscriptionOperationExecutor returns FAIL_NORETRY and never creates the user, but isUnsendable still treats it as the creator when externalId is set. The session op then stays queued with a local OneSignal id. Nothing enqueues session ops yet, and the v4 migration usually has a null external id. Remove that branch when you touch this.

[abdulraqeeb33]

This returns for any map that does not translate the pinned onesignalId, including a create-subscription response whose only entry is the subscription id. That pin stays local and is persisted. Skip the subscription write only when the map translates a different user's OneSignal id.

[fadi-george]

Closing this one. #2790 is merged and covers the retry side (per-op backoff, Retry-After, foreground and network wake), so this PR's retryNow and OperationRetryTrigger are now redundant. I'll open a fresh PR from main with just the session parts.