[fix]: clean up cdp session event handlers - #2288
Merged
seanmcguire12 merged 8 commits intoJul 6, 2026
Merged
Conversation
🦋 Changeset detectedLatest commit: 5e7d04a 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
There was a problem hiding this comment.
1 issue found across 3 files
Confidence score: 3/5
- In
packages/core/lib/v3/understudy/cdp.ts, theTarget.targetDestroyedcleanup currently exits after the first matching session, so other session-bound handlers on the same target can remain attached and keep receiving events unexpectedly; this can cause stale callbacks/memory leaks and cross-session side effects after a target closes — update the cleanup to continue iterating and remove handlers for all sessions tied to that target before merging.
Architecture diagram
sequenceDiagram
participant Chrome as Chrome DevTools (CDP)
participant Connection as CdpConnection
participant Handlers as eventHandlers Map
participant Sessions as sessions Map
participant GlobalHandler as Root event handler
participant OtherSession as CdpSession (session-b)
Note over Chrome,OtherSession: Session attach and handler registration
Chrome->>Connection: Target.attachedToTarget (sessionId: "session-a")
Connection->>Sessions: store session metadata
Connection-->>Chrome: ack
Connection->>Connection: session.on("Fetch.requestPaused", handler)
Connection->>Handlers: set key "session-a:Fetch.requestPaused"
Connection->>Connection: session.on("Network.requestWillBeSent", handler)
Connection->>Handlers: set key "session-a:Network.requestWillBeSent"
Note over Chrome,OtherSession: Another session attaches (to verify preservation)
Chrome->>Connection: Target.attachedToTarget (sessionId: "session-b")
Connection->>Sessions: store session-b metadata
Connection->>Connection: otherSession.on("Fetch.requestPaused", handler)
Connection->>Handlers: set key "session-b:Fetch.requestPaused"
GlobalHandler->>Connection: on("Target.targetCreated", rootHandler)
Connection->>Handlers: set key "Target.targetCreated" (root-level, no prefix)
Note over Chrome,GlobalHandler: --- Detach triggers cleanup ---
Chrome->>Connection: Target.detachedFromTarget (sessionId: "session-a", targetId: "target-a")
Connection->>Connection: NEW: clearSessionEventHandlers("session-a")
Connection->>Handlers: delete keys starting with "session-a:"
Handlers-->>Connection: removed "session-a:Fetch.requestPaused" & "session-a:Network.requestWillBeSent"
Connection->>Sessions: delete session-a metadata
Connection-->>Chrome: ack
Chrome->>Connection: Target.targetCreated (target-b) [root event]
Connection->>Handlers: lookup "Target.targetCreated"
Handlers-->>Connection: rootHandler
Connection->>GlobalHandler: invoke root handler
GlobalHandler-->>Connection: done
Note over Connection,Handlers: Verify other session handlers remain
alt Other handler preserved
Connection->>Handlers: check key "session-b:Fetch.requestPaused"
Handlers-->>Connection: still present
end
Note over Chrome,GlobalHandler: --- Alternative: Target.targetDestroyed ---
Chrome->>Connection: Target.targetDestroyed (targetId: "target-b")
Connection->>Sessions: find sessionId for targetId "target-b"
Sessions-->>Connection: "session-b"
Connection->>Connection: NEW: clearSessionEventHandlers("session-b")
Connection->>Handlers: delete keys starting with "session-b:"
Handlers-->>Connection: removed "session-b:Fetch.requestPaused"
Connection->>Sessions: delete session-b metadata
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
seanmcguire12
force-pushed
the
seanmcguire/stg-2415-clean-up-session-handlers-on-target-detach
branch
from
July 1, 2026 20:13
4ceadba to
1d7e608
Compare
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 3 files
Confidence score: 3/5
- In
packages/core/lib/v3/understudy/cdp.ts, theTarget.targetDestroyedcleanup path for mapped sessions can drop session state without rejecting pendingsend/dispatch waiters, which can leave CDP calls hanging and cause user-visible stalls or timeouts. Mirror the teardown behavior used in theT...branch so pending promises are explicitly rejected before removing handlers, then add a regression test for destroyed-session in-flight requests before merging.
Architecture diagram
sequenceDiagram
participant Chrome as CDP Agent
participant Conn as CdpConnection
participant Handlers as eventHandlers Map
participant Sessions as sessions/maps
Note over Chrome,Sessions: Target.detachedFromTarget Flow
Chrome->>Conn: "Target.detachedFromTarget" (sessionId, targetId)
Conn->>Conn: Extract sessionId
Conn->>Conn: NEW: clearSessionEventHandlers(sessionId)
Conn->>Handlers: Iterate keys starting with `${sessionId}:`
Handlers-->>Conn: Delete all matching entries
Conn->>Sessions: Remove sessionId from sessions map
Conn->>Sessions: Remove sessionId from sessionToTarget map
Conn->>Sessions: Remove sessionId from latestCdpCallEvent map
Conn-->>Chrome: (event processed)
Note over Chrome,Sessions: Target.targetDestroyed Flow
Chrome->>Conn: "Target.targetDestroyed" (targetId)
Conn->>Conn: Find all sessionIds for targetId
loop For each sessionId mapped to this target
Conn->>Conn: NEW: clearSessionEventHandlers(sessionId)
Conn->>Handlers: Delete `${sessionId}:*` keys
Conn->>Sessions: Delete sessionId from sessions map
Conn->>Sessions: Delete sessionId from sessionToTarget map
Conn->>Sessions: Delete sessionId from latestCdpCallEvent map
end
Conn-->>Chrome: (event processed)
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Member
Author
Contributor
|
@seanmcguire12 I have started the AI code review. It will take a few minutes to complete. |
tkattkat
approved these changes
Jul 6, 2026
This was referenced Jul 6, 2026
Merged
seanmcguire12
pushed a commit
that referenced
this pull request
Jul 13, 2026
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/stagehand@3.7.0 ### Minor Changes - [#2283](#2283) [`871ca7e`](871ca7e) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - add `context.setDomainPolicy({ allowedDomains: ["allowed.domain"] })` which allows users to define a set of domains that are accessible to stagehand - [#2274](#2274) [`f31980f`](f31980f) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - add `context.setDomainPolicy({blockedDomains: ["some.domain"]})` which allows users to define a list of domains that will be blocked by stagehand ### Patch Changes - [#2305](#2305) [`cd1daad`](cd1daad) Thanks [@shrey150](https://github.com/shrey150)! - Remove the noisy AI SDK "system message in messages" warning logged on every hybrid/DOM `agent.execute()` call. - [#2328](#2328) [`d287ff4`](d287ff4) Thanks [@miguelg719](https://github.com/miguelg719)! - Allow modelName "auto" in the constructor and per-primitive model overrides when running through the Stagehand API - [#2294](#2294) [`3938590`](3938590) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - automatically close popups that violate user defined domain policy - [#2298](#2298) [`892701a`](892701a) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - Fix CUA `keypress` actions to press key combinations as a single chord. - [#2345](#2345) [`21826c7`](21826c7) Thanks [@monadoid](https://github.com/monadoid)! - Repair malformed UTF-16 snapshot text before it reaches model prompts. - [#2306](#2306) [`8dcef1b`](8dcef1b) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - Use the screenshot provider's declared media type when sending CUA image payloads. The `setScreenshotProvider` callback now returns `ScreenshotProviderResult` (`{ base64, mediaType }`) instead of a bare base64 string. - [#2273](#2273) [`93a23d3`](93a23d3) Thanks [@miguelg719](https://github.com/miguelg719)! - Add support for the new `google/gemini-3.5-flash` computer-use tools model - [#2278](#2278) [`022d68f`](022d68f) Thanks [@shrey150](https://github.com/shrey150)! - Fix `TypeError: Converting circular structure to JSON` when creating an agent with MCP `integrations` that include a `Client` instance (e.g. a local/stdio server from `connectToMCPServer`). The agent-creation log serialized the raw `integrations` array, and a live MCP `Client` is circular. It now logs a safe descriptor (URL strings kept, client instances summarized) so `agent({ integrations: [client] })` works. - [#2288](#2288) [`bb5ffa6`](bb5ffa6) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - clean up cdp session event handlers on target detach ## @browserbasehq/stagehand-evals@2.0.4 ### Patch Changes - Updated dependencies \[[`cd1daad`](cd1daad), [`d287ff4`](d287ff4), [`3938590`](3938590), [`892701a`](892701a), [`21826c7`](21826c7), [`8dcef1b`](8dcef1b), [`93a23d3`](93a23d3), [`871ca7e`](871ca7e), [`022d68f`](022d68f), [`bb5ffa6`](bb5ffa6), [`f31980f`](f31980f)]: - @browserbasehq/stagehand@3.7.0 ## @browserbasehq/stagehand-server-v3@3.7.2 ### Patch Changes - Updated dependencies \[[`cd1daad`](cd1daad), [`d287ff4`](d287ff4), [`3938590`](3938590), [`892701a`](892701a), [`21826c7`](21826c7), [`8dcef1b`](8dcef1b), [`93a23d3`](93a23d3), [`871ca7e`](871ca7e), [`022d68f`](022d68f), [`bb5ffa6`](bb5ffa6), [`f31980f`](f31980f)]: - @browserbasehq/stagehand@3.7.0 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
felipeofdev-ai
pushed a commit
to felipeofdev-ai/stagehand
that referenced
this pull request
Aug 4, 2026
# why
`CdpSession.on(...)` stores session scoped listeners on the root
`CdpConnection` under keys like `${sessionId}:${event}`. when chrome
sends `Target.detachedFromTarget`, `CdpConnection` removes session
metadata, but the actual event handlers were not being removed
this was problematic because stale handler functions remained reachable
for the lifetime of the root connection. this could cause memory to
grow, particularly for long sessions with a lot of attach/detach churn
# what changed
added `CdpConnection.clearSessionEventHandlers(sessionId)`, which:
- clears session scoped handlers when `Target.detachedFromTarget` or
`Target.targetDestroyed` fires
# test plan
added tests in
`packages/core/dist/esm/tests/unit/cdp-connection-close.test.js`, which
verify:
- detached session handlers are removed
- multiple handlers for the same detached session are removed
- handlers for other live sessions are preserved
- root-level handlers are preserved
- stale session handlers are removed when `Target.targetDestroyed` fires
<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
Fixes memory leaks and prevents hangs by cleaning up CDP session event
handlers and rejecting pending work when targets detach or are
destroyed. Aligns with Linear STG-2415.
- **Bug Fixes**
- Added `clearSessionEventHandlers(sessionId)` to remove
`${sessionId}:*` listeners.
- Added `rejectSessionPendingWork(sessionId, targetId)` to reject
in-flight `.send` calls and session dispatch waiters with
`PageNotFoundError`.
- On `Target.detachedFromTarget` and `Target.targetDestroyed`: call both
methods and delete affected sessions/mappings (for destroyed targets,
all sessions mapped to the target). Tests cover cleanup across
detach/destroy, multiple sessions per target, rejection of pending work,
and preservation of other sessions and root-level handlers.
<sup>Written for commit 5e7d04a.
Summary will update on new commits.</sup>
<a
href="https://cubic.dev/pr/browserbase/stagehand/pull/2288?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
felipeofdev-ai
pushed a commit
to felipeofdev-ai/stagehand
that referenced
this pull request
Aug 4, 2026
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/stagehand@3.7.0 ### Minor Changes - [browserbase#2283](browserbase#2283) [`871ca7e`](browserbase@871ca7e) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - add `context.setDomainPolicy({ allowedDomains: ["allowed.domain"] })` which allows users to define a set of domains that are accessible to stagehand - [browserbase#2274](browserbase#2274) [`f31980f`](browserbase@f31980f) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - add `context.setDomainPolicy({blockedDomains: ["some.domain"]})` which allows users to define a list of domains that will be blocked by stagehand ### Patch Changes - [browserbase#2305](browserbase#2305) [`cd1daad`](browserbase@cd1daad) Thanks [@shrey150](https://github.com/shrey150)! - Remove the noisy AI SDK "system message in messages" warning logged on every hybrid/DOM `agent.execute()` call. - [browserbase#2328](browserbase#2328) [`d287ff4`](browserbase@d287ff4) Thanks [@miguelg719](https://github.com/miguelg719)! - Allow modelName "auto" in the constructor and per-primitive model overrides when running through the Stagehand API - [browserbase#2294](browserbase#2294) [`3938590`](browserbase@3938590) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - automatically close popups that violate user defined domain policy - [browserbase#2298](browserbase#2298) [`892701a`](browserbase@892701a) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - Fix CUA `keypress` actions to press key combinations as a single chord. - [browserbase#2345](browserbase#2345) [`21826c7`](browserbase@21826c7) Thanks [@monadoid](https://github.com/monadoid)! - Repair malformed UTF-16 snapshot text before it reaches model prompts. - [browserbase#2306](browserbase#2306) [`8dcef1b`](browserbase@8dcef1b) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - Use the screenshot provider's declared media type when sending CUA image payloads. The `setScreenshotProvider` callback now returns `ScreenshotProviderResult` (`{ base64, mediaType }`) instead of a bare base64 string. - [browserbase#2273](browserbase#2273) [`93a23d3`](browserbase@93a23d3) Thanks [@miguelg719](https://github.com/miguelg719)! - Add support for the new `google/gemini-3.5-flash` computer-use tools model - [browserbase#2278](browserbase#2278) [`022d68f`](browserbase@022d68f) Thanks [@shrey150](https://github.com/shrey150)! - Fix `TypeError: Converting circular structure to JSON` when creating an agent with MCP `integrations` that include a `Client` instance (e.g. a local/stdio server from `connectToMCPServer`). The agent-creation log serialized the raw `integrations` array, and a live MCP `Client` is circular. It now logs a safe descriptor (URL strings kept, client instances summarized) so `agent({ integrations: [client] })` works. - [browserbase#2288](browserbase#2288) [`bb5ffa6`](browserbase@bb5ffa6) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - clean up cdp session event handlers on target detach ## @browserbasehq/stagehand-evals@2.0.4 ### Patch Changes - Updated dependencies \[[`cd1daad`](browserbase@cd1daad), [`d287ff4`](browserbase@d287ff4), [`3938590`](browserbase@3938590), [`892701a`](browserbase@892701a), [`21826c7`](browserbase@21826c7), [`8dcef1b`](browserbase@8dcef1b), [`93a23d3`](browserbase@93a23d3), [`871ca7e`](browserbase@871ca7e), [`022d68f`](browserbase@022d68f), [`bb5ffa6`](browserbase@bb5ffa6), [`f31980f`](browserbase@f31980f)]: - @browserbasehq/stagehand@3.7.0 ## @browserbasehq/stagehand-server-v3@3.7.2 ### Patch Changes - Updated dependencies \[[`cd1daad`](browserbase@cd1daad), [`d287ff4`](browserbase@d287ff4), [`3938590`](browserbase@3938590), [`892701a`](browserbase@892701a), [`21826c7`](browserbase@21826c7), [`8dcef1b`](browserbase@8dcef1b), [`93a23d3`](browserbase@93a23d3), [`871ca7e`](browserbase@871ca7e), [`022d68f`](browserbase@022d68f), [`bb5ffa6`](browserbase@bb5ffa6), [`f31980f`](browserbase@f31980f)]: - @browserbasehq/stagehand@3.7.0
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
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
CdpSession.on(...)stores session scoped listeners on the rootCdpConnectionunder keys like${sessionId}:${event}. when chrome sendsTarget.detachedFromTarget,CdpConnectionremoves session metadata, but the actual event handlers were not being removedthis was problematic because stale handler functions remained reachable for the lifetime of the root connection. this could cause memory to grow, particularly for long sessions with a lot of attach/detach churn
what changed
added
CdpConnection.clearSessionEventHandlers(sessionId), which:Target.detachedFromTargetorTarget.targetDestroyedfirestest plan
added tests in
packages/core/dist/esm/tests/unit/cdp-connection-close.test.js, which verify:Target.targetDestroyedfiresSummary by cubic
Fixes memory leaks and prevents hangs by cleaning up CDP session event handlers and rejecting pending work when targets detach or are destroyed. Aligns with Linear STG-2415.
clearSessionEventHandlers(sessionId)to remove${sessionId}:*listeners.rejectSessionPendingWork(sessionId, targetId)to reject in-flight.sendcalls and session dispatch waiters withPageNotFoundError.Target.detachedFromTargetandTarget.targetDestroyed: call both methods and delete affected sessions/mappings (for destroyed targets, all sessions mapped to the target). Tests cover cleanup across detach/destroy, multiple sessions per target, rejection of pending work, and preservation of other sessions and root-level handlers.Written for commit 5e7d04a. Summary will update on new commits.