feat(replay): keep trigger groups on v2 when last group deleted#61738
Merged
Conversation
Deleting the last trigger group now replaces it with a record-everything group instead of leaving an empty config (which silently fell back to V1 and recorded nothing). Also trims the verbose V1/V2 compatibility banner and shows the legacy-migration button whenever legacy config exists. Generated-By: PostHog Code Task-Id: 38e0faab-250b-4b33-8b29-da5d9b5a7c4b
Contributor
|
ksvat
requested review from
a team,
TueHaulund,
arnohillen and
fasyy612
and removed request for
a team
ksvat
added
stamphog
Graphite App
Contributor
|
Size Change: -8.56 kB (-0.01%) Total Size: 81.9 MB 📦 View Changed
ℹ️ View Unchanged
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
No showstoppers found. The core change (substituting a "record all sessions" group instead of leaving zero groups and falling back to V1) is sound, well-documented in comments, and covered by new tests. The UI simplification and condition change are coherent with the new behavior.
Contributor
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
Contributor
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
arnohillen
approved these changes
Jun 4, 2026
Contributor
arnohillen
left a comment
arnohillen
left a comment
There was a problem hiding this comment.
lgtm! a test is failing fyi
Contributor
Author
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
2 participants
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.
Problem
Session replay's V2 trigger groups carry an overloaded "empty" state. A team is treated as "on V2" only while
session_recording_trigger_groupsis non-null, but the meaning of an empty/zero-group config was ambiguous:null→ backend sendsversion: 1and the legacy fields (records everything via V1's permissive default).{version: 2, groups: [...]}→ V2, conditional recording.{version: 2, groups: []}→ a truthy dict, so the backend sentversion: 2with no groups → the SDK recorded nothing. But the delete-last-group modal told the user it would "revert to legacy recording triggers." So deleting the last group was silently inconsistent with its own copy.Because "no groups" was overloaded to mean "fall back to V1," a team that removed its last group was pushed back onto the V1 code path — which is exactly what keeps V1 impossible to retire. The settings UI also carried a verbose three-bullet V1/V2 compatibility banner that duplicated the dedicated "Legacy recording conditions" note below it.
Changes
{groups: []}or dropping tonull. Recording behavior is unchanged (record all), but the config stays in V2 (version: 2) so the legacy V1 fields become dead weight rather than something the team relies on.{groups: []}is never produced;nullkeeps meaning only "never adopted V2."legacyTriggersPreviewselector.hasLegacyTriggers) rather than only when there are zero groups — otherwise, with sticky-delete keeping at least one group, the button would become permanently unreachable after adoption.Backend changes to stop emitting the V1 fields once a team's posthog-js floor is ≥ the trigger-groups minimum are intentionally out of scope here (that needs SDK-version detection) and are a follow-up.
This is a frontend-only change; no backend, API, or schema changes.
How did you test this code?
Clicked around -- deleting final group now gives a warning/placeholder group
replayTriggersV2Logic.test.ts— extended with cases for the sticky delete: deleting the only group yields a single record-everything group (nevernull/{groups: []}); deleting one of several keeps the rest. 6/6 pass.tsc --noEmit— clean for the changed files (kea logic types regenerated).oxlint+oxfmt— clean on the changed files.Manual verification in the running app (banner copy, the "Record all sessions" modal flow, and confirming
session_recording_trigger_groupsstays{version: 2, groups: [<record-all>]}after deleting the last group) is still recommended before merge.Automatic notifications
🤖 Agent context
Authored with PostHog Code (Claude Opus). The work started as "only show V2 to teams that never used older SDKs," but during exploration we found the more impactful issue was the empty-state ambiguity in the V2 trigger-groups data model and its delete-last-group footgun.
Key decisions made along the way:
null(matching the old modal copy), but reversed course — that re-introduces the V1 dependency and works against retiring V1. Sticky-delete (convert to record-all stub) keeps the team on V2.Agent-authored; requires human review.
Created with PostHog Code