Skip to content

[api] update root frame ID on init #920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 28, 2025

Conversation

seanmcguire12
Copy link
Member

why

  • tab handling for act, extract and observe was failing because we sometimes sent frameId: undefined after a new page had been opened
  • the rootFrameId only became available after Page.frameNavigated fired, so the time between the event firing and a stagehand API call caused undefined frameIds

what changed

  • Added getCurrentRootFrameId helper which calls Page.getFrameTree to fetch the current root frame ID on page creation
  • attachFrameNavigatedListener is now called right after a page is wrapped with createStagehandPage
  • handleNewPlaywrightPage now runs before attaching the listener

test plan

  • tested in local dev
  • run all evals

Copy link

changeset-bot bot commented Jul 27, 2025

🦋 Changeset detected

Latest commit: 52f0974

The changes in this PR will be included in the next version bump.

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

@seanmcguire12 seanmcguire12 added act These changes pertain to the act function extract These changes pertain to the extract function observe These changes pertain to the observe function targeted-extract These changes pertain to targeted extract labels Jul 27, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR fixes a critical race condition in frame ID handling that was causing tab operations (act, extract, and observe) to fail. The core issue was that frameId: undefined was being sent to API calls when operations occurred on newly opened pages before the frame ID was properly initialized.

The problem stemmed from the timing of frame ID availability - the rootFrameId only became available after the Page.frameNavigated event fired, creating a window where API calls could be made with undefined frame IDs. This was particularly problematic for tab handling scenarios where pages are created dynamically.

The solution involves three key architectural changes:

  1. Proactive Frame ID Fetching: A new getCurrentRootFrameId helper function was added that uses the Chrome DevTools Protocol (CDP) Page.getFrameTree command to immediately fetch the root frame ID during page initialization, eliminating the need to wait for the Page.frameNavigated event.

  2. Restructured Initialization Flow: The page creation flow in StagehandContext.ts was reorganized so that attachFrameNavigatedListener is called immediately after page wrapping, and handleNewPlaywrightPage runs before listener attachment. This ensures frame IDs are available before any operations can occur.

  3. Consistent Frame ID Usage: All API calls throughout StagehandPage.ts were updated to use this.rootFrameId instead of this.frameId, providing better semantic clarity and ensuring consistent frame identification across operations.

The changes integrate well with the existing CDP-based architecture and maintain compatibility with Stagehand's page proxy system while solving the timing issues that were causing tab handling failures.

Confidence score: 4/5

  • This PR addresses a specific, well-identified race condition with a targeted technical solution
  • The changes show careful consideration of initialization order and timing issues
  • The files StagehandPage.ts and StagehandContext.ts need attention due to the complexity of the event handling and CDP integration changes

2 files reviewed, no comments

Edit Code Review Bot Settings | Greptile

@seanmcguire12 seanmcguire12 changed the title update root frame ID on init [api] update root frame ID on init Jul 27, 2025
@miguelg719 miguelg719 merged commit c20adb9 into main Jul 28, 2025
19 of 30 checks passed
seanmcguire12 pushed a commit that referenced this pull request Jul 31, 2025
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @browserbasehq/[email protected]

### Patch Changes

- [#865](#865)
[`6b4e6e3`](6b4e6e3)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - improve
type safety for trimTrailingTextNode

- [#897](#897)
[`e77d018`](e77d018)
Thanks [@miguelg719](https://github.com/miguelg719)! - Fix selfHeal to
remember intially received arguments

- [#920](#920)
[`c20adb9`](c20adb9)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - fix: tab
handling on API

- [#882](#882)
[`b86df93`](b86df93)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - remove
elements that don't have xpaths from observe response

- [#905](#905)
[`023c2c2`](023c2c2)
Thanks [@tkattkat](https://github.com/tkattkat)! - Delete old images
from anthropic cua client

- [#925](#925)
[`8c28647`](8c28647)
Thanks [@miguelg719](https://github.com/miguelg719)! - Remove
\_refreshPageFromApi()

- [#887](#887)
[`87e09c6`](87e09c6)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - fix: allow
xpaths with prepended 'xpath=' for targeted extract

- [#864](#864)
[`a611115`](a611115)
Thanks [@miguelg719](https://github.com/miguelg719)! - Temporarily patch
custom clients serialization error on api

- [#881](#881)
[`69913fe`](69913fe)
Thanks [@miguelg719](https://github.com/miguelg719)! - Pass sdk version
number to API for debugging

- [#913](#913)
[`b1b83a1`](b1b83a1)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - move iframe
out of 'experimental'

- [#891](#891)
[`be8497c`](be8497c)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - fix: nested
iframe xpath bug

- [#883](#883)
[`98704c9`](98704c9)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - add timeout
for JS click

- [#907](#907)
[`04978bd`](04978bd)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - store
mapping of CDP frame ID -> page

## @browserbasehq/[email protected]

### Patch Changes

- Updated dependencies
\[[`6b4e6e3`](6b4e6e3),
[`e77d018`](e77d018),
[`c20adb9`](c20adb9),
[`b86df93`](b86df93),
[`023c2c2`](023c2c2),
[`8c28647`](8c28647),
[`87e09c6`](87e09c6),
[`a611115`](a611115),
[`69913fe`](69913fe),
[`b1b83a1`](b1b83a1),
[`be8497c`](be8497c),
[`98704c9`](98704c9),
[`04978bd`](04978bd)]:
    -   @browserbasehq/[email protected]

## @browserbasehq/[email protected]

### Patch Changes

- Updated dependencies
\[[`6b4e6e3`](6b4e6e3),
[`e77d018`](e77d018),
[`c20adb9`](c20adb9),
[`b86df93`](b86df93),
[`023c2c2`](023c2c2),
[`8c28647`](8c28647),
[`87e09c6`](87e09c6),
[`a611115`](a611115),
[`69913fe`](69913fe),
[`b1b83a1`](b1b83a1),
[`be8497c`](be8497c),
[`98704c9`](98704c9),
[`04978bd`](04978bd)]:
    -   @browserbasehq/[email protected]

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
act These changes pertain to the act function extract These changes pertain to the extract function observe These changes pertain to the observe function targeted-extract These changes pertain to targeted extract
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants