Repository navigation
fix(flags): drop flag loads in flight across reset() - #1044
Closed
turnipdabeets wants to merge 1 commit into
Closed
turnipdabeets wants to merge 1 commit into
turnipdabeets wants to merge 1 commit into
Conversation
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 and notified onFeatureFlags listeners 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 or reach listeners, 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 result instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
posthog-ios Compliance ReportDate: 2026-11-09T17:52:36.624738+00:00 ✅ All Tests Passed!234/234 tests passed Capture_V1 Tests✅ 106/106 tests passed View Details
Capture_Ai_V1 Tests✅ 111/111 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
PR overviewThe PR updates remote feature-flag loading to discard in-flight responses that arrive after reset(). One issue remains open: a pre-reset response can still reactivate session replay after reset, even when its cache update and notification are discarded. This can allow screenshot capture based on the previous user's replay state, and the stale state can persist if the post-reset reload fails. Open issues (1)
Fixed/addressed: 0 · PR risk: 6/10 Reviews
|
Contributor
Author
|
Closing in favour of #1048, which makes this fix on |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| self.featureFlagsLock.withLock { | ||
| let isStale = self.featureFlagsLock.withLock { () -> Bool in | ||
| // Re-checked under the cache lock, so a reset() since the check above can't be undone. | ||
| guard self.featureFlagsResetGeneration == generation else { return true } |
There was a problem hiding this comment.
Medium: Replay activation remains outside the generation guard
This locked check runs after processSessionRecordingConfig, so if reset() finishes between the initial check and that call, the old response can restore the previous user's sessionReplayFlagActive = true even though its cache write and notification are discarded; isSessionReplayActive() trusts that state when allowing screenshot capture, and a failed post-reset reload leaves it unchanged. Make the generation check and replay-state mutation atomic with clear() in both successful and quota-limited paths, keeping exposure callbacks outside the locks.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 Motivation and Context
sdk-specs requires that a flag load in flight when
reset()orclose()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 forclose()but notreset(), and it's the only gap on the iOS On Feature Flags compliance row (PostHog/sdk-specs#187). posthog-flutter on iOS/macOS inherits it.reset()cleared the flag cache and queued a reload for the new identity, but the/flagsrequest already in flight for the previous identity had no staleness check. When it completed it wrote the previous user's flags into the cache and notifiedonFeatureFlagslisteners, 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
PostHogRemoteConfigkeeps a reset generation, bumped byclear()(called fromreset()) underfeatureFlagsLock, like posthog-android 4.0 and posthog-js:onFeatureFlagsLoaded, so it's never stored for replay to a late listener. The check is repeated under the cache lock, so areset()that lands mid-response can't be undone.reset()ran since it was sent.reset()queued still runs. AreloadFeatureFlags(_:)callback of the dropped load gets that reload's result instead. If no reload is queued, it gets a failed result (errorsLoading: true) with the current, cleared flags.close()is unchanged (it already drops these). The changeset gets a "Fix" line.💚 How did you test it?
New tests in
PostHogFeatureFlagsTest/TestOnFeatureFlags, each holding the first/flagsresponse until afterreset():reloadFeatureFlagscallback gets the post-reset flags.With the fix reverted, both fail (5 issues: the listener receives
previous-user, the cache holdsprevious-user, and so does the callback). With the fix:make test filter="'PostHogFeatureFlags|PostHogRemoteConfig|PostHogSDKTest|Survey'": 388 tests in 58 suites passed.make test(full): failures only inPostHogLogsQueueTest,PostHogPushNotificationTestandPostHogPropertiesSerializationTest(timeouts). Rerun alone, only onePostHogLogsQueueTestcase failed, and aPostHogLogsQueueTestcase fails the same way on the base branch.make lint: 0 serious violations.iOS simulator (iPhone 17):
xcodebuild build-for-testingsucceeded, butxcodebuild test-without-building -scheme PostHog -destination 'id=CEF9AA9B-6E3E-4A94-BCA4-892ED863F9C3' -only-testing:PostHogTests/PostHogFeatureFlagsTest -only-testing:PostHogTests/PostHogRemoteConfigTesthung three times with "The test runner hung before establishing connection", before any test ran. CI'stest-ios-simulator (unit tests)job ran both new tests on a simulator, and they passed.PostHogExample, changed locally (not committed) and pointed at a local stub server that holds/flagsforprevious-userfor 3 s. It callsidentify("previous-user"), thenreset()0.5 s later, with a listener registered and the flag read every second.previous-user, thennew-usernew-useronlynew-user📝 Checklist
If releasing new changes
.changeset/v4-major-release.md)🤖 Agent context
Autonomy: Human-driven (agent-assisted)
PostHogRemoteConfigrather than only at listener delivery (as posthog-android does), so it also never reaches the cache or flag getters.reloadFeatureFlags(_:)callback onto the post-reset reload was a choice: the alternative was answering it with the previous identity's flags or a failure.main) has the same gap; this PR only targets 4.0.🤖 Generated with Claude Code