diff --git a/src/browser_harness/daemon.py b/src/browser_harness/daemon.py index de910171e..f21b99a5b 100644 --- a/src/browser_harness/daemon.py +++ b/src/browser_harness/daemon.py @@ -360,12 +360,30 @@ def __init__(self): self.cdp = None self.session = None self.target_id = None + self.owns_target = False # True when this daemon created its own tab self.events = deque(maxlen=BUF) self.dialog = None self.stop = None # asyncio.Event, set inside start() async def attach_first_page(self): """Attach to a real page (or any page). Sets self.session. Returns attached target or None.""" + # Named daemons (BU_NAME != "default") share one browser with other + # daemons — attaching to the first page makes parallel daemons fight + # over a single tab (navigations clobber each other). Give each named + # daemon its own dedicated tab instead. REMOTE_ID (cloud) browsers are + # already exclusive to this daemon, so first-page attach stays. + if NAME != "default" and not REMOTE_ID: + tid = (await self.cdp.send_raw("Target.createTarget", {"url": "about:blank"}))["targetId"] + self.owns_target = True + log(f"named daemon {NAME}: created dedicated tab ({tid})") + page = {"targetId": tid, "url": "about:blank", "type": "page"} + self.session = (await self.cdp.send_raw( + "Target.attachToTarget", {"targetId": tid, "flatten": True} + ))["sessionId"] + self.target_id = tid + log(f"attached {tid} (about:blank) session={self.session}") + await self._enable_default_domains(self.session) + return page targets = (await self.cdp.send_raw("Target.getTargets"))["targetInfos"] pages = [t for t in targets if is_real_page(t)] if not pages: @@ -615,7 +633,21 @@ async def handler(reader, writer): async def main(): d = Daemon() await d.start() - await serve(d) + try: + await serve(d) + finally: + # A named daemon owns the tab it created — close it on shutdown so + # parallel workers don't leak about:blank/leftover tabs into the + # shared browser. Best-effort: the WS may already be gone. + if d.owns_target and d.target_id: + try: + await asyncio.wait_for( + d.cdp.send_raw("Target.closeTarget", {"targetId": d.target_id}), + timeout=2, + ) + log(f"closed owned tab {d.target_id}") + except Exception as e: + log(f"close owned tab {d.target_id}: {e}") def already_running(): diff --git a/tests/unit/test_daemon.py b/tests/unit/test_daemon.py index 90c5bc855..5f49a77fa 100644 --- a/tests/unit/test_daemon.py +++ b/tests/unit/test_daemon.py @@ -293,3 +293,81 @@ def test_current_tab_meta_returns_not_attached_when_no_target_id(): assert result == {"error": "not_attached"} # No CDP call should have been issued. assert d.cdp.calls == [] + + +class _AttachCDP(_FakeCDP): + """FakeCDP with realistic responses for the attach flow.""" + + def __init__(self, targets=None): + super().__init__() + self.targets = targets or [] + self.created = 0 + + async def send_raw(self, method, params=None, session_id=None): + self.calls.append((method, params, session_id)) + if method == "Target.getTargets": + return {"targetInfos": self.targets} + if method == "Target.createTarget": + self.created += 1 + return {"targetId": f"created-{self.created}"} + if method == "Target.attachToTarget": + return {"sessionId": f"session-for-{params['targetId']}"} + return {} + + +def test_named_daemon_creates_dedicated_tab(monkeypatch): + """A named daemon (BU_NAME != default) on a shared local/CDP browser must + create its own tab rather than attaching to the first existing page — + otherwise parallel named daemons all grab the same tab and clobber each + other's navigations (#375 / #582).""" + monkeypatch.setattr(daemon, "NAME", "worker-a") + monkeypatch.setattr(daemon, "REMOTE_ID", None) + existing = [{"targetId": "someone-elses-tab", "url": "https://example.com/", "type": "page"}] + d = daemon.Daemon() + d.cdp = _AttachCDP(existing) + + page = asyncio.run(d.attach_first_page()) + + assert page["targetId"] == "created-1" + assert d.target_id == "created-1" + assert d.owns_target is True + assert d.session == "session-for-created-1" + # It must NOT have attached to the pre-existing tab. + attach_calls = [p for (m, p, _s) in d.cdp.calls if m == "Target.attachToTarget"] + assert attach_calls == [{"targetId": "created-1", "flatten": True}] + # Domains enabled on the new session (parity with default attach). + enabled = {m for (m, _p, s) in d.cdp.calls if s == d.session and m.endswith(".enable")} + assert enabled == {"Page.enable", "DOM.enable", "Runtime.enable", "Network.enable"} + + +def test_default_daemon_still_attaches_first_page(monkeypatch): + """The default daemon keeps the existing attach-to-first-real-page + behavior (single-user flow: reuse the tab the user is looking at).""" + monkeypatch.setattr(daemon, "NAME", "default") + monkeypatch.setattr(daemon, "REMOTE_ID", None) + existing = [{"targetId": "user-tab", "url": "https://example.com/", "type": "page"}] + d = daemon.Daemon() + d.cdp = _AttachCDP(existing) + + page = asyncio.run(d.attach_first_page()) + + assert page["targetId"] == "user-tab" + assert d.owns_target is False + assert d.cdp.created == 0 + + +def test_named_remote_daemon_keeps_first_page_attach(monkeypatch): + """A named CLOUD daemon (REMOTE_ID set) has the whole browser to itself — + creating an extra tab would just leak one. First-page attach stays.""" + monkeypatch.setattr(daemon, "NAME", "r7k2") + monkeypatch.setattr(daemon, "REMOTE_ID", "remote-browser-id") + monkeypatch.setattr(daemon, "BROWSER_KIND", "cloud") + existing = [{"targetId": "cloud-blank", "url": "about:blank", "type": "page"}] + d = daemon.Daemon() + d.cdp = _AttachCDP(existing) + + page = asyncio.run(d.attach_first_page()) + + assert page["targetId"] == "cloud-blank" + assert d.owns_target is False + assert d.cdp.created == 0