mirror of
https://github.com/usestrix/strix.git
synced 2026-09-14 23:21:15 +00:00
fix: properly isolate context
This commit is contained in:
parent
c7eb83bec9
commit
48ea4d15d6
8 changed files with 205 additions and 37 deletions
17
Makefile
17
Makefile
|
|
@ -1,4 +1,4 @@
|
|||
.PHONY: help install dev-install format lint type-check test test-cov clean pre-commit setup-dev
|
||||
.PHONY: help install dev-install format lint type-check test test-cov clean pre-commit setup-dev integration
|
||||
|
||||
help:
|
||||
@echo "Available commands:"
|
||||
|
|
@ -14,8 +14,10 @@ help:
|
|||
@echo " check-all - Run all code quality checks"
|
||||
@echo ""
|
||||
@echo "Testing:"
|
||||
@echo " test - Run tests with pytest"
|
||||
@echo " test-cov - Run tests with coverage reporting"
|
||||
@echo " test - Run tests with pytest"
|
||||
@echo " test-cov - Run tests with coverage reporting"
|
||||
@echo " integration - Run integration tests (verbose)"
|
||||
@echo " PRETTY=1 integration - Run integration tests (clean TUI)"
|
||||
@echo ""
|
||||
@echo "Development:"
|
||||
@echo " pre-commit - Run pre-commit hooks on all files"
|
||||
|
|
@ -70,6 +72,15 @@ test-cov:
|
|||
@echo "✅ Tests with coverage complete!"
|
||||
@echo "📊 Coverage report generated in htmlcov/"
|
||||
|
||||
integration:
|
||||
ifdef PRETTY
|
||||
poetry run pytest tests/integration/ -m integration --no-cov --no-header -q --tb=no -p no:logging -s --pretty
|
||||
else
|
||||
@echo "🧪 Running integration tests..."
|
||||
poetry run pytest tests/integration/ -v -s --log-cli-level=INFO -m integration --no-cov
|
||||
@echo "✅ Integration tests complete!"
|
||||
endif
|
||||
|
||||
pre-commit:
|
||||
@echo "🔧 Running pre-commit hooks..."
|
||||
uv run pre-commit run --all-files
|
||||
|
|
|
|||
|
|
@ -359,6 +359,7 @@ asyncio_mode = "auto"
|
|||
asyncio_default_fixture_loop_scope = "session"
|
||||
markers = [
|
||||
"integration: end-to-end tests requiring Docker and network access",
|
||||
"browsers(n): launch n isolated browser sessions for the test",
|
||||
]
|
||||
|
||||
[tool.coverage.run]
|
||||
|
|
|
|||
|
|
@ -156,7 +156,7 @@ async def _run_browser_tool(
|
|||
metadata: dict[str, Any] | None = None,
|
||||
) -> Any:
|
||||
if not session.browser.is_cdp_connected:
|
||||
await session.browser.start()
|
||||
await session.start()
|
||||
|
||||
llm, _ = _build_llm(metadata=metadata)
|
||||
tools = Tools()
|
||||
|
|
|
|||
|
|
@ -60,6 +60,7 @@ class BrowserSession:
|
|||
__slots__ = (
|
||||
"auth_token",
|
||||
"browser",
|
||||
"browser_context_id",
|
||||
"cdp_url",
|
||||
"local",
|
||||
"profile_directory",
|
||||
|
|
@ -83,9 +84,45 @@ class BrowserSession:
|
|||
self.auth_token = auth_token
|
||||
self.local = local
|
||||
self.profile_directory = profile_directory
|
||||
self.browser_context_id: str | None = None
|
||||
self.task_count = 0
|
||||
|
||||
async def start(self) -> None:
|
||||
from cdp_use.cdp.target.commands import CreateTargetParameters
|
||||
|
||||
await self.browser.start()
|
||||
|
||||
# [info] Really annoying discovery: despite being isolated in
|
||||
# different internal sessions, browser use does not
|
||||
# *actually* isolate the cookies and internals.
|
||||
#
|
||||
# [fix] We use CDP browser contexts manually to make the
|
||||
# sessions unique per CDP connection. Nothing groundbreaking
|
||||
cdp = self.browser.cdp_client
|
||||
ctx = await cdp.send.Target.createBrowserContext(params={"disposeOnDetach": True})
|
||||
self.browser_context_id = ctx["browserContextId"]
|
||||
|
||||
async def _scoped(
|
||||
url: str = "about:blank", background: bool = False, new_window: bool = False
|
||||
) -> str:
|
||||
params = CreateTargetParameters(
|
||||
url=url, background=background, browserContextId=self.browser_context_id
|
||||
)
|
||||
if new_window:
|
||||
params["newWindow"] = True
|
||||
return (await cdp.send.Target.createTarget(params=params))["targetId"] # type: ignore[no-any-return]
|
||||
|
||||
self.browser._cdp_create_new_page = _scoped
|
||||
target_id = await _scoped(new_window=True)
|
||||
await self.browser.get_or_create_cdp_session(target_id, focus=True)
|
||||
|
||||
async def close(self) -> None:
|
||||
if self.browser_context_id and self.browser and self.browser.is_cdp_connected:
|
||||
with contextlib.suppress(Exception):
|
||||
# [fix] properly dispose the session on closure.
|
||||
await self.browser.cdp_client.send.Target.disposeBrowserContext(
|
||||
params={"browserContextId": self.browser_context_id},
|
||||
)
|
||||
await _close_browser(self.browser)
|
||||
self.browser = None
|
||||
|
||||
|
|
@ -164,7 +201,9 @@ async def _launch_browser(api_url: str, agent_id: str, auth_token: str = "") ->
|
|||
task.add_done_callback(_manager.background_tasks.discard)
|
||||
return session
|
||||
|
||||
return _manager.create(agent_id, browser, api_url, ws_url, auth_token=auth_token)
|
||||
session = _manager.create(agent_id, browser, api_url, ws_url, auth_token=auth_token)
|
||||
await session.start()
|
||||
return session
|
||||
|
||||
|
||||
async def _launch_local_browser(
|
||||
|
|
|
|||
|
|
@ -131,19 +131,32 @@ def agent_state(sandbox_info):
|
|||
)
|
||||
|
||||
|
||||
@pytest.fixture(scope="session")
|
||||
def browser_session(agent_state):
|
||||
set_current_agent_id(_SESSION_AGENT_ID)
|
||||
ui.status("Launching browser…")
|
||||
result = _run_in_bg(browser_action(action="launch", agent_state=agent_state))
|
||||
if "error" in result:
|
||||
pytest.fail(f"Browser launch failed: {result}")
|
||||
ui.log(f"Browser ready: mode={result.get('mode')}")
|
||||
yield agent_state
|
||||
@pytest.fixture
|
||||
def browsers(agent_state, request):
|
||||
from strix.tools.browser.browser_manager import _manager
|
||||
|
||||
_manager.sessions.pop(_SESSION_AGENT_ID, None)
|
||||
_bg_loop.call_soon_threadsafe(_bg_loop.stop)
|
||||
from .helpers import Browser
|
||||
|
||||
marker = request.node.get_closest_marker("browsers")
|
||||
count = marker.args[0] if marker else 1
|
||||
|
||||
agent_ids = [f"{_SESSION_AGENT_ID}-{i}" for i in range(count)]
|
||||
for aid in agent_ids:
|
||||
set_current_agent_id(aid)
|
||||
result = _run_in_bg(browser_action(action="launch", agent_state=agent_state))
|
||||
if "error" in result:
|
||||
pytest.fail(f"Browser launch failed for {aid}: {result}")
|
||||
|
||||
yield [Browser(aid) for aid in agent_ids]
|
||||
|
||||
for aid in agent_ids:
|
||||
_manager.sessions.pop(aid, None)
|
||||
set_current_agent_id(_SESSION_AGENT_ID)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def browser(browsers):
|
||||
return browsers[0]
|
||||
|
||||
|
||||
def pytest_runtest_logstart(nodeid, location):
|
||||
|
|
@ -164,10 +177,3 @@ def pytest_runtest_logreport(report):
|
|||
@pytest.fixture(autouse=True)
|
||||
def _set_agent_context():
|
||||
set_current_agent_id(_SESSION_AGENT_ID)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def browser(browser_session):
|
||||
from .helpers import Browser
|
||||
|
||||
return Browser(browser_session)
|
||||
|
|
|
|||
|
|
@ -6,7 +6,6 @@ from pathlib import Path
|
|||
from pytest_check import check
|
||||
|
||||
from . import console as ui
|
||||
from .conftest import _run_in_bg
|
||||
|
||||
|
||||
SCREENSHOTS_DIR = Path(__file__).parent / "screenshots"
|
||||
|
|
@ -19,20 +18,24 @@ def setup_screenshots_dir():
|
|||
|
||||
|
||||
class Browser:
|
||||
def __init__(self, agent_state):
|
||||
self._state = agent_state
|
||||
def __init__(self, agent_id):
|
||||
self._agent_id = agent_id
|
||||
|
||||
def __getattr__(self, action):
|
||||
import asyncio
|
||||
|
||||
from strix.tools.browser.browser_actions import browser_action
|
||||
from strix.tools.context import set_current_agent_id
|
||||
|
||||
from .conftest import _bg_loop
|
||||
|
||||
def call(**kwargs):
|
||||
result = _run_in_bg(
|
||||
browser_action(
|
||||
action=action,
|
||||
agent_state=self._state,
|
||||
**kwargs,
|
||||
)
|
||||
)
|
||||
async def _run():
|
||||
set_current_agent_id(self._agent_id)
|
||||
return await browser_action(agent_state=None, action=action, **kwargs)
|
||||
|
||||
future = asyncio.run_coroutine_threadsafe(_run(), _bg_loop)
|
||||
result = future.result(timeout=120)
|
||||
if "error" in result:
|
||||
Fail(result).error(result["error"])
|
||||
return result
|
||||
|
|
@ -40,6 +43,18 @@ class Browser:
|
|||
return call
|
||||
|
||||
|
||||
def act_parallel(tasks):
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
|
||||
def _run_one(browser, kwargs):
|
||||
action = kwargs.pop("action")
|
||||
return getattr(browser, action)(**kwargs)
|
||||
|
||||
with ThreadPoolExecutor(max_workers=len(tasks)) as pool:
|
||||
futures = [pool.submit(_run_one, b, dict(kw)) for b, kw in tasks]
|
||||
return [f.result(timeout=120) for f in futures]
|
||||
|
||||
|
||||
def _caller_test_name():
|
||||
for frame in inspect.stack():
|
||||
if frame.function.startswith("test_"):
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@ import base64
|
|||
import binascii
|
||||
|
||||
import pytest
|
||||
from pytest_check import check
|
||||
|
||||
from . import console as ui
|
||||
from .helpers import Fail, setup_screenshots_dir
|
||||
|
|
@ -18,9 +17,10 @@ def test_navigate(browser):
|
|||
result = browser.navigate(url="https://example.com")
|
||||
ui.log(f"navigate → url={result.get('url')} title={result.get('title')}")
|
||||
|
||||
with check:
|
||||
check.is_in("example.com", result.get("url", "").lower())
|
||||
check.is_in("example", result.get("title", "").lower())
|
||||
if "example.com" not in result.get("url", "").lower():
|
||||
Fail(result).expected("url containing 'example.com'").got(result.get("url"))
|
||||
if "example" not in result.get("title", "").lower():
|
||||
Fail(result).expected("title containing 'example'").got(result.get("title"))
|
||||
|
||||
|
||||
def test_click(browser):
|
||||
|
|
|
|||
96
tests/integration/test_browser_isolation.py
Normal file
96
tests/integration/test_browser_isolation.py
Normal file
|
|
@ -0,0 +1,96 @@
|
|||
import pytest
|
||||
|
||||
from . import console as ui
|
||||
from .helpers import Fail, act_parallel
|
||||
|
||||
|
||||
pytestmark = [pytest.mark.integration, pytest.mark.browsers(2)]
|
||||
|
||||
|
||||
def test_session_objects_are_distinct(browsers):
|
||||
from strix.tools.browser.browser_manager import _manager
|
||||
|
||||
a, b = browsers
|
||||
session_a = _manager.sessions.get(a._agent_id)
|
||||
session_b = _manager.sessions.get(b._agent_id)
|
||||
|
||||
if not session_a or not session_b:
|
||||
Fail().error("one or both sessions missing from manager")
|
||||
return
|
||||
if session_a is session_b:
|
||||
Fail().error("both agents share the same session object")
|
||||
if session_a.browser_context_id == session_b.browser_context_id:
|
||||
Fail().expected("different browser_context_ids").got(
|
||||
f"A={session_a.browser_context_id}, B={session_b.browser_context_id}"
|
||||
)
|
||||
|
||||
|
||||
def test_parallel_navigate_isolated(browsers):
|
||||
a, b = browsers
|
||||
ui.status("isolation → navigating both agents concurrently")
|
||||
result_a, result_b = act_parallel(
|
||||
[
|
||||
(a, {"action": "navigate", "url": "https://example.com"}),
|
||||
(b, {"action": "navigate", "url": "https://www.iana.org"}),
|
||||
]
|
||||
)
|
||||
ui.log(f"agent A → {result_a.get('url')}")
|
||||
ui.log(f"agent B → {result_b.get('url')}")
|
||||
|
||||
if "example.com" not in result_a.get("url", ""):
|
||||
Fail(result_a).expected("url containing 'example.com'").got(result_a.get("url"))
|
||||
if "iana.org" not in result_b.get("url", ""):
|
||||
Fail(result_b).expected("url containing 'iana.org'").got(result_b.get("url"))
|
||||
|
||||
state_a, state_b = act_parallel(
|
||||
[
|
||||
(a, {"action": "screenshot"}),
|
||||
(b, {"action": "screenshot"}),
|
||||
]
|
||||
)
|
||||
|
||||
if "example.com" not in state_a.get("url", ""):
|
||||
Fail(state_a).expected("agent A still on example.com").got(state_a.get("url"))
|
||||
if "iana.org" not in state_b.get("url", ""):
|
||||
Fail(state_b).expected("agent B still on iana.org").got(state_b.get("url"))
|
||||
|
||||
|
||||
def test_cookie_isolation(browsers):
|
||||
a, b = browsers
|
||||
a.navigate(url="https://example.com")
|
||||
a.evaluate(code="document.cookie = 'agent=A; path=/'")
|
||||
|
||||
b.navigate(url="https://example.com")
|
||||
result_b = b.evaluate(code="document.cookie")
|
||||
cookie_b = str(result_b.get("result", ""))
|
||||
ui.log(f"agent B cookies: {cookie_b}")
|
||||
|
||||
if "agent=A" in cookie_b:
|
||||
Fail(result_b).expected("no cookie leakage").got(cookie_b)
|
||||
|
||||
result_a = a.evaluate(code="document.cookie")
|
||||
cookie_a = str(result_a.get("result", ""))
|
||||
ui.log(f"agent A cookies: {cookie_a}")
|
||||
|
||||
if "agent=A" not in cookie_a:
|
||||
Fail(result_a).expected("cookie 'agent=A' present").got(cookie_a)
|
||||
|
||||
|
||||
def test_concurrent_mixed_actions(browsers):
|
||||
a, b = browsers
|
||||
a.navigate(url="https://example.com")
|
||||
b.navigate(url="https://www.iana.org")
|
||||
|
||||
ui.status("isolation → mixed actions concurrently")
|
||||
results = act_parallel(
|
||||
[
|
||||
(a, {"action": "scroll", "direction": "down", "amount": 3}),
|
||||
(b, {"action": "screenshot"}),
|
||||
(a, {"action": "screenshot"}),
|
||||
(b, {"action": "evaluate", "code": "document.title"}),
|
||||
]
|
||||
)
|
||||
|
||||
for r in results:
|
||||
if "error" in r:
|
||||
Fail(r).error(r["error"])
|
||||
Loading…
Add table
Reference in a new issue