[mkuritsu]

This PR setups BrowserRun wrapped binding with the methods available to later move every call to go through it.

It also adds ability for Websocket initiated connections to be auto upgraded, so users don't need to do it manually, improving the API in this regard.

[clanky-the-clanker]

💬 Approved with comments — The wrapper and the WebSocket auto-upgrade look correct, and the tests cover both the success and failure paths. One WARNING: the public Browser Run TypeScript declarations don't include the new websocket option or the upgraded WebSocket return type.

Findings: 1 warning

_{The decision is advisory: this comment never approves, blocks or requests changes on the pull request.}

🔧 Fix with your agent

Paste the following into your coding agent to investigate and fix these issues.

Fix the following code review issues found in pull request #7999.

Before making changes, review each issue and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g., the fix would require disproportionate changes
  for minimal ROI, or the finding is factually incorrect), explain why
- If you need to clarify something before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order

## WARNING (1)

### [Documentation] The public `types/defines/browser-run.d.ts` doesn't declare the new `websocket` option for `launch`/`connectSession` or the upgraded `webSocket: WebSocket` return type, so TypeScript users can't use the feature without a type error. The new internal copy `src/cloudflare/internal/browser-run.d.ts` could also drift from the public one.
- **File:** src/cloudflare/internal/br-api.ts:46

---

📝 Documentation: ⚠️ 1 warning

The public Browser Run TypeScript declarations don't include the new WebSocket option.

Recommendation: 📝 Minor Comments

• [WARNING] The public types/defines/browser-run.d.ts doesn't declare the new websocket option for launch/connectSession or the upgraded webSocket: WebSocket return type, so TypeScript users can't use the feature without a type error. The new internal copy src/cloudflare/internal/browser-run.d.ts could also drift from the public one. (src/cloudflare/internal/br-api.ts:46)

View full review

src/cloudflare/internal/br-api.ts:46-56 and :83-106 add a websocket?: boolean option to launch() and connectSession(). With that option set, the returned connection's webSocket is a WebSocket, not a Fetcher.

The public declarations in types/defines/browser-run.d.ts (for example webSocket: Fetcher at line 512) still only accept BrowserRunAcquireOptions / BrowserRunConnectOptions. TypeScript users therefore get a type error when they pass { websocket: true }, and the result is typed wrongly when the upgrade happens.

The pull request also adds src/cloudflare/internal/browser-run.d.ts (+916 lines), which looks like a copy of the public defines. Please update the public declarations to match, and either keep the two files in sync or have the internal module reuse the public one, so they don't drift apart.

---

✅ All other checks passed

🔒 Security: ✅ LGTM

No security concerns in this change.

---

📖 AGENTS.md: ✅ LGTM

No AGENTS.md updates are needed for this change.

---

📋 Code Quality: ✅ LGTM

No issues in the changed code.

• Wrappers: each BrowserRendering method passes straight through to the fetcher binding. The class now uses the #fetcher private field instead of private readonly, which follows the repository's TypeScript convention.
• launch({ websocket: true }): if the upgrade fails, the session it just created is closed. A failure in closeSession is ignored, so the original upgrade error is the one that reaches the caller.
• connectSession({ websocket: true }): if the upgrade fails, the existing session is left open, which fits connecting to a session the caller didn't create.
• Tests: the new tests cover the default return, a successful upgrade, and a failed upgrade, for both launch and connectSession.

---

⚡ Performance: ✅ LGTM

No performance concerns in this change.

---

_{Reviewed commit: 2172ee53}

[ask-bonk]

Review: 1 finding (1 warning).

Adds Browser Run forwarding methods and optional WebSocket auto-upgrade coverage.

Specialist findings not posted (4)

• api-compat#1 [BLOCKING] src/cloudflare/internal/br-api.ts:59: Switching from the previous this.fetcher = fetcher assignment to an ECMAScript private field removes the binding instance's own fetcher property. That was observable to existing JavaScript Workers… — fetcher was an undocumented TypeScript-private implementation field, so removing it is not a supported Browser Run API change.
• tests#1 [WARNING] src/cloudflare/internal/br-api.ts:83: websocket is intentionally a wrapper-only option, but the mock ignores unknown options. Consequently all of the launch/connect tests would still pass if either method forwarded websocket to the un… — The review does not establish that the underlying Browser Run binding rejects unknown RPC option fields, so forwarding this wrapper-only field is not a demonstrated defect.
• tests#2 [WARNING] src/cloudflare/internal/br-api.ts:152: The failed-upgrade tests only make the mock return HTTP 503, so they still pass if the || !webSocket half of this condition is removed. UpgradeResponse explicitly permits a null socket, and a 101 … — The Worker Response implementation refuses status 101 unless a WebSocket is supplied, so this missing mock case is not shown reachable by the exercised binding path.
• design-simplicity#1 [WARNING] src/cloudflare/internal/br-api.ts:20: Fetcher restates every non-generic member already declared on BrowserRun in browser-run.d.ts. That makes this forwarding-only interface another API surface that must be updated for each Browser … — The raw fetcher must not expose the wrapper-only websocket option, so inheriting the public BrowserRun methods would become incorrect once the declarations are fixed.

---

_{Reviewed commit: 2172ee53 · github run}

[danlapid]

I'd rather we model launch as just jsrpc too.

[mkuritsu]

If I understood correctly you are referring to the websocket upgrade in that function? We wanted to have that to not require users to manually call the websocket upgrade (since WebSocket does not go through JSRPC).

iirc the AI binding does something similar of calling the upgrade inside the wrapped binding itself