fix: session rotation was relying on in-memory values and so could get out of sync with reality

Author: pauldambra

.changeset/calm-coats-brake.md

@@ -0,0 +1,5 @@
+---
+'posthog-js': patch
+---
+
+fix: session id rotation relied on in-memory cache which would be stale after log idle periods - particularly with multiple windows in play

packages/browser/playwright/mocked/session-recording/session-recording-idle-timeout.spec.ts

@@ -146,17 +146,127 @@ test.describe('Session recording - idle timeout behavior', () => {
             ph?.capture('test_after_idle_restart')
         })
 
-        await page.expectCapturedEventsToBe(['$snapshot', '$snapshot', 'test_after_idle_restart'])
         const capturedEvents = await page.capturedEvents()
+        const snapshots = capturedEvents.filter((e) => e.event === '$snapshot')
+        const testEvent = capturedEvents.find((e) => e.event === 'test_after_idle_restart')
 
-        expect(capturedEvents[0]['properties']['$session_id']).toEqual(initialSessionId)
-        expect(getSnapshotTimestamp(capturedEvents[0], 'last')).toBeLessThan(timestampAfterRestart)
+        // Should have at least 2 snapshots (old session final, new session data)
+        expect(snapshots.length).toBeGreaterThanOrEqual(2)
 
-        expect(capturedEvents[1]['properties']['$session_id']).toEqual(newSessionId)
-        expect(getSnapshotTimestamp(capturedEvents[1], 'first')).toBeGreaterThan(timestampBeforeIdle)
+        // First snapshot should be old session final data
+        const oldSessionSnapshots = snapshots.filter((s) => s['properties']['$session_id'] === initialSessionId)
+        expect(oldSessionSnapshots.length).toBeGreaterThanOrEqual(1)
+        expect(getSnapshotTimestamp(oldSessionSnapshots[0], 'last')).toBeLessThan(timestampAfterRestart)
 
