Remove the legacy Go Stagehand lifecycle - #2549
Merged
miguelg719 merged 10 commits intoAug 3, 2026
Merged
Conversation
Review finding: after a failed first Close, every later caller got nil, hiding the cleanup failure and diverging from the TS memoized closePromise (and from Browser.Close in this SDK). Close now records its joined result and repeated/concurrent calls return it without re-running teardown.
|
This was referenced Aug 1, 2026
miguelg719
marked this pull request as ready for review
August 1, 2026 06:09
Contributor
There was a problem hiding this comment.
4 issues found across 26 files
Confidence score: 3/5
- In
packages/sdk-go/browserbase_session.go(LaunchBrowserbase), allowing caller-supplied extension IDs can bypass the Stagehand extension even though the factory reports it as preloaded, which can cause runtime feature failures and hard-to-diagnose behavior mismatches — reject both extension-ID inputs at the factory boundary and remove those fields from launch options. - In
packages/sdk-go/stagehand.go(newStagehandWithClient), running the fullcreateWithAdapterspath and issuing init viacontext.Background()means init calls may hang without timeout/cancellation, increasing risk of stuck startup and leaked work under failure conditions — thread through caller context (or a bounded timeout) for the init RPC. - In
packages/sdk-go/chrome_launcher.go, the KeepAlive ownership shift is under-tested, so close semantics may regress and either terminate Chrome unexpectedly or leak processes depending onKeepAlivestate — add focused true/false regression tests with a closable mock source to verifyBrowser.Closebehavior. - In
packages/sdk-go/browser_factories.go,materializeBrowserExtensionis now a pass-through alias, which adds indirection without behavior and can obscure where extension resolution dependencies were removed — either inline/remove the alias or restore meaningful separation with explicit dependency handling.
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:196">
P2: `LaunchBrowserbase` still accepts caller-provided extension IDs, so callers can launch without the Stagehand extension while the factory marks it preloaded; reject both extension fields at the factory boundary and remove them from the public launch shape. This conflicts with the factory schema contract and can leave the returned Browser unable to find its Stagehand service worker.
(Based on your team's feedback about Browserbase launch schema boundary.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/sdk-go/chrome_launcher.go">
<violation number="1" location="packages/sdk-go/chrome_launcher.go:95">
P3: Local-browser `KeepAlive` ownership moved to the factory but has no regression coverage. Add focused true/false tests with a closable mocked source so `Browser.Close` is verified to retain or terminate launched Chrome as configured.
(Based on your team's feedback about adding unit tests for new behavior.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/sdk-go/browser_factories.go">
<violation number="1" location="packages/sdk-go/browser_factories.go:248">
P3: materializeBrowserExtension is now a pointless one-line alias for materializeStagehandExtension with an identical signature and no transformation. Since this change removed the previous browserSourceResolverDependencies adaptation, the indirection no longer serves a purpose and adds a small amount of confusion about which function actually owns the logic. Consider calling materializeStagehandExtension directly from the two call sites (launchLocalBrowserWithDependencies and connectLocalBrowserWithDependencies) and dropping the wrapper, or keeping only one named entry point.</violation>
</file>
<file name="packages/sdk-go/stagehand.go">
<violation number="1" location="packages/sdk-go/stagehand.go:463">
P3: The `newStagehandWithClient` helper was rewritten to run the entire `createWithAdapters` flow, and it calls the init RPC with `context.Background()` (no timeout, no cancellation). It lives in the production `stagehand.go` yet is only ever referenced from `_test.go` files. Since it drives `stagehand.init` through the real create path, an unbounded background context means a hung/blocked transport would stall forever with no way to cancel. Consider moving it into a `_test.go` file and passing a caller-supplied context through to `createWithAdapters` to avoid the footgun in production code.</violation>
</file>
Architecture diagram
sequenceDiagram
participant App as Application Code
participant Factory as Browser Factory
participant Stagehand as Stagehand.Create()
participant RPC as CDP Transport
participant Worker as Stagehand Worker
Note over App,Worker: NEW: Two-phase lifecycle (Browser then Stagehand)
App->>Factory: LaunchLocalBrowser/ConnectLocalBrowser/LaunchBrowserbase(ctx, opts)
Factory->>Factory: Resolve browser source (launch Chrome / connect CDP / create Browserbase session)
Factory->>Factory: Materialize Stagehand extension
Factory-->>App: *Browser handle
App->>Stagehand: Create(ctx, CreateOptions{Browser: browser, ...})
Stagehand->>RPC: Connect via claimed browser CDP
RPC-->>Stagehand: protocolClient
Stagehand->>RPC: onNotification("stagehand.log")
Stagehand->>RPC: onRequest("llm.generate") [optional]
Stagehand->>Worker: stagehand.init(StagehandInitParams)
Worker-->>Stagehand: StagehandInitResult{Initialized: true}
Stagehand-->>App: *Stagehand (initialized)
Note over App,Stagehand: Operations use Stagehand's rpc/context
App->>Stagehand: Act/Extract/Observe/Context()
Stagehand->>RPC: RPC calls via CDP
RPC-->>Stagehand: Results
Stagehand-->>App: Results
Note over App,Worker: CHANGED: Close only stops runtime, not browser
App->>Stagehand: Close(ctx)
Stagehand->>Stagehand: Check closed flag (memoized)
alt First close
Stagehand->>RPC: stagehand.close
RPC-->>Stagehand: Result (or ErrCDPConnectionClosed)
Stagehand->>Stagehand: Remove LLM handler
Stagehand->>Stagehand: Remove notification handler
Stagehand->>RPC: close() transport
Stagehand->>Stagehand: Set initialized=false, closed=true
Stagehand-->>App: errors.Join(closeErr, rpcErr) [memoized]
else Subsequent close
Stagehand-->>App: memoized closeResult
end
Note over App,Factory: Browser lifetime is always explicit
App->>Factory: browser.Close(ctx) [deferred after client.Close]
alt Local Browser
Factory->>Factory: Kill Chrome process
Factory->>Factory: Remove extension directory
else Browserbase Session
Factory->>Factory: Release session
else Existing CDP
Factory->>Factory: Close CDP client (no transport ownership)
end
Factory-->>App: nil or error
alt Error: client.Close fails before browser.Close
Note over App,Factory: Deferred browser.Close still runs (errors.Join)
App->>Factory: browser.Close(ctx)
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…d-go-remove-legacy-lifecycle # Conflicts: # packages/sdk-go/browser_factories.go # packages/sdk-go/browserbase_session_test.go # packages/sdk-go/stagehand.go
Call materializeStagehandExtension directly now that the wrapper no longer adapts anything, move the newStagehandWithClient test helper out of the production client into client_test.go, and add a factory-level regression test asserting Browser.Close terminates a launched local browser only when KeepAlive is false.
…lifecycle' into feat/stagehand-go-remove-legacy-lifecycle
…d-go-remove-legacy-lifecycle
akeimach
approved these changes
Aug 3, 2026
…d-go-remove-legacy-lifecycle # Conflicts: # packages/sdk-go/examples/custom-logging.go
…d-go-remove-legacy-lifecycle
…d-go-remove-legacy-lifecycle Migrate the model-gateway example onto the Browser handle lifecycle, mirroring packages/sdk-ts/examples/model-gateway.ts: launch the Browserbase browser with LaunchBrowserbase and attach with Create, so the Browserbase API key travels on the handle instead of StagehandClientInitParams.
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
# 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
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.
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.New,Init,StagehandClientInitParams, theBrowserSourceunion, and the publicResolvedBrowserSource; the raw-CDP-with-headers path has no replacement (ConnectLocalBrowsertakes a bare CDP URL)Browser()returns the exact handle passed toCreateOptionsStagehand.Closenever closes the CDP transport, Chrome process, or Browserbase session; browser lifetime is exclusivelyBrowser.Close(ctx);Closeresults are memoized for TSclosePromiseparityErrNotInitializedmessage to point at the new lifecycle (exported var name unchanged)Create→client.Close(ctx)→browser.Close(ctx)(deferred in that order so a failed client close can't leak the process)stagehand.Createshape in the same commit (they gate TS CI)Reviewer focus
Stagehand.Closestops the runtime;Browser.Closeowns browser/session cleanup — under no configuration does Stagehand touch the browser-owned transport.stagehand.initwire payload is unchanged (models.gen.gountouched, no regeneration).{Browser, Context, Initialized}; the centralStagehandInitParamsliteral stays instagehand.go.Follow-up (not in this PR)
packages/docs/v4/**Go snippets still show the deletedNew/Initlifecycle (~10 files); docs migration should follow once this stack settles.Stack
stagehand.CreateVerification
go vet,go build,go test, generator--check+ generator tests, all 7 examples compilepnpm run test:unitgreen (ast-grep example-parity + sdk-parity against the migrated Go examples/source)CHROME_PATH) migrated and exercised by CISummary by cubic
Make
stagehand.Create(ctx, CreateOptions)the only way to build the Go client and finalize split lifecycles: the client stops the worker; theBrowserowns its cleanup.Stagehand.Closememoizes the first result (including failures) for repeated or concurrent calls.Refactors
New,Init,StagehandClientInitParams, theBrowserSourceunion, and publicResolvedBrowserSource; deleted the legacy resolver and tests.Stagehand.Browser()returns the exact*Browserpassed toCreate; the client never closes Chrome or a Browserbase session.LaunchLocalBrowser,ConnectLocalBrowser,LaunchBrowserbase(usesBrowserbaseLaunchOptions); factories materialize the bundled extension and setextensionDironBrowser.KeepAliveat the factory:Browser.Closeterminates a launched Chrome only whenKeepAliveis false (covered by tests).Create→client.Close(ctx)→browser.Close(ctx); Go parity now usescreate()(Python stillinit()).Migration
client := stagehand.New(...); client.Init(ctx)with:browser := stagehand.LaunchLocalBrowser(...) | ConnectLocalBrowser(...) | LaunchBrowserbase(...)client, _ := stagehand.Create(ctx, stagehand.CreateOptions{Browser: browser, ...})defer client.Close(ctx)anddefer browser.Close(ctx).ConnectLocalBrowser(ctx, LocalBrowserConnectOptions{CDPURL: ...}).LaunchBrowserbase(ctx, stagehand.BrowserbaseLaunchOptions{APIKey: ...}).Written for commit ad12189. Summary will update on new commits.