Skip to content

Conversation

@Kylejeong2
Copy link
Member

what

upgrade to stagehand v3

greptile-apps[bot]

This comment was marked as outdated.

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 Overview

Greptile Summary

This PR upgrades Stagehand from v2.5.2 to v3.0.1, bringing the MCP server in line with the latest version of the browser automation library.

Key Changes:

  • Upgraded @browserbasehq/stagehand dependency from ^2.5.2 to ^3.0.1
  • Moved mcpvals from dependencies to devDependencies (^0.4.0)
  • Removed cookies configuration field and playwright-core import from config.d.ts
  • Updated version to 2.3.0 and added CHANGELOG entry

Issues Found:

  • Incomplete cleanup: The cookies feature was removed from the TypeScript type definition (config.d.ts) and implementation (sessionManager.ts), but remnants remain in src/program.ts (CLI option) and src/index.ts (schema definition). This creates a mismatch where users can specify cookies via CLI or config, but they won't be processed, leading to confusion.
  • The obsolete code should be removed to prevent users from thinking cookies are still supported.

Confidence Score: 3/5

  • This PR requires cleanup before merging due to incomplete removal of cookies functionality
  • Score reflects a successful dependency upgrade with proper version management and changelog, but incomplete code cleanup creates a critical inconsistency. The cookies CLI option and schema still exist but are non-functional, which will mislead users and cause runtime issues. The core Stagehand v3 integration appears sound, but the leftover code must be removed to prevent breaking changes for users expecting cookies to work.
  • Pay close attention to src/program.ts and src/index.ts - both contain obsolete cookies-related code that must be removed

Important Files Changed

File Analysis

Filename Score Overview
package.json 5/5 Version bump to 2.3.0, Stagehand upgraded from ^2.5.2 to ^3.0.1, mcpvals moved from dependencies to devDependencies
config.d.ts 5/5 Removed Cookie import from playwright-core and cookies field from Config type
src/program.ts 2/5 Contains obsolete --cookies CLI option that should be removed
src/index.ts 2/5 Contains obsolete cookieSchema and cookies config field that should be removed

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as CLI/Program
    participant Config as Config Resolver
    participant Server as MCP Server
    participant Context as Context
    participant SessionMgr as SessionManager
    participant Stagehand as Stagehand v3
    participant Browserbase as Browserbase API

    User->>CLI: Start with options
    CLI->>Config: resolveConfig(options)
    Config-->>CLI: Merged Config
    CLI->>Server: createServerFunction(config)
    Server->>Context: new Context(server, config)
    Context->>SessionMgr: new SessionManager()
    
    User->>Server: Execute tool
    Server->>Context: run(tool, args)
    Context->>SessionMgr: getSession(sessionId, config)
    
    alt Session doesn't exist
        SessionMgr->>SessionMgr: createNewBrowserSession()
        SessionMgr->>Stagehand: new Stagehand({env: "BROWSERBASE", ...})
        Stagehand->>Browserbase: Create session with v3 config
        Browserbase-->>Stagehand: Session created
        Stagehand->>Stagehand: init()
        Stagehand-->>SessionMgr: Initialized instance
    end
    
    SessionMgr-->>Context: BrowserSession
    Context->>Context: Execute tool action
    Context-->>Server: Tool result
    Server-->>User: Response
Loading

Additional Comments (3)

  1. src/program.ts, line 54-56 (link)

    logic: The --cookies CLI option should be removed. The cookies functionality was removed from config.d.ts and sessionManager.ts as part of the Stagehand v3 upgrade, but this CLI option wasn't cleaned up. This will mislead users into thinking cookies are supported when they aren't.

  2. src/index.ts, line 21-30 (link)

    logic: The cookieSchema is no longer used and should be removed as part of the Stagehand v3 cleanup.

  3. src/index.ts, line 77-80 (link)

    logic: The cookies field in the config schema should be removed. Cookies functionality was removed from the implementation but this schema definition remains, causing a mismatch with the TypeScript Config type definition in config.d.ts.

6 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

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 Overview

Greptile Summary

This PR upgrades @browserbasehq/stagehand from v2.5.2 to v3.0.1, which introduces breaking API changes that required updates across the codebase.

