Repository navigation
[fix]: adopt pre-existing and nested cross-origin iframes - #3262
Open
ForrestMoret-at wants to merge 5 commits into
Open
ForrestMoret-at wants to merge 5 commits into
ForrestMoret-at wants to merge 5 commits into
Conversation
An OOPIF session was only adopted when its main frame was already in a page's frame graph, or when the parent session later emitted Page.frameAttached for it. Frames that were already loaded when the context bootstrapped (reconnect, service-worker restart) and OOPIFs nested inside another OOPIF never get that event, so they stayed pending and snapshots/act could not see inside them. - fall back to the page owning the child's parentId, and adopt pending children once that parent page registers - keep an adopted OOPIF session as owner when the parent session reports the frame late Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A Page.frameAttached can add the frame a pending OOPIF is waiting on (for example a same-site wrapper frame the page had not reported yet) without re-checking the other pending OOPIFs, leaving them unadopted. Retry the cascading adoption after every frameAttached, route that path through adoptOopif, and resync the embedded Go extension asset. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: dc0cd99 The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
This PR is from an external contributor and must be approved by a stagehand team member with write access before CI can run. |
github-actions
Bot
added
external-contributor
…-oopifs # Conflicts: # packages/sdk-go/internal/extensionassets/stagehand-extension.zip
…-oopifs # Conflicts: # packages/sdk-go/internal/extensionassets/stagehand-extension.zip
This branch has not been deployed
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.
why
BrowserContextonly adopts an OOPIF session when its main frame is already in a page's frame graph, or when the parent session later emitsPage.frameAttachedfor it. Two cases never get that event, so the child session stays inpendingOopifByMainFrameforever and snapshots/actcan't see inside the iframe:Page.getFrameTreeomits remote frames, and no newframeAttachedfires.Two related gaps:
Page.frameAttachedthat arrives after the OOPIF was adopted gives ownership back to the parent session (FrameRegistry.onFrameAttachedtrusts the emitter).frameAttachedthat adds the frame a pending OOPIF is waiting on (e.g. a same-site wrapper frame) didn't retry the other pending OOPIFs.Repro: open a page with a cross-origin iframe, then start or reconnect Stagehand against that tab. The iframe's frame id never appears in the page's frame tree, and its contents are missing from snapshots.
what changed
parentId, taken from the child's ownPage.getFrameTree. If that page isn't known yet, the parent id is recorded, and the child is adopted once the page registers, a parent OOPIF is adopted, or aframeAttachedreports the parent frame. This also covers nesting.frameAttachedno longer takes ownership while the child session is alive.findPageOwningFrameand the adoption steps intoadoptOopif.packages/extension/artifacts/stagehand-extension.zip→sdk-go/internal/extensionassets; manifest version 1.1.1 matches). Go isn't installed locally, so I copied the zip by hand instead of runningextensionpack. Please let CI'sextensionpack --checkconfirm it.test plan
packages/extension/tests/understudy-context-oopif-adoption.test.tsdrives the realBrowserContext+CdpConnectionover a scripted fake transport. Cases: pre-existing iframe; iframe attaching before its page registers; late parentframeAttachedkeeps the child owner; nested OOPIF; waiting OOPIFs adopted once their wrapper frame is reported. Each fails onmainwithout its part of the fix.pnpm check(43/43) and extension unit tests (1008 passed). Go and Python checks weren't run locally.🤖 Generated with Claude Code
Summary by cubic
Fixes adoption of cross-origin iframes that were already loaded when Stagehand attached (reconnect, extension restarts) or that are nested inside another cross-origin iframe, so snapshots and actions can see inside them.
Bug Fixes
Page.frameAttachedno longer takes ownership of an already-adopted OOPIF session.BrowserContextover a scripted fake transport; each case fails onmain.mainthat resolved its zip; CI'sextensionpack --checkshould verify it.Written for commit dc0cd99. Summary will update on new commits.