Skip to content

fix(flags): drop flag loads in flight across reset() - #1048

Merged
turnipdabeets merged 3 commits into
mainfrom
fix/drop-flag-loads-across-reset
Nov 9, 2026
Merged

turnipdabeets merged 3 commits into
mainfrom
fix/drop-flag-loads-across-reset

Conversation

@turnipdabeets

@turnipdabeets turnipdabeets commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

sdk-specs requires that a flag load in flight when reset() or close() runs never reaches a listener and isn't replayed to one registered later (on-feature-flags, "Loads discarded by reset or close reach no listener", from PostHog/sdk-specs#161). iOS met this for close() but not reset().

reset() cleared the flag cache and queued a reload for the new identity, but the /flags request already in flight for the previous identity had no staleness check. When it completed it wrote the previous user's flags into the cache, notified onFeatureFlags listeners and posted didReceiveFeatureFlags, until the queued reload replaced them. Listeners and getters briefly saw the previous user's flags, and a listener registered in that window got them replayed.

Changes

PostHogRemoteConfig keeps a reset generation, bumped by clear() (called from reset()) under featureFlagsLock:

  • A load records the generation before reading the identity. A response from an older generation is dropped: it doesn't write the cache, isn't sent to onFeatureFlagsLoaded, doesn't post didReceiveFeatureFlags, and so is never stored for replay to a late listener. The check is repeated under the cache lock, so a reset() that lands mid-response can't be undone.
  • A flags-loaded notification already queued on main is also dropped if a reset() ran since it was sent.
  • The reload reset() queued still runs. A reloadFeatureFlags callback of the dropped load gets that reload's flags instead, or nil if no reload is queued (the callback already takes an optional, so there's no API change).
  • A queued request that read the old identity can't replace the queued post-reset one.

close() is unchanged (it already drops these). No public API change; patch changeset added.

Merging into 4.0