Key Changes

  • API Migration: Updated all tool methods from stagehand.page.act() / stagehand.page.extract() / stagehand.page.observe() to the v3 API (stagehand.act(), stagehand.extract(), stagehand.observe())
  • Model Configuration: Changed from modelClientOptions object to a model field that accepts either a string (model name) or an object with apiKey and modelName
  • Session Property: Updated from stagehand.browserbaseSessionID (camelCase) to stagehand.browserbaseSessionId (lowercase 'd')
  • Page Access: Changed from stagehand.page direct access to stagehand.context.pages()[0]
  • Browser Removal: Removed Browser type and references, simplified session management by removing browser disconnect handlers
  • Cookie Support Removed: Removed the cookies configuration field and cookie injection functionality - potentially breaking for users who rely on this feature
  • Observe Tool: Removed the returnAction parameter that was previously available

Breaking Changes

  • Users with existing configs using the cookies field will need to remove it or find alternative cookie injection methods
  • The returnAction parameter for the observe tool is no longer available

Minor Issues

  • Unused cookieSchema definition in src/index.ts (lines 21-30) that should be removed

Confidence Score: 4/5

  • This PR is generally safe to merge with careful consideration of breaking changes for users relying on cookie injection
  • The upgrade is well-executed with comprehensive API migration across all tool files. However, the removal of cookie support is a breaking change that may affect existing users. The code changes are consistent and follow Stagehand v3's API patterns correctly. The unused cookieSchema is a minor cleanup item but doesn't affect functionality.
  • config.d.ts and src/index.ts (cookie-related changes) - ensure users are notified about cookie support removal

Important Files Changed

File Analysis

Filename Score Overview
config.d.ts 4/5 Removed Cookie import from playwright-core and removed the cookies configuration field, potentially breaking existing configurations that use cookies
src/index.ts 4/5 Removed AvailableModelSchema import, changed modelName from enum to string, added tools: {} to capabilities, updated version to 2.3.0, and improved modelApiKey validation with type check - contains unused cookieSchema
src/sessionManager.ts 4/5 Updated Stagehand v3 API usage: changed modelClientOptions to model object, removed BrowserContext and Cookie imports, removed cookie injection method, changed stagehand.browserbaseSessionID to stagehand.browserbaseSessionId, removed browser disconnect handler, and updated page access from stagehand.page to stagehand.context.pages()[0]
src/tools/act.ts 5/5 Updated to Stagehand v3 API: changed from stagehand.page.act({ action, variables }) to stagehand.act(action, { variables })
src/tools/extract.ts 5/5 Updated to Stagehand v3 API: changed from stagehand.page.extract() to stagehand.extract()
src/tools/observe.ts 5/5 Updated to Stagehand v3 API: removed returnAction parameter from input schema and changed from stagehand.page.observe({ instruction, returnAction }) to stagehand.observe(instruction)
src/types/types.ts 5/5 Removed Browser and Page type imports, changed page and browser types to any with TODO comments for Stagehand v3, and removed browser field from BrowserSession type

Sequence Diagram

sequenceDiagram
    participant User
    participant Context
    participant SessionManager
    participant Stagehand
    participant Browserbase
    
    User->>Context: Call tool (act/extract/observe)
    Context->>Context: getStagehand()
    Context->>SessionManager: getSession()
    
    alt Session exists
        SessionManager->>SessionManager: Validate session
        SessionManager->>Context: Return existing session
    else Session needs creation
        SessionManager->>SessionManager: createStagehandInstance()
        SessionManager->>Stagehand: new Stagehand({ env: "BROWSERBASE", model: {...} })
        Stagehand->>Browserbase: Create/resume session
        Browserbase-->>Stagehand: Return session ID
        Stagehand->>Stagehand: init()
        Stagehand-->>SessionManager: Initialized instance
        SessionManager->>Stagehand: context.pages()[0]
        Stagehand-->>SessionManager: Page instance
        SessionManager->>SessionManager: Store in browsers Map
        SessionManager-->>Context: Return new session
    end
    
    Context->>Stagehand: Call v3 API (act/extract/observe)
    Note over Stagehand: V2: stagehand.page.act({action, variables})<br/>V3: stagehand.act(action, {variables})
    Stagehand->>Browserbase: Execute action
    Browserbase-->>Stagehand: Return result
    Stagehand-->>Context: Return result
    Context-->>User: Return tool result
Loading

Additional Comments (1)

  1. src/index.ts, line 21-30 (link)

    style: cookieSchema is defined but never used after the cookies field was removed from configSchema

16 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

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