Add Go browser transport and Browserbase session foundations - #2547
Merged
Conversation
Review finding: a blank (whitespace) caller extension ID was treated as absent, so Stagehand silently uploaded and substituted its own extension, diverging from the TS presence (??) semantics. browserbaseCallerExtensionID now uses pointer presence; blank IDs are rejected by the existing create-request validation with zero extension upload/create/delete calls.
|
This was referenced Aug 1, 2026
miguelg719
marked this pull request as ready for review
August 1, 2026 06:11
Contributor
There was a problem hiding this comment.
3 issues found across 7 files
Confidence score: 3/5
- In
packages/sdk-go/browserbase_session.goandpackages/sdk-go/browserbase_client.go, session create/retrieve failures currently propagate raw Browserbase response details through returned errors, which can leak upstream internals to SDK consumers and make error contracts inconsistent — map these paths to the fixed session-safe error and drop the upstream cause/body. - In
packages/sdk-go/browserbase_client.go, retrievedConnectURLvalues are not validated forws/wssand can behttp, relative, or malformed, which risks runtime CDP connection failures or unsafe endpoint usage — apply the same strict WebSocket URL validation used in create-session before accepting the endpoint.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/sdk-go/browserbase_session.go">
<violation number="1" location="packages/sdk-go/browserbase_session.go:171">
P2: Failed Browserbase lookups expose upstream response details through the returned error. Return a fixed session error without retaining the upstream cause.
(Based on your team's feedback about sanitizing Browserbase session errors.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/sdk-go/browserbase_client.go">
<violation number="1" location="packages/sdk-go/browserbase_client.go:183">
P2: Session retrieval failures preserve the raw Browserbase response body and message through the connect error path, so upstream details can reach SDK consumers. Convert create/retrieve failures to the session-safe error before wrapping or returning them, without retaining `BrowserbaseAPIError` as a cause.
(Based on your team's feedback about sanitizing Browserbase session errors.) [FEEDBACK_USED]</violation>
<violation number="2" location="packages/sdk-go/browserbase_client.go:827">
P2: A retrieved `connectUrl` with `http`, a relative URL, or malformed WebSocket URL passes validation and is later used as the CDP endpoint. Validate a present `ConnectURL` with the same `ws`/`wss` check as create-session responses, while still allowing it to be omitted.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Ext as External Caller
participant SessClient as browserbaseSessionClient
participant BBClient as browserbaseHTTPClient
participant FakeAPI as fakeBrowserbaseAPI (test)
participant RPC as rpcClient
participant Transport as rpcTransport (WebSocket)
Note over Ext,Transport: NEW: Borrow-Don't-Own Transport
Ext->>RPC: newRPCClient(transport, ownsTransport)
alt ownsTransport = true
RPC->>RPC: Store transport ownership flag
RPC->>RPC: Cancel context, reject pending calls
RPC->>Transport: Close transport on shutdown
else ownsTransport = false
RPC->>RPC: Store transport ownership flag
RPC->>RPC: Cancel context, reject pending calls
RPC->>RPC: Skip transport.Close() on shutdown
end
Transport-->>RPC: (transport not closed)
Note over Ext,BBClient: NEW: Browserbase Session Connect
Ext->>SessClient: connectSession(ctx, sessionID)
SessClient->>SessClient: Normalize & validate sessionID
SessClient->>BBClient: retrieveSession(ctx, sessionID)
BBClient->>BBClient: Encode GET /v1/sessions/{sessionID}
BBClient->>BBClient: Validate session ID required
BBClient->>External API: HTTP GET /v1/sessions/{id}
External API-->>BBClient: browserbaseRetrieveSessionResponse
BBClient->>BBClient: Validate response (id required, connectUrl/region optional)
BBClient-->>SessClient: browserbaseRetrieveSessionResponse
SessClient->>SessClient: Validate retrieved session
SessClient->>SessClient: Extract ConnectURL (trim spaces)
alt ConnectURL empty
SessClient-->>Ext: Error: session not available
else ConnectURL present
SessClient-->>Ext: browserbaseSessionConnection{ sessionID, cdpURL, region }
end
Note over Ext,FakeAPI: NEW: Caller-Supplied Extension IDs
Ext->>SessClient: createSession(ctx, params)
SessClient->>SessClient: browserbaseCallerExtensionID(params)
alt Caller provided ExtensionID (top-level or BrowserSettings)
SessClient->>SessClient: Use caller's ExtensionID directly
SessClient->>SessClient: Set request.ExtensionID = callerExtensionID
SessClient->>SessClient: ownsExtension = false
else No caller ExtensionID
SessClient->>SessClient: Use pending-stagehand-extension placeholder
SessClient->>SessClient: Upload Stagehand extension via api.uploadExtension()
SessClient->>SessClient: Set request.ExtensionID = uploadedID
SessClient->>SessClient: ownsExtension = true
end
SessClient->>BBClient: createSession(ctx, request)
BBClient-->>SessClient: browserbaseCreateSessionResponse
alt Create failed
SessClient->>SessClient: deleteExtensionBestEffort(ctx, extensionID, ownsExtension)
alt ownsExtension
SessClient->>BBClient: deleteExtension(ctx, extensionID)
end
SessClient-->>Ext: Error
else Session invalid
SessClient->>SessClient: cleanupInvalidSession(ctx, sessionID, extensionID, ownsExtension)
SessClient->>BBClient: releaseSession(ctx, sessionID)
alt ownsExtension
SessClient->>BBClient: deleteExtension(ctx, extensionID)
end
SessClient-->>Ext: Error
end
Note over Ext,FakeAPI: NEW: Extension cleanup respects ownership
Ext->>SessClient: resources.close(ctx)
alt ownsExtension AND not already deleted
SessClient->>BBClient: deleteExtension(ctx, extensionID)
end
SessClient->>BBClient: releaseSession(ctx, sessionID)
Note over Ext,FakeAPI: TEST: Caller extensions never uploaded/deleted
Ext->>FakeAPI: createSession with caller ExtensionID
FakeAPI->>SessClient: Handle session creation
alt Success path
FakeAPI-->>Ext: no uploadExtension/deleteExtension calls
else Create failure path
FakeAPI-->>Ext: no uploadExtension/deleteExtension calls
else Invalid session path
FakeAPI-->>Ext: no uploadExtension/deleteExtension calls
end
alt Close after success
SessClient->>FakeAPI: releaseSession called
FakeAPI-->>Ext: no deleteExtension calls
end
Note over Ext,FakeAPI: TEST: Owned extension gets uploaded/deleted
Ext->>FakeAPI: createSession without caller extension
FakeAPI->>FakeAPI: uploadExtension called
FakeAPI->>SessClient: Handle session
SessClient->>FakeAPI: releaseSession called
alt Close session
FakeAPI->>FakeAPI: deleteExtension called
end
Note over Ext,Transport: CHANGED: RPC transport ownership in connectRPCClient
Ext->>RPC: connectRPCClient(wsURL)
RPC->>Transport: WebSocket connect
RPC->>RPC: newRPCClient(transport, ownsTransport=true)
Transport-->>Ext: rpcClient with owned transport
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…onnectUrl Mirror the TypeScript client's error boundaries in the Browserbase session client: session create and retrieve failures now return fixed errors instead of wrapping the upstream *BrowserbaseAPIError, so raw response bodies no longer reach SDK consumers. Session/extension cleanup on the create failure paths runs on context.WithoutCancel(ctx) so a timed-out create still releases the session and deletes the uploaded extension. Retrieved sessions validate a present connectUrl as ws/wss, matching the create-session response.
…tions' into feat/stagehand-go-browser-foundations
akeimach
approved these changes
Aug 3, 2026
…-browser-foundations
…-browser-foundations
miguelg719
added a commit
that referenced
this pull request
Aug 3, 2026
# why With transport ownership and session foundations in place, this stack entry adds the full new lifecycle additively: browser factories and `stagehand.Create`, while legacy `New`/`Init` keeps working untouched. # what changed - adds exported `Browser` handle (unexported fields; `Provider`/`Origin`/`Closed`/`Close` only) with one-time Stagehand claiming and idempotent, memoized, race-safe `Close` - adds `LaunchLocalBrowser` / `ConnectLocalBrowser` / `LaunchBrowserbase` / `ConnectBrowserbase`, each resolving only after the Stagehand extension service worker is ready - implements the ownership rule `ownsSource = launched && !keepAlive`; failed connects clean up owned sources with `errors.Join`, keep-alive sources are left running - implements factory-path downloads via root-session `Browser.setDownloadBehavior` - Browserbase launches merge `userMetadata` with `stagehand_sdk_language: "go"` and honor caller extension IDs per the foundations PR - adds `stagehand.Create(ctx, CreateOptions)`: claims the handle, attaches over the browser-owned transport (never closes it), releases the claim on failure so `Create` can be retried on the same handle - keeps the central `StagehandInitParams` literal in `stagehand.go` feeding both lifecycles (ast-grep sdk-parity constraint) # compatibility - legacy `New`/`Init`/`Close`, all examples, and the ast-grep rules pass unchanged — this layer is purely additive # stack 1. #2547 — transport and Browserbase session foundations 2. **this PR — browser factories and `stagehand.Create`** 3. #2549 — remove the legacy lifecycle and migrate consumers # test plan - new `browser_test.go`: claim/release/re-claim, close idempotence and concurrent-close context handling, ownership matrix, extension routing, download validation and command capture, metadata/region propagation - `Create` wire-shape tests over a recording protocol client: handle API key wins, local handles omit `Browser`, failed init releases the claim - full package gates green: gofmt, `go vet`, `go build`, `go test`, generator `--check`, examples compile, root `pnpm run test:unit`, changeset check
miguelg719
added a commit
that referenced
this pull request
Aug 3, 2026
## Summary Completes the Go port of the browser-lifecycle stack (#2517–#2523) by making `stagehand.Create(ctx, CreateOptions)` the sole construction path, mirroring the TypeScript end state. - removes `New`, `Init`, `StagehandClientInitParams`, the `BrowserSource` union, and the public `ResolvedBrowserSource`; the raw-CDP-with-headers path has no replacement (`ConnectLocalBrowser` takes a bare CDP URL) - `Browser()` returns the exact handle passed to `CreateOptions` - `Stagehand.Close` never closes the CDP transport, Chrome process, or Browserbase session; browser lifetime is exclusively `Browser.Close(ctx)`; `Close` results are memoized for TS `closePromise` parity - updates `ErrNotInitialized` message to point at the new lifecycle (exported var name unchanged) - migrates all 7 examples and the live tests to launch → `Create` → `client.Close(ctx)` → `browser.Close(ctx)` (deferred in that order so a failed client close can't leak the process) - updates the Go ast-grep example-parity patterns to the multi-value `stagehand.Create` shape in the same commit (they gate TS CI) ## Reviewer focus 1. `Stagehand.Close` stops the runtime; `Browser.Close` owns browser/session cleanup — under no configuration does Stagehand touch the browser-owned transport. 2. Browser acquisition options stay client-side; `stagehand.init` wire payload is unchanged (`models.gen.go` untouched, no regeneration). 3. Parity accessors remain exactly `{Browser, Context, Initialized}`; the central `StagehandInitParams` literal stays in `stagehand.go`. ## Follow-up (not in this PR) - `packages/docs/v4/**` Go snippets still show the deleted `New`/`Init` lifecycle (~10 files); docs migration should follow once this stack settles. ## Stack - #2547 — transport and Browserbase session foundations - #2548 — browser factories and `stagehand.Create` - **this PR** — remove the legacy lifecycle and migrate consumers ## Verification - full package gates green: gofmt, `go vet`, `go build`, `go test`, generator `--check` + generator tests, all 7 examples compile - root `pnpm run test:unit` green (ast-grep example-parity + sdk-parity against the migrated Go examples/source) - changeset check passed; live tests (`CHROME_PATH`) migrated and exercised by CI <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Make `stagehand.Create(ctx, CreateOptions)` the only way to build the Go client and finalize split lifecycles: the client stops the worker; the `Browser` owns its cleanup. `Stagehand.Close` memoizes the first result (including failures) for repeated or concurrent calls. - **Refactors** - Removed `New`, `Init`, `StagehandClientInitParams`, the `BrowserSource` union, and public `ResolvedBrowserSource`; deleted the legacy resolver and tests. - `Stagehand.Browser()` returns the exact `*Browser` passed to `Create`; the client never closes Chrome or a Browserbase session. - Standardized factories: `LaunchLocalBrowser`, `ConnectLocalBrowser`, `LaunchBrowserbase` (uses `BrowserbaseLaunchOptions`); factories materialize the bundled extension and set `extensionDir` on `Browser`. - Enforced local `KeepAlive` at the factory: `Browser.Close` terminates a launched Chrome only when `KeepAlive` is false (covered by tests). - Updated examples, live tests, and ast-grep rules to launch/connect → `Create` → `client.Close(ctx)` → `browser.Close(ctx)`; Go parity now uses `create()` (Python still `init()`). - **Migration** - Replace: - `client := stagehand.New(...); client.Init(ctx)` with: - `browser := stagehand.LaunchLocalBrowser(...) | ConnectLocalBrowser(...) | LaunchBrowserbase(...)` - `client, _ := stagehand.Create(ctx, stagehand.CreateOptions{Browser: browser, ...})` - Manage lifetimes separately: `defer client.Close(ctx)` and `defer browser.Close(ctx)`. - For existing CDP, use `ConnectLocalBrowser(ctx, LocalBrowserConnectOptions{CDPURL: ...})`. - For Browserbase, create via `LaunchBrowserbase(ctx, stagehand.BrowserbaseLaunchOptions{APIKey: ...})`. <sup>Written for commit ad12189. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/browserbase/stagehand/pull/2549?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. -->
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
Ports the merged TypeScript browser-lifecycle stack (#2517–#2523) to the Go SDK. This bottom PR lands the internal plumbing the factory lifecycle needs — borrow-don't-own transports and Browserbase session connect semantics — with zero public API change.
what changed
rpcClient: shutdown still cancels, rejects pending calls, and clears handlers, but only closes the transport when ownedretrieveSession(GET/v1/sessions/{id}) to the Browserbase client with deliberately lenient validation (idrequired;connectUrl/regionoptional)connectSessionthat never takes release ownership of an existing sessionintentionally not included
New/Init/Closebehavior is unchangedBrowserhandle, noCreatestack
stagehand.Createtest plan
httptestcoverage forretrieveSession, fake-API cases proving caller extension IDs are never uploaded/deletedgo vet,go build,go test, generator--check, examples compile, rootpnpm run test:unit, changeset check