Skip to content

fix: read-your-write consistency defects - #1706

Open
nan-li wants to merge 4 commits into
mainfrom
nan/jwt-pr2-consistency
Open

fix: read-your-write consistency defects#1706
nan-li wants to merge 4 commits into
mainfrom
nan/jwt-pr2-consistency

Conversation

@nan-li

@nan-li nan-li commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Description

One Line Summary

Fixes a set of bugs in how OneSignal makes sure an in-app message fetch sees your own recent changes (a "read-your-write" wait)

  • a wait that could block forever, a wait that could be satisfied by a different user's data after a login switch, and a response with no token for one user that could release — and disarm — another user's wait.
  • Also includes a one-time cleanup of project.pbxproj so future diffs aren't buried in Xcode's reorder noise.

(This targets main, not the Identity Verification/JWT work.)

Details

Motivation

When you update your profile or subscription, the SDK briefly waits before fetching in-app messages, to make sure that fetch reflects what you just wrote instead of stale data. That waiting logic had several sharp edges:

  • A wait could block forever if the expected confirmation never arrived.
  • After a user switch, a stale wait could be satisfied using the previous user's data.
  • A response with no token for one user could release — and clear the raised "bar" on — a different user's wait, not just its own.
  • Even for the right user, a wait that timed out left that "bar" raised, so the next fetch paid a full wait for a token that was never coming.

Fixed here, across two passes: an initial fix, plus a follow-up that closed gaps the first one left (the "release across indexes" and "bar not lowered on timeout" cases above).

main's project.pbxproj was also out of the gem's canonical UUID order, so the first PR to touch the project file would otherwise bury real changes in reorder noise. Landing that normalize here (no separate PR) keeps follow-ups focused.

Scope

  • Waits now time out instead of blocking forever, and clean up after themselves when they do (OSConsistencyManager).
  • Releasing a wait early (e.g., no token came back from the server) is now scoped to one user — resolveConditions(conditionId:forId:) only touches that user's waiters, never another's.
  • A wait that times out now also lowers any bar it raised, so later fetches for that user aren't held to a token that's never coming.
  • Each user gets their own wait condition, so a fetch can't be released by someone else's tokens (OSIamFetchReadyCondition).
  • Conditions can now react when their wait is released, so a raised "bar" comes back down instead of sticking around for the next fetch (OSCondition.onConditionSatisfied).
  • Added shared test helpers (ConsistencyManagerTestHelpers, XCTestCase+WaitUntil) so future consistency tests don't need to reinvent polling/reset logic.
  • project.pbxproj — pure formatting, no source or behavior changes.
  • Not Identity Verification / JWT.

Testing

Unit testing

  • Expanded OSConsistencyManagerTests
  • New OSIamFetchReadyConditionTests
  • Existing IAM / early-trigger tests updated to use ConsistencyManagerTestHelpers.reset()

Manual testing

Built UnitTestApp for iOS Simulator as part of the local JWT stack; this PR's commits compiled in sequence.

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

nan-li and others added 2 commits August 11, 2026 16:34
Canonical sort applied by the xcodeproj tool, so later commits in this
stack show only their own file additions. No project changes.

Co-authored-by: Cursor <cursoragent@cursor.com>
Four faults in how a fetch waits for its own write to be readable:

resolveConditionsWithID looked up waiters by condition id, but they are
registered under the id passed to getRywTokenFromAwaitableCondition, so
the lookup found nothing and the waiter it meant to release stayed
blocked. It now scans every index for waiters on that condition.

A waiter blocked on an unbounded semaphore, so a response that never
arrived held the calling thread for the life of the process. Waits now
time out and deregister rather than leaving an entry that the next token
signals to nobody.

OSIamFetchReadyCondition was a singleton pinned to the first id it ever
saw, so after a user switch a fetch consulted the previous user's tokens.
Conditions are now per id, with reset() as the test seam.

hasSubscriptionUpdatePending was never lowered, so one in-session
subscription change held every later fetch to waiting for a subscription
token with no update behind it. The new optional onConditionSatisfied
lets a condition lower a bar it raised once its waiter is released.

Shared state moves behind the serial queue and locks throughout.