When main is next merged into chore/v4-remove-deprecated, this conflicts in PostHog/PostHogRemoteConfig.swift: 4.0 passes a FeatureFlagsLoadResult to the reload callback (#913) and no longer posts didReceiveFeatureFlags (#965). Keep the generation checks and adapt the callback to the 4.0 type. With no reload queued, the 4.0 callback should get a failed result (errorsLoading: true) with the cleared flags.

💚 How did you test it?

New tests in PostHogFeatureFlagsTest / TestOnFeatureFlags, each holding the first /flags response until after reset():

  • "drops a load in flight across reset() and delivers the reload reset() started": the listener gets only the post-reset flags, the held response doesn't write the cache, and the held load's reloadFeatureFlags callback gets the post-reset flags.
  • "does not replay a load dropped by reset() to a listener registered afterwards": with the post-reset reload skipped, a listener registered afterwards gets nothing and the cache stays empty.
  • "does not post didReceiveFeatureFlags for a load dropped by reset()": with the post-reset reload skipped, no didReceiveFeatureFlags post happens while the previous user's flags are cached.

With the fix reverted, all three fail (7 issues: the listener, cache, callback and notification all see previous-user). With the fix:

  • make test filter="'PostHogFeatureFlagsTest'": 79 tests in 14 suites passed.
  • make test filter="'PostHogFeatureFlags|PostHogRemoteConfig|PostHogSDKTest|Survey'": 367 tests in 58 suites passed.
  • make lint: 0 serious violations (same 30 warnings as main).

iOS simulator (iPhone 17, iOS 26.4.1):

  • Unit tests: xcodebuild test-without-building hung after "unable to connect to com.apple.instruments.deviceservice.lockdown", before any test ran. So the bundle from xcodebuild build-for-testing -scheme PostHog -destination 'id=<udid>' -derivedDataPath <dd> was run directly in the booted simulator:

    xcrun simctl spawn <udid> "$(xcode-select -p)/Platforms/iPhoneSimulator.platform/Developer/Library/Xcode/Agents/xctest" \
      -XCTest PostHogTests.PostHogFeatureFlagsTest \
      <dd>/Build/Products/Testing-iphonesimulator/PostHogTests.xctest
    PostHogFeatureFlagsTest PostHogRemoteConfigTest
    This branch 79 tests in 14 suites passed 51 tests in 7 suites passed
    main's PostHogRemoteConfig.swift with the new tests the 3 new tests fail (7 issues, as above); the other 76 pass —
  • Example app: PostHogExample, changed locally (not committed) and pointed at a local stub server that holds /flags for previous-user for 3 s. It calls identify("previous-user"), then reset() 0.5 s later, with a listener registered and the flag read every second.

    Listener received Late listener received
    main previous-user, then new-user new-user
    This branch new-user only new-user

📝 Checklist

  • All commits are signed (see Commit signing)
  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm changeset to generate a changeset file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

  • Claude Code (Claude Opus 5.5).
  • The stale response is dropped inside PostHogRemoteConfig rather than only at listener delivery, so it never reaches the cache or flag getters either.

🤖 Generated with Claude Code

reset() cleared the flag cache and queued a reload for the new identity,
but a /flags request already in flight for the previous identity still
wrote its flags into the cache, notified onFeatureFlags listeners and
posted didReceiveFeatureFlags when it completed, until the queued reload
corrected it.

PostHogRemoteConfig now bumps a reset generation in clear() and drops a
response, or a queued notification, from an older generation: it doesn't
write the cache, reach listeners or post didReceiveFeatureFlags, so it
can't be replayed to a late listener either. The queued post-reset reload
still runs, and a reloadFeatureFlags callback of the dropped load gets
that reload's flags instead, or nil when no reload is queued.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

posthog-ios Compliance Report

Date: 2026-11-09T23:54:47.388965+00:00
Duration: 235234ms

⚠️ Some Tests Failed

46/47 tests passed, 1 failed


Capture Tests

⚠️ 29/30 tests passed, 1 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 3709ms
Format Validation.Event Has Uuid ✅ 451ms
Format Validation.Event Has Lib Properties ✅ 3132ms
Format Validation.Distinct Id Is String ✅ 2930ms
Format Validation.Token Is Present ✅ 2922ms
Format Validation.Custom Properties Preserved ✅ 2919ms
Format Validation.Event Has Timestamp ✅ 3012ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 2999ms
Retry Behavior.Retries On 503 ✅ 9209ms
Retry Behavior.Does Not Retry On 400 ✅ 4986ms
Retry Behavior.Does Not Retry On 401 ✅ 2560ms
Retry Behavior.Respects Retry After Header ✅ 7969ms
Retry Behavior.Implements Backoff ✅ 21845ms
Retry Behavior.Retries On 500 ✅ 9427ms
Retry Behavior.Retries On 502 ✅ 9324ms
Retry Behavior.Retries On 504 ✅ 9372ms
Retry Behavior.Max Retries Respected ❌ 15542ms
Deduplication.Generates Unique Uuids ✅ 3325ms
Deduplication.Preserves Uuid On Retry ✅ 9190ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 16347ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 8016ms
Deduplication.No Duplicate Events In Batch ✅ 3051ms
Deduplication.Different Events Have Different Uuids ✅ 2943ms
Compression.Sends Gzip When Enabled ✅ 2827ms
Batch Format.Uses Proper Batch Structure ✅ 2875ms
Batch Format.Flush With No Events Sends Nothing ✅ 297ms
Batch Format.Multiple Events Batched Together ✅ 3052ms
Error Handling.Does Not Retry On 403 ✅ 4825ms
Error Handling.Does Not Retry On 413 ✅ 4704ms
Error Handling.Retries On 408 ✅ 9269ms

Failures

retry_behavior.max_retries_respected

Expected 4 requests, got 5

Feature_Flags Tests

✅ 17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 3030ms
Request Payload.Flags Request Uses V2 Query Param ✅ 2866ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 2774ms
Request Payload.Flags Request Omits Authorization Header ✅ 2818ms
Request Payload.Token In Flags Body Matches Init ✅ 2870ms
Request Payload.Groups Round Trip ✅ 2868ms
Request Payload.Groups Default To Empty Object ✅ 2830ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 2755ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 2872ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 2873ms
Request Lifecycle.No Flags Request On Init Alone ✅ 89ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 2841ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 5366ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 2839ms
Retry Behavior.Retries Flags On 502 ✅ 2909ms
Retry Behavior.Retries Flags On 504 ✅ 2820ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 2967ms

@turnipdabeets
turnipdabeets marked this pull request as ready for review November 9, 2026 21:53
@turnipdabeets
turnipdabeets requested a review from a team as a code owner November 9, 2026 21:53
@veria-ai

veria-ai Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Fixed/addressed: 1 · PR risk: 0/10

Reviews

Comment thread PostHog/PostHogRemoteConfig.swift Resolved
Re-check the reset generation under sessionReplayLock before a /flags response
(success or quota-limited) applies its linked-flag decision, so a reset() that
lands between the early staleness check and the replay update can't re-enable
recording for the new identity with the previous user's flags.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@turnipdabeets
turnipdabeets enabled auto-merge (squash) November 9, 2026 23:36
@turnipdabeets
turnipdabeets merged commit 6d7e886 into main Nov 9, 2026
53 checks passed
@turnipdabeets
turnipdabeets deleted the fix/drop-flag-loads-across-reset branch November 9, 2026 23:55
turnipdabeets added a commit that referenced this pull request Nov 10, 2026
…cated

Brings in #1048 (drop flag loads in flight across reset()). PostHogRemoteConfig.swift
and its tests take the 4.0 resolution from #1044 (fix/v4-drop-flag-loads-across-reset),
which keeps 4.0's FeatureFlagsLoadResult callback and the removed didReceiveFeatureFlags.
PostHog.podspec stays deleted.
@turnipdabeets turnipdabeets mentioned this pull request Nov 12, 2026
6 of 18 tasks
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.

2 participants