Skip to content

fix: share local server across client copies - #357

Open
abhinavkr26104 wants to merge 1 commit into
browserbase:mainfrom
abhinavkr26104:fix/350-share-local-server
Open

fix: share local server across client copies#357
abhinavkr26104 wants to merge 1 commit into
browserbase:mainfrom
abhinavkr26104:fix/350-share-local-server

Conversation

@abhinavkr26104

@abhinavkr26104 abhinavkr26104 commented Aug 15, 2026

Copy link
Copy Markdown

Summary

  • reuse the existing SEA server manager when copy() / with_options() only changes HTTP client options
  • track server ownership so closing a derived client does not terminate the original client's process
  • retain independent servers when local process configuration is overridden

Testing

  • uv run --frozen pytest tests/test_local_server.py -q (12 passed)
  • uv run --frozen ruff check .
  • uv run --frozen pyright -p .
  • uv run --frozen mypy --platform linux .
  • full Python 3.9 and 3.14 suites run on Windows (503 passed; existing Windows proxy/memory-accounting failures are covered by Make byte fixtures checkout-stable and run tests on Windows CI #355)

Fixes #350


Summary by cubic

Shares the existing local SEA server between client copies when only HTTP client options change. Tracks server ownership so closing a derived client no longer shuts down the original client’s local process; if local process settings are overridden, a new independent server is created. Fixes #350.

  • Behavior and review notes
    • Stagehand.copy()/with_options() reuse the local server when server="local" and no local-process or model_api_key overrides are provided; otherwise they start a new server.
    • Adds _owns_sea_server to track ownership. We set it only when launching a server; close()/aclose() terminate the server only if the client owns it.
    • New helper reuse_local_mode_server wires shared state; configure_client_base_url initializes ownership; close paths guard on ownership for sync and async clients.
    • Migration: If you relied on closing a derived client to stop the local server, call close() on the original (owning) client, or force an independent server by overriding a local option (for example, local_port).

Written for commit 7d68e18. Summary will update on new commits.

Review in cubic

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

2 issues found across 3 files

Confidence score: 3/5

  • In src/stagehand/_custom/sea_server.py, the owner/derived-client lifecycle can leave a restarted shared SEA process with no client able to shut it down, which risks orphaned background processes and resource leaks in local mode — add shared reference tracking (or equivalent ownership handoff) so the last live client can always close the process.
  • In src/stagehand/_custom/sea_server.py, local-mode copy()/with_options() appears to create a new SeaServerManager and resolve binaries before immediately replacing _sea_server, which adds avoidable setup overhead and can make lifecycle behavior harder to reason about — skip manager construction when reusing the existing local server.
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="src/stagehand/_custom/sea_server.py">

<violation number="1" location="src/stagehand/_custom/sea_server.py:409">
P3: Every local-mode `copy()`/`with_options()` constructs a fresh `SeaServerManager` (plus binary-path resolution) inside `self.__class__(...)` before `reuse_local_mode_server` overwrites `target._sea_server` with the source's server, so that newly built manager is allocated and immediately discarded whenever reuse applies. Because the server starts lazily, this is not a process leak, but you can avoid the redundant construction by deciding reuse before building the copy (e.g., reuse the source server into the copy without letting the constructor allocate a fresh manager), or by having `reuse_local_mode_server` short-circuit so the constructor path for shared copies skips creating a new manager.</violation>

<violation number="2" location="src/stagehand/_custom/sea_server.py:410">
P2: If the owner client closes before a derived client, the derived client can restart the shared SEA process but can never close it because ownership stays false. Add shared reference tracking (or equivalent ownership handoff) so one active client can always shut down a restarted shared manager.</violation>
</file>
Architecture diagram
sequenceDiagram
    participant App as Application
    participant C1 as Original Stagehand Client
    participant C2 as Derived Client (copy)
    participant SEA as SEA Server Manager
    participant Process as Local SEA Process

    Note over App,Process: Client Copy with Local Server Sharing

    App->>C1: Stagehand(server="local", ...)
    C1->>C1: _owns_sea_server = True
    C1->>SEA: Start local server
    SEA->>Process: Launch SEA process
    Process-->>SEA: Server running
    SEA-->>C1: _sea_server reference

    App->>C1: with_options(max_retries=3)
    C1->>C1: Check local overrides (none provided)
    C1->>SEA: Reuse existing server (share reference)
    SEA-->>C2: _sea_server = same server
    C2->>C2: _owns_sea_server = False

    App->>C2: close()
    alt C2 owns server?
        Note over C2: False - shared reference only
        C2-->>App: Close without terminating server
    end

    App->>C1: close()
    alt C1 owns server?
        Note over C1: True - original owner
        C1->>SEA: Terminate server
        SEA->>Process: Shutdown
        Process-->>SEA: Stopped
        SEA-->>C1: Server closed
    end

    Note over C1,Process: Copy with Local Override

    App->>C1: with_options(local_port=43124)
    C1->>C1: Detect local process override
    alt Server must be independent
        C1->>SEA: Keep original server for source
        SEA-->>C2: _sea_server = null
        C2->>SEA: Start new independent server
        SEA->>Process: Launch new SEA process
        C2->>C2: _owns_sea_server = True
    end
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

and all(value is None for value in local_overrides)
):
target._sea_server = source._sea_server
target._owns_sea_server = False

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: If the owner client closes before a derived client, the derived client can restart the shared SEA process but can never close it because ownership stays false. Add shared reference tracking (or equivalent ownership handoff) so one active client can always shut down a restarted shared manager.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/stagehand/_custom/sea_server.py, line 410:

