From f668a4abf21272c959d94a815218e6bba375da3e Mon Sep 17 00:00:00 2001 From: Sean McGuire Date: Mon, 31 Aug 2026 10:58:16 -0300 Subject: [PATCH 1/9] add repo-wide launch flag fixture & local launcher tests --- packages/sdk-go/chrome_launcher_test.go | 77 +++++++++++-------- packages/sdk-python/tests/test_browser.py | 63 +++++++++++++++ .../sdk-ts/tests/browser/localBrowser.test.ts | 37 ++------- .../fixtures/local-browser-default-flags.json | 27 +++++++ 4 files changed, 143 insertions(+), 61 deletions(-) create mode 100644 tests/fixtures/local-browser-default-flags.json diff --git a/packages/sdk-go/chrome_launcher_test.go b/packages/sdk-go/chrome_launcher_test.go index e1e9f7afb..844719206 100644 --- a/packages/sdk-go/chrome_launcher_test.go +++ b/packages/sdk-go/chrome_launcher_test.go @@ -10,7 +10,6 @@ import ( "os" "os/exec" "path/filepath" - "reflect" "slices" "strings" "testing" @@ -20,37 +19,16 @@ import ( const testWebMCPChromeFlag = "--enable-features=WebMCPTesting,DevToolsWebMCPSupport" func TestDefaultChromeFlags(t *testing.T) { - want := []string{ - "--disable-features=Translate,OptimizationHints,MediaRouter,DialMediaRouteProvider," + - "CalculateNativeWinOcclusion,InterestFeedContentSuggestions," + - "CertificateTransparencyComponentUpdater,AutofillServerCommunication," + - "PrivacySandboxSettings4,RenderDocument", - "--disable-component-extensions-with-background-pages", - "--disable-background-networking", - "--disable-component-update", - "--disable-client-side-phishing-detection", - "--disable-sync", - "--metrics-recording-only", - "--disable-default-apps", - "--mute-audio", - "--no-default-browser-check", - "--no-first-run", - "--disable-backgrounding-occluded-windows", - "--disable-renderer-backgrounding", - "--disable-background-timer-throttling", - "--disable-ipc-flooding-protection", - "--password-store=basic", - "--use-mock-keychain", - "--force-fieldtrials=*BackgroundTracing/default/", - "--disable-hang-monitor", - "--disable-prompt-on-repost", - "--disable-domain-reliability", - "--propagate-iph-for-testing", - "--enable-unsafe-extension-debugging", - "--remote-allow-origins=*", - testWebMCPChromeFlag, + fixturePath := filepath.Join("..", "..", "tests", "fixtures", "local-browser-default-flags.json") + fixture, err := os.ReadFile(fixturePath) + if err != nil { + t.Fatalf("read default Chrome flags fixture: %v", err) + } + var want []string + if err := json.Unmarshal(fixture, &want); err != nil { + t.Fatalf("decode default Chrome flags fixture: %v", err) } - if !reflect.DeepEqual(defaultChromeFlags, want) { + if !slices.Equal(defaultChromeFlags, want) { t.Fatalf("defaultChromeFlags = %#v, want %#v", defaultChromeFlags, want) } if slices.Contains(defaultChromeFlags, "--disable-extensions") { @@ -58,6 +36,43 @@ func TestDefaultChromeFlags(t *testing.T) { } } +func TestLaunchedChromeProfileOwnership(t *testing.T) { + tests := []struct { + name string + removeDir bool + wantExist bool + }{ + {name: "SDK-owned", removeDir: true, wantExist: false}, + {name: "caller-owned or preserved", removeDir: false, wantExist: true}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + profile := filepath.Join(t.TempDir(), "profile") + if err := os.Mkdir(profile, 0o700); err != nil { + t.Fatalf("create profile: %v", err) + } + done := make(chan struct{}) + close(done) + launched := &launchedChrome{ + userDataDir: profile, + process: &chromeProcess{done: done}, + removeDir: test.removeDir, + } + + if err := launched.close(context.Background()); err != nil { + t.Fatalf("close launched Chrome: %v", err) + } + _, err := os.Stat(profile) + if test.wantExist && err != nil { + t.Fatalf("preserved profile is unavailable: %v", err) + } + if !test.wantExist && !errors.Is(err, os.ErrNotExist) { + t.Fatalf("SDK-owned profile still exists: %v", err) + } + }) + } +} + func TestBuildChromeArgsSupportsLocalBrowserOptions(t *testing.T) { t.Setenv("CI", "") sandbox := false diff --git a/packages/sdk-python/tests/test_browser.py b/packages/sdk-python/tests/test_browser.py index 8827f7ede..45460ec47 100644 --- a/packages/sdk-python/tests/test_browser.py +++ b/packages/sdk-python/tests/test_browser.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +import json from pathlib import Path from typing import ClassVar, Literal, cast @@ -32,6 +33,17 @@ from stagehand.cdp_client import CDPConnectionClosedError from stagehand.client_models import LocalBrowserLaunchOptions, LocalViewport +EXPECTED_DEFAULT_CHROME_FLAGS = tuple( + json.loads( + ( + Path(__file__).resolve().parents[3] + / "tests" + / "fixtures" + / "local-browser-default-flags.json" + ).read_text() + ) +) + class FakeCDPClient: connect_arguments: ClassVar[list[dict[str, object]]] = [] @@ -616,6 +628,23 @@ def test_local_browser_flags_are_unchanged_for_launch_options(tmp_path: Path) -> ] +def test_local_browser_default_flags_match_shared_fixture(tmp_path: Path) -> None: + assert _DEFAULT_CHROME_FLAGS == EXPECTED_DEFAULT_CHROME_FLAGS + assert "--disable-extensions" not in _DEFAULT_CHROME_FLAGS + assert _local_browser_flags( + LocalBrowserLaunchOptions(), + port=9222, + user_data_dir=tmp_path, + is_ci=False, + ) == [ + *EXPECTED_DEFAULT_CHROME_FLAGS, + "--window-size=1280,800", + "--remote-debugging-port=9222", + f"--user-data-dir={tmp_path}", + "about:blank", + ] + + class FakeBrowserbaseSession: def __init__( self, @@ -811,6 +840,40 @@ async def create_subprocess_exec(*_args: object, **_kwargs: object) -> FakeProce assert not profile.exists() +@pytest.mark.parametrize( + "uses_temporary_profile", + [False, True], +) +async def test_local_browser_close_preserves_non_owned_profiles( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + uses_temporary_profile: bool, +) -> None: + profile = tmp_path / "profile" + profile.mkdir() + + class FakeProcess: + returncode = 0 + pid = 123 + + async def create_subprocess_exec(*_args: object, **_kwargs: object) -> FakeProcess: + return FakeProcess() + + monkeypatch.setattr(browser, "_find_chrome_path", lambda: "/path/to/chrome") + monkeypatch.setattr(browser, "_available_port", lambda: 9222) + monkeypatch.setattr(browser.asyncio, "create_subprocess_exec", create_subprocess_exec) + if uses_temporary_profile: + monkeypatch.setattr(browser.tempfile, "mkdtemp", lambda **_kwargs: str(profile)) + options = LocalBrowserLaunchOptions(preserve_user_data_dir=True) + else: + options = LocalBrowserLaunchOptions(user_data_dir=str(profile)) + + source = await _launch_local_browser(options) + await source.close() + + assert profile.exists() + + def test_local_browser_flags_keep_explicit_viewport_without_defaults(tmp_path: Path) -> None: flags = _local_browser_flags( LocalBrowserLaunchOptions( diff --git a/packages/sdk-ts/tests/browser/localBrowser.test.ts b/packages/sdk-ts/tests/browser/localBrowser.test.ts index af3cdbc47..b8aed845d 100644 --- a/packages/sdk-ts/tests/browser/localBrowser.test.ts +++ b/packages/sdk-ts/tests/browser/localBrowser.test.ts @@ -1,5 +1,6 @@ import type { ChildProcess } from "node:child_process"; import { EventEmitter } from "node:events"; +import { readFileSync } from "node:fs"; import { afterEach, describe, expect, it, vi } from "vitest"; import { createLocalBrowserLauncherForTest, @@ -10,36 +11,12 @@ import { const WEBMCP_CHROME_FLAG = "--enable-features=WebMCPTesting,DevToolsWebMCPSupport"; -const EXPECTED_DEFAULT_CHROME_FLAGS = [ - "--disable-features=Translate,OptimizationHints,MediaRouter,DialMediaRouteProvider," + - "CalculateNativeWinOcclusion,InterestFeedContentSuggestions," + - "CertificateTransparencyComponentUpdater,AutofillServerCommunication," + - "PrivacySandboxSettings4,RenderDocument", - "--disable-component-extensions-with-background-pages", - "--disable-background-networking", - "--disable-component-update", - "--disable-client-side-phishing-detection", - "--disable-sync", - "--metrics-recording-only", - "--disable-default-apps", - "--mute-audio", - "--no-default-browser-check", - "--no-first-run", - "--disable-backgrounding-occluded-windows", - "--disable-renderer-backgrounding", - "--disable-background-timer-throttling", - "--disable-ipc-flooding-protection", - "--password-store=basic", - "--use-mock-keychain", - "--force-fieldtrials=*BackgroundTracing/default/", - "--disable-hang-monitor", - "--disable-prompt-on-repost", - "--disable-domain-reliability", - "--propagate-iph-for-testing", - "--enable-unsafe-extension-debugging", - "--remote-allow-origins=*", - WEBMCP_CHROME_FLAG, -] as const; +const EXPECTED_DEFAULT_CHROME_FLAGS = JSON.parse( + readFileSync( + new URL("../../../../tests/fixtures/local-browser-default-flags.json", import.meta.url), + "utf8", + ), +) as string[]; class FakeChromeProcess extends EventEmitter { pid = 123; diff --git a/tests/fixtures/local-browser-default-flags.json b/tests/fixtures/local-browser-default-flags.json new file mode 100644 index 000000000..4571da828 --- /dev/null +++ b/tests/fixtures/local-browser-default-flags.json @@ -0,0 +1,27 @@ +[ + "--disable-features=Translate,OptimizationHints,MediaRouter,DialMediaRouteProvider,CalculateNativeWinOcclusion,InterestFeedContentSuggestions,CertificateTransparencyComponentUpdater,AutofillServerCommunication,PrivacySandboxSettings4,RenderDocument", + "--disable-component-extensions-with-background-pages", + "--disable-background-networking", + "--disable-component-update", + "--disable-client-side-phishing-detection", + "--disable-sync", + "--metrics-recording-only", + "--disable-default-apps", + "--mute-audio", + "--no-default-browser-check", + "--no-first-run", + "--disable-backgrounding-occluded-windows", + "--disable-renderer-backgrounding", + "--disable-background-timer-throttling", + "--disable-ipc-flooding-protection", + "--password-store=basic", + "--use-mock-keychain", + "--force-fieldtrials=*BackgroundTracing/default/", + "--disable-hang-monitor", + "--disable-prompt-on-repost", + "--disable-domain-reliability", + "--propagate-iph-for-testing", + "--enable-unsafe-extension-debugging", + "--remote-allow-origins=*", + "--enable-features=WebMCPTesting,DevToolsWebMCPSupport" +] From 5f97a8d70b2f75486fa925ca52c005d94d1b145c Mon Sep 17 00:00:00 2001 From: Sean McGuire Date: Mon, 31 Aug 2026 11:17:41 -0300 Subject: [PATCH 2/9] make python wait for chrome readiness on local launch --- packages/sdk-python/src/stagehand/browser.py | 268 +++++++++++++---- packages/sdk-python/tests/test_browser.py | 286 ++++++++++++++++++- 2 files changed, 502 insertions(+), 52 deletions(-) diff --git a/packages/sdk-python/src/stagehand/browser.py b/packages/sdk-python/src/stagehand/browser.py index 312b554a7..30f10a318 100644 --- a/packages/sdk-python/src/stagehand/browser.py +++ b/packages/sdk-python/src/stagehand/browser.py @@ -1,15 +1,19 @@ from __future__ import annotations import asyncio +import json +import math import os import shutil import signal import socket import sys import tempfile +import urllib.request from collections.abc import Awaitable, Callable, Mapping, Sequence +from contextlib import suppress from dataclasses import dataclass, field -from pathlib import Path +from pathlib import Path, PureWindowsPath from typing import TYPE_CHECKING, Any, Literal, Protocol if TYPE_CHECKING: @@ -65,6 +69,8 @@ ) _BROWSER_TOKEN = object() +_CHROME_POLL_INTERVAL_SECONDS = 0.1 +_CHROME_REQUEST_TIMEOUT_SECONDS = 0.1 @dataclass(frozen=True) @@ -94,6 +100,12 @@ async def close(self) -> None: await self._close_callback() +@dataclass(frozen=True) +class _ChromeProfile: + path: Path + remove: bool + + class StagehandBrowser: __slots__ = ( "_attachment", @@ -258,6 +270,20 @@ class _LocalBrowserOptions(Protocol): keep_alive: bool | None +class _WaitableChromeProcess(Protocol): + returncode: int | None + + async def wait(self) -> int: ... + + +class _ChromeProcess(_WaitableChromeProcess, Protocol): + pid: int + + def terminate(self) -> None: ... + + def kill(self) -> None: ... + + async def _connect_browser( *, provider: Literal["local", "browserbase"], @@ -622,54 +648,39 @@ async def connect( async def _launch_local_browser(options: _LocalBrowserOptions) -> ResolvedBrowserSource: - chrome_path = options.executable_path or _find_chrome_path() + _validate_local_browser_options(options) + chrome_path = _find_chrome_path(options.executable_path) port = options.port or _available_port() - temporary_profile = options.user_data_dir is None - user_data_dir = Path(options.user_data_dir or tempfile.mkdtemp(prefix="stagehand-chrome-")) + profile = _resolve_chrome_profile(options) flags = _local_browser_flags( options, port=port, - user_data_dir=user_data_dir, + user_data_dir=profile.path, is_ci=bool(os.environ.get("CI")), ) + process: _ChromeProcess | None = None try: - process = await asyncio.create_subprocess_exec( - chrome_path, - *flags, - stdin=asyncio.subprocess.DEVNULL, - stdout=asyncio.subprocess.DEVNULL, - stderr=asyncio.subprocess.DEVNULL, - start_new_session=sys.platform != "win32", - ) + try: + process = await asyncio.create_subprocess_exec( + chrome_path, + *flags, + stdin=asyncio.subprocess.DEVNULL, + stdout=asyncio.subprocess.DEVNULL, + stderr=asyncio.subprocess.DEVNULL, + start_new_session=sys.platform != "win32", + ) + except OSError as error: + raise RuntimeError(f"Failed to start Chrome: {error}") from error + await _wait_for_chrome(f"http://127.0.0.1:{port}", process) except BaseException: - if temporary_profile and options.preserve_user_data_dir is not True: - await asyncio.to_thread(shutil.rmtree, user_data_dir, True) + if process is not None: + await _close_local_chrome(process, profile) + elif profile.remove: + await _remove_chrome_profile(profile.path) raise async def close() -> None: - try: - if process.returncode is None: - try: - if sys.platform == "win32": - process.terminate() - else: - os.killpg(process.pid, signal.SIGTERM) - except ProcessLookupError: - pass - try: - await asyncio.wait_for(process.wait(), timeout=3) - except TimeoutError: - try: - if sys.platform == "win32": - process.kill() - else: - os.killpg(process.pid, signal.SIGKILL) - except ProcessLookupError: - pass - await process.wait() - finally: - if temporary_profile and options.preserve_user_data_dir is not True: - await asyncio.to_thread(shutil.rmtree, user_data_dir, True) + await _close_local_chrome(process, profile) return ResolvedBrowserSource( cdp_url=f"http://127.0.0.1:{port}", @@ -678,6 +689,133 @@ async def close() -> None: ) +def _validate_local_browser_options(options: _LocalBrowserOptions) -> None: + if options.viewport is not None and ( + options.viewport.width <= 0 or options.viewport.height <= 0 + ): + raise ValueError("Chrome viewport dimensions must be positive integers") + if options.device_scale_factor is not None and ( + not math.isfinite(options.device_scale_factor) or options.device_scale_factor <= 0 + ): + raise ValueError("Chrome device scale factor must be positive and finite") + if options.proxy is not None: + if not options.proxy.server: + raise ValueError("Chrome proxy server is required") + if options.proxy.username is not None or options.proxy.password is not None: + raise NotImplementedError("Authenticated local browser proxies are not implemented yet") + + +def _resolve_chrome_profile(options: _LocalBrowserOptions) -> _ChromeProfile: + if options.user_data_dir is not None: + path = Path(options.user_data_dir) + path.mkdir(mode=0o700, parents=True, exist_ok=True) + return _ChromeProfile(path=path, remove=False) + path = Path(tempfile.mkdtemp(prefix="stagehand-chrome-")) + return _ChromeProfile(path=path, remove=options.preserve_user_data_dir is not True) + + +async def _wait_for_chrome( + cdp_url: str, + process: _WaitableChromeProcess, +) -> None: + exited = asyncio.create_task(process.wait()) + ready: asyncio.Task[bool] | None = None + delay: asyncio.Task[None] | None = None + try: + while True: + if process.returncode is not None: + raise _chrome_exited_before_ready_error(process.returncode) + + ready = asyncio.create_task(_chrome_debugging_ready(cdp_url)) + done, _ = await asyncio.wait((ready, exited), return_when=asyncio.FIRST_COMPLETED) + if exited in done: + ready.cancel() + with suppress(asyncio.CancelledError): + await ready + raise _chrome_exited_before_ready_error(exited.result()) + if ready.result(): + if process.returncode is not None or exited.done(): + raise _chrome_exited_before_ready_error(process.returncode) + return + + delay = asyncio.create_task(asyncio.sleep(_CHROME_POLL_INTERVAL_SECONDS)) + done, _ = await asyncio.wait((delay, exited), return_when=asyncio.FIRST_COMPLETED) + if exited in done: + delay.cancel() + with suppress(asyncio.CancelledError): + await delay + raise _chrome_exited_before_ready_error(exited.result()) + finally: + for pending in (ready, delay): + if pending is not None and not pending.done(): + pending.cancel() + with suppress(asyncio.CancelledError): + await pending + if not exited.done(): + exited.cancel() + with suppress(asyncio.CancelledError): + await exited + + +async def _chrome_debugging_ready(cdp_url: str) -> bool: + try: + version = await asyncio.to_thread(_read_chrome_version, cdp_url) + except Exception: + return False + debugger_url = version.get("webSocketDebuggerUrl") + return isinstance(debugger_url, str) and bool(debugger_url.strip()) + + +def _read_chrome_version(cdp_url: str) -> dict[str, object]: + url = f"{cdp_url.rstrip('/')}/json/version" + with urllib.request.urlopen( # noqa: S310 -- The launcher owns this loopback URL. + url, + timeout=_CHROME_REQUEST_TIMEOUT_SECONDS, + ) as response: + value: Any = json.load(response) + if not isinstance(value, dict): + raise RuntimeError("Chrome version endpoint returned invalid JSON") + return value + + +def _chrome_exited_before_ready_error(returncode: int | None) -> RuntimeError: + detail = "unknown" if returncode is None else str(returncode) + return RuntimeError(f"Chrome exited before its debugging port was ready with code {detail}") + + +async def _close_local_chrome( + process: _ChromeProcess, + profile: _ChromeProfile, +) -> None: + try: + if process.returncode is None: + try: + if sys.platform == "win32": + process.terminate() + else: + os.killpg(process.pid, signal.SIGTERM) + except ProcessLookupError: + pass + try: + await asyncio.wait_for(process.wait(), timeout=3) + except TimeoutError: + try: + if sys.platform == "win32": + process.kill() + else: + os.killpg(process.pid, signal.SIGKILL) + except ProcessLookupError: + pass + await process.wait() + finally: + if profile.remove: + await _remove_chrome_profile(profile.path) + + +async def _remove_chrome_profile(path: Path) -> None: + await asyncio.to_thread(shutil.rmtree, path, True) + + def _local_browser_flags( options: _LocalBrowserOptions, *, @@ -729,29 +867,52 @@ def _local_browser_flags( ] -def _find_chrome_path() -> str: - configured = os.environ.get("CHROME_PATH") - if configured and Path(configured).is_file(): +def _find_chrome_path( + explicit_path: str | None = None, + *, + platform: str | None = None, + environment: Mapping[str, str] | None = None, + which: Callable[[str], str | None] = shutil.which, + is_executable: Callable[[str, str], bool] | None = None, +) -> str: + platform = sys.platform if platform is None else platform + environment = os.environ if environment is None else environment + is_executable = _is_executable_file if is_executable is None else is_executable + + if explicit_path is not None: + if is_executable(explicit_path, platform): + return explicit_path + raise RuntimeError(f"Chrome executable {json.dumps(explicit_path)} does not exist") + + configured = environment.get("CHROME_PATH") + if configured and is_executable(configured, platform): return configured - if sys.platform == "darwin": + if platform == "darwin": candidates = ( "/Applications/Google Chrome Canary.app/Contents/MacOS/Google Chrome Canary", "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome", + "/Applications/Google Chrome Beta.app/Contents/MacOS/Google Chrome Beta", + "/Applications/Chromium.app/Contents/MacOS/Chromium", ) - elif sys.platform == "win32": + elif platform == "win32": roots = filter( None, ( - os.environ.get("LOCALAPPDATA"), - os.environ.get("PROGRAMFILES"), - os.environ.get("PROGRAMFILES(X86)"), + environment.get(name) + for name in ( + "LOCALAPPDATA", + "PROGRAMFILES", + "PROGRAMFILES(X86)", + ) ), ) candidates = tuple( - str(Path(root) / "Google" / "Chrome" / "Application" / "chrome.exe") for root in roots + str(PureWindowsPath(root) / "Google" / product / "Application" / "chrome.exe") + for root in roots + for product in ("Chrome SxS", "Chrome") ) - else: + elif platform == "linux": candidates = tuple( path for name in ( @@ -760,15 +921,22 @@ def _find_chrome_path() -> str: "chromium-browser", "chromium", ) - if (path := shutil.which(name)) is not None + if (path := which(name)) is not None ) + else: + raise RuntimeError(f"Chrome launching is not supported on {platform}") for candidate in candidates: - if Path(candidate).is_file(): + if is_executable(candidate, platform): return candidate raise RuntimeError("Chrome installation not found; set CHROME_PATH") +def _is_executable_file(path: str, platform: str) -> bool: + candidate = Path(path) + return candidate.is_file() and (platform == "win32" or os.access(candidate, os.X_OK)) + + def _available_port() -> int: with socket.socket() as candidate: candidate.bind(("127.0.0.1", 0)) diff --git a/packages/sdk-python/tests/test_browser.py b/packages/sdk-python/tests/test_browser.py index 45460ec47..e3993f705 100644 --- a/packages/sdk-python/tests/test_browser.py +++ b/packages/sdk-python/tests/test_browser.py @@ -828,10 +828,11 @@ async def wait(self) -> int: async def create_subprocess_exec(*_args: object, **_kwargs: object) -> FakeProcess: return FakeProcess() - monkeypatch.setattr(browser, "_find_chrome_path", lambda: "/path/to/chrome") + monkeypatch.setattr(browser, "_find_chrome_path", lambda _explicit: "/path/to/chrome") monkeypatch.setattr(browser, "_available_port", lambda: 9222) monkeypatch.setattr(browser.tempfile, "mkdtemp", lambda **_kwargs: str(profile)) monkeypatch.setattr(browser.asyncio, "create_subprocess_exec", create_subprocess_exec) + monkeypatch.setattr(browser, "_wait_for_chrome", _ready_chrome) monkeypatch.setattr(browser.sys, "platform", "win32") source = await _launch_local_browser(LocalBrowserLaunchOptions()) @@ -859,9 +860,10 @@ class FakeProcess: async def create_subprocess_exec(*_args: object, **_kwargs: object) -> FakeProcess: return FakeProcess() - monkeypatch.setattr(browser, "_find_chrome_path", lambda: "/path/to/chrome") + monkeypatch.setattr(browser, "_find_chrome_path", lambda _explicit: "/path/to/chrome") monkeypatch.setattr(browser, "_available_port", lambda: 9222) monkeypatch.setattr(browser.asyncio, "create_subprocess_exec", create_subprocess_exec) + monkeypatch.setattr(browser, "_wait_for_chrome", _ready_chrome) if uses_temporary_profile: monkeypatch.setattr(browser.tempfile, "mkdtemp", lambda **_kwargs: str(profile)) options = LocalBrowserLaunchOptions(preserve_user_data_dir=True) @@ -874,6 +876,286 @@ async def create_subprocess_exec(*_args: object, **_kwargs: object) -> FakeProce assert profile.exists() +async def _ready_chrome(_cdp_url: str, _process: object) -> None: + return None + + +async def test_local_browser_validation_precedes_executable_discovery( + monkeypatch: pytest.MonkeyPatch, +) -> None: + def find_chrome(_explicit: str | None) -> str: + raise AssertionError("executable discovery should not run") + + monkeypatch.setattr(browser, "_find_chrome_path", find_chrome) + + with pytest.raises(ValueError, match="viewport dimensions"): + await _launch_local_browser( + LocalBrowserLaunchOptions(viewport=LocalViewport(width=0, height=800)) + ) + + +async def test_invalid_explicit_executable_precedes_profile_creation( + monkeypatch: pytest.MonkeyPatch, +) -> None: + def create_profile(**_kwargs: object) -> str: + raise AssertionError("profile creation should not run") + + monkeypatch.setattr(browser.tempfile, "mkdtemp", create_profile) + + with pytest.raises(RuntimeError, match="Chrome executable.*does not exist"): + await _launch_local_browser(LocalBrowserLaunchOptions(executable_path="/missing/chrome")) + + +async def test_spawn_failure_removes_sdk_owned_profile( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + profile = tmp_path / "profile" + profile.mkdir() + + async def create_subprocess_exec(*_args: object, **_kwargs: object) -> object: + raise OSError("spawn failed") + + monkeypatch.setattr(browser, "_find_chrome_path", lambda _explicit: "/path/to/chrome") + monkeypatch.setattr(browser, "_available_port", lambda: 9222) + monkeypatch.setattr(browser.tempfile, "mkdtemp", lambda **_kwargs: str(profile)) + monkeypatch.setattr(browser.asyncio, "create_subprocess_exec", create_subprocess_exec) + + with pytest.raises(RuntimeError, match="Failed to start Chrome: spawn failed"): + await _launch_local_browser(LocalBrowserLaunchOptions()) + + assert not profile.exists() + + +def test_find_chrome_path_uses_explicit_then_environment() -> None: + def executable(path: str, _platform: str) -> bool: + return path in {"/explicit", "/configured"} + + assert ( + browser._find_chrome_path( + "/explicit", + platform="linux", + environment={"CHROME_PATH": "/configured"}, + is_executable=executable, + ) + == "/explicit" + ) + assert ( + browser._find_chrome_path( + platform="linux", + environment={"CHROME_PATH": "/configured"}, + is_executable=executable, + ) + == "/configured" + ) + + +@pytest.mark.parametrize( + ("platform", "environment", "expected"), + [ + ( + "darwin", + {}, + [ + "/Applications/Google Chrome Canary.app/Contents/MacOS/Google Chrome Canary", + "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome", + "/Applications/Google Chrome Beta.app/Contents/MacOS/Google Chrome Beta", + "/Applications/Chromium.app/Contents/MacOS/Chromium", + ], + ), + ( + "win32", + {"LOCALAPPDATA": r"C:\Users\me\AppData", "PROGRAMFILES": r"C:\Program Files"}, + [ + r"C:\Users\me\AppData\Google\Chrome SxS\Application\chrome.exe", + r"C:\Users\me\AppData\Google\Chrome\Application\chrome.exe", + r"C:\Program Files\Google\Chrome SxS\Application\chrome.exe", + r"C:\Program Files\Google\Chrome\Application\chrome.exe", + ], + ), + ], +) +def test_find_chrome_path_checks_platform_candidates_in_order( + platform: str, + environment: dict[str, str], + expected: list[str], +) -> None: + checked: list[str] = [] + + def executable(path: str, _platform: str) -> bool: + checked.append(path) + return path == expected[-1] + + assert ( + browser._find_chrome_path( + platform=platform, + environment=environment, + is_executable=executable, + ) + == expected[-1] + ) + assert checked == expected + + +def test_find_chrome_path_checks_linux_candidates_in_order() -> None: + names: list[str] = [] + + def which(name: str) -> str: + names.append(name) + return f"/bin/{name}" + + assert ( + browser._find_chrome_path( + platform="linux", + environment={}, + which=which, + is_executable=lambda path, _platform: path == "/bin/chromium", + ) + == "/bin/chromium" + ) + assert names == [ + "google-chrome-stable", + "google-chrome", + "chromium-browser", + "chromium", + ] + + +def test_find_chrome_path_rejects_unsupported_platform() -> None: + with pytest.raises(RuntimeError, match="not supported on freebsd"): + browser._find_chrome_path( + platform="freebsd", + environment={}, + is_executable=lambda _path, _platform: False, + ) + + +async def test_launch_creates_caller_profile_before_spawn( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + profile = tmp_path / "nested" / "profile" + spawned = False + + class FakeProcess: + returncode = 0 + pid = 123 + + async def create_subprocess_exec(*_args: object, **_kwargs: object) -> FakeProcess: + nonlocal spawned + spawned = True + assert profile.is_dir() + return FakeProcess() + + monkeypatch.setattr(browser, "_find_chrome_path", lambda _explicit: "/path/to/chrome") + monkeypatch.setattr(browser, "_available_port", lambda: 9222) + monkeypatch.setattr(browser.asyncio, "create_subprocess_exec", create_subprocess_exec) + monkeypatch.setattr(browser, "_wait_for_chrome", _ready_chrome) + + source = await _launch_local_browser(LocalBrowserLaunchOptions(user_data_dir=str(profile))) + await source.close() + + assert spawned + assert profile.is_dir() + + +async def test_wait_for_chrome_requires_debugger_url( + monkeypatch: pytest.MonkeyPatch, +) -> None: + never_exits = asyncio.Event() + responses = iter((False, True)) + + class FakeProcess: + returncode = None + + async def wait(self) -> int: + await never_exits.wait() + return 0 + + async def debugging_ready(_cdp_url: str) -> bool: + return next(responses) + + async def no_delay(_seconds: float) -> None: + return None + + monkeypatch.setattr(browser, "_chrome_debugging_ready", debugging_ready) + monkeypatch.setattr(browser.asyncio, "sleep", no_delay) + + await browser._wait_for_chrome("http://127.0.0.1:9222", FakeProcess()) + + +async def test_wait_for_chrome_reports_early_exit( + monkeypatch: pytest.MonkeyPatch, +) -> None: + class FakeProcess: + returncode = None + + async def wait(self) -> int: + return 17 + + async def never_ready(_cdp_url: str) -> bool: + await asyncio.Event().wait() + return False + + monkeypatch.setattr(browser, "_chrome_debugging_ready", never_ready) + + with pytest.raises(RuntimeError, match="ready with code 17"): + await browser._wait_for_chrome("http://127.0.0.1:9222", FakeProcess()) + + +@pytest.mark.parametrize( + ("version", "ready"), + [ + ({}, False), + ({"webSocketDebuggerUrl": " "}, False), + ({"webSocketDebuggerUrl": "ws://127.0.0.1/devtools/browser/id"}, True), + ], +) +async def test_chrome_debugging_ready_requires_nonempty_websocket_url( + monkeypatch: pytest.MonkeyPatch, + version: dict[str, object], + ready: bool, +) -> None: + monkeypatch.setattr(browser, "_read_chrome_version", lambda _url: version) + assert await browser._chrome_debugging_ready("http://127.0.0.1:9222") is ready + + +async def test_launch_cancellation_closes_process_and_removes_profile( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + profile = tmp_path / "profile" + closed: list[Path] = [] + + class FakeProcess: + returncode = None + pid = 123 + + async def create_subprocess_exec(*_args: object, **_kwargs: object) -> FakeProcess: + return FakeProcess() + + async def cancel_wait(_cdp_url: str, _process: object) -> None: + raise asyncio.CancelledError + + async def close_process(_process: object, chrome_profile: object) -> None: + closed.append(cast(browser._ChromeProfile, chrome_profile).path) + profile.rmdir() + + monkeypatch.setattr(browser, "_find_chrome_path", lambda _explicit: "/path/to/chrome") + monkeypatch.setattr(browser, "_available_port", lambda: 9222) + monkeypatch.setattr(browser.tempfile, "mkdtemp", lambda **_kwargs: str(profile)) + monkeypatch.setattr(browser.asyncio, "create_subprocess_exec", create_subprocess_exec) + monkeypatch.setattr(browser, "_wait_for_chrome", cancel_wait) + monkeypatch.setattr(browser, "_close_local_chrome", close_process) + profile.mkdir() + + with pytest.raises(asyncio.CancelledError): + await _launch_local_browser(LocalBrowserLaunchOptions()) + + assert closed == [profile] + assert not profile.exists() + + def test_local_browser_flags_keep_explicit_viewport_without_defaults(tmp_path: Path) -> None: flags = _local_browser_flags( LocalBrowserLaunchOptions( From e43eeace2eb5c9afe81913fef4e0281e2368f097 Mon Sep 17 00:00:00 2001 From: Sean McGuire Date: Mon, 31 Aug 2026 11:26:41 -0300 Subject: [PATCH 3/9] enforce explicit debugging port ownership --- packages/sdk-go/chrome_launcher.go | 58 +++++++++++--- packages/sdk-go/chrome_launcher_test.go | 79 ++++++++++++++++++++ packages/sdk-python/src/stagehand/browser.py | 27 ++++++- packages/sdk-python/tests/test_browser.py | 76 ++++++++++++++++++- 4 files changed, 226 insertions(+), 14 deletions(-) diff --git a/packages/sdk-go/chrome_launcher.go b/packages/sdk-go/chrome_launcher.go index 2cbbd47fd..eb421e0cc 100644 --- a/packages/sdk-go/chrome_launcher.go +++ b/packages/sdk-go/chrome_launcher.go @@ -16,6 +16,7 @@ import ( "strconv" "strings" "sync" + "syscall" "time" ) @@ -91,6 +92,14 @@ func launchLocalBrowser( func launchChrome( ctx context.Context, options LocalBrowserLaunchOptions, +) (*launchedChrome, error) { + return launchChromeWithPortResolver(ctx, options, resolveChromePort) +} + +func launchChromeWithPortResolver( + ctx context.Context, + options LocalBrowserLaunchOptions, + resolvePort func(int) (int, error), ) (*launchedChrome, error) { if ctx == nil { return nil, errors.New("stagehand Chrome launch context is required") @@ -106,12 +115,9 @@ func launchChrome( if err != nil { return nil, err } - port := options.Port - if port == 0 { - port, err = availablePort() - if err != nil { - return nil, err - } + port, err := resolvePort(options.Port) + if err != nil { + return nil, err } userDataDir := options.UserDataDir @@ -356,15 +362,49 @@ func isFile(path string) bool { } func availablePort() (int, error) { - listener, err := net.Listen("tcp4", "127.0.0.1:0") + port, err := inspectChromePort(0) if err != nil { return 0, fmt.Errorf("select Chrome debugging port: %w", err) } - defer listener.Close() - port := listener.Addr().(*net.TCPAddr).Port return port, nil } +func resolveChromePort(requestedPort int) (int, error) { + return resolveChromePortWith(requestedPort, inspectChromePort) +} + +func resolveChromePortWith( + requestedPort int, + inspect func(int) (int, error), +) (int, error) { + if requestedPort == 0 { + port, err := inspect(0) + if err != nil { + return 0, fmt.Errorf("select Chrome debugging port: %w", err) + } + return port, nil + } + if _, err := inspect(requestedPort); err != nil { + if errors.Is(err, syscall.EADDRINUSE) { + return 0, fmt.Errorf("Chrome debugging port %d is already in use: %w", requestedPort, err) + } + return 0, fmt.Errorf("inspect Chrome debugging port %d: %w", requestedPort, err) + } + return requestedPort, nil +} + +func inspectChromePort(port int) (int, error) { + listener, err := net.Listen("tcp4", net.JoinHostPort("127.0.0.1", strconv.Itoa(port))) + if err != nil { + return 0, err + } + assignedPort := listener.Addr().(*net.TCPAddr).Port + if err := listener.Close(); err != nil { + return 0, err + } + return assignedPort, nil +} + func waitForChrome( ctx context.Context, cdpURL string, diff --git a/packages/sdk-go/chrome_launcher_test.go b/packages/sdk-go/chrome_launcher_test.go index 844719206..93f60c0ed 100644 --- a/packages/sdk-go/chrome_launcher_test.go +++ b/packages/sdk-go/chrome_launcher_test.go @@ -4,7 +4,9 @@ import ( "context" "encoding/json" "errors" + "fmt" "math" + "net" "net/http" "net/http/httptest" "os" @@ -12,6 +14,7 @@ import ( "path/filepath" "slices" "strings" + "syscall" "testing" "time" ) @@ -467,4 +470,80 @@ func TestAvailablePort(t *testing.T) { if port < 1 || port > 65_535 { t.Fatalf("availablePort() = %d, want valid TCP port", port) } + listener, err := net.Listen("tcp4", net.JoinHostPort("127.0.0.1", fmt.Sprint(port))) + if err != nil { + t.Fatalf("automatic port %d was not released: %v", port, err) + } + if err := listener.Close(); err != nil { + t.Fatalf("close automatic-port listener: %v", err) + } +} + +func TestResolveChromePort(t *testing.T) { + t.Run("automatic", func(t *testing.T) { + got, err := resolveChromePortWith(0, func(port int) (int, error) { + if port != 0 { + t.Fatalf("inspect port = %d, want 0", port) + } + return 4567, nil + }) + if err != nil || got != 4567 { + t.Fatalf("resolveChromePortWith() = (%d, %v), want (4567, nil)", got, err) + } + }) + + t.Run("explicit available", func(t *testing.T) { + got, err := resolveChromePortWith(9222, func(port int) (int, error) { + return port, nil + }) + if err != nil || got != 9222 { + t.Fatalf("resolveChromePortWith() = (%d, %v), want (9222, nil)", got, err) + } + }) + + t.Run("explicit occupied", func(t *testing.T) { + _, err := resolveChromePortWith(9222, func(int) (int, error) { + return 0, syscall.EADDRINUSE + }) + if !errors.Is(err, syscall.EADDRINUSE) || + !strings.Contains(err.Error(), "Chrome debugging port 9222 is already in use") { + t.Fatalf("resolveChromePortWith() error = %v, want occupied-port error", err) + } + }) + + t.Run("other socket error", func(t *testing.T) { + socketErr := errors.New("socket unavailable") + _, err := resolveChromePortWith(9222, func(int) (int, error) { + return 0, socketErr + }) + if !errors.Is(err, socketErr) || strings.Contains(err.Error(), "already in use") { + t.Fatalf("resolveChromePortWith() error = %v, want preserved socket error", err) + } + }) +} + +func TestLaunchChromeRejectsOccupiedPortBeforeProfileCreation(t *testing.T) { + profile := filepath.Join(t.TempDir(), "profile") + portChecked := false + + _, err := launchChromeWithPortResolver(context.Background(), LocalBrowserLaunchOptions{ + ExecutablePath: os.Args[0], + Port: 9222, + UserDataDir: profile, + }, func(port int) (int, error) { + portChecked = true + if port != 9222 { + t.Fatalf("resolve port = %d, want 9222", port) + } + return 0, fmt.Errorf("Chrome debugging port %d is already in use: %w", port, syscall.EADDRINUSE) + }) + if err == nil || !strings.Contains(err.Error(), "already in use") { + t.Fatalf("launchChrome() error = %v, want occupied-port error", err) + } + if !portChecked { + t.Fatal("Chrome port was not checked") + } + if _, statErr := os.Stat(profile); !errors.Is(statErr, os.ErrNotExist) { + t.Fatalf("profile was created before occupied-port rejection: %v", statErr) + } } diff --git a/packages/sdk-python/src/stagehand/browser.py b/packages/sdk-python/src/stagehand/browser.py index 30f10a318..b1061966e 100644 --- a/packages/sdk-python/src/stagehand/browser.py +++ b/packages/sdk-python/src/stagehand/browser.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +import errno import json import math import os @@ -650,7 +651,7 @@ async def connect( async def _launch_local_browser(options: _LocalBrowserOptions) -> ResolvedBrowserSource: _validate_local_browser_options(options) chrome_path = _find_chrome_path(options.executable_path) - port = options.port or _available_port() + port = _resolve_chrome_port(options.port) profile = _resolve_chrome_profile(options) flags = _local_browser_flags( options, @@ -937,7 +938,25 @@ def _is_executable_file(path: str, platform: str) -> bool: return candidate.is_file() and (platform == "win32" or os.access(candidate, os.X_OK)) -def _available_port() -> int: - with socket.socket() as candidate: - candidate.bind(("127.0.0.1", 0)) +def _resolve_chrome_port(requested_port: int | None) -> int: + if requested_port is None: + return _available_port() + try: + _inspect_chrome_port(requested_port) + except OSError as error: + if error.errno == errno.EADDRINUSE: + raise RuntimeError( + f"Chrome debugging port {requested_port} is already in use" + ) from error + raise + return requested_port + + +def _inspect_chrome_port(port: int) -> int: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as candidate: + candidate.bind(("127.0.0.1", port)) return int(candidate.getsockname()[1]) + + +def _available_port() -> int: + return _inspect_chrome_port(0) diff --git a/packages/sdk-python/tests/test_browser.py b/packages/sdk-python/tests/test_browser.py index e3993f705..c89a5185f 100644 --- a/packages/sdk-python/tests/test_browser.py +++ b/packages/sdk-python/tests/test_browser.py @@ -1,9 +1,10 @@ from __future__ import annotations import asyncio +import errno import json from pathlib import Path -from typing import ClassVar, Literal, cast +from typing import ClassVar, Literal, Self, cast import pytest from pydantic import ValidationError @@ -906,6 +907,79 @@ def create_profile(**_kwargs: object) -> str: await _launch_local_browser(LocalBrowserLaunchOptions(executable_path="/missing/chrome")) +async def test_occupied_explicit_port_precedes_profile_creation_and_spawn( + monkeypatch: pytest.MonkeyPatch, +) -> None: + profile_created = False + spawned = False + + def inspect_port(_port: int) -> int: + raise OSError(errno.EADDRINUSE, "address already in use") + + def create_profile(**_kwargs: object) -> str: + nonlocal profile_created + profile_created = True + return "/unused" + + async def create_subprocess_exec(*_args: object, **_kwargs: object) -> object: + nonlocal spawned + spawned = True + raise AssertionError("Chrome should not spawn") + + monkeypatch.setattr(browser, "_find_chrome_path", lambda _explicit: "/path/to/chrome") + monkeypatch.setattr(browser, "_inspect_chrome_port", inspect_port) + monkeypatch.setattr(browser.tempfile, "mkdtemp", create_profile) + monkeypatch.setattr(browser.asyncio, "create_subprocess_exec", create_subprocess_exec) + + with pytest.raises(RuntimeError, match="Chrome debugging port 9222 is already in use"): + await _launch_local_browser(LocalBrowserLaunchOptions(port=9222)) + + assert not profile_created + assert not spawned + + +def test_resolve_chrome_port_preserves_non_occupancy_socket_errors( + monkeypatch: pytest.MonkeyPatch, +) -> None: + socket_error = OSError(errno.EACCES, "permission denied") + + def inspect_port(_port: int) -> int: + raise socket_error + + monkeypatch.setattr(browser, "_inspect_chrome_port", inspect_port) + + with pytest.raises(OSError) as raised: + browser._resolve_chrome_port(9222) + assert raised.value is socket_error + + +def test_available_port_releases_automatic_reservation( + monkeypatch: pytest.MonkeyPatch, +) -> None: + class FakeSocket: + closed = False + + def __enter__(self) -> Self: + return self + + def __exit__(self, *_args: object) -> None: + self.closed = True + + def bind(self, address: tuple[str, int]) -> None: + assert address == ("127.0.0.1", 0) + + def getsockname(self) -> tuple[str, int]: + return ("127.0.0.1", 4567) + + reservation = FakeSocket() + monkeypatch.setattr(browser.socket, "socket", lambda *_args: reservation) + + port = browser._available_port() + + assert port == 4567 + assert reservation.closed + + async def test_spawn_failure_removes_sdk_owned_profile( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, From b8ad7ac50da66552d72029c64abd2303053e23c1 Mon Sep 17 00:00:00 2001 From: Sean McGuire Date: Mon, 31 Aug 2026 12:08:58 -0300 Subject: [PATCH 4/9] align local shutdown semantics for python --- packages/sdk-python/src/stagehand/browser.py | 153 ++++++++--- packages/sdk-python/tests/test_browser.py | 265 ++++++++++++++++++- 2 files changed, 374 insertions(+), 44 deletions(-) diff --git a/packages/sdk-python/src/stagehand/browser.py b/packages/sdk-python/src/stagehand/browser.py index b1061966e..0fabdb55c 100644 --- a/packages/sdk-python/src/stagehand/browser.py +++ b/packages/sdk-python/src/stagehand/browser.py @@ -91,14 +91,18 @@ class ResolvedBrowserSource: cdp_url: str keep_alive: bool _close_callback: Callable[[], Awaitable[None]] | None = field(default=None, repr=False) - _closed: bool = field(default=False, init=False, repr=False) + _close_task: asyncio.Task[None] | None = field(default=None, init=False, repr=False) async def close(self) -> None: - if self._closed: + if self._close_callback is None: return - self._closed = True - if self._close_callback is not None: - await self._close_callback() + if self._close_task is None: + self._close_task = asyncio.create_task(_invoke_close(self._close_callback)) + await asyncio.shield(self._close_task) + + +async def _invoke_close(callback: Callable[[], Awaitable[None]]) -> None: + await callback() @dataclass(frozen=True) @@ -280,9 +284,11 @@ async def wait(self) -> int: ... class _ChromeProcess(_WaitableChromeProcess, Protocol): pid: int - def terminate(self) -> None: ... - def kill(self) -> None: ... +class _TaskkillError(RuntimeError): + def __init__(self, returncode: int) -> None: + self.returncode = returncode + super().__init__(f"taskkill exited with code {returncode}") async def _connect_browser( @@ -657,7 +663,7 @@ async def _launch_local_browser(options: _LocalBrowserOptions) -> ResolvedBrowse options, port=port, user_data_dir=profile.path, - is_ci=bool(os.environ.get("CI")), + disable_sandbox=_should_disable_chromium_sandbox(options), ) process: _ChromeProcess | None = None try: @@ -673,11 +679,17 @@ async def _launch_local_browser(options: _LocalBrowserOptions) -> ResolvedBrowse except OSError as error: raise RuntimeError(f"Failed to start Chrome: {error}") from error await _wait_for_chrome(f"http://127.0.0.1:{port}", process) - except BaseException: - if process is not None: - await _close_local_chrome(process, profile) - elif profile.remove: - await _remove_chrome_profile(profile.path) + except BaseException as launch_error: + try: + if process is not None: + await _close_local_chrome(process, profile) + elif profile.remove: + await _remove_chrome_profile(profile.path) + except BaseException as cleanup_error: + raise _combined_error( + "Chrome launch failed and browser cleanup also failed", + [launch_error, cleanup_error], + ) from launch_error raise async def close() -> None: @@ -788,33 +800,83 @@ async def _close_local_chrome( process: _ChromeProcess, profile: _ChromeProfile, ) -> None: + errors: list[BaseException] = [] try: - if process.returncode is None: - try: - if sys.platform == "win32": - process.terminate() - else: - os.killpg(process.pid, signal.SIGTERM) - except ProcessLookupError: - pass - try: - await asyncio.wait_for(process.wait(), timeout=3) - except TimeoutError: - try: - if sys.platform == "win32": - process.kill() - else: - os.killpg(process.pid, signal.SIGKILL) - except ProcessLookupError: - pass - await process.wait() - finally: - if profile.remove: + await _close_chrome_process(process) + except BaseException as error: + errors.append(error) + if profile.remove: + try: await _remove_chrome_profile(profile.path) + except BaseException as error: + errors.append(error) + if errors: + raise _combined_error("Chrome termination and profile cleanup failed", errors) + + +async def _close_chrome_process(process: _ChromeProcess) -> None: + if process.returncode is not None: + return + try: + await _terminate_chrome_process(process.pid, force=False) + except BaseException as error: + if not _is_finished_process_error(error): + raise + try: + await asyncio.wait_for(process.wait(), timeout=3) + return + except TimeoutError: + pass + try: + await _terminate_chrome_process(process.pid, force=True) + except BaseException as error: + if not _is_finished_process_error(error): + raise + await process.wait() + + +async def _terminate_chrome_process(pid: int, *, force: bool) -> None: + if sys.platform == "win32": + await _run_taskkill(pid, force=force) + return + os.killpg(pid, signal.SIGKILL if force else signal.SIGTERM) + + +async def _run_taskkill(pid: int, *, force: bool) -> None: + taskkill = await asyncio.create_subprocess_exec( + "taskkill", + "/PID", + str(pid), + "/T", + *(["/F"] if force else []), + stdin=asyncio.subprocess.DEVNULL, + stdout=asyncio.subprocess.DEVNULL, + stderr=asyncio.subprocess.DEVNULL, + ) + returncode = await taskkill.wait() + if returncode != 0: + raise _TaskkillError(returncode) + + +def _is_finished_process_error(error: BaseException) -> bool: + return isinstance(error, ProcessLookupError) or ( + isinstance(error, _TaskkillError) and error.returncode == 128 + ) + + +def _combined_error(message: str, errors: list[BaseException]) -> BaseException: + if len(errors) == 1: + return errors[0] + if all(isinstance(error, Exception) for error in errors): + return ExceptionGroup( + message, + [error for error in errors if isinstance(error, Exception)], + ) + return BaseExceptionGroup(message, errors) async def _remove_chrome_profile(path: Path) -> None: - await asyncio.to_thread(shutil.rmtree, path, True) + await asyncio.to_thread(shutil.rmtree, path) def _local_browser_flags( @@ -822,7 +884,7 @@ def _local_browser_flags( *, port: int, user_data_dir: Path, - is_ci: bool, + disable_sandbox: bool, ) -> list[str]: ignored_default_args = options.ignore_default_args ignored_flags = set(ignored_default_args) if isinstance(ignored_default_args, list) else set() @@ -848,7 +910,7 @@ def _local_browser_flags( f"--user-data-dir={user_data_dir}", *(["--headless"] if options.headless is True else []), *(["--auto-open-devtools-for-tabs"] if options.devtools is True else []), - *(["--no-sandbox"] if is_ci or options.chromium_sandbox is False else []), + *(["--no-sandbox"] if disable_sandbox else []), *([f"--proxy-server={options.proxy.server}"] if options.proxy else []), *( [f"--proxy-bypass-list={options.proxy.bypass}"] @@ -868,6 +930,23 @@ def _local_browser_flags( ] +def _should_disable_chromium_sandbox( + options: _LocalBrowserOptions, + *, + platform: str | None = None, + environment: Mapping[str, str] | None = None, + getuid: Callable[[], int] | None = None, +) -> bool: + platform = sys.platform if platform is None else platform + environment = os.environ if environment is None else environment + getuid = getattr(os, "getuid", None) if getuid is None else getuid + return ( + bool(environment.get("CI")) + or options.chromium_sandbox is False + or (platform == "linux" and getuid is not None and getuid() == 0) + ) + + def _find_chrome_path( explicit_path: str | None = None, *, diff --git a/packages/sdk-python/tests/test_browser.py b/packages/sdk-python/tests/test_browser.py index c89a5185f..f3fe2cd62 100644 --- a/packages/sdk-python/tests/test_browser.py +++ b/packages/sdk-python/tests/test_browser.py @@ -4,7 +4,7 @@ import errno import json from pathlib import Path -from typing import ClassVar, Literal, Self, cast +from typing import Any, ClassVar, Literal, Self, cast import pytest from pydantic import ValidationError @@ -552,7 +552,12 @@ async def test_launch_converts_argument_tuples_to_flag_lists( async def launch(options: LocalBrowserLaunchOptions) -> FakeSource: captured_flags.extend( - _local_browser_flags(options, port=9222, user_data_dir=tmp_path, is_ci=False) + _local_browser_flags( + options, + port=9222, + user_data_dir=tmp_path, + disable_sandbox=False, + ) ) return FakeSource(keep_alive=False) @@ -618,7 +623,12 @@ def test_local_browser_flags_are_unchanged_for_launch_options(tmp_path: Path) -> devtools=True, args=["--custom-flag"], ) - flags = _local_browser_flags(options, port=9222, user_data_dir=tmp_path, is_ci=True) + flags = _local_browser_flags( + options, + port=9222, + user_data_dir=tmp_path, + disable_sandbox=True, + ) assert flags[-5:] == [ "--headless", @@ -636,7 +646,7 @@ def test_local_browser_default_flags_match_shared_fixture(tmp_path: Path) -> Non LocalBrowserLaunchOptions(), port=9222, user_data_dir=tmp_path, - is_ci=False, + disable_sandbox=False, ) == [ *EXPECTED_DEFAULT_CHROME_FLAGS, "--window-size=1280,800", @@ -1230,6 +1240,247 @@ async def close_process(_process: object, chrome_profile: object) -> None: assert not profile.exists() +async def test_resolved_browser_source_concurrent_close_waits_for_shared_task() -> None: + started = asyncio.Event() + release = asyncio.Event() + close_calls = 0 + + async def close_callback() -> None: + nonlocal close_calls + close_calls += 1 + started.set() + await release.wait() + + source = browser.ResolvedBrowserSource( + cdp_url="http://127.0.0.1:9222", + keep_alive=False, + _close_callback=close_callback, + ) + first = asyncio.create_task(source.close()) + await started.wait() + second = asyncio.create_task(source.close()) + await asyncio.sleep(0) + + assert close_calls == 1 + assert not first.done() + assert not second.done() + + release.set() + await asyncio.gather(first, second) + await source.close() + assert close_calls == 1 + + +@pytest.mark.parametrize( + ("platform", "environment", "uid", "sandbox_option", "expected"), + [ + ("linux", {}, 0, None, True), + ("linux", {}, 1000, None, False), + ("darwin", {}, 0, None, False), + ("darwin", {"CI": "1"}, 1000, None, True), + ("win32", {}, 1000, False, True), + ], +) +def test_should_disable_chromium_sandbox( + platform: str, + environment: dict[str, str], + uid: int, + sandbox_option: bool | None, + expected: bool, +) -> None: + assert ( + browser._should_disable_chromium_sandbox( + LocalBrowserLaunchOptions(chromium_sandbox=sandbox_option), + platform=platform, + environment=environment, + getuid=lambda: uid, + ) + is expected + ) + + +async def test_close_chrome_process_terminates_unix_process_group( + monkeypatch: pytest.MonkeyPatch, +) -> None: + signals: list[tuple[int, int]] = [] + + class FakeProcess: + returncode = None + pid = 123 + + async def wait(self) -> int: + return 0 + + monkeypatch.setattr(browser.sys, "platform", "linux") + monkeypatch.setattr(browser.os, "killpg", lambda pid, sig: signals.append((pid, sig))) + + await browser._close_chrome_process(FakeProcess()) + + assert signals == [(123, browser.signal.SIGTERM)] + + +async def test_close_chrome_process_force_kills_after_timeout( + monkeypatch: pytest.MonkeyPatch, +) -> None: + signals: list[tuple[int, int]] = [] + waits = 0 + + class FakeProcess: + returncode = None + pid = 123 + + async def wait(self) -> int: + nonlocal waits + waits += 1 + return 0 + + async def timeout_wait(awaitable: object, *, timeout: float) -> int: + assert timeout == 3 + cast(Any, awaitable).close() + raise TimeoutError + + monkeypatch.setattr(browser.sys, "platform", "linux") + monkeypatch.setattr(browser.os, "killpg", lambda pid, sig: signals.append((pid, sig))) + monkeypatch.setattr(browser.asyncio, "wait_for", timeout_wait) + + await browser._close_chrome_process(FakeProcess()) + + assert signals == [ + (123, browser.signal.SIGTERM), + (123, browser.signal.SIGKILL), + ] + assert waits == 1 + + +async def test_run_taskkill_terminates_windows_process_tree( + monkeypatch: pytest.MonkeyPatch, +) -> None: + calls: list[tuple[tuple[object, ...], dict[str, object]]] = [] + + class FakeTaskkill: + async def wait(self) -> int: + return 0 + + async def create_subprocess_exec( + *args: object, + **kwargs: object, + ) -> FakeTaskkill: + calls.append((args, kwargs)) + return FakeTaskkill() + + monkeypatch.setattr(browser.asyncio, "create_subprocess_exec", create_subprocess_exec) + + await browser._run_taskkill(123, force=False) + await browser._run_taskkill(123, force=True) + + assert [args for args, _ in calls] == [ + ("taskkill", "/PID", "123", "/T"), + ("taskkill", "/PID", "123", "/T", "/F"), + ] + assert all( + kwargs + == { + "stdin": asyncio.subprocess.DEVNULL, + "stdout": asyncio.subprocess.DEVNULL, + "stderr": asyncio.subprocess.DEVNULL, + } + for _, kwargs in calls + ) + + +async def test_close_chrome_process_ignores_finished_windows_tree( + monkeypatch: pytest.MonkeyPatch, +) -> None: + class FakeProcess: + returncode = None + pid = 123 + + async def wait(self) -> int: + return 0 + + async def taskkill(_pid: int, *, force: bool) -> None: + assert not force + raise browser._TaskkillError(128) + + monkeypatch.setattr(browser.sys, "platform", "win32") + monkeypatch.setattr(browser, "_run_taskkill", taskkill) + + await browser._close_chrome_process(FakeProcess()) + + +async def test_close_chrome_process_skips_already_exited_process( + monkeypatch: pytest.MonkeyPatch, +) -> None: + class FakeProcess: + returncode: int | None = 0 + pid = 123 + + async def wait(self) -> int: + raise AssertionError("already-exited process should not be awaited") + + async def terminate(_pid: int, *, force: bool) -> None: + raise AssertionError(f"already-exited process received force={force}") + + monkeypatch.setattr(browser, "_terminate_chrome_process", terminate) + + await browser._close_chrome_process(FakeProcess()) + + +async def test_close_local_chrome_combines_shutdown_and_profile_errors( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + termination_error = OSError("termination failed") + profile_error = OSError("profile cleanup failed") + + async def close_process(_process: object) -> None: + raise termination_error + + async def remove_profile(_path: Path) -> None: + raise profile_error + + monkeypatch.setattr(browser, "_close_chrome_process", close_process) + monkeypatch.setattr(browser, "_remove_chrome_profile", remove_profile) + + with pytest.raises(ExceptionGroup) as raised: + await browser._close_local_chrome( + cast(browser._ChromeProcess, object()), + browser._ChromeProfile(path=tmp_path, remove=True), + ) + + assert raised.value.message == "Chrome termination and profile cleanup failed" + assert raised.value.exceptions == (termination_error, profile_error) + + +async def test_launch_combines_spawn_and_profile_cleanup_errors( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + profile = tmp_path / "profile" + profile.mkdir() + profile_error = OSError("profile cleanup failed") + + async def create_subprocess_exec(*_args: object, **_kwargs: object) -> object: + raise OSError("spawn failed") + + async def remove_profile(_path: Path) -> None: + raise profile_error + + monkeypatch.setattr(browser, "_find_chrome_path", lambda _explicit: "/path/to/chrome") + monkeypatch.setattr(browser, "_available_port", lambda: 9222) + monkeypatch.setattr(browser.tempfile, "mkdtemp", lambda **_kwargs: str(profile)) + monkeypatch.setattr(browser.asyncio, "create_subprocess_exec", create_subprocess_exec) + monkeypatch.setattr(browser, "_remove_chrome_profile", remove_profile) + + with pytest.raises(ExceptionGroup) as raised: + await _launch_local_browser(LocalBrowserLaunchOptions()) + + assert raised.value.message == "Chrome launch failed and browser cleanup also failed" + assert isinstance(raised.value.exceptions[0], RuntimeError) + assert str(raised.value.exceptions[0]) == "Failed to start Chrome: spawn failed" + assert raised.value.exceptions[1] is profile_error + + def test_local_browser_flags_keep_explicit_viewport_without_defaults(tmp_path: Path) -> None: flags = _local_browser_flags( LocalBrowserLaunchOptions( @@ -1238,7 +1489,7 @@ def test_local_browser_flags_keep_explicit_viewport_without_defaults(tmp_path: P ), port=9222, user_data_dir=tmp_path, - is_ci=False, + disable_sandbox=False, ) assert "--window-size=1440,900" in flags @@ -1253,7 +1504,7 @@ def test_local_browser_flags_keep_ignored_explicit_viewport(tmp_path: Path) -> N ), port=9222, user_data_dir=tmp_path, - is_ci=False, + disable_sandbox=False, ) assert "--window-size=1440,900" in flags @@ -1264,7 +1515,7 @@ def test_local_browser_flags_can_omit_implicit_default_viewport(tmp_path: Path) LocalBrowserLaunchOptions(ignore_default_args=["--window-size=1280,800"]), port=9222, user_data_dir=tmp_path, - is_ci=False, + disable_sandbox=False, ) assert "--window-size=1280,800" not in flags From d52b2fdb67b8f54d22efa9c722a340080d29fb94 Mon Sep 17 00:00:00 2001 From: Sean McGuire Date: Mon, 31 Aug 2026 12:17:16 -0300 Subject: [PATCH 5/9] changeset --- .changeset/strict-tigers-read.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/strict-tigers-read.md diff --git a/.changeset/strict-tigers-read.md b/.changeset/strict-tigers-read.md new file mode 100644 index 000000000..1066cc0da --- /dev/null +++ b/.changeset/strict-tigers-read.md @@ -0,0 +1,6 @@ +--- +"@browserbasehq/stagehand-python": patch +"@browserbasehq/stagehand-go": patch +--- + +Make python and golang SDKs reject occupied local chrome debugging ports, and make local browser launch wait for readiness in python. From 68b5af93b9a7fa2c70846cbb9fbe689d0a626a67 Mon Sep 17 00:00:00 2001 From: Sean McGuire Date: Mon, 31 Aug 2026 12:51:38 -0300 Subject: [PATCH 6/9] treat empty string in userdatadir as omitted --- packages/sdk-go/chrome_launcher.go | 27 ++++++++++------- packages/sdk-go/chrome_launcher_test.go | 18 +++++++++++ packages/sdk-python/src/stagehand/browser.py | 2 +- packages/sdk-python/tests/test_browser.py | 30 +++++++++++++++++++ packages/sdk-ts/src/browser/localBrowser.ts | 7 +++-- .../sdk-ts/tests/browser/localBrowser.test.ts | 23 ++++++++++++++ 6 files changed, 93 insertions(+), 14 deletions(-) diff --git a/packages/sdk-go/chrome_launcher.go b/packages/sdk-go/chrome_launcher.go index eb421e0cc..13c7bc355 100644 --- a/packages/sdk-go/chrome_launcher.go +++ b/packages/sdk-go/chrome_launcher.go @@ -120,18 +120,11 @@ func launchChromeWithPortResolver( return nil, err } - userDataDir := options.UserDataDir - temporaryProfile := userDataDir == "" - if temporaryProfile { - userDataDir, err = os.MkdirTemp("", "stagehand-chrome-") - if err != nil { - return nil, fmt.Errorf("create Chrome profile: %w", err) - } - } else if err := os.MkdirAll(userDataDir, 0o700); err != nil { - return nil, fmt.Errorf("create Chrome profile %q: %w", userDataDir, err) + userDataDir, removeDir, err := resolveChromeProfile(options) + if err != nil { + return nil, err } - removeDir := temporaryProfile && !options.PreserveUserDataDir cleanupProfile := func() error { if !removeDir { return nil @@ -179,6 +172,20 @@ func launchChromeWithPortResolver( return launched, nil } +func resolveChromeProfile(options LocalBrowserLaunchOptions) (string, bool, error) { + if options.UserDataDir != "" { + if err := os.MkdirAll(options.UserDataDir, 0o700); err != nil { + return "", false, fmt.Errorf("create Chrome profile %q: %w", options.UserDataDir, err) + } + return options.UserDataDir, false, nil + } + userDataDir, err := os.MkdirTemp("", "stagehand-chrome-") + if err != nil { + return "", false, fmt.Errorf("create Chrome profile: %w", err) + } + return userDataDir, !options.PreserveUserDataDir, nil +} + func validateLocalBrowserOptions(options LocalBrowserLaunchOptions) error { if options.Port < 0 || options.Port > 65_535 { return errors.New("stagehand Chrome port must be 0 or between 1 and 65535") diff --git a/packages/sdk-go/chrome_launcher_test.go b/packages/sdk-go/chrome_launcher_test.go index 93f60c0ed..a48b2de11 100644 --- a/packages/sdk-go/chrome_launcher_test.go +++ b/packages/sdk-go/chrome_launcher_test.go @@ -76,6 +76,24 @@ func TestLaunchedChromeProfileOwnership(t *testing.T) { } } +func TestResolveChromeProfileTreatsEmptyPathAsTemporary(t *testing.T) { + profile, remove, err := resolveChromeProfile(LocalBrowserLaunchOptions{UserDataDir: ""}) + if err != nil { + t.Fatalf("resolveChromeProfile() error = %v", err) + } + t.Cleanup(func() { + if err := os.RemoveAll(profile); err != nil { + t.Errorf("remove temporary Chrome profile: %v", err) + } + }) + if !remove { + t.Fatal("resolveChromeProfile() remove = false, want true") + } + if filepath.Base(profile) == "." || !strings.HasPrefix(filepath.Base(profile), "stagehand-chrome-") { + t.Fatalf("resolveChromeProfile() path = %q, want Stagehand temporary profile", profile) + } +} + func TestBuildChromeArgsSupportsLocalBrowserOptions(t *testing.T) { t.Setenv("CI", "") sandbox := false diff --git a/packages/sdk-python/src/stagehand/browser.py b/packages/sdk-python/src/stagehand/browser.py index 0fabdb55c..bdc7ccf62 100644 --- a/packages/sdk-python/src/stagehand/browser.py +++ b/packages/sdk-python/src/stagehand/browser.py @@ -719,7 +719,7 @@ def _validate_local_browser_options(options: _LocalBrowserOptions) -> None: def _resolve_chrome_profile(options: _LocalBrowserOptions) -> _ChromeProfile: - if options.user_data_dir is not None: + if options.user_data_dir not in (None, ""): path = Path(options.user_data_dir) path.mkdir(mode=0o700, parents=True, exist_ok=True) return _ChromeProfile(path=path, remove=False) diff --git a/packages/sdk-python/tests/test_browser.py b/packages/sdk-python/tests/test_browser.py index f3fe2cd62..8301d23b9 100644 --- a/packages/sdk-python/tests/test_browser.py +++ b/packages/sdk-python/tests/test_browser.py @@ -852,6 +852,36 @@ async def create_subprocess_exec(*_args: object, **_kwargs: object) -> FakeProce assert not profile.exists() +async def test_empty_user_data_dir_uses_and_removes_temporary_profile( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + profile = tmp_path / "profile" + profile.mkdir() + spawned_args: tuple[object, ...] = () + + class FakeProcess: + returncode = 0 + pid = 123 + + async def create_subprocess_exec(*args: object, **_kwargs: object) -> FakeProcess: + nonlocal spawned_args + spawned_args = args + return FakeProcess() + + monkeypatch.setattr(browser, "_find_chrome_path", lambda _explicit: "/path/to/chrome") + monkeypatch.setattr(browser, "_available_port", lambda: 9222) + monkeypatch.setattr(browser.tempfile, "mkdtemp", lambda **_kwargs: str(profile)) + monkeypatch.setattr(browser.asyncio, "create_subprocess_exec", create_subprocess_exec) + monkeypatch.setattr(browser, "_wait_for_chrome", _ready_chrome) + + source = await _launch_local_browser(LocalBrowserLaunchOptions(user_data_dir="")) + + assert f"--user-data-dir={profile}" in spawned_args + await source.close() + assert not profile.exists() + + @pytest.mark.parametrize( "uses_temporary_profile", [False, True], diff --git a/packages/sdk-ts/src/browser/localBrowser.ts b/packages/sdk-ts/src/browser/localBrowser.ts index ddcef8276..0fbc9cd0b 100644 --- a/packages/sdk-ts/src/browser/localBrowser.ts +++ b/packages/sdk-ts/src/browser/localBrowser.ts @@ -118,9 +118,10 @@ async function launchChrome( const chromePath = await findChromePath(options.executablePath, dependencies); const port = await resolveChromePort(options.port, dependencies); - const temporaryProfile = options.userDataDir === undefined; - const userDataDir = - options.userDataDir ?? (await dependencies.mkdtemp(path.join(tmpdir(), "stagehand-chrome-"))); + const temporaryProfile = options.userDataDir === undefined || options.userDataDir === ""; + const userDataDir = temporaryProfile + ? await dependencies.mkdtemp(path.join(tmpdir(), "stagehand-chrome-")) + : options.userDataDir; const removeProfile = temporaryProfile && options.preserveUserDataDir !== true; if (!temporaryProfile) { diff --git a/packages/sdk-ts/tests/browser/localBrowser.test.ts b/packages/sdk-ts/tests/browser/localBrowser.test.ts index b8aed845d..891afbd07 100644 --- a/packages/sdk-ts/tests/browser/localBrowser.test.ts +++ b/packages/sdk-ts/tests/browser/localBrowser.test.ts @@ -214,6 +214,29 @@ describe("local browser launch lifecycle", () => { await browser.close(); }); + it("treats an empty profile path as an SDK-owned temporary profile", async () => { + const mkdir = vi.fn(async () => undefined); + const mkdtemp = vi.fn(async () => "/tmp/empty-stagehand-chrome-profile"); + const { launch, removeProfile, spawnChrome } = fakeLauncher({ mkdir, mkdtemp }); + + const browser = await launch({ + executablePath: "/path/to/chrome", + userDataDir: "", + }); + + expect(mkdtemp).toHaveBeenCalledOnce(); + expect(mkdir).not.toHaveBeenCalled(); + expect(spawnChrome.mock.calls[0]?.[1]).toContain( + "--user-data-dir=/tmp/empty-stagehand-chrome-profile", + ); + + await browser.close(); + expect(removeProfile).toHaveBeenCalledWith("/tmp/empty-stagehand-chrome-profile", { + force: true, + recursive: true, + }); + }); + it("uses CHROME_PATH before platform candidates", async () => { const checked: string[] = []; const { launch, spawnChrome } = fakeLauncher({ From e11ea4fbc715ef6f4721ef0206678dc6fd12a3cd Mon Sep 17 00:00:00 2001 From: Sean McGuire Date: Mon, 31 Aug 2026 12:54:37 -0300 Subject: [PATCH 7/9] fix test --- packages/sdk-python/tests/test_browser.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/sdk-python/tests/test_browser.py b/packages/sdk-python/tests/test_browser.py index 8301d23b9..a41545bf3 100644 --- a/packages/sdk-python/tests/test_browser.py +++ b/packages/sdk-python/tests/test_browser.py @@ -830,25 +830,30 @@ class FakeProcess: returncode = None pid = 123 - def terminate(self) -> None: - raise ProcessLookupError - async def wait(self) -> int: return 0 async def create_subprocess_exec(*_args: object, **_kwargs: object) -> FakeProcess: return FakeProcess() + taskkill_calls: list[tuple[int, bool]] = [] + + async def taskkill(pid: int, *, force: bool) -> None: + taskkill_calls.append((pid, force)) + raise browser._TaskkillError(128) + monkeypatch.setattr(browser, "_find_chrome_path", lambda _explicit: "/path/to/chrome") monkeypatch.setattr(browser, "_available_port", lambda: 9222) monkeypatch.setattr(browser.tempfile, "mkdtemp", lambda **_kwargs: str(profile)) monkeypatch.setattr(browser.asyncio, "create_subprocess_exec", create_subprocess_exec) monkeypatch.setattr(browser, "_wait_for_chrome", _ready_chrome) + monkeypatch.setattr(browser, "_run_taskkill", taskkill) monkeypatch.setattr(browser.sys, "platform", "win32") source = await _launch_local_browser(LocalBrowserLaunchOptions()) await source.close() + assert taskkill_calls == [(123, False)] assert not profile.exists() From 5fc6d57c3e917547c4b81bd47b914aa5d9aff4cd Mon Sep 17 00:00:00 2001 From: Sean McGuire Date: Mon, 31 Aug 2026 13:07:01 -0300 Subject: [PATCH 8/9] fix type error --- packages/sdk-ts/src/browser/localBrowser.ts | 25 +++++++++++++-------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/packages/sdk-ts/src/browser/localBrowser.ts b/packages/sdk-ts/src/browser/localBrowser.ts index 0fbc9cd0b..8918017ca 100644 --- a/packages/sdk-ts/src/browser/localBrowser.ts +++ b/packages/sdk-ts/src/browser/localBrowser.ts @@ -118,15 +118,7 @@ async function launchChrome( const chromePath = await findChromePath(options.executablePath, dependencies); const port = await resolveChromePort(options.port, dependencies); - const temporaryProfile = options.userDataDir === undefined || options.userDataDir === ""; - const userDataDir = temporaryProfile - ? await dependencies.mkdtemp(path.join(tmpdir(), "stagehand-chrome-")) - : options.userDataDir; - const removeProfile = temporaryProfile && options.preserveUserDataDir !== true; - - if (!temporaryProfile) { - await dependencies.mkdir(userDataDir, { recursive: true, mode: 0o700 }); - } + const { userDataDir, removeProfile } = await resolveChromeProfile(options, dependencies); let child: ChildProcess | undefined; let close: (() => Promise) | undefined; @@ -175,6 +167,21 @@ async function launchChrome( } } +async function resolveChromeProfile( + options: LocalBrowserLaunchOptions, + dependencies: Pick, +): Promise<{ userDataDir: string; removeProfile: boolean }> { + if (options.userDataDir !== undefined && options.userDataDir !== "") { + await dependencies.mkdir(options.userDataDir, { recursive: true, mode: 0o700 }); + return { userDataDir: options.userDataDir, removeProfile: false }; + } + const userDataDir = await dependencies.mkdtemp(path.join(tmpdir(), "stagehand-chrome-")); + return { + userDataDir, + removeProfile: options.preserveUserDataDir !== true, + }; +} + export function localBrowserChromeFlags( options: LocalBrowserLaunchOptions, port: number, From a7bccfa92f8f445d6ffcd8d6a143915faefdffac Mon Sep 17 00:00:00 2001 From: Sean McGuire Date: Wed, 2 Sep 2026 10:35:03 -0300 Subject: [PATCH 9/9] ensure requested port is not 0 --- packages/sdk-python/src/stagehand/browser.py | 2 ++ packages/sdk-python/tests/test_browser.py | 14 ++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/packages/sdk-python/src/stagehand/browser.py b/packages/sdk-python/src/stagehand/browser.py index bdc7ccf62..6cde74d7a 100644 --- a/packages/sdk-python/src/stagehand/browser.py +++ b/packages/sdk-python/src/stagehand/browser.py @@ -1020,6 +1020,8 @@ def _is_executable_file(path: str, platform: str) -> bool: def _resolve_chrome_port(requested_port: int | None) -> int: if requested_port is None: return _available_port() + if requested_port < 1 or requested_port > 65_535: + raise ValueError("Chrome port must be between 1 and 65535") try: _inspect_chrome_port(requested_port) except OSError as error: diff --git a/packages/sdk-python/tests/test_browser.py b/packages/sdk-python/tests/test_browser.py index a41545bf3..78061a94e 100644 --- a/packages/sdk-python/tests/test_browser.py +++ b/packages/sdk-python/tests/test_browser.py @@ -983,6 +983,20 @@ async def create_subprocess_exec(*_args: object, **_kwargs: object) -> object: assert not spawned +@pytest.mark.parametrize("port", [0, -1, 65_536]) +def test_resolve_chrome_port_rejects_invalid_explicit_ports( + monkeypatch: pytest.MonkeyPatch, + port: int, +) -> None: + def inspect_port(_port: int) -> int: + raise AssertionError("invalid ports should not be inspected") + + monkeypatch.setattr(browser, "_inspect_chrome_port", inspect_port) + + with pytest.raises(ValueError, match="between 1 and 65535"): + browser._resolve_chrome_port(port) + + def test_resolve_chrome_port_preserves_non_occupancy_socket_errors( monkeypatch: pytest.MonkeyPatch, ) -> None: