[fix]: ctx addInitScript on popup pages#1642
Merged
seanmcguire12 merged 3 commits intomainfrom Feb 2, 2026
Merged
Conversation
🦋 Changeset detectedLatest commit: 21193c2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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
Greptile OverviewGreptile SummaryThis PR fixes a race condition where Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant Context
participant CDP
participant Session
participant Browser
Note over Client,Browser: newPage(url) Flow
Client->>Context: newPage(targetUrl)
Context->>CDP: Target.createTarget({url: "about:blank"})
CDP->>Browser: Create target at about:blank
Browser-->>CDP: targetId
CDP-->>Context: {targetId}
Context->>Context: Store targetId with "about:blank"
Note over Browser,Session: Auto-attach with waitForDebuggerOnStart
Browser->>CDP: Target.attachedToTarget (paused)
CDP->>Context: onAttachedToTarget(info, sessionId)
alt Worker Target
Context->>Session: Runtime.runIfWaitingForDebugger
Session->>Browser: Resume worker
else Page Target
Context->>Context: executionContexts.attachSession()
Context->>Session: installInitScriptsOnSession()
Session->>Browser: Page.enable
Session->>Browser: Page.addScriptToEvaluateOnNewDocument(script1)
Session->>Browser: Page.addScriptToEvaluateOnNewDocument(script2)
Context->>Session: Runtime.runIfWaitingForDebugger
Session->>Browser: Resume target
Browser-->>Context: Scripts settled
Context->>Context: Page.create()
Context->>Context: applyInitScriptsToPage({seedOnly: true})
Note over Context: Seed only to avoid double-install
Context->>Context: Register page in pagesByTarget
Context-->>Client: page object returned
Client->>Context: (newPage continues)
Context->>Context: page.seedCurrentUrl(targetUrl)
Context->>Session: Page.navigate({url: targetUrl})
Note over Session,Browser: Fire-and-forget navigation
Session->>Browser: Navigate to actual URL
Browser->>Browser: Init scripts run on first document
end
Note over Client,Browser: Popup Flow
Client->>Browser: Click link with target="_blank"
Browser->>CDP: Target.attachedToTarget (paused, with openerId)
CDP->>Context: onAttachedToTarget(info, sessionId)
Context->>Session: installInitScriptsOnSession()
Session->>Browser: Page.enable
Session->>Browser: Page.addScriptToEvaluateOnNewDocument(scripts)
Context->>Session: Runtime.runIfWaitingForDebugger
Session->>Browser: Resume popup target
Browser->>Browser: Init scripts run on popup document
Context->>Context: Page.create() and register
Context->>Context: applyInitScriptsToPage({seedOnly: true})
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| "@browserbasehq/stagehand": patch | ||
| --- | ||
|
|
||
| fix issue where scripts added via context.addInitScripts() were not being injected into new pages that were opened via popups (eg, clicking a link that opens a new page) and/or calling context.newPage(url) |
Contributor
There was a problem hiding this comment.
typo: addInitScripts() should be addInitScript() (no 's' at the end)
Prompt To Fix With AI
This is a comment left during a code review.
Path: .changeset/small-jobs-drum.md
Line: 5:5
Comment:
typo: `addInitScripts()` should be `addInitScript()` (no 's' at the end)
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 5 files
Confidence score: 2/5
- High risk due to
packages/core/lib/v3/understudy/context.tsnot awaiting CDPsend()calls, so scripts may not be installed before the target resumes, leading to inconsistent behavior. - The issue is concrete and user-facing, which lowers merge confidence despite the change being limited in scope.
- Pay close attention to
packages/core/lib/v3/understudy/context.ts- CDPsend()calls are fire-and-forget, so setup may race with target resume.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/lib/v3/understudy/context.ts">
<violation number="1" location="packages/core/lib/v3/understudy/context.ts:66">
P1: CDP `send()` calls are not awaited - scripts may not be installed before target resumes.
The method is `async` but none of the `send()` calls use `await`. The `.catch(() => {})` only handles errors, it doesn't wait for the promises. This defeats the PR's goal of guaranteeing scripts are registered before the first document executes.</violation>
</file>
Architecture diagram
sequenceDiagram
participant C as Client
participant VC as V3Context
participant CDP as CdpConnection (Browser)
participant S as Target Session (CDP)
participant P as Page Object
Note over VC,CDP: Initialization
VC->>>CDP: CHANGED: Target.setAutoAttach(waitForDebuggerOnStart: true)
rect rgb(23, 37, 84)
Note over C,S: NEW: Page Creation Flow (e.g. newPage(url) or Popup)
C->>>VC: newPage(targetUrl)
VC->>>CDP: Target.createTarget(url: "about:blank")
CDP-->>VC: targetId
CDP->>VC: onAttachedToTarget(info, sessionId)
Note right of VC: Target is PAUSED by debugger
VC->>>VC: NEW: installInitScriptsOnSession(session)
VC->>>S: Page.addScriptToEvaluateOnNewDocument(sources)
VC->>>S: NEW: Runtime.runIfWaitingForDebugger
Note right of S: Target resumes execution
VC->>>P: NEW: Page.create() & seedInitScript()
VC->>>P: seedCurrentUrl(targetUrl)
opt targetUrl != "about:blank"
VC->>>S: CHANGED: Page.navigate(targetUrl)
end
VC-->>C: return Page
end
rect rgb(69, 26, 3)
Note over VC,S: Worker Handling
CDP->>VC: onAttachedToTarget(workerInfo)
VC->>>S: NEW: Runtime.runIfWaitingForDebugger
Note right of S: Resume workers immediately (not tracked)
end
Note over VC,P: Dynamic Script Addition
C->>>VC: addInitScript(script)
VC->>>P: registerInitScript(source)
P->>>S: Page.addScriptToEvaluateOnNewDocument(source)
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Member
Author
Contributor
|
@seanmcguire12 I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
1 issue found across 5 files
Confidence score: 3/5
- Potential user-impacting bug:
scriptsInstalledinpackages/core/lib/v3/understudy/context.tsdoesn’t reflect async failures fromPromise.allSettled, so session-level install failures may prevent page fallback script installation. - Score lowered due to a concrete, medium-high severity logic gap around failure handling that could affect runtime behavior.
- Pay close attention to
packages/core/lib/v3/understudy/context.ts- ensure settled results are checked so failures trigger fallback installation.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/lib/v3/understudy/context.ts">
<violation number="1" location="packages/core/lib/v3/understudy/context.ts:464">
P1: The `scriptsInstalled` flag never reflects async failures from `Promise.allSettled`. If session-level script installation fails, scripts won't be installed via the page fallback. Check the settled results to update the flag.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client
participant Ctx as V3Context
participant CDP as Browser (CDP)
participant Page as Page Object
Note over Ctx,CDP: CHANGED: Target.setAutoAttach<br/>(waitForDebuggerOnStart: true)
Client->>Ctx: newPage("https://site.com")
Note over Ctx,CDP: Phase 1: Create Paused Target
Ctx->>CDP: CHANGED: Target.createTarget(url="about:blank")
Note right of CDP: Target created PAUSED
CDP-->>Ctx: targetId
par Event Processing
CDP->>Ctx: event: attachedToTarget
alt Target is Worker
Ctx->>CDP: Runtime.runIfWaitingForDebugger
else Target is Page
Note over Ctx,CDP: Phase 2: Secure Init Injection
Ctx->>CDP: NEW: Page.enable
Ctx->>CDP: NEW: Page.addScriptToEvaluateOnNewDocument
Ctx->>CDP: NEW: Runtime.runIfWaitingForDebugger
Note right of CDP: Resumes (at about:blank)
Ctx->>Page: create(session)
Ctx->>Page: NEW: seedInitScript(source)
Note right of Page: Record script (skip re-install)
Ctx->>Ctx: Register Page instance
end
and Waiting & Navigation
loop Wait for Page instance
Ctx->>Ctx: Check pagesByTarget.get(targetId)
end
Note over Ctx,CDP: Phase 3: Navigate to Real URL
Ctx->>Page: seedCurrentUrl("https://site.com")
Ctx->>CDP: NEW: Page.navigate("https://site.com")
end
Ctx-->>Client: return Page
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Member
Author
Contributor
|
@seanmcguire12 I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
No issues found across 5 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant C as V3Context
participant CONN as CdpConnection
participant B as Browser (CDP)
participant S as Target Session
participant P as Page Object
Note over C,B: NEW: page/popup creation flow with guaranteed script injection
C->>CONN: newPage(url)
CONN->>B: Target.createTarget(url: "about:blank")
Note right of B: NEW: waitForDebuggerOnStart pauses execution
B-->>CONN: targetId
B->>CONN: Target.attachedToTarget (Event)
CONN->>C: onAttachedToTarget(info, sessionId)
rect rgb(23, 37, 84)
Note over C,S: NEW: Pre-execution script injection
C->>S: Page.enable
loop for each initScript
C->>S: Page.addScriptToEvaluateOnNewDocument(source)
end
C->>S: CHANGED: Runtime.runIfWaitingForDebugger
end
alt is worker target
C->>S: CHANGED: Resume immediately (not tracked)
else is top-level page
C->>P: Page.create()
C->>P: CHANGED: seedInitScript() (track scripts without re-injecting)
opt url is not about:blank
P->>P: seedCurrentUrl(targetUrl)
P->>S: CHANGED: Page.navigate(targetUrl)
end
end
C-->>C: resolve newPage()
Note over S,B: Browser executes init scripts before actual document loads
Member
Author
pirate
reviewed
Feb 2, 2026
| { type: "shared_worker", exclude: true }, | ||
| { type: "service_worker", exclude: true }, | ||
| ], | ||
| waitForDebuggerOnStart: true, |
Member
There was a problem hiding this comment.
I think this might have a stealth/reliability impact, gaps in runtime performance metrics as a result of debuggerOnStart are one of the signals bot-blockers use iirc. Also there are weird situations with using waitForDebuggerOnStart on service workers.
might be worth only setting this if there are initScripts needed
Further reading on issues with waitForDebuggerOnStart:
-
Intercept target creation puppeteer/puppeteer#3667 (comment)
-
https://issues.chromium.org/issues/376417348 reports one such weirdness:
This works fine (as expected) while a new target is opened with a normal click (left click). The new tab is stuck in hanging/loading state until runIfWaitingForDebugger command is sent. We receive waitingForDebugger: true for attachedTotTarget event.
However, when a tab is opened using ctrl + click (command + click on MAC), browser behaves differently. The tab starts loading at once, without waiting for runIfWaitingForDebugger command. In attachedTotTarget waitingForDebugger parameter is still true.
pirate
approved these changes
Feb 2, 2026
seanmcguire12
changed the title
addInitScript on popup pages
This was referenced Feb 1, 2026
This was referenced Feb 16, 2026
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
2 participants
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
context.addInitScript()were not reliably applied to pages that were created via a popup (eg, clicking a link that opens a new page), or, when a new page was created viacontext.newPage(url).what changed
waitForDebuggerOnStartinTarget.setAutoAttachincdp.ts, so that new targets pause until we explicitly resume themunderstudy/context.ts:onAttachedToTarget()now installs context init scripts on the session before resuming, then resumes withRuntime.runIfWaitingForDebugger. this guarantees scripts are registered before the first real document executes.seedOnlytoapplyInitScriptsToPage()so thepageobject records scripts without re‑installing themnewPage()first creates targets atabout:blankand then navigates after attach (fire‑and‑forget). this avoids the race where the first document loads before init scripts are installedunderstudy/page.ts, i addedseedInitScript()so thepagecan track scripts that were already installed on the sessiontest plan
tests/context-addInitScript.spec.tswhich explicitly coverscontext.newPage(url)and popup pages to ensure init scripts run on the initial documentSummary by cubic
Fixes a race where context.addInitScript() didn’t run on the first document for new pages opened via popups or context.newPage(url). New targets now pause until init scripts are registered, ensuring scripts run reliably.
Written for commit 21193c2. Summary will update on new commits. Review in cubic