Co-authored-by: Cursor <cursoragent@cursor.com>
@cursor cursor Bot mentioned this pull request Aug 12, 2026
18 tasks

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

Multimodal adversarial review (interrogate)

Verdict: The four named defects are real and largely hit, but two of the fixes reintroduce the same class of failure under slightly different shapes. Highest confidence from consensus across 4 models.

Intent

Fix four IAM read-your-write defects: broken resolveConditionsWithID lookup, unbounded waiter blocking, singleton condition answering the wrong user after switch, and a stuck hasSubscriptionUpdatePending bar — via cross-index resolve, 30s timeout + deregister, per-id conditions, and onConditionSatisfied.

Reviewers

  • A: claude-fable-5-thinking-xhigh — 7 findings
  • B: gpt-5.6-sol-xhigh — 4 findings
  • C: cursor-grok-4.5-high-fast — 3 findings
  • D: claude-opus-5-thinking-high — 9 findings

Act On

  1. Cross-user resolve broadcast (A/B/C/D) — resolveConditionsWithID scans every index and releases every waiter with conditionId == "OSIamFetchReadyCondition", then clears every user's subscription bar via onConditionSatisfied. That undoes per-id isolation under login/user-switch overlap. All four executor call sites already have the relevant onesignalId in scope.
  2. Timeout never lowers the subscription bar (A/B/C/D) — timeout deregisters the waiter but skips onConditionSatisfied. After a failed/missing subscription update, every later IAM fetch for that id pays another full 30s. Defect (4) survives as a permanent per-fetch tax.
  3. Boolean bar + unconditional clear cannot represent overlapping subscription writes (A/B/C/D) — hasSubscriptionUpdatePending is a single bool; any waiter release clears it. A second in-flight subscription change can be dropped when the first token/release fires. Stale subscriptionUpdate tokens also make later arms inert because isMet only checks presence.

Consider

  • Make the no-ryw_token fallback level-triggered (record a sentinel) so a response that lands before the fetch registers does not force a full timeout (A/D).
  • OSSubscriptionOperationExecutor update path stores tokens under current onesignalId, not request.identityModel.onesignalId (B) — adjacent cross-user footgun at the same boundary.
  • Flaky short-timeout tests that assert waiterCount == 1 while the clock is already running (A/D).
  • Call onConditionSatisfied outside the serial queue to avoid future deadlocks if a conformer re-enters the manager (D).

Noted

  • Per-id condition map never evicted in production (A/D).
  • Spurious timeout warn if release races deregistration (A).
  • static var waitTimeout as unsynchronized test seam (D).
  • Missing import Foundation in OSIamFetchReadyCondition.swift (D).

Dismissed

  • Broad “rewrite waiters with async/await” style preferences — out of scope; ObjC call site in OSMessagingController.
  • File-size / abstraction nits — not applicable here.

Agreement map

All four models independently flagged (1) cross-user resolve fan-out and (2) timeout leaving the bar raised. Three-plus also flagged the boolean/generation problem. Divergence was mostly on severity labels and on whether the edge-triggered resolve race is Act On vs Consider.

Skill: Cursor interrogate (pstack).

Open in Web View Automation 

Sent by Cursor Automation: Untitled

nan-li and others added 2 commits August 11, 2026 18:25
resolveConditionsWithID walked every index and cleared every matching
condition's subscription bar, so a response with no ryw_token for one
user could unblock — and disarm — another. Resolve now takes the
onesignal id and only touches that bucket.

A timed-out waiter only deregistered itself, leaving hasSubscriptionUpdatePending
up, so later IAM fetches for that id paid another full wait for a
subscription token that was never coming. Timeout now runs the same
onConditionSatisfied clear, if the waiter is still registered.

Co-authored-by: Cursor <cursoragent@cursor.com>
Cut hazard essays and cross-user narration down to short whys hitched to
the action, and trim the resolveConditions doc to the contract.

Co-authored-by: Cursor <cursoragent@cursor.com>
@nan-li
nan-li force-pushed the nan/jwt-pr2-consistency branch 2 times, most recently from 6aec0fa to c09d5a9 Compare August 12, 2026 05:40
@nan-li
nan-li changed the base branch from nan/jwt-pr1-path-encoding to main August 12, 2026 05:40
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.

1 participant