Skip to content

Fix Client(model_api_key=...) and remove OPENAI_API_KEY/MODEL_API_KEY env var auto loading#328

Merged
pirate merged 7 commits intomainfrom
model-api-key-fixes
Apr 1, 2026
Merged

Fix Client(model_api_key=...) and remove OPENAI_API_KEY/MODEL_API_KEY env var auto loading#328
pirate merged 7 commits intomainfrom
model-api-key-fixes

Conversation

@pirate
Copy link
Copy Markdown
Member

@pirate pirate commented Apr 1, 2026

  • removes auto loading of AI api keys from env in favor of explicit constructor params only
  • removes explicit openai_api_key param in favor of only model_api_key
  • fixes propagation of BROWSERBASE_FLOW_LOGS/BROWSERBASE_CONFIG_DIR to SEA binary

Summary by cubic

Stop auto-reading model API keys from env. Only send x-model-api-key when model_api_key is provided, set the SEA child MODEL_API_KEY explicitly in local mode, and force production NODE_ENV for the SEA process.

  • Bug Fixes

    • Client no longer reads MODEL_API_KEY or provider env vars; header is sent only if model_api_key is set.
    • Local mode sets MODEL_API_KEY for the SEA process from model_api_key (or empty), never infers from OPENAI_API_KEY, and forwards BROWSERBASE_FLOW_LOGS/BROWSERBASE_CONFIG_DIR.
    • SEA child env masks inherited MODEL_API_KEY while leaving unrelated env (like OPENAI_API_KEY) intact.
    • SEA child always runs with NODE_ENV=production to avoid dev-only dependency issues.
    • .github/publish-pypi.yml and scripts/download-binary.py select the highest stable stagehand-server-v3/vX.Y.Z release and ignore dev/pre-release tags; binary cache version defaults to the Python package version when STAGEHAND_VERSION is unset.
  • Migration

    • Pass model_api_key explicitly if you need LLM auth; env vars are no longer auto-read.
    • Remove local_openai_api_key; use model_api_key instead (examples updated).

Written for commit ec6ff4d. Summary will update on new commits. Review in cubic

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 12 files

Confidence score: 4/5

  • This PR looks safe to merge overall, with risk concentrated in test stability rather than production behavior.
  • The most significant issue is in tests/test_client.py: not clearing ANTHROPIC_API_KEY can cause _resolve_model_api_key to pick the wrong key based on runner environment, leading to non-deterministic test outcomes.
  • A similar environment-leak risk exists in tests/test_sea_binary.py, where missing monkeypatch.delenv("STAGEHAND_SEA_BINARY", raising=False) can bypass the intended monkeypatched path resolution and make the test depend on external env state.
  • Pay close attention to tests/test_client.py, tests/test_sea_binary.py - ensure env vars are explicitly cleared so tests remain deterministic across different runners.
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="tests/test_client.py">

<violation number="1" location="tests/test_client.py:506">
P2: This test doesn't omit `ANTHROPIC_API_KEY`, which has higher priority than `GEMINI_API_KEY` in `_MODEL_API_KEY_ENV_VARS`. If `ANTHROPIC_API_KEY` is set in the test environment, `_resolve_model_api_key` will return it instead of the intended `"gemini-key"`, causing the assertion to fail.</violation>
</file>

<file name="tests/test_sea_binary.py">

<violation number="1" location="tests/test_sea_binary.py:33">
P2: Missing `monkeypatch.delenv("STAGEHAND_SEA_BINARY", raising=False)`. If this env var is set in the runner's environment, `resolve_binary_path()` returns early before reaching the monkeypatched `_resource_binary_path`/`_copy_to_cache`, causing the test to fail.</violation>
</file>
Architecture diagram
sequenceDiagram
    participant User as Developer / App
    participant Client as Stagehand Client
    participant Env as OS Environment
    participant Manager as SeaServerManager
    participant Binary as SEA Server Binary
    participant GitHub as GitHub Releases API

    Note over User, GitHub: Initialization & Key Resolution Flow

    User->>Client: NEW: Stagehand(model_api_key=None, ...)
    Client->>Env: NEW: _resolve_model_api_key()
    Note right of Env: Iterates provider list:<br/>MODEL_API_KEY, OPENAI_API_KEY,<br/>ANTHROPIC_API_KEY, GEMINI_API_KEY, etc.
    Env-->>Client: Return first matching value

    alt No API key found in args or environment
        Client-->>User: NEW: Raise StagehandError (lists all supported env vars)
    end

    Note over Client, Binary: Local Server Startup (Server="local")

    Client->>Manager: NEW: Initialize SeaServerConfig(model_api_key=...)
    Note right of Client: local_openai_api_key now<br/>maps to model_api_key

    Manager->>Binary: resolve_binary_path()
    opt Version check required
        Binary->>GitHub: NEW: resolve_latest_server_tag()
        Note right of GitHub: NEW: Filters for stable tags only<br/>(ignores -dev or +build)
        GitHub-->>Binary: Latest stable stagehand-server-v3/vX.Y.Z
    end
    Binary-->>Manager: Local path to binary

    Manager->>Manager: NEW: _build_process_env()
    Note right of Manager: Sets MODEL_API_KEY in child env<br/>regardless of provider source

    Manager->>Binary: CHANGED: subprocess.Popen(env=proc_env)
    
    Binary->>Binary: Start Stagehand Server
    Note over Binary: Server uses MODEL_API_KEY<br/>for all LLM provider requests

    Binary-->>Manager: Server ready
    Manager-->>Client: Connection established
    Client-->>User: Client ready
Loading

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread tests/test_client.py Outdated
Comment thread tests/test_sea_binary.py
@pirate pirate force-pushed the model-api-key-fixes branch from 839a7ef to dd3124d Compare April 1, 2026 18:39
@pirate pirate merged commit 99b6033 into main Apr 1, 2026
8 checks passed
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