-        expect(capturedEvents[2]['properties']['$session_id']).toEqual(newSessionId)
-        expect(capturedEvents[2]['properties']['$session_recording_start_reason']).toEqual('session_id_changed')
+        // New session snapshots should exist
+        const newSessionSnapshots = snapshots.filter((s) => s['properties']['$session_id'] === newSessionId)
+        expect(newSessionSnapshots.length).toBeGreaterThanOrEqual(1)
+        expect(getSnapshotTimestamp(newSessionSnapshots[0], 'first')).toBeGreaterThan(timestampBeforeIdle)
+
+        // Test event should be on new session with correct start reason
+        expect(testEvent?.['properties']['$session_id']).toEqual(newSessionId)
+        expect(testEvent?.['properties']['$session_recording_start_reason']).toEqual('session_id_changed')
+    })
+
+    test('rotates session when event timestamp shows idle timeout exceeded (frozen tab scenario)', async ({ page }) => {
+        // This tests the scenario where:
+        // 1. A browser tab is frozen/backgrounded for a long time
+        // 2. The forcedIdleReset timer never fires (because JS timers don't run when tab is frozen)
+        // 3. When the tab unfreezes, rrweb emits events with timestamps far in the future
+        // 4. We should detect this via timestamp-based idle detection and rotate the session
+
+        // Start recording normally
+        await ensureActivitySendsSnapshots(page)
+
+        const initialSessionId = await page.evaluate(() => {
+            const ph = (window as WindowWithPostHog).posthog
+            return ph?.get_session_id()
+        })
+        expect(initialSessionId).toBeDefined()
+
+        await page.resetCapturedEvents()
+
+        // Simulate "frozen tab" scenario:
+        // Make the session appear to have been inactive for 35+ minutes
+        // by manipulating the lastActivityTimestamp in persistence and clearing the in-memory cache
+        // This simulates what happens when a tab is frozen and the forcedIdleReset timer never fires
+        await page.evaluate(() => {
+            const ph = (window as WindowWithPostHog).posthog
+            const persistence = ph?.persistence as any
+            const sessionManager = ph?.sessionManager as any
+
+            if (!persistence) {
+                throw new Error('Persistence not available')
+            }
+
+            if (!sessionManager) {
+                throw new Error('SessionManager not available')
+            }
+
+            // Get current session data (stored as [lastActivityTimestamp, sessionId, sessionStartTimestamp])
+            const sessionIdKey = '$sesid'
+            const currentSessionData = persistence.props[sessionIdKey]
+
+            if (!currentSessionData) {
+                throw new Error('Session data not found')
+            }
+
+            // Set the lastActivityTimestamp to 35 minutes ago
+            // This simulates a frozen tab where no activity was recorded
+            const thirtyFiveMinutesAgo = Date.now() - 35 * 60 * 1000
+            currentSessionData[0] = thirtyFiveMinutesAgo
+
+            // Write back the modified session data
+            persistence.register({ [sessionIdKey]: currentSessionData })
+
+            // Also clear the session manager's in-memory cache so it reads from persistence
+            // This simulates what happens when a tab unfreezes and state needs to be re-read
+            sessionManager._sessionActivityTimestamp = null
+        })
+
+        // Now trigger user activity
+        // This should detect that the session has been idle too long and rotate
+        await page.waitingForNetworkCausedBy({
+            urlPatternsToWaitFor: ['**/ses/*'],
+            action: async () => {
+                await page.locator('[data-cy-input]').type('activity after simulated freeze!')
+            },
+        })
+
+        const newSessionId = await page.evaluate(() => {
+            const ph = (window as WindowWithPostHog).posthog
+            return ph?.get_session_id()
+        })
+
+        // The session should have rotated because we exceeded the idle timeout
+        expect(newSessionId).not.toEqual(initialSessionId)
+
+        // Capture all snapshot data to see exactly what happened
+        const capturedEvents = await page.capturedEvents()
+        const snapshots = capturedEvents.filter((e) => e.event === '$snapshot')
+
+        // Collapse to essential fields: session_id, type, tag (for custom events), timestamp
+        const snapshotSummary = snapshots.flatMap((snapshot) => {
+            const sessionId = snapshot['properties']['$session_id']
+            const snapshotData = snapshot['properties']['$snapshot_data'] as any[]
+            return snapshotData.map((event) => ({
+                sessionId: sessionId === initialSessionId ? 'initial' : sessionId === newSessionId ? 'new' : 'unknown',
+                type: event.type,
+                tag: event.data?.tag || null,
+                timestamp: event.timestamp,
+            }))
+        })
+
+        // The key assertion: bootup events ($session_options, $posthog_config, $remote_config_received)
+        // should be on the NEW session, not the initial one
+        const bootupEventsOnInitialSession = snapshotSummary.filter(
+            (e) =>
+                e.sessionId === 'initial' &&
+                e.type === 5 && // CustomEvent type
+                ['$session_options', '$posthog_config', '$remote_config_received'].includes(e.tag)
+        )
+
+        expect(bootupEventsOnInitialSession).toEqual([])
     })
 })
 

packages/browser/src/__tests__/sessionid.test.ts

@@ -524,6 +524,52 @@ describe('Session ID manager', () => {
         })
     })
 
