-
Notifications
You must be signed in to change notification settings - Fork 117
fix: share local server across client copies #357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,7 @@ class _HasLocalModeState(Protocol): | |
| _local_ready_timeout_s: float | ||
| _local_shutdown_on_close: bool | ||
| _sea_server: SeaServerManager | None | ||
| _owns_sea_server: bool | ||
|
|
||
|
|
||
| class LocalModeKwargs(TypedDict): | ||
|
|
@@ -309,6 +310,7 @@ def configure_client_base_url( | |
| client._local_ready_timeout_s = local_ready_timeout_s | ||
| client._local_shutdown_on_close = local_shutdown_on_close | ||
| client._sea_server = None | ||
| client._owns_sea_server = False | ||
|
|
||
| if server == "local": | ||
| if base_url is None: | ||
|
|
@@ -326,6 +328,7 @@ def configure_client_base_url( | |
| ), | ||
| _local_stagehand_binary_path=_local_stagehand_binary_path, | ||
| ) | ||
| client._owns_sea_server = True | ||
| return base_url | ||
|
|
||
| if base_url is None: | ||
|
|
@@ -373,6 +376,40 @@ def copy_local_mode_kwargs( | |
| } | ||
|
|
||
|
|
||
| def reuse_local_mode_server( | ||
| source: _HasLocalModeState, | ||
| target: _HasLocalModeState, | ||
| *, | ||
| server: Literal["remote", "local"] | None, | ||
| model_api_key: str | None, | ||
| _local_stagehand_binary_path: str | os.PathLike[str] | None, | ||
| local_host: str | None, | ||
| local_port: int | None, | ||
| local_headless: bool | None, | ||
| local_chrome_path: str | None, | ||
| local_ready_timeout_s: float | None, | ||
| local_shutdown_on_close: bool | None, | ||
| ) -> None: | ||
| """Share an unchanged local server with a derived client.""" | ||
| local_overrides = ( | ||
| model_api_key, | ||
| _local_stagehand_binary_path, | ||
| local_host, | ||
| local_port, | ||
| local_headless, | ||
| local_chrome_path, | ||
| local_ready_timeout_s, | ||
| local_shutdown_on_close, | ||
| ) | ||
| if ( | ||
| source._sea_server is not None | ||
| 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 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
|
||
|
|
||
| def prepare_sync_client_base_url(client: _HasLocalModeState) -> str | None: | ||
| if client._sea_server is None: | ||
| return None | ||
|
|
@@ -386,10 +423,10 @@ async def prepare_async_client_base_url(client: _HasLocalModeState) -> str | Non | |
|
|
||
|
|
||
| def close_sync_client_sea_server(client: _HasLocalModeState) -> None: | ||
| if client._sea_server is not None: | ||
| if client._sea_server is not None and client._owns_sea_server: | ||
| client._sea_server.close() | ||
|
|
||
|
|
||
| async def close_async_client_sea_server(client: _HasLocalModeState) -> None: | ||
| if client._sea_server is not None: | ||
| if client._sea_server is not None and client._owns_sea_server: | ||
| await client._sea_server.aclose() | ||
There was a problem hiding this comment.
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 freshSeaServerManager(plus binary-path resolution) insideself.__class__(...)beforereuse_local_mode_serveroverwritestarget._sea_serverwith 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 havingreuse_local_mode_servershort-circuit so the constructor path for shared copies skips creating a new manager.Prompt for AI agents