Repository navigation
fix(replay): skip unpatchable adopted stylesheet properties - #5617
Merged
Merged
Conversation
Contributor
Replay incident risk checkThis diff touches code involved in past incidents. This is a heads-up, not a verdict: read the matched sections of INCIDENTS.md and answer their review questions before merging. For a judgment on whether this diff has the same failure mode, run the |
marandaneto
marked this pull request as ready for review
November 8, 2026 11:33
marandaneto
requested review from
TueHaulund,
fasyy612 and
ksvat
and removed request for
a team
November 8, 2026 11:33
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed e05f338: 2 Should fix.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
Contributor
Coherence📄 0 spec file errors · ⛔ 0 broken rules · 🚪 0 unchecked inputs · 🐛 0 bugs that came back What is Coherence?Coherence compares the code with its spec files ( This check read 44 parts of the code and 7 rules. Tip Nothing to fix. |
Contributor
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
Contributor
|
Size Change: +24.3 kB (+0.09%) Total Size: 26.5 MB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
SDK performance: posthog-nodeBenchmarks completed. No changes flagged for review. Flags require at least a 5% change in the median and all six pairs agreeing on direction. This is a noise filter, not proof of a performance change or a CI gate. View all results
Values are medians of paired measurements. Not flagged does not mean unchanged. Unavailable means a measurement could not be provided, not zero. Estimated allocation measurements retain their instrumentation uncertainty. RSS measures the whole process, not just SDK allocations; observed peaks are sampled checkpoints. Paired ranges in the diagnostics describe variability, not confidence intervals. Each shard uses its own runner; compare Before PR against With PR within a row, not across shards. Compared 102 metric rows; 102 rows shown in all results. Omitted or shortened data remain in |
Member
Author
Live PostHog US validationValidated commit Result: all 8 scenarios passed. PostHog MCP independently confirmed 8 ingested recordings and 10 passing validation-result events (the overlapping-recorder cases each use two SDK instances).
Assertions and backend evidence
Method: Real Brave with a fresh temporary test profile and default sandbox. Shields were disabled only in that disposable profile to permit PostHog traffic; the sample explicitly enabled automated-user-agent capture. No project settings were changed. The full SDK was built from the PR worktree using Scope: This validates browser lifecycle behavior and backend ingestion, not manual playback or inspection of individual stored snapshot bodies. Locked-property cases intentionally cannot observe later stylesheet assignments on the unpatchable host; they verify preserved native behavior and continued recording elsewhere. Short cases can display a rounded duration of zero seconds despite containing replay data and recorded clicks. Final run identifier: |
ioannisj
approved these changes
Nov 9, 2026
ioannisj
left a comment
ioannisj
left a comment
Contributor
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
No findings from me. I went through the install and cleanup descriptor handling, the overlapping-observer stop orders, the sentinel through chained setters, and the new unit and browser tests, and the fixes from the earlier threads are all in at this head.
TueHaulund
approved these changes
Nov 9, 2026
marandaneto
enabled auto-merge
November 9, 2026 15:49
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Nov 9, 2026
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.
Problem
Session recording can throw
TypeError: Cannot redefine property: adoptedStyleSheetswhen another script locks that property on a document or shadow root. Document setup contains the error, but shadow-root setup runs in a timer and lets it escape to the page.Fixes #5613.
Changes
An unpatchable host keeps its existing page behavior. Later stylesheet assignments on that host cannot be observed, but other recording continues. No public API, stylesheet serialization, sampling, or session controls change. This follows the SDK spec's requirement to contain replay setup failures.
Validation
efcfba394): wrapper/locked-hook unit tests reproduced post-stop recording; overlapping-observer tests reproduced restoring disposed hooks. Both document and shadow-root Brave regressions emitted one extra replay event after stop on the previous PR build and zero after the fix, while native style application remained intact. This follows the stop-session-recording spec's no-new-capture requirement; no spec divergence.LocalNetworkAccessChecksdisabled for the local test servers.dcf0cf8f4): four unit and two real-Brave regressions failed on the previous head and now pass. Chained hooks propagate rejection status without recording rejected CSS, and subsequent accepted assignments still record. PostHog US validation confirmed two nonempty recordings and four passing result events through MCP; full evidence.git diff --checkpassed.e05f3385e1ca6c6575d80ac69b12cb5a8e98c9f1with no actionable findings. The test-authoring audit found no actionable issues.Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
posthog-jspatch changeset (file written directly)🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Implemented with Pi, shell tools, GitHub CLI, and Vitest/Puppeteer in Brave. Autoreview used Claude Opus 5.5. The fix stays in the shared property hook so it covers document and deferred shadow-root setup without a broad timer catch. All browser testing was automated, not manual.