Skip to content

Conversation

@seanmcguire12
Copy link
Member

@seanmcguire12 seanmcguire12 commented Dec 9, 2025

why

what changed

test plan


Summary by cubic

Retries DOM.getDocument with decreasing depths and hydrates missing nodes to avoid CBOR stack limit errors on complex pages. Also injects the Stagehand init script at the Playwright context level and removes window-scoped helpers to eliminate page.evaluate dependencies.

  • Bug Fixes

    • Added adaptive DOM.getDocument retries (depths: -1 → 256 → … → 1) and DOM.describeNode hydration for nodes with childNodeCount gaps, with CBOR error detection.
    • Updated DOM types to include nodeId and childNodeCount.
  • Refactors

    • Inject helper script once per Playwright context via addInitScript, guarded by window.__stagehandInjected; removed per-page injection and internal page.evaluate checks.
    • Removed window.getScrollableElementXpaths, getNodeFromXpath, waitForElementScrollEnd and related utils/xpath modules; adjusted dom/index to only initialize process; deleted the test that depended on re-injecting deleted globals.

Written for commit e9d058c. Summary will update automatically on new commits.

@changeset-bot
Copy link

changeset-bot bot commented Dec 9, 2025

🦋 Changeset detected

Latest commit: e9d058c

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
Copy link
Member Author

@greptileai

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 9, 2025

Greptile Overview

Greptile Summary

This PR adds resilient DOM.getDocument handling to avoid CBOR stack limit errors on complex pages. When the initial request fails due to stack limits, the code retries with progressively smaller depth values (-1 → 256 → 128 → ... → 1) and hydrates missing child nodes using DOM.describeNode calls.

Key changes:

  • DOM retry logic: New getDomTreeWithFallback() and hydrateDomTree() functions in lib/a11y/utils.ts that gracefully handle CBOR stack errors by retrying with reduced depth and expanding truncated subtrees
  • Context-level script injection: Moved the Stagehand init script injection from per-page to context level in StagehandContext.init(), using a WeakSet to prevent duplicate injection
  • Code cleanup: Removed unused window-scoped helpers (getScrollableElementXpaths, getNodeFromXpath, waitForElementScrollEnd) and their supporting modules (lib/dom/utils.ts, lib/dom/xpathUtils.ts, lib/dom/elementCheckUtils.ts)
  • Type updates: Added nodeId and childNodeCount fields to DOMNode type to support the hydration logic

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk - the retry logic is defensive and the code cleanup removes dead code.
  • The core DOM retry logic is well-structured with proper error handling and fallback mechanisms. The refactoring to context-level script injection is clean. Minor deduction because the changeset description mentions "exponential backoff" but the implementation uses fixed depth levels, and there are no unit tests for the new retry logic.
  • lib/a11y/utils.ts contains the new retry logic that would benefit from unit test coverage.

Important Files Changed

File Analysis

Filename Score Overview
lib/a11y/utils.ts 4/5 Added adaptive DOM.getDocument retry logic with depth fallbacks and DOM.describeNode hydration for handling CBOR stack limit errors on complex pages. Clean implementation.
lib/StagehandContext.ts 5/5 Moved Stagehand init script injection to context level with WeakSet tracking to prevent duplicate injection across contexts.
lib/StagehandPage.ts 5/5 Removed per-page script injection and $$eval hook since injection now happens at context level.
lib/dom/process.ts 5/5 Removed window-scoped helper functions (getScrollableElementXpaths, getNodeFromXpath, waitForElementScrollEnd) that are no longer used.
types/context.ts 5/5 Added nodeId and childNodeCount fields to DOMNode type to support DOM hydration logic.
evals/deterministic/tests/page/addInitScript.test.ts 4/5 Removed test that depended on deleted window.getScrollableElementXpaths function. Remaining test still validates init script injection.

Sequence Diagram

sequenceDiagram
    participant Client as StagehandPage
    participant Utils as lib/a11y/utils.ts
    participant CDP as CDP Session
    
    Client->>Utils: buildBackendIdMaps()
    Utils->>Utils: getDomTreeWithFallback()
    Utils->>CDP: DOM.getDocument(depth=-1)
    
    alt CBOR stack error
        CDP-->>Utils: Error: CBOR stack limit exceeded
        Utils->>CDP: DOM.getDocument(depth=256)
        alt Success with truncated tree
            CDP-->>Utils: Partial DOM tree
            Utils->>Utils: hydrateDomTree()
            loop For each node needing expansion
                Utils->>CDP: DOM.describeNode(nodeId, depth)
                alt CBOR error
                    CDP-->>Utils: Error
                    Utils->>CDP: DOM.describeNode(lower depth)
                end
                CDP-->>Utils: Node children
                Utils->>Utils: mergeDomNodes()
            end
        end
    else Success
        CDP-->>Utils: Complete DOM tree
    end
    
    Utils-->>Client: BackendIdMaps
Loading

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.

Additional Comments (1)

  1. lib/index.ts, line 825-833 (link)

    style: This init script registration is now redundant since StagehandContext.init() on line 816 already adds the same guarded script to the context. The runtime guard (if (!window.__stagehandInjected)) prevents double execution, but this block could be removed to clean up the code.

12 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@seanmcguire12 seanmcguire12 force-pushed the retry-on-max-depth-exceeded branch from 15f6d8a to 6f252b7 Compare December 9, 2025 04:37
@seanmcguire12 seanmcguire12 marked this pull request as ready for review December 9, 2025 15:55
@seanmcguire12 seanmcguire12 merged commit 575226f into v2 Dec 9, 2025
8 checks passed
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 12 files

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.

11 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@github-actions github-actions bot mentioned this pull request Dec 9, 2025
miguelg719 pushed a commit that referenced this pull request Dec 9, 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 v2, this PR will
be updated.


# Releases
## @browserbasehq/[email protected]

### Patch Changes

- [#1389](#1389)
[`575226f`](575226f)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - Retry
DOM.getDocument on max depth exceeded with exponential backoff

- [#1386](#1386)
[`dbffb75`](dbffb75)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - Remove
deprecated internal usage of page.evaluate

## @browserbasehq/[email protected]

### Patch Changes

- Updated dependencies
\[[`575226f`](575226f),
[`dbffb75`](dbffb75)]:
    -   @browserbasehq/[email protected]

## @browserbasehq/[email protected]

### Patch Changes

- Updated dependencies
\[[`575226f`](575226f),
[`dbffb75`](dbffb75)]:
    -   @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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants