Solve init script race with Debugger.resume at frame init script injection time

[pirate]

why

Init script injection was racing with Debugger.resume() sometimes, causing frames to load without init scripts running sometimes. This led to flaky init script tests, which were legitimately catching the issue.

• https://github.com/browserbase/stagehand/actions/runs/22233062982/job/64336364420?pr=1580

what changed

• queues calls on page load to run before we resume
• catches oopifs and lazy frames and click-triggered popups the same way playwright does
• removes flaky timeout/retry based prior approach

https://deepwiki.com/search/how-does-playwright-guarantee_8cf2339b-c060-4cfc-bc62-f3baaf57b229?mode=deep

test plan

---

Summary by cubic

Fixes the init‑script race by guaranteeing pre‑resume setup and correcting popup attach order. Init scripts now run reliably in same‑ and cross‑process popups, OOPIF iframes, and across reloads; race tests verify addScript is sent before resume per session.

• Bug Fixes
  • Enforce pre‑resume ordering: per‑session dispatch waiters ensure Page/Runtime enables, Target.setAutoAttach(waitForDebuggerOnStart), Network.enable/setExtraHTTPHeaders, and Page.addScriptToEvaluateOnNewDocument(runImmediately) are sent before Runtime.runIfWaitingForDebugger; resume only after dispatch; log ordering issues only for top‑level pages.
  • Stabilize attach and evaluation: fix popup attach ordering; fan out Target.* events to root listeners; retry Runtime.evaluate once on stale context ids; pre‑register the piercer script before resume and lazy‑install if needed.
  • Harden lifecycle: convert detach errors to PageNotFoundError and propagate; treat Page.enable/lifecycle acks as best‑effort; never drop top‑level Page.create due to local timeouts.
  • Expand tests and deflake: add delayed‑CDP‑send popup/iframe race repro with real URLs; assert addScript precedes resume per session; cover in‑process and cross‑process popups, window.open, OOPIF iframes, and reload persistence; update detach expectations and timeouts.

^{Written for commit 6f464d3. Summary will update on new commits. Review in cubic}

[changeset-bot]

⚠️ No Changeset found

Latest commit: 6f464d3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[greptile-apps]

Greptile Summary

This PR fixes a race condition in init script injection by ensuring all CDP commands are queued and sent before resuming the debugger. Previously, Page.addScriptToEvaluateOnNewDocument could race with Runtime.runIfWaitingForDebugger, causing init scripts to miss some frames.

Key changes:

• Queues Page.enable, Runtime.enable, Target.setAutoAttach, init scripts, and piercer registration before a single resume call
• Removes timeout-based fallback injection in favor of deterministic pre-resume setup
• Forwards Target.* events from child sessions to root listeners for consistent tracking across browsers
• Marks Page.enable calls in Page.create() as non-blocking since context already enables them
• Adds comprehensive tests for in-process popups, cross-process popups, and reload persistence

Architecture insight:
The fix matches Playwright's approach of batching all setup commands before resume. The queuePreResume helper returns promises that resolve to boolean success flags, allowing parallel queueing while maintaining CDP message order on the session. After all promises resolve, the code checks if resume succeeded before proceeding with page creation.

Minor observation:
The resume() function defined at context.ts:584-590 appears to be a safety guard that will always no-op in the success path (since resumed = true is set at line 682 before the try-finally block). It serves as a fallback if exceptions occur, though the current control flow makes this unlikely.

Confidence Score: 4/5

• This PR is mostly safe to merge with thorough test coverage, though the complex race condition fix warrants careful production monitoring
• The changes eliminate flaky race conditions by queuing all pre-resume setup before a single resume call, matching Playwright's approach. The logic is sound and well-tested with new test cases for popups and reloads. Score is 4/5 rather than 5/5 due to the complexity of the CDP session lifecycle management and one minor redundancy in the resume guard logic
• Pay close attention to packages/core/lib/v3/understudy/context.ts due to the complex refactoring of the resume logic

Important Files Changed

Filename	Overview
packages/core/lib/v3/tests/context-addInitScript.spec.ts	Added helper function and new tests for in-process and cross-process popups with reload verification
packages/core/lib/v3/understudy/cdp.ts	Added event forwarding for Target.* events from child sessions to root listeners for consistent tracking
packages/core/lib/v3/understudy/context.ts	Refactored init script injection to queue all pre-resume commands, then resume once; removed timeout-based fallback
packages/core/lib/v3/understudy/page.ts	Changed Page.enable and lifecycle enable to non-blocking void calls since context handles them pre-resume

Sequence Diagram