+    describe('persistence is source of truth over in-memory cache', () => {
+        // This test verifies that when persistence is updated (e.g., by another tab or after a frozen tab thaws),
+        // the session manager reads from persistence rather than trusting stale in-memory cache
+
+        const memoryConfig = {
+            persistence_name: 'test-session-memory',
+            persistence: 'memory',
+            token: 'test-token',
+        } as PostHogConfig
+
+        it.each([
+            { description: 'with stale timestamp from simulated frozen tab', offsetMs: 1000 },
+            { description: 'with exactly expired timestamp', offsetMs: 1 },
+        ])('should detect activity timeout $description', ({ offsetMs }) => {
+            const realPersistence = new PostHogPersistence(memoryConfig)
+            const testTimestamp = 1603107479471
+
+            const sessionIdManager = new SessionIdManager(
+                createMockPostHog({
+                    config: memoryConfig,
+                    persistence: realPersistence,
+                    register: jest.fn(),
+                }),
+                () => 'newUUID',
+                () => 'newUUID'
+            )
+
+            // First call establishes the session
+            const firstResult = sessionIdManager.checkAndGetSessionAndWindowId(false, testTimestamp)
+            expect(firstResult.sessionId).toBe('newUUID')
+
+            // Simulate persistence being updated externally to have a stale timestamp
+            // In a frozen tab scenario, another tab might have updated persistence,
+            // or time passed while the tab was frozen
+            const staleTimestamp = testTimestamp - (DEFAULT_SESSION_IDLE_TIMEOUT_SECONDS * 1000 + offsetMs)
+            realPersistence.register({ [SESSION_ID]: [staleTimestamp, 'oldSessionID', staleTimestamp] })
+
+            // Second call should read from persistence and detect the activity timeout
+            const secondResult = sessionIdManager.checkAndGetSessionAndWindowId(false, testTimestamp)
+
+            // The session SHOULD rotate because persistence shows idle timeout exceeded
+            expect(secondResult.changeReason?.activityTimeout).toBe(true)
+            expect(secondResult.sessionId).not.toBe('oldSessionID')
+        })
+    })
+
     describe('destroy()', () => {
         it('clears the idle timeout timer', () => {
             jest.useFakeTimers()

packages/browser/src/extensions/replay/external/lazy-loaded-session-recorder.ts

@@ -687,6 +687,7 @@ export class LazyLoadedSessionRecording implements LazyLoadedSessionRecordingInt
         }
 
         // We want to ensure the sessionManager is reset if necessary on loading the recorder
+        // This is idempotent - if session already rotated, it returns the same session ID
         const { sessionId, windowId } = this._sessionManager.checkAndGetSessionAndWindowId()
         this._sessionId = sessionId
         this._windowId = windowId
@@ -794,15 +795,20 @@ export class LazyLoadedSessionRecording implements LazyLoadedSessionRecordingInt
     private _onSessionIdCallback: SessionIdChangedCallback = (sessionId, windowId, changeReason) => {
         if (!changeReason) return
 
+        // Skip if session hasn't actually changed (callback might fire multiple times)
+        if (sessionId === this._sessionId && windowId === this._windowId) {
+            return
+        }
+
         const wasLikelyReset = changeReason.noSessionId
         const shouldLinkSessions =
             !wasLikelyReset && (changeReason.activityTimeout || changeReason.sessionPastMaximumLength)
 
-        let oldSessionId, oldWindowId
+        // Capture old IDs before start() updates them
+        const oldSessionId = this._sessionId
+        const oldWindowId = this._windowId
 
         if (shouldLinkSessions) {
-            oldSessionId = this._sessionId
-            oldWindowId = this._windowId
             this._tryAddCustomEvent('$session_ending', {
                 nextSessionId: sessionId,
                 nextWindowId: windowId,
@@ -823,9 +829,15 @@ export class LazyLoadedSessionRecording implements LazyLoadedSessionRecordingInt
 
         this._clearConditionalRecordingPersistence()
 
-        if (!this._stopRrweb) {
-            this.start('session_id_changed')
+        // Restart recording to ensure new session gets proper bootup events
+        // If rrweb IS running, we need to stop and restart to flush the old buffer
+        // and emit fresh bootup events for the new session
+        if (this._stopRrweb) {
+            this.stop()
         }
+        // start() calls checkAndGetSessionAndWindowId() which is idempotent -
+        // session already rotated so it will return the same new session ID
+        this.start('session_id_changed')
 
         if (shouldLinkSessions) {
             this._tryAddCustomEvent('$session_starting', {

packages/browser/src/sessionid.ts

@@ -180,9 +180,9 @@ export class SessionIdManager {
     }
 
     private _getSessionId(): [number, string, number] {
-        if (this._sessionId && this._sessionActivityTimestamp && this._sessionStartTimestamp) {
-            return [this._sessionActivityTimestamp, this._sessionId, this._sessionStartTimestamp]
-        }
+        // Always read from persistence - it's the source of truth
+        // The in-memory cache could become stale (e.g., in a frozen tab scenario where
+        // time passes but the cache isn't updated)
         const sessionIdInfo = this._persistence.props[SESSION_ID]
 
         if (isArray(sessionIdInfo) && sessionIdInfo.length === 2) {