cleanup + concurrency test

strix/tools/browser/browser_manager.py

@@ -92,12 +92,6 @@ class BrowserSession:
 
         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"]
@@ -116,13 +110,16 @@ class BrowserSession:
         target_id = await _scoped(new_window=True)
         await self.browser.get_or_create_cdp_session(target_id, focus=True)
 
-    async def close(self) -> None:
+    async def dispose_context(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},
                 )
+            self.browser_context_id = None
+
+    async def close(self) -> None:
+        await self.dispose_context()
         await _close_browser(self.browser)
         self.browser = None
 

tests/integration/conftest.py

@@ -39,7 +39,6 @@ def _preflight():
 
 _preflight()
 
-# in pretty mode, silence all loggers and suppress pytest's own terminal output
 if ui.is_pretty():
     for _name in (
         "strix.tests.integration",
@@ -147,10 +146,12 @@ def browsers(agent_state, request):
         if "error" in result:
             pytest.fail(f"Browser launch failed for {aid}: {result}")
 
-    yield [Browser(aid) for aid in agent_ids]
+    yield [Browser(aid, agent_state) for aid in agent_ids]
 
     for aid in agent_ids:
-        _manager.sessions.pop(aid, None)
+        session = _manager.remove(aid)
+        if session:
+            _run_in_bg(session.dispose_context())
     set_current_agent_id(_SESSION_AGENT_ID)
 
 

tests/integration/helpers.py

@@ -18,8 +18,9 @@ def setup_screenshots_dir():
 
 
 class Browser:
-    def __init__(self, agent_id):
+    def __init__(self, agent_id, agent_state=None):
         self._agent_id = agent_id
+        self._agent_state = agent_state
 
     def __getattr__(self, action):
         import asyncio
@@ -32,12 +33,17 @@ class Browser:
         def call(**kwargs):
             async def _run():
                 set_current_agent_id(self._agent_id)
-                return await browser_action(agent_state=None, action=action, **kwargs)
+                return await browser_action(
+                    agent_state=self._agent_state,
+                    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"])
+            _strip_screenshot(result, _caller_test_name())
             return result
 
         return call
@@ -62,20 +68,20 @@ def _caller_test_name():
     return "unknown"
 
 
-def _save_screenshot(result, name):
-    b64 = result.get("screenshot")
+def _strip_screenshot(result, name):
+    b64 = result.pop("screenshot", None)
     if not b64 or not isinstance(b64, str) or len(b64) < 100:
-        return None
+        return
     path = SCREENSHOTS_DIR / f"{name}.png"
     path.write_bytes(base64.b64decode(b64))
-    return str(path)
+    result["screenshot_path"] = str(path)
 
 
 class Fail:
     def __init__(self, result=None):
         self._result = result
         self._name = _caller_test_name()
-        self._screenshot = _save_screenshot(result, self._name) if result else None
+        self._screenshot = result.get("screenshot_path") if result else None
 
     def expected(self, value):
         self._expected = value

tests/integration/test_browser_actions.py

@@ -1,5 +1,4 @@
-import base64
-import binascii
+from pathlib import Path
 
 import pytest
 
@@ -89,15 +88,13 @@ def test_screenshot(browser):
     browser.navigate(url="https://example.com")
 
     result = browser.screenshot()
-    screenshot = result.get("screenshot", "")
-    ui.log(f"screenshot → {len(screenshot)} bytes base64")
+    path = result.get("screenshot_path")
+    ui.log(f"screenshot → {path}")
 
-    if len(screenshot) <= 100:
-        Fail(result).expected("> 100 bytes").got(f"{len(screenshot)} bytes")
-    try:
-        base64.b64decode(screenshot)
-    except (ValueError, binascii.Error) as e:
-        Fail(result).error(f"invalid base64: {e}")
+    if not path:
+        Fail(result).error("no screenshot saved")
+    elif not Path(path).exists():
+        Fail(result).error(f"screenshot file missing: {path}")
 
 
 def test_input(browser):

tests/integration/test_browser_isolation.py

@@ -1,13 +1,13 @@
 import pytest
 
 from . import console as ui
-from .helpers import Fail, act_parallel
+from .helpers import Browser, Fail, act_parallel
 
 
 pytestmark = [pytest.mark.integration, pytest.mark.browsers(2)]
 
 
-def test_session_objects_are_distinct(browsers):
+def test_session_objects_are_distinct(browsers: list[Browser]) -> None:
     from strix.tools.browser.browser_manager import _manager
 
     a, b = browsers
@@ -25,7 +25,7 @@ def test_session_objects_are_distinct(browsers):
         )
 
 
-def test_parallel_navigate_isolated(browsers):
+def test_parallel_navigate_isolated(browsers: list[Browser]) -> None:
     a, b = browsers
     ui.status("isolation → navigating both agents concurrently")
     result_a, result_b = act_parallel(
@@ -55,7 +55,7 @@ def test_parallel_navigate_isolated(browsers):
         Fail(state_b).expected("agent B still on iana.org").got(state_b.get("url"))
 
 
-def test_cookie_isolation(browsers):
+def test_cookie_isolation(browsers: list[Browser]) -> None:
     a, b = browsers
     a.navigate(url="https://example.com")
     a.evaluate(code="document.cookie = 'agent=A; path=/'")
@@ -76,7 +76,7 @@ def test_cookie_isolation(browsers):
         Fail(result_a).expected("cookie 'agent=A' present").got(cookie_a)
 
 
-def test_concurrent_mixed_actions(browsers):
+def test_concurrent_mixed_actions(browsers: list[Browser]) -> None:
     a, b = browsers
     a.navigate(url="https://example.com")
     b.navigate(url="https://www.iana.org")
@@ -94,3 +94,56 @@ def test_concurrent_mixed_actions(browsers):
     for r in results:
         if "error" in r:
             Fail(r).error(r["error"])
+
+
+def test_session_close_does_not_affect_other(browsers: list[Browser]) -> None:
+    a, b = browsers
+    a.navigate(url="https://example.com")
+    b.navigate(url="https://www.iana.org")
+
+    a.close_browser()
+    ui.log("agent A closed")
+
+    result = b.navigate(url="https://example.com")
+    ui.log(f"agent B navigation result: {result}")
+    if "error" in result:
+        Fail(result).error(f"agent B broken after A closed: {result['error']}")
+    if "example.com" not in result.get("url", ""):
+        Fail(result).expected("url containing 'example.com'").got(result.get("url"))
+
+
+def test_relaunch_parallel_no_side_effects(browsers: list[Browser]) -> None:
+    a, b = browsers
+    a.navigate(url="https://example.com")
+    b.navigate(url="https://www.iana.org")
+
+    # close A, verify B is unaffected
+    a.close_browser()
+    b_check = b.screenshot()
+    if "iana.org" not in b_check.get("url", ""):
+        Fail(b_check).expected("agent B still on iana.org").got(b_check.get("url"))
+
+    # relaunch A
+    a.launch()
+    a.navigate(url="https://example.com")
+
+    # parallel actions on both after relaunch
+    results = act_parallel(
+        [
+            (a, {"action": "evaluate", "code": "document.title"}),
+            (b, {"action": "evaluate", "code": "document.title"}),
+            (a, {"action": "screenshot"}),
+            (b, {"action": "screenshot"}),
+        ]
+    )
+
+    for r in results:
+        if "error" in r:
+            Fail(r).error(r["error"])
+
+    # verify no cross-contamination after relaunch (use url, titles vary)
+    a_state, b_state = results[2], results[3]
+    if "example.com" not in a_state.get("url", ""):
+        Fail(a_state).expected("agent A on example.com").got(a_state.get("url"))
+    if "iana.org" not in b_state.get("url", ""):
+        Fail(b_state).expected("agent B on iana.org").got(b_state.get("url"))