Implement Go browser factories and stagehand.Create - #2548
Merged
miguelg719 merged 9 commits intoAug 3, 2026
Merged
Conversation
- reject zero-value Browser handles in the default Create adapter instead of dereferencing a typed-nil cdpClient (panic) - make concurrent Browser.Close waiters honor their context while preserving the memoized close result - defensively copy the Browserbase region into worker init metadata - retype launchChrome plainly to LocalBrowserLaunchOptions, dropping the generic union bridge, and touch up the live test call site - assert LaunchBrowserbase forwards the session-create payload
|
This was referenced Aug 1, 2026
miguelg719
marked this pull request as ready for review
August 1, 2026 06:10
Contributor
There was a problem hiding this comment.
7 issues found across 12 files
Confidence score: 2/5
- In
packages/sdk-go/browser_factories.go, extension discovery during launch is not filtered by the caller’s extension ID, so a session can attach to the wrong installed Stagehand worker and cross wires between environments—thread the supplied top-level/BrowserSettingsextension ID into ID-filtered worker discovery. - In
packages/sdk-go/browser_factories.go, timed-out factory connects perform cleanup with an already-canceled context, which can leave Browserbase sessions and uploaded extensions orphaned—run teardown with a fresh non-canceled cleanup context (while preserving needed values) so owned resources are reliably released. packages/sdk-go/browser_source.goregresses the legacyNew(...).Init()path by failing local download configurations (including validAcceptDownloads: &false), andpackages/sdk-go/stagehand.goletsCreateclients callInitin invalid lifecycle states; both can cause unexpected runtime failures—restore equivalent post-connect CDP setup for local sources and rejectInitwhenattachedBrowserindicates a factory-managed handle.- In
packages/sdk-go/browser_factories.go, local launch withKeepAlive=truecan skip source ownership teardown, so launched sources and temp user-data directories may never be cleaned up; timeout input validation is also deferred too deep and fails unclearly—ensure launched sources are closed when owned and reject negativeConnectTimeoutMsat API boundaries.
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/browser_source.go">
<violation number="1" location="packages/sdk-go/browser_source.go:129">
P2: Legacy `New(...).Init()` now fails for every local download configuration, including valid `AcceptDownloads: &false`, instead of preserving its `LocalBrowserSource` contract. Apply the equivalent post-connect CDP setup in the legacy path, or retain support rather than rejecting these options.</violation>
</file>
<file name="packages/sdk-go/stagehand.go">
<violation number="1" location="packages/sdk-go/stagehand.go:103">
P2: `Create` clients silently accept `Init` while live and can enter legacy initialization after `Close`, rather than rejecting factory-handle re-entry. Check `attachedBrowser` at the start of `Init` and return an error while retaining the existing idempotent behavior for `New` clients.
(Based on your team's feedback about rejecting init re-entry on Stagehand.create instances.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/sdk-go/browser_factories.go">
<violation number="1" location="packages/sdk-go/browser_factories.go:149">
P2: When a local browser is launched with KeepAlive=true, ownsSource evaluates to false, so the launched createSource/closeSource (and its temp user-data directory cleanup in launched.close) is never invoked either by Browser.Close() or by the failure path in connectBrowser. The SDK-spawned Chromium process keeps running and its os.MkdirTemp "stagehand-chrome-" profile directory is left on disk forever, since removeChromeProfile only runs inside launched.close. The ownership matrix intentionally leaves the process running, but for a locally launched browser there is no later SDK path to reclaim that temp profile or process. Consider either disallowing/keeping the browser for local keep-alive (e.g., treating local launch keep-alive as connection-only), or ensuring the temp profile is removed when the process is eventually terminated, and documenting the keep-alive semantics for local launches.</violation>
<violation number="2" location="packages/sdk-go/browser_factories.go:173">
P3: ConnectLocalBrowser and ConnectBrowserbase pass a caller-supplied ConnectTimeoutMs straight into connectBrowser without validating that it is non-negative. A negative value eventually fails deep in validateCDPClientOptions with the generic "CDP connect timeout must be positive" message rather than a stagehand options error, and a value of 0 silently falls back to the default timeout. For consistency with LaunchLocalBrowser (which rejects negative ConnectTimeoutMs in validateLocalBrowserOptions), consider validating this field up front in the connect paths (or short-circuiting to the default when <= 0) so callers get a clear, symmetric error.</violation>
<violation number="3" location="packages/sdk-go/browser_factories.go:204">
P1: Launching with a caller extension ID can attach to another installed Stagehand worker because preloaded discovery is unfiltered. Route the supplied top-level or `BrowserSettings` extension ID into ID-filtered service-worker discovery.</violation>
<violation number="4" location="packages/sdk-go/browser_factories.go:283">
P1: Timed-out factory connects can leak Browserbase sessions and uploaded extensions because cleanup reuses the already-canceled connect context. Use a non-canceled cleanup context while retaining values so owned resources are actually released.</violation>
</file>
<file name="packages/sdk-go/browser.go">
<violation number="1" location="packages/sdk-go/browser.go:153">
P3: The `commandTimeout` field is copied from Browser into claimedBrowser and then never consumed anywhere in the Create flow. Stagehand.Create only reads `claimed.workerAPIKey`, `claimed.workerBrowser`, and `claimed.cdp`, and the actual CDP command timeout is already baked into the cdpClient that newRPCClient wraps. This leaves a dead, unexported field that adds confusion about whether a per-attachment timeout can be configured. Consider dropping `commandTimeout` from claimedBrowser (and/or Browser) or documenting that the CDP client owns the timeout if it's meant to be retained for future wiring.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Caller as Caller
participant Browser as Browser Factory
participant CDP as CDP Client
participant Stagehand as Stagehand Client
participant RPC as RPC Client
participant Browserbase as Browserbase API
participant Chrome as Chrome Process
participant Extension as Stagehand Extension
Note over Caller,Extension: NEW: Browser lifecycle with factory functions
alt LaunchLocalBrowser
Caller->>Browser: LaunchLocalBrowser(options)
Browser->>Browser: Validate downloadsPath
Browser->>Extension: materializeBrowserExtension()
Extension-->>Browser: extensionDir, cleanup
Browser->>Chrome: launchLocalBrowser(options)
Chrome-->>Browser: resolvedBrowserSource (cdpURL)
alt AcceptDownloads set
Browser->>CDP: sendCommand("Browser.setDownloadBehavior", params)
end
else ConnectLocalBrowser
Caller->>Browser: ConnectLocalBrowser(options)
alt No ExtensionID
Browser->>Extension: materializeBrowserExtension()
Extension-->>Browser: extensionDir, cleanup
end
Browser->>CDP: connectCDP(cdpURL, extensionDir)
else LaunchBrowserbase
Caller->>Browser: LaunchBrowserbase(options)
Browser->>Browserbase: createSession() with userMetadata
Note over Browserbase: Merges stagehand_sdk_language: "go"
Browserbase-->>Browser: resolvedBrowserSource (cdpURL, sessionID)
Browser->>CDP: connectCDP(cdpURL, preloadedExtension)
else ConnectBrowserbase
Caller->>Browser: ConnectBrowserbase(options)
Browser->>Browserbase: connectSession(sessionID)
Browserbase-->>Browser: cdpURL, sessionID, region
Browser->>CDP: connectCDP(cdpURL, extensionID)
end
CDP-->>Browser: cdpClient
Browser-->>Caller: *Browser (unexported fields)
Note over Caller,Stagehand: NEW: Create flow over a Browser handle
Caller->>Stagehand: Create(ctx, CreateOptions{Browser})
Stagehand->>Browser: claimBrowser(browser)
alt Claim fails
Browser-->>Stagehand: error (already claimed/closed)
Stagehand-->>Caller: error
else Claim succeeds
Browser-->>Stagehand: claimedBrowser (cdp, workerAPIKey, workerBrowser)
Stagehand->>RPC: connectClaimedBrowser(claimed)
RPC-->>Stagehand: protocolClient
Stagehand->>RPC: stagehand.init(initParams)
Note over Stagehand,RPC: APIKey from workerAPIKey wins over options
alt Init fails
RPC-->>Stagehand: error
Stagehand->>Browser: releaseBrowserClaim()
Stagehand->>RPC: rpc.close()
Stagehand-->>Caller: error
else Init succeeds
RPC-->>Stagehand: StagehandInitResult
Stagehand-->>Caller: *Stagehand (initialized, attached)
end
end
Note over Caller,Browser: NEW: Close semantics
Caller->>Browser: Close(ctx)
Browser->>Browser: Lock mu, mark closeRequested
Browser->>CDP: cdp.Close()
alt ownsSource = launched && !keepAlive
Browser->>Chrome: closeSource(ctx) (kill process)
Browser->>Extension: cleanup() (remove temp dir)
else keepAlive or connected
Note over Browser: Skip source cleanup
end
Browser-->>Caller: errors.Join(result)
Browser-->>Caller: Memoized on subsequent calls
Note over Caller,Browser: Error path: failed connect cleans owned sources
alt Provider factory fails
Browser->>Extension: cleanup()
Browser-->>Caller: errors.Join(factoryErr, cleanupErr)
else Connect fails (owned)
Browser->>Extension: cleanup()
Browser->>Chrome: closeSource(ctx)
Browser-->>Caller: errors.Join(connectErr, cleanupErr, sourceErr)
else Connect fails (keepAlive)
Note over Browser: Skip source cleanup
Browser-->>Caller: connectErr only
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…and-go-browser-factories
Close an owned browser source on context.WithoutCancel(ctx) when a factory connect fails, so a timed-out connect still releases the Browserbase session and its uploaded extension. Reject legacy Init on a Stagehand built by Create, mirroring the TypeScript client's attachedBrowser guard. Drop the dead commandTimeout field from Browser and claimedBrowser: the CDP client owns the command timeout. Validate a negative ConnectTimeoutMs at both connect factory boundaries before any resource is allocated, matching LaunchLocalBrowser and the TypeScript connect schemas. Document local KeepAlive ownership.
…ies' into feat/stagehand-go-browser-factories
…and-go-browser-factories
akeimach
approved these changes
Aug 3, 2026
…and-go-browser-factories
…and-go-browser-factories
miguelg719
added a commit
that referenced
this pull request
Aug 3, 2026
# 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 - adds transport ownership to `rpcClient`: shutdown still cancels, rejects pending calls, and clears handlers, but only closes the transport when owned - adds `retrieveSession` (GET `/v1/sessions/{id}`) to the Browserbase client with deliberately lenient validation (`id` required; `connectUrl`/`region` optional) - adds internal `connectSession` that never takes release ownership of an existing session - caller-supplied extension IDs suppress Stagehand extension provisioning **and** cleanup on every path (success, create failure, close) # intentionally not included - no exported identifier added or changed; legacy `New`/`Init`/`Close` behavior is unchanged - no factories, no `Browser` handle, no `Create` # stack 1. **this PR — transport and Browserbase session foundations** 2. #2548 — browser factories and `stagehand.Create` 3. #2549 — remove the legacy lifecycle and migrate consumers # test plan - table tests for un-owned shutdown (transport left open, pending calls still rejected), `httptest` coverage for `retrieveSession`, fake-API cases proving caller extension IDs are never uploaded/deleted - 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
With transport ownership and session foundations in place, this stack entry adds the full new lifecycle additively: browser factories and
stagehand.Create, while legacyNew/Initkeeps working untouched.what changed
Browserhandle (unexported fields;Provider/Origin/Closed/Closeonly) with one-time Stagehand claiming and idempotent, memoized, race-safeCloseLaunchLocalBrowser/ConnectLocalBrowser/LaunchBrowserbase/ConnectBrowserbase, each resolving only after the Stagehand extension service worker is readyownsSource = launched && !keepAlive; failed connects clean up owned sources witherrors.Join, keep-alive sources are left runningBrowser.setDownloadBehavioruserMetadatawithstagehand_sdk_language: "go"and honor caller extension IDs per the foundations PRstagehand.Create(ctx, CreateOptions): claims the handle, attaches over the browser-owned transport (never closes it), releases the claim on failure soCreatecan be retried on the same handleStagehandInitParamsliteral instagehand.gofeeding both lifecycles (ast-grep sdk-parity constraint)compatibility
New/Init/Close, all examples, and the ast-grep rules pass unchanged — this layer is purely additivestack
stagehand.Createtest plan
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 propagationCreatewire-shape tests over a recording protocol client: handle API key wins, local handles omitBrowser, failed init releases the claimgo vet,go build,go test, generator--check, examples compile, rootpnpm run test:unit, changeset check