[fix]: wait for the locator world when a deep XPath crosses an iframe - #2978
seanmcguire12 wants to merge 5 commits into
Conversation
🦋 Changeset detectedLatest commit: d6ef703 The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 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 |
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Architecture diagram
sequenceDiagram
participant Client as Client App
participant SDK as Stagehand SDK
participant Page as Page
participant Deep as DeepLocator
participant FrameLoc as FrameLocator
participant CDP as CDP Session
participant ExecCtx as ExecutionContext registry
participant ChildDoc as Child Frame Document
Note over Client,ChildDoc: Deep XPath iframe navigation flow
Client->>SDK: Request deep XPath click
SDK->>Page: resolve deep target
Note over Page,Deep: XPath planning phase
Page->>Deep: planDeepXPathTarget(selector)
alt Trailing iframe XPath (e.g. .../iframe[1])
Deep->>Deep: Detect no steps after iframe
Deep-->>Page: Return parent frame + selector
Page->>Page: Target iframe element directly in parent
Page->>CDP: Click iframe element (no child navigation)
CDP-->>Page: Done
Page-->>Client: Click complete
else Crossing iframe XPath (e.g. .../iframe[1]/html/body/button)
Deep->>Deep: Detect iframe followed by more steps
Deep-->>Page: Return frameHopSelectors + final selector
Note over Page,FrameLoc: Frame hop execution
Page->>FrameLoc: Resolve frame for iframe step
FrameLoc->>CDP: DOM.getFrameOwner()
CDP-->>FrameLoc: Backend node ID
Note over FrameLoc,ExecCtx: Readiness wait (15s budget)
loop Until deadline (15s)
FrameLoc->>CDP: getSessionForFrame(childFrameId)
alt OOPIF adoption during attempt
CDP-->>FrameLoc: New session for child
FrameLoc->>ExecCtx: waitForLocatorWorld(session, childId, 200ms)
ExecCtx-->>FrameLoc: Locator world ready OR error
else Stable session
CDP-->>FrameLoc: Same session
FrameLoc->>ExecCtx: waitForLocatorWorld(session, childId, 200ms)
ExecCtx-->>FrameLoc: Locator world ready
end
alt Locator world ready
FrameLoc->>CDP: Verify session still owns frame
alt Session unchanged
FrameLoc-->>Page: Child frame resolved
Page->>CDP: Evaluate on child frame
CDP->>ChildDoc: Execute click/action
ChildDoc-->>CDP: Result
CDP-->>Page: Action complete
Page-->>Client: Success
else Session changed during verification
Note over FrameLoc: Retry with new session
end
else Timeout or error
FrameLoc-->>Page: Throw readiness error
Page-->>Client: Error propagated
end
end
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
@trippyogi waiting for a max of 15s per frame seems excessive to me here. I am gonna bring that allowance closer to the previous ~1.2s while keeping your readiness and session ownership fixes. also gonna follow up with another PR that adds shared operation deadlines which will let iframe readiness, element resolution, and execution share one timeout. this means users will be able to |
…2342) Fixes #2324. > Rebuilt against current v4 `main`. The original implementation in this PR targeted `packages/core/lib/v3/understudy/`, which no longer exists, and waited on the **main world**. v4 fails on the **Stagehand extension/locator world** instead, so this branch was rewritten rather than rebased. ## Why Clicking through a still-loading iframe fails deterministically on v4: ``` Stagehand extension world not ready for frame <id>; checked contexts: none ``` The same click succeeds immediately once the child document commits. The reproduction in #2324 sharpens the original report in two ways: the trigger is **frame descent** rather than the target element, and it is **not cross-origin specific** — a same-site, in-process delayed iframe fails identically. That surfaces two distinct defects: | Case | Selector | Expected | Before | | --- | --- | --- | --- | | A. Selector ends at the iframe | `/html/body/iframe[1]` | Stay in the parent document and target the `<iframe>` element | `resolveDeepXPathTarget` flushed the trailing iframe into a `FrameLocator`, entered the child with `xpath=/`, and needed a child execution context it never required | | B. Selector crosses the iframe | `/html/body/iframe[1]/html/body/button` | Descend, then wait for the child's locator world | `ensureChildFrameReady` waited ~1200ms best-effort for the **main** world and swallowed failures, so evaluation reached the child before the extension world existed | A plain CSS `iframe` click already worked while loading, which is the semantic target for case A. ## What changed **`packages/extension/understudy/deepLocator.ts`** — an iframe step becomes a frame hop only when further selector steps follow it. A trailing `iframe[n]` stays an element target in the parent frame. Hop planning moved into `planDeepXPathTarget()` so the semantics are unit-testable without CDP. **`packages/extension/understudy/frameLocator.ts`** — `ensureChildFrameReady()` now blocks on `executionContexts.waitForLocatorWorld()` instead of the main world: - one bounded `15s` deadline, applied only when crossing a frame; - session ownership is re-resolved per attempt so OOPIF adoption cannot pin the wait to a stale session, and re-verified after a successful wait; - readiness failure throws instead of returning best-effort; - the per-child `try/catch` in `resolveFrame()` was narrowed to the `DOM.getFrameOwner` lookup, so a readiness failure surfaces its real message rather than the generic `Unable to obtain a content frame for selector: ...`. `executionContextRegistry.ts` is untouched, and the generic `1s` `waitForLocatorWorld` call sites are unchanged — after a gated transition that budget is only a short recovery window for context churn, not the loading timeout. ## Test plan New unit coverage: - `packages/extension/tests/deep-locator.test.ts` — trailing, crossing, nested-trailing, and nested-crossing XPath semantics, plus `frame` parity with `iframe`. - `packages/extension/tests/frame-locator.test.ts` — readiness errors propagate out of `resolveFrame()`, and readiness retries against the new session when OOPIF ownership changes mid-attempt. New browser regression, `packages/sdk-ts/tests/integration/iframeLocatorReadiness.test.ts`, registered in the `local/frames-shadow` group. The child document response is delayed while the `<iframe>` element is in the parent DOM immediately: | Case | Same-process | OOPIF (`--site-per-process` + host-resolver rules) | | --- | --- | --- | | Trailing iframe XPath | clicks without waiting for the child document | same | | Deep XPath into the child button | waits for readiness, then the button's server-side counter increments | waits across session adoption, then increments | Case B asserts a real side effect (the child button issues an HTTP request the fixture counts) rather than only elapsed time, so it works for OOPIF without cross-origin DOM inspection. Results locally: extension unit suite passes, all 4 integration cases pass, `oxfmt` and `oxlint` clean, and typecheck reports no errors in the touched files. ## Changeset Extension patch plus the SDKs that embed it; no protocol bump, since there is no wire or API change.
d297f1f to
d6ef703
Compare
Thanks @trippyogi for the contribution!
Fixes #2324.
Fix locator failures when entering an iframe whose Stagehand helpers are still initializing, and correctly target the iframe element when an XPath ends at the iframe.
xpath=/html/body/iframe[1]stays in the parent document and targets the iframe element without waiting for its contents.xpath=/html/body/iframe[1]/html/body/buttonenters the child frame and waits for its locator helpers before resolving the button.Frame transitions now wait for the locator world instead of the main JavaScript world. Readiness retries use a best-effort 1.2-second budget per transition, with attempts requesting up to 200 ms. Each attempt rechecks session ownership so out-of-process iframe adoption does not leave the wait on a stale session. Already-ready frames proceed immediately.
If readiness fails, the error identifies the frame and configured readiness budget and preserves the underlying failure as its cause. It is no longer swallowed and replaced with a generic content-frame lookup error.
This budget is not a hard operation timeout: fallback checks and pending browser commands can extend an attempt. The generic one-second locator-world waits remain unchanged. Shared operation deadlines and locator auto-waiting are follow-up work.
Validation:
Summary by cubic
Fixes #2324: deep XPath actions through a still-loading iframe now wait for the child frame's locator world instead of failing with
Stagehand extension world not ready. Trailing iframe XPaths no longer descend into the child frame; they target the<iframe>element in the parent frame, matching CSS iframe click behavior.What changed
resolveFrame()no longer hides readiness failures behind the generic "Unable to obtain a content frame" message.@browserbasehq/stagehand-extensionand the SDKs that embed it, including a regenerated Go SDK asset bundle; no protocol or API change.Test plan
Written for commit d6ef703. Summary will update on new commits.