sequenceDiagram
    participant Browser
    participant Context as V3Context
    participant Session as CDPSession
    participant Page

    Browser->>Context: Target.attachedToTarget
    Context->>Session: Queue Page.enable
    Context->>Session: Queue Runtime.enable
    Context->>Session: Queue Target.setAutoAttach
    Context->>Session: Queue Network.enable (if headers)
    Context->>Session: Queue Page.addScriptToEvaluateOnNewDocument (init scripts)
    Context->>Session: Queue Page.addScriptToEvaluateOnNewDocument (piercer)
    Context->>Session: Queue Runtime.runIfWaitingForDebugger
    Note over Context,Session: All commands queued before resume
    Session-->>Context: Await all promises together
    
    alt resumeOk
        Context->>Context: Set resumed = true
        Context->>Session: Page.setLifecycleEventsEnabled
        Context->>Page: Page.create()
        Page->>Session: Page.enable (void, best-effort)
        Page->>Session: Page.setLifecycleEventsEnabled (void)
        Page-->>Context: Page instance
        Context->>Context: Wire session to page
        Context->>Context: Install frame bridges
        alt piercer pre-registered
            Context->>Context: Mark piercer installed
        else piercer failed
            Context->>Session: ensurePiercer() (void, lazy)
        end
    else !resumeOk
        Context->>Context: Log error and return
    end

Loading

_{Last reviewed commit: ef94203}

[greptile-apps]

_{4 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[cubic-dev-ai]

No issues found across 4 files

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

Architecture diagram

sequenceDiagram
    participant CDP as Browser (CDP)
    participant Conn as CdpConnection
    participant Ctx as V3Context
    participant Sess as Target Session
    participant P as Page Object

    Note over CDP, P: Target Creation (New Page/Popup/OOPIF)

    CDP->>Conn: Target.attachedToTarget (Paused)
    Conn->>Ctx: onAttachedToTarget()
    
    Note over Ctx, Sess: NEW: Batch Setup (Pre-Resume Queue)
    
    Ctx->>Sess: 1. Page.enable / Runtime.enable
    Ctx->>Sess: 2. Target.setAutoAttach (waitForDebuggerOnStart)
    
    opt Init Scripts defined
        Ctx->>Sess: 3. Page.addScriptToEvaluateOnNewDocument
    end
    
    Ctx->>Sess: 4. Register Piercer (Shadow DOM Hook)
    
    Note over Ctx, Sess: CHANGED: Resume queued as last operation in batch
    Ctx->>Sess: 5. Runtime.runIfWaitingForDebugger (Resume)
    
    Ctx->>Ctx: await Promise.all([setup_ops, resume_op])
    Sess-->>Ctx: All commands ACK'd (Execution resumed)

    alt isTopLevelPage
        Ctx->>P: Page.create()
        P->>Sess: Page.enable / Lifecycle.enable
        P-->>Ctx: pageInstance
        Ctx->>Ctx: wireSessionToOwnerPage()
        Ctx->>Ctx: CHANGED: applyInitScriptsToPage (seedOnly=true)
    else isChildFrame (OOPIF/Iframe)
        Ctx->>Sess: Page.getFrameTree
        Sess-->>Ctx: frameTree
        Ctx->>P: installFrameEventBridges()
    end

    Note over CDP, Conn: NEW: Event Fan-out Logic
    
    CDP->>Sess: Target.* event (Child Session)
    Sess->>Conn: dispatch(method, params)
    Conn->>Conn: NEW: Forward Target.* events to root listeners
    Conn-->>Ctx: Emit unified Target event

Loading

[cubic-dev-ai]

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/core/lib/v3/tests/iframe-ctx-addInitScript-race.spec.ts">

<violation number="1" location="packages/core/lib/v3/tests/iframe-ctx-addInitScript-race.spec.ts:15">
P2: The monkey-patching of `_sendViaSession` is a no-op by default because `INIT_SCRIPT_DELAY_MS` defaults to `0`. The `if (INIT_SCRIPT_DELAY_MS > 0 && ...)` guard will never be true, so this test doesn't actually reproduce the race condition it claims to test unless the env var `IFRAME_INIT_SCRIPT_SEND_DELAY_MS` is explicitly set. Consider giving it a non-zero default (e.g., `50` or `100`) so the test exercises the delayed code path by default, or add a comment explaining that the delay is opt-in.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

1 issue found across 3 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/core/lib/v3/understudy/cdp.ts">

<violation number="1" location="packages/core/lib/v3/understudy/cdp.ts:270">
P2: Custom agent: **Exception and error message sanitization**

This uses a generic `new Error()` instead of a typed error class. The rule requires all exceptions raised to users to be individually typed, never generic `new Error()`. Use `CdpConnectionClosedError` (or introduce a new `CdpSessionDetachedError`) to match the project's error hierarchy. Note that two lines above (line 264, pre-existing) has the same issue — consider fixing both together.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}