From ea68f75d96dea7654af16c79612bb1c7da41c994 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Thu, 3 Sep 2026 18:37:57 +0000 Subject: [PATCH 1/2] fix(replay): gate event-trigger starts behind the replay checks A matching event trigger called start() directly, so it skipped the master switch, the project flag and the sampling decision that every other start path applies. An app that gates replay behind its own feature flag kept recording the users the flag excludes. start() then set startedWithAutomaticDisabled, so the recording counted as manually started and survived every later check. onEvent now reuses isRecordingPermittedForCurrentSession() before it starts, after it records the trigger activation, so the trigger only lifts the event-trigger gate. start() remembers an explicit start asked for while automatic replay is off. The trigger gate defers that start, so without this the manual intent is lost and the deferred recording is refused once the trigger matches. Generated-By: PostHog Desktop Task-Id: d4a23d7a-bccc-4e65-9061-9223a7937811 --- ...ession-replay-gate-event-trigger-starts.md | 6 + .../replay/PostHogReplayIntegration.kt | 18 ++- .../replay/PostHogReplayIntegrationTest.kt | 108 +++++++++++++++++- 3 files changed, 127 insertions(+), 5 deletions(-) create mode 100644 .changeset/session-replay-gate-event-trigger-starts.md diff --git a/.changeset/session-replay-gate-event-trigger-starts.md b/.changeset/session-replay-gate-event-trigger-starts.md new file mode 100644 index 000000000..0d98a620a --- /dev/null +++ b/.changeset/session-replay-gate-event-trigger-starts.md @@ -0,0 +1,6 @@ +--- +"posthog": patch +"posthog-android": patch +--- + +Fix: a session recording started by an event trigger now checks the same gates as every other start path. A matching event used to start recording even when `PostHogConfig.sessionReplay` was false, the project flag was off, or sampling excluded the session, so an app that gates replay behind its own feature flag recorded the users the flag excluded. The recording was also treated as manually started, so no later check stopped it. A recording that `PostHog.startSessionReplay` asked for while automatic replay is off still starts when the trigger matches. diff --git a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt index 1ebc1d138..2e8f63f26 100644 --- a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt +++ b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt @@ -2254,6 +2254,12 @@ public class PostHogReplayIntegration( override fun start(resumeCurrent: Boolean) { // Check if we should wait for event triggers before starting if (shouldWaitForEventTriggers()) { + // Remember an explicit start asked for while automatic replay is off. The trigger + // gate defers it, so without this the manual intent is lost and the deferred + // recording is refused as an automatic one once the trigger matches. + if (!config.sessionReplay) { + startedWithAutomaticDisabled = true + } val triggers = config.remoteConfigHolder?.getEventTriggers() config.logger.log( "[Session Replay] Event triggers configured. Integration will not start until any of these events are captured: $triggers", @@ -2306,7 +2312,7 @@ public class PostHogReplayIntegration( /** * Called when an event is captured. Checks if the event matches any configured triggers - * and starts session recording if so. + * and starts session recording if so, provided the other gates permit the session. */ override fun onEvent( event: String, @@ -2334,6 +2340,16 @@ public class PostHogReplayIntegration( synchronized(eventTriggersLock) { triggerActivatedSessionId = currentSessionId } + // A matched trigger only lifts the event-trigger gate. The master switch, the project + // flag and the sampling decision still decide, as on every other start path. Without + // this, an app that turns replay off records the users it excludes, and start() marks + // the recording as manually started, so no later check stops it. + if (!isRecordingPermittedForCurrentSession()) { + config.logger.log( + "[Session Replay] Event trigger matched: $event, but recording is not permitted for session $currentSessionId.", + ) + return + } config.logger.log("[Session Replay] Event trigger matched: $event. Starting replay for session $currentSessionId.") // Start the integration now that a trigger has matched start(resumeCurrent = true) diff --git a/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt b/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt index c7c7c0fa9..d1d5f24a9 100644 --- a/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt +++ b/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt @@ -188,12 +188,13 @@ internal class PostHogReplayIntegrationTest { flagActive: Boolean, samplingPasses: Boolean, sessionReplay: Boolean = true, + triggers: Set = emptySet(), ): PostHogAndroidConfig { val remoteConfig = mock { on { isSessionReplayFlagActive() } doReturn flagActive on { makeSamplingDecision(any()) } doReturn samplingPasses - on { getEventTriggers() } doReturn emptySet() + on { getEventTriggers() } doReturn triggers on { hasRemoteConfigFetched() } doReturn true } return PostHogAndroidConfig(API_KEY).apply { @@ -573,7 +574,7 @@ internal class PostHogReplayIntegrationTest { val config = PostHogAndroidConfig(API_KEY).apply { remoteConfigHolder = remoteConfig - sessionReplay = false + sessionReplay = true } val sut = getSut(config) val postHog = mock() @@ -585,8 +586,8 @@ internal class PostHogReplayIntegrationTest { shadowOf(Looper.getMainLooper()).idle() assertTrue(sut.isActive()) - // Rotating into a session the trigger has not matched must stop recording, and the - // preserved manual intent must not let remote config resume it behind the trigger gate. + // Rotating into a session the trigger has not matched must stop recording, and remote + // config must not resume it behind the trigger gate. PostHogSessionManager.endSession() PostHogSessionManager.startSession() sut.onSessionIdChanged() @@ -602,6 +603,105 @@ internal class PostHogReplayIntegrationTest { } } + @Test + fun `event trigger does not start recording for a session the other gates reject`() { + val triggers = setOf("checkout_started") + val cases = + listOf( + "master switch off" to + configWithSampling( + flagActive = true, + samplingPasses = true, + sessionReplay = false, + triggers = triggers, + ), + "project flag off" to configWithSampling(flagActive = false, samplingPasses = true, triggers = triggers), + "sampled out" to configWithSampling(flagActive = true, samplingPasses = false, triggers = triggers), + ) + + for ((name, config) in cases) { + val sut = getSut(config) + val postHog = mock() + whenever(postHog.getSessionId()).thenAnswer { PostHogSessionManager.peekSessionId() } + sut.install(postHog) + try { + PostHogSessionManager.startSession() + sut.onEvent("checkout_started", null) + shadowOf(Looper.getMainLooper()).idle() + + assertFalse(sut.isActive(), "started recording despite $name") + } finally { + sut.uninstall() + PostHogSessionManager.endSession() + } + } + } + + @Test + fun `event trigger starts a manually requested recording while the master switch is off`() { + val config = + configWithSampling( + flagActive = true, + samplingPasses = true, + sessionReplay = false, + triggers = setOf("checkout_started"), + ) + val sut = getSut(config) + val postHog = mock() + whenever(postHog.getSessionId()).thenAnswer { PostHogSessionManager.peekSessionId() } + sut.install(postHog) + try { + PostHogSessionManager.startSession() + // The trigger gate defers this start, but the intent must survive it. + sut.start(resumeCurrent = true) + shadowOf(Looper.getMainLooper()).idle() + assertFalse(sut.isActive()) + + sut.onEvent("checkout_started", null) + shadowOf(Looper.getMainLooper()).idle() + + assertTrue(sut.isActive()) + } finally { + sut.uninstall() + } + } + + @Test + fun `event trigger start refused by the master switch does not become a manual recording`() { + val config = + configWithSampling( + flagActive = true, + samplingPasses = true, + sessionReplay = false, + triggers = setOf("checkout_started"), + ) + val sut = getSut(config) + val postHog = mock() + whenever(postHog.getSessionId()).thenAnswer { PostHogSessionManager.peekSessionId() } + sut.install(postHog) + try { + PostHogSessionManager.startSession() + sut.onEvent("checkout_started", null) + shadowOf(Looper.getMainLooper()).idle() + assertFalse(sut.isActive()) + + // A refused start must not be treated as manually started, so the master switch keeps + // deciding on every later remote config delivery. + sut.onRemoteConfig() + shadowOf(Looper.getMainLooper()).idle() + assertFalse(sut.isActive()) + + // The trigger did fire though, so turning the switch back on records the rest of the session. + config.sessionReplay = true + sut.onSessionReplayConfigChanged() + shadowOf(Looper.getMainLooper()).idle() + + assertTrue(sut.isActive()) + } finally { + sut.uninstall() + } + } + @Test fun `clears buffer when PostHogReplayIntegration is installed`() { val config = PostHogAndroidConfig(API_KEY) From 3ad26f1b0e36a76d3c97367a65b56c7ac6d99ed4 Mon Sep 17 00:00:00 2001 From: Dustin Byrne Date: Fri, 4 Sep 2026 12:23:00 -0400 Subject: [PATCH 2/2] fix(replay): make deferred trigger starts cancellable --- ...ession-replay-gate-event-trigger-starts.md | 2 +- .../replay/PostHogReplayIntegration.kt | 42 +++--- .../replay/PostHogReplayIntegrationTest.kt | 140 +++++++++++++++--- posthog/src/main/java/com/posthog/PostHog.kt | 9 +- .../src/test/java/com/posthog/PostHogTest.kt | 11 ++ 5 files changed, 151 insertions(+), 53 deletions(-) diff --git a/.changeset/session-replay-gate-event-trigger-starts.md b/.changeset/session-replay-gate-event-trigger-starts.md index 0d98a620a..e629e7766 100644 --- a/.changeset/session-replay-gate-event-trigger-starts.md +++ b/.changeset/session-replay-gate-event-trigger-starts.md @@ -3,4 +3,4 @@ "posthog-android": patch --- -Fix: a session recording started by an event trigger now checks the same gates as every other start path. A matching event used to start recording even when `PostHogConfig.sessionReplay` was false, the project flag was off, or sampling excluded the session, so an app that gates replay behind its own feature flag recorded the users the flag excluded. The recording was also treated as manually started, so no later check stopped it. A recording that `PostHog.startSessionReplay` asked for while automatic replay is off still starts when the trigger matches. +Fix: a session recording started by an event trigger now checks the same gates as every other start path. A matching event used to start recording even when `PostHogConfig.sessionReplay` was false, the project flag was off, or sampling excluded the session, so an app that gates replay behind its own feature flag recorded the users the flag excluded. A manual start can still wait for a matching event, and `PostHog.stopSessionReplay` cancels that pending request. diff --git a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt index 2e8f63f26..52795fdf2 100644 --- a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt +++ b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt @@ -180,9 +180,9 @@ public class PostHogReplayIntegration( @Volatile private var isSessionReplayActive: Boolean = false - // Set by any start that happens while config.sessionReplay is false — the manual API or an - // event trigger. Survives stopRecording() so an internal stop (session cleared, sampled out, - // flag off) can still resume later; only an explicit stop() or uninstall() clears it. + // Set only by an explicit start request made while config.sessionReplay is false. It survives + // stopRecording() so an internal stop (session cleared, sampled out, flag off) can still resume + // later; only an explicit stop() or uninstall() clears it. @Volatile private var startedWithAutomaticDisabled: Boolean = false @@ -2252,14 +2252,13 @@ public class PostHogReplayIntegration( } override fun start(resumeCurrent: Boolean) { - // Check if we should wait for event triggers before starting + // Remember an explicit start asked for while automatic replay is off. The event gate can + // defer it, so the intent must be recorded before checking that gate. + if (!config.sessionReplay) { + startedWithAutomaticDisabled = true + } + if (shouldWaitForEventTriggers()) { - // Remember an explicit start asked for while automatic replay is off. The trigger - // gate defers it, so without this the manual intent is lost and the deferred - // recording is refused as an automatic one once the trigger matches. - if (!config.sessionReplay) { - startedWithAutomaticDisabled = true - } val triggers = config.remoteConfigHolder?.getEventTriggers() config.logger.log( "[Session Replay] Event triggers configured. Integration will not start until any of these events are captured: $triggers", @@ -2267,10 +2266,13 @@ public class PostHogReplayIntegration( return } + startRecording(resumeCurrent) + } + + private fun startRecording(resumeCurrent: Boolean) { val currentSessionId = postHog?.getSessionId()?.toString() resetSessionStateIfNeeded(currentSessionId, force = !resumeCurrent) - startedWithAutomaticDisabled = !config.sessionReplay isSessionReplayActive = true if (!resumeCurrent) { @@ -2341,9 +2343,7 @@ public class PostHogReplayIntegration( triggerActivatedSessionId = currentSessionId } // A matched trigger only lifts the event-trigger gate. The master switch, the project - // flag and the sampling decision still decide, as on every other start path. Without - // this, an app that turns replay off records the users it excludes, and start() marks - // the recording as manually started, so no later check stops it. + // flag and the sampling decision still decide, as on every other automatic start path. if (!isRecordingPermittedForCurrentSession()) { config.logger.log( "[Session Replay] Event trigger matched: $event, but recording is not permitted for session $currentSessionId.", @@ -2351,8 +2351,8 @@ public class PostHogReplayIntegration( return } config.logger.log("[Session Replay] Event trigger matched: $event. Starting replay for session $currentSessionId.") - // Start the integration now that a trigger has matched - start(resumeCurrent = true) + // Do not call start(): only an explicit request may establish manual-start provenance. + startRecording(resumeCurrent = true) } } @@ -2413,7 +2413,8 @@ public class PostHogReplayIntegration( return@post } if (isSessionReplayActive) stopRecording() - start(resumeCurrent = false) + // Do not call start(): session rotation is an automatic transition. + startRecording(resumeCurrent = false) } } @@ -2667,15 +2668,16 @@ public class PostHogReplayIntegration( if (!isSessionReplayActive) { config.logger.log("[Session Replay] Remote config enabled recording. Resuming.") mainHandler.handler.post { - if (!isSessionReplayActive) { + // Re-check at the transition so a queued automatic resume cannot use stale gates. + if (!isSessionReplayActive && isRecordingPermittedForCurrentSession()) { // Force a fresh keyframe for the resumed segment. While stopped, per-view snapshot // state is frozen and can reference a full snapshot that was never delivered (e.g. a // first-config-off opening window that was dropped), so resuming against it would emit // orphaned incremental snapshots the player can't anchor. Clear the state and force a // redraw so the resumed segment starts with meta + full snapshot — without rotating the - // session or touching the cold-start buffering state (unlike start(resumeCurrent = false)). + // session or touching the cold-start buffering state (unlike startRecording(false)). clearSnapshotStates() - start(resumeCurrent = true) + startRecording(resumeCurrent = true) synchronized(decorViews) { decorViews.keys.forEach { it.postInvalidate() } } diff --git a/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt b/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt index d1d5f24a9..b4a36c25f 100644 --- a/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt +++ b/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt @@ -604,32 +604,44 @@ internal class PostHogReplayIntegrationTest { } @Test - fun `event trigger does not start recording for a session the other gates reject`() { - val triggers = setOf("checkout_started") + fun `event trigger starts only when all replay gates pass`() { + data class GateCase( + val name: String, + val localEnabled: Boolean = true, + val flagActive: Boolean = true, + val samplingPasses: Boolean = true, + val triggerMatches: Boolean = true, + val expectedActive: Boolean = false, + ) + val cases = listOf( - "master switch off" to - configWithSampling( - flagActive = true, - samplingPasses = true, - sessionReplay = false, - triggers = triggers, - ), - "project flag off" to configWithSampling(flagActive = false, samplingPasses = true, triggers = triggers), - "sampled out" to configWithSampling(flagActive = true, samplingPasses = false, triggers = triggers), + GateCase("flag off, trigger not matched", flagActive = false, triggerMatches = false), + GateCase("flag off, trigger matched", flagActive = false), + GateCase("flag on, trigger not matched", triggerMatches = false), + GateCase("all gates pass", expectedActive = true), + GateCase("local switch off", localEnabled = false), + GateCase("sampled out", samplingPasses = false), ) - for ((name, config) in cases) { + for (case in cases) { + val config = + configWithSampling( + flagActive = case.flagActive, + samplingPasses = case.samplingPasses, + sessionReplay = case.localEnabled, + triggers = setOf("checkout_started"), + ) val sut = getSut(config) val postHog = mock() whenever(postHog.getSessionId()).thenAnswer { PostHogSessionManager.peekSessionId() } sut.install(postHog) try { PostHogSessionManager.startSession() - sut.onEvent("checkout_started", null) + sut.onEvent(if (case.triggerMatches) "checkout_started" else "product_viewed", null) shadowOf(Looper.getMainLooper()).idle() - assertFalse(sut.isActive(), "started recording despite $name") + assertEquals(case.expectedActive, sut.isActive(), case.name) } finally { sut.uninstall() PostHogSessionManager.endSession() @@ -667,7 +679,52 @@ internal class PostHogReplayIntegrationTest { } @Test - fun `event trigger start refused by the master switch does not become a manual recording`() { + fun `event trigger activation is retained when another gate initially rejects recording`() { + val triggers = setOf("checkout_started") + val localConfig = + configWithSampling( + flagActive = true, + samplingPasses = true, + sessionReplay = false, + triggers = triggers, + ) + val linkedFlag = AtomicBoolean(false) + val linkedFlagConfig = configWithSampling(flagActive = false, samplingPasses = true, triggers = triggers) + whenever(linkedFlagConfig.remoteConfigHolder!!.isSessionReplayFlagActive()).thenAnswer { linkedFlag.get() } + val sampling = AtomicBoolean(false) + val samplingConfig = configWithSampling(flagActive = true, samplingPasses = false, triggers = triggers) + whenever(samplingConfig.remoteConfigHolder!!.makeSamplingDecision(any())).thenAnswer { sampling.get() } + val cases = + listOf( + Triple("master switch", localConfig) { localConfig.sessionReplay = true }, + Triple("linked flag", linkedFlagConfig) { linkedFlag.set(true) }, + Triple("sampling", samplingConfig) { sampling.set(true) }, + ) + + for ((name, config, openGate) in cases) { + val sut = getSut(config) + val postHog = mock() + whenever(postHog.getSessionId()).thenAnswer { PostHogSessionManager.peekSessionId() } + sut.install(postHog) + try { + PostHogSessionManager.startSession() + sut.onEvent("checkout_started", null) + assertFalse(sut.isActive(), "started before the $name gate opened") + + openGate() + sut.onRemoteConfig() + shadowOf(Looper.getMainLooper()).idle() + + assertTrue(sut.isActive(), "did not reuse trigger activation after the $name gate opened") + } finally { + sut.uninstall() + PostHogSessionManager.endSession() + } + } + } + + @Test + fun `explicit stop cancels a manual start waiting for an event trigger`() { val config = configWithSampling( flagActive = true, @@ -681,23 +738,58 @@ internal class PostHogReplayIntegrationTest { sut.install(postHog) try { PostHogSessionManager.startSession() - sut.onEvent("checkout_started", null) - shadowOf(Looper.getMainLooper()).idle() + sut.start(resumeCurrent = true) assertFalse(sut.isActive()) - // A refused start must not be treated as manually started, so the master switch keeps - // deciding on every later remote config delivery. - sut.onRemoteConfig() + sut.stop() + sut.onEvent("checkout_started", null) shadowOf(Looper.getMainLooper()).idle() + assertFalse(sut.isActive()) + } finally { + sut.uninstall() + } + } - // The trigger did fire though, so turning the switch back on records the rest of the session. - config.sessionReplay = true - sut.onSessionReplayConfigChanged() - shadowOf(Looper.getMainLooper()).idle() + @Test + fun `concurrent local disable does not give an event trigger manual start provenance`() { + val samplingStarted = CountDownLatch(1) + val continueSampling = CountDownLatch(1) + val config = + configWithSampling( + flagActive = true, + samplingPasses = true, + triggers = setOf("checkout_started"), + ) + whenever(config.remoteConfigHolder!!.makeSamplingDecision(any())).thenAnswer { + samplingStarted.countDown() + assertTrue(continueSampling.await(2, TimeUnit.SECONDS)) + true + } + val sut = getSut(config) + val postHog = mock() + whenever(postHog.getSessionId()).thenAnswer { PostHogSessionManager.peekSessionId() } + sut.install(postHog) + val eventExecutor = Executors.newSingleThreadExecutor() + try { + PostHogSessionManager.startSession() + val event = eventExecutor.submit { sut.onEvent("checkout_started", null) } + assertTrue(samplingStarted.await(2, TimeUnit.SECONDS)) + config.sessionReplay = false + continueSampling.countDown() + event.get(2, TimeUnit.SECONDS) assertTrue(sut.isActive()) + + // The racing automatic start may win, but it must remain automatic so the next + // reevaluation can stop it instead of preserving it as a manual recording. + sut.onRemoteConfig() + shadowOf(Looper.getMainLooper()).idle() + + assertFalse(sut.isActive()) } finally { + continueSampling.countDown() + eventExecutor.shutdownNow() sut.uninstall() } } diff --git a/posthog/src/main/java/com/posthog/PostHog.kt b/posthog/src/main/java/com/posthog/PostHog.kt index 7b5ecd004..d325ec780 100644 --- a/posthog/src/main/java/com/posthog/PostHog.kt +++ b/posthog/src/main/java/com/posthog/PostHog.kt @@ -2209,14 +2209,7 @@ public class PostHog private constructor( return } - sessionReplayHandler?.let { - // already inactive - if (!it.isActive()) { - return - } - - it.stop() - } ?: run { + sessionReplayHandler?.stop() ?: run { config?.logger?.log("Session replay isn't installed.") } } diff --git a/posthog/src/test/java/com/posthog/PostHogTest.kt b/posthog/src/test/java/com/posthog/PostHogTest.kt index 6376d21bf..5f8ad4f8e 100644 --- a/posthog/src/test/java/com/posthog/PostHogTest.kt +++ b/posthog/src/test/java/com/posthog/PostHogTest.kt @@ -2756,6 +2756,17 @@ internal class PostHogTest { assertTrue(integration.stopCalled) } + @Test + fun `stopSessionReplay forwards stop when replay is inactive`() { + val http = mockHttp() + val integration = PostHogSessionReplayHandlerFake(false) + val sut = getSut(http.url("/").toString(), preloadFeatureFlags = false, integration = integration) + + sut.stopSessionReplay() + + assertTrue(integration.stopCalled) + } + @Test @Suppress("DEPRECATION") fun `captureException captures exception with correct properties`() {