[fadi-george]

Description

One Line Summary

Sends sessions API create/update requests as persisted OperationRepo operations, so they survive app kills and are retried with the same idempotency key.

Details

Motivation

Session requests need to survive process death, offline periods and server errors without double counting. Replaces the earlier approach in #2777 now that OperationRepo retries failed operations per operation (#2790).

Scope

• CreateSessionOperation / UpdateSessionOperation, persisted via OperationModelStore. Each stores an idempotency key at creation that is reused on every retry.
  • Create waits for backend onesignal/subscription IDs. Update waits for the backend session ID (local ID translated after create).
• SessionOperationExecutor: retries on network errors, 408, 429 (honors Retry-After) and 5xx; drops other 4xx. Update 404 is retried within the post-create window since create is async (202). No retry logic of its own; backoff comes from OperationRepo.
• SessionQueuePolicy, hooked into OperationRepo enqueue/drop:
  • Unsent duration-only updates coalesce to the highest duration.
  • An end replaces an unsent update; updates after a queued end are dropped.
  • Cap of 100 queued session operations (matches iOS); oldest whole session dropped and logged.
  • A dropped create drops its waiting updates.
  • Updates built from a local session ID just after the create succeeded still get the server ID.
  • One persisted SessionStateModelStore is the only record of a session's create IDs, server session ID and whether it is closed, so updates after a restart still get them and an update after an end that already left the queue is still dropped. Capped at 100 sessions, never dropping one with operations still queued.
• SessionModel.localSessionId, set on each new session. SessionModel.serverSessionId is removed, since nothing reads it.
• Operations with requiresJwt = false are no longer suppressed for anonymous users under identity verification, otherwise IV apps would send no sessions.

Known tradeoff: a successful create returns ID translations, so OperationRepo's existing post-create delay (5s default) pauses the queue once per session. This also gives the backend time to process the create before an update.

Not included: wiring session lifecycle to enqueue these operations (downstream PRs).

Testing

Unit testing

• SessionOperationExecutorTests: request bodies, ms to seconds, retry vs drop status mapping, Retry-After, 404 in missing window, idempotency keys surviving persistence.
• SessionOperationQueueTests (through OperationRepo): coalescing, end replacing update, update after end dropped, cap, create before update ordering, late local-ID update, dropped create drops updates, replay after restart with same key, IV exemption. SessionQueuePolicy unit cases cover the create IDs, the backend session ID and a closed session surviving a restart, and the state cap keeping queued sessions.
• Full testDebugUnitTest, spotless, detekt, apiCheck and assembleRelease pass locally.

Manual testing

Not tested on device; nothing enqueues these operations until the lifecycle wiring lands.

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: 27/27 touched executable lines (100.0%) (52 touched lines in diff)
• SessionQueuePolicy.kt: 96/97 touched executable lines (99.0%) (187 touched lines in diff)
• ⚠️ SessionModule.kt: Not in coverage report (may not be compiled/tested)
• SessionModel.kt: 2/2 touched executable lines (100.0%) (4 touched lines in diff)
• SessionService.kt: 1/1 touched executable lines (100.0%) (1 touched lines in diff)
• CreateSessionOperation.kt: 35/37 touched executable lines (94.6%) (85 touched lines in diff)
• SessionStateModel.kt: 19/19 touched executable lines (100.0%) (49 touched lines in diff)
• UpdateSessionOperation.kt: 43/47 touched executable lines (91.5%) (104 touched lines in diff)
• SessionOperationExecutor.kt: 56/57 touched executable lines (98.2%) (121 touched lines in diff)

Overall (aggregate gate)

281/289 touched executable lines covered (97.2% — requires ≥ 80%)

📥 View workflow run

[cursor]

Multi-model review

Models: Claude Opus 5.5, GPT 5.6 Sol, Grok 4.7

Persist-and-retry via OperationRepo is the right shape, and the queue/executor tests cover the in-queue rules. Three integration gaps will drop or evict session work once lifecycle wiring starts enqueueing these ops.

Act on

1. Offline (HttpClient status -1) is FAIL_NORETRY here; every other executor treats it as retryable via NetworkUtils.
2. The 100-op cap runs on every enqueue and adds +1 even for non-session ops, so a tag/login at the cap evicts the oldest session.
3. Coalesce / drop-after-end only see queue. In-flight ends and FAIL_RETRY / startup requeues skip the policy, so an update can still send after an end.

Consider

• Update 404 retry uses in-memory NewRecordsState only — a kill during the 202 window permanently drops the restored update.
• Create 404 does not retry in the user/subscription missing window.
• Coalescing a retrying update resets backoff and replaces the idempotency key.

Noted: the whole-queue 5s post-create delay is the documented tradeoff.

  

_{Sent by Cursor Automation: PR Reviews}

[fadi-george]

Also: merging updates now carries over retries and retryAt, so new updates no longer skip backoff. Keeping the fresh idempotency key since the merged update carries the max duration.

[abdulraqeeb33]

Requesting changes.

1. A session create with requiresJwt false survives the anonymous suppress and the hydrate purge. If its ids are still local, it never starts, and it stays queued until the cap.
2. A 2xx with no session_id drops the create and its updates. Retry that response on the same idempotency key.
3. An update whose session id is still local is accepted when no create is left.
4. loadSavedOperations calls internalEnqueue, so a restored end is not recorded in endedSessions. Once that end is taken out to send, a later update is accepted. Record restored ends at load.
5. The local-id check and the serverSessionId write take the model lock separately. A focus between them can stamp the next session with the previous server id.

[fadi-george]

Thanks, 4 is fixed. restored ends are now recorded as closed sessions at load

[abdulraqeeb33]

The four items from the last review are fixed in 12eedf7. ab34f50 only adjusts two tests.

1. A create with a local onesignal id or subscription id is dropped at enqueue and at hydrate, with its updates.
2. A 202 with no session_id retries on the same idempotency key, three times, then the create and its updates are dropped.
3. An update with a local session id is rejected unless its create is known, including one restored from the store.
4. Restored ends are recorded at load. The server session id compare and write run in one runOnSerialIO block.

closedSessions drops the oldest id after 50, and an end that already left the store is not restored.