fix: propagate recording session to screencast stream#1045
Open
shtefcs wants to merge 1 commit intovercel-labs:mainfrom
Open
fix: propagate recording session to screencast stream#1045shtefcs wants to merge 1 commit intovercel-labs:mainfrom
shtefcs wants to merge 1 commit intovercel-labs:mainfrom
Conversation
Contributor
|
@shtefcs is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
shtefcs
force-pushed
the
fix/recording-screencast-session
branch
from
da5070c to
08c8227
Compare
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| @@ -0,0 +1,9 @@ | |||
| The recording function currently: | |||
Contributor
There was a problem hiding this comment.
A development scratch note file (cli/src/native/actions.rs.patch) was accidentally committed to the repository.
`record start` previously created a new BrowserContext via `Target.createBrowserContext`. This creates an Off-The-Record (incognito) profile where extensions loaded via `--load-extension` are NOT available — extensions require explicit user opt-in for incognito, and CDP-created contexts have no such opt-ins. This breaks CAPTCHA solvers (NopeCHA), ad blockers, and any other extension that relies on content script injection during recording. Confirmed by: - Chromium source: `DevToolsBrowserContextManager::CreateBrowserContext` calls `GetOffTheRecordProfile()` which creates an OTR profile - Puppeteer issue #3442: extensions blocked in createIncognitoBrowserContext - Playwright issue #5328: extensions unavailable in browser.newContext() Fix: replace `Target.createBrowserContext` + `Target.createTarget` with `browserContextId` with just `Target.createTarget` (no browserContextId). This creates a new tab in the DEFAULT browser context where extensions work normally. The recording still gets its own CDP session via `Target.attachToTarget`, so there are no command conflicts — CDP supports multiple concurrent in-flight commands via ID-based multiplexing. Also removed `Browser.setDownloadBehavior` re-application since the new tab shares the default context where downloads are already configured.
shtefcs
force-pushed
the
fix/recording-screencast-session
branch
from
08c8227 to
094d7c7
Compare
Collaborator
|
Thanks for the fix @shtefcs! One thing: please remove |
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
After
record start, the live preview (screencast) stays on the old pre-recording context while CLI commands execute on the new recording context. This causes:Root Cause
handle_recording_startcreates a newBrowserContextviaTarget.createBrowserContext, adds a new page, and switchesactive_page_indexto it. All subsequent CLI commands (snapshot,eval,click) usemgr.active_session_id()which returns the new recording session.However, the
StreamServer'scdp_session_id(used forPage.startScreencast) is never updated.update_stream_client()is called by 13+ other commands that switch contexts (open, close, connect, tab switch, etc.) but was missing from bothhandle_recording_startandhandle_recording_stop.Fix
Add
state.update_stream_client().awaitin bothhandle_recording_start(afterset_recording) andhandle_recording_stop(afterset_recording). This propagates the active session to the screencast stream, matching the pattern used everywhere else.Same class of bug as #1019
PR #1019 fixed downloads not working in the recording context by re-applying
Browser.setDownloadBehaviorto the new context. This PR fixes the screencast stream not following the recording context by callingupdate_stream_client().Testing
Verified that without this fix:
record start→ live preview stays on old context--load-extension) appear to not work because the screencast shows the wrong contextWith this fix:
record stopsyncs the screencast back to the current page