Skip to content

Conversation

chrisreadsf
Copy link
Member

@chrisreadsf chrisreadsf commented Sep 7, 2025

why

if you add custom browserbase create params, you need to include the project id even if the project id is already passed to the stagehand instance

what changed

browserbaseSessionCreateParams type changed to omit project id

test plan

build stagehand locally, use local stagehand to run sessions with

  • two project ids: one in Stagehand part, one in Browserbase Session Create Params
  • one project id: only in Stagehand part

Copy link

changeset-bot bot commented Sep 7, 2025

🦋 Changeset detected

Latest commit: 2ef2d94

The changes in this PR will be included in the next version bump.

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

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR modifies the type definition for browserbaseSessionCreateParams to make the projectId field optional, allowing users to omit it when they've already provided a project ID to the Stagehand constructor. The change affects three files: lib/index.ts, types/api.ts, and types/stagehand.ts, all updating the type to Omit<Browserbase.Sessions.SessionCreateParams, "projectId"> & { projectId?: string }.

Key changes:

  • Modified type definitions to make projectId optional in browserbaseSessionCreateParams
  • Added helpful documentation comment explaining that projectId will use the main parameter if not provided
  • Maintains backward compatibility by keeping projectId as an optional field

Issues found:

  • Potential conflict resolution issue where browserbaseSessionCreateParams.projectId could override the main projectId parameter

Confidence score: 3/5

  • This PR has a moderate risk due to a potential logical issue with parameter precedence
  • The type changes are well-implemented and backward compatible, but there's a logical issue where browserbaseSessionCreateParams.projectId could unexpectedly override the main projectId parameter due to spread operator precedence, which could lead to confusing behavior
  • lib/index.ts requires attention to fix the parameter precedence issue in session creation

3 files reviewed, no comments

Edit Code Review Bot Settings | Greptile

Copy link
Member

@seanmcguire12 seanmcguire12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrisreadsf thanks for this!! can you add a changeset? you can run npx changeset and just do a patch, with a 1 line description. then we'll get it merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants