fix: share local server across client copies - #357
Conversation
There was a problem hiding this comment.
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-modecopy()/with_options()appears to create a newSeaServerManagerand 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
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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>
Summary
copy()/with_options()only changes HTTP client optionsTesting
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 .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.
Stagehand.copy()/with_options()reuse the local server whenserver="local"and no local-process ormodel_api_keyoverrides are provided; otherwise they start a new server._owns_sea_serverto track ownership. We set it only when launching a server;close()/aclose()terminate the server only if the client owns it.reuse_local_mode_serverwires shared state;configure_client_base_urlinitializes ownership; close paths guard on ownership for sync and async clients.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.