<comment>If the owner client closes before a derived client, the derived client can restart the shared SEA process but can never close it because ownership stays false. Add shared reference tracking (or equivalent ownership handoff) so one active client can always shut down a restarted shared manager.</comment>

<file context>
@@ -373,6 +376,40 @@ def copy_local_mode_kwargs(
+        and all(value is None for value in local_overrides)
+    ):
+        target._sea_server = source._sea_server
+        target._owns_sea_server = False
+
+
</file context>

and (server is None or server == "local")
and all(value is None for value in local_overrides)
):
target._sea_server = source._sea_server

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: Every local-mode copy()/with_options() constructs a fresh SeaServerManager (plus binary-path resolution) inside self.__class__(...) before reuse_local_mode_server overwrites target._sea_server with the source's server, so that newly built manager is allocated and immediately discarded whenever reuse applies. Because the server starts lazily, this is not a process leak, but you can avoid the redundant construction by deciding reuse before building the copy (e.g., reuse the source server into the copy without letting the constructor allocate a fresh manager), or by having reuse_local_mode_server short-circuit so the constructor path for shared copies skips creating a new manager.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/stagehand/_custom/sea_server.py, line 409:

<comment>Every local-mode `copy()`/`with_options()` constructs a fresh `SeaServerManager` (plus binary-path resolution) inside `self.__class__(...)` before `reuse_local_mode_server` overwrites `target._sea_server` with the source's server, so that newly built manager is allocated and immediately discarded whenever reuse applies. Because the server starts lazily, this is not a process leak, but you can avoid the redundant construction by deciding reuse before building the copy (e.g., reuse the source server into the copy without letting the constructor allocate a fresh manager), or by having `reuse_local_mode_server` short-circuit so the constructor path for shared copies skips creating a new manager.</comment>

<file context>
@@ -373,6 +376,40 @@ def copy_local_mode_kwargs(
+        and (server is None or server == "local")
+        and all(value is None for value in local_overrides)
+    ):
+        target._sea_server = source._sea_server
+        target._owns_sea_server = False
+
</file context>

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.

Local with_options() clones create separate SEA servers and can leak child processes

1 participant