From 15433dda2ac0c06a57a966f6fa6520c2e31619b8 Mon Sep 17 00:00:00 2001 From: Lucien <164857476+dakjdakd@users.noreply.github.com> Date: Fri, 30 Oct 2026 07:20:02 +0800 Subject: [PATCH] fix(browser): preserve the browser context after login popups close (#7674) * fix(browser): preserve context when active popup closes * fix(browser): focus recovered managed tabs and clarify session guidance --- README.md | 2 + backend/packages/harness/deerflow/AGENTS.md | 10 +- .../community/browser_automation/AGENTS.md | 17 ++ .../community/browser_automation/session.py | 14 ++ backend/tests/test_agent_guidance_check.py | 1 + backend/tests/test_browser_automation.py | 181 ++++++++++++++++++ 6 files changed, 217 insertions(+), 8 deletions(-) create mode 100644 backend/packages/harness/deerflow/community/browser_automation/AGENTS.md diff --git a/README.md b/README.md index b7a30db8b5..06bdfc5c12 100644 --- a/README.md +++ b/README.md @@ -3226,6 +3226,8 @@ Then uncomment the `group: browser` tool entries in `config.yaml` (`browser_navi Existing, non-mock Custom Agent chats expose the same Browser Live controls when browser control is available and the agent either leaves `tool_groups` unrestricted or includes the `browser` group. An explicit tool-group allowlist without `browser` keeps those controls hidden. +Closing a login popup returns browser control to a surviving tab in the same session, preserving cookies and site storage. If every tab closes, the next browser action opens a tab in that same session. Recovered tabs are brought to the front in DeerFlow-managed browsers; CDP-attached Chrome keeps the user's current tab focus. + The workspace Browser Live client negotiates binary JPEG WebSocket frames, keeps only the newest pending frame per display refresh, and revokes replaced object URLs. Gateway control messages remain JSON, and clients that do not diff --git a/backend/packages/harness/deerflow/AGENTS.md b/backend/packages/harness/deerflow/AGENTS.md index d6999c6384..cdb90112bb 100644 --- a/backend/packages/harness/deerflow/AGENTS.md +++ b/backend/packages/harness/deerflow/AGENTS.md @@ -58,15 +58,9 @@ logout and restores the complete old tree on any switch failure. Offload config/DDGS together. Only returned empties mean `No images found`; the SDK sentinel can hide timeouts. See `tests/test_image_search_errors.py`. -### Browser Progress Screenshots (`community/browser_automation/`) +### Browser Automation -Hidden per-action browser progress frames use JPEG at quality 80 to keep their -storage and transfer cost bounded relative to lossless PNG. The explicit -`browser_screenshot` tool remains PNG because it creates a user-requested -artifact. New automatic capture entry points must reuse the shared progress -encoding definition in `tools.py` so the byte encoding and `.jpg` suffix cannot -drift. `browser_screenshot` never replaces an `outputs/` file: it hard-links -onto the first free `-N` name. +Screenshot and session rules: [browser guidance](community/browser_automation/AGENTS.md). ### Embedded Client (`packages/harness/deerflow/client.py`) diff --git a/backend/packages/harness/deerflow/community/browser_automation/AGENTS.md b/backend/packages/harness/deerflow/community/browser_automation/AGENTS.md new file mode 100644 index 0000000000..424eafc499 --- /dev/null +++ b/backend/packages/harness/deerflow/community/browser_automation/AGENTS.md @@ -0,0 +1,17 @@ +### Browser Progress Screenshots + +Hidden per-action browser progress frames use JPEG at quality 80 to keep their +storage and transfer cost bounded relative to lossless PNG. The explicit +`browser_screenshot` tool remains PNG because it creates a user-requested +artifact. New automatic capture entry points must reuse the shared progress +encoding definition in `tools.py` so the byte encoding and `.jpg` suffix cannot +drift. `browser_screenshot` never replaces an `outputs/` file: it hard-links +onto the first free `-N` name. + +### Session Lifecycle + +Contexts own cookies and origin storage: retain a live context after page +closure; rebuild only when the browser disconnects or the context is gone. +Reset `_request_guard_bound` / `_page_listener_bound` only on context replacement. +Await `bring_to_front()` before `_set_active_page()` during managed-page +recovery; CDP sessions preserve user focus and intentionally skip the request guard. diff --git a/backend/packages/harness/deerflow/community/browser_automation/session.py b/backend/packages/harness/deerflow/community/browser_automation/session.py index d1a6d061ad..208ab8fe73 100644 --- a/backend/packages/harness/deerflow/community/browser_automation/session.py +++ b/backend/packages/harness/deerflow/community/browser_automation/session.py @@ -411,11 +411,25 @@ class BrowserSession: async with self._ensure_page_lock: if self._page is not None and not self._page.is_closed(): return self._page + # A closed popup/tab does not end the browsing session. Reuse a + # surviving tab, or open one in the same context to retain cookies + # and storage. Closed contexts disappear from browser.contexts. + if self._browser is not None and self._browser.is_connected() and self._context is not None and self._context in self._browser.contexts: + pages = [page for page in self._context.pages if not page.is_closed()] + page = pages[-1] if pages else await self._context.new_page() + if not self._connected_over_cdp: + await page.bring_to_front() + self._set_active_page(page) + return self._page + from playwright.async_api import async_playwright if self._playwright is None: self._playwright = await async_playwright().start() + # These registrations belong to the context being replaced. + self._request_guard_bound = False + self._page_listener_bound = False if self._cdp_url: # Attach to the user's running Chrome (started with # --remote-debugging-port). Reuse its default context + an existing diff --git a/backend/tests/test_agent_guidance_check.py b/backend/tests/test_agent_guidance_check.py index 60f9649a93..500498c196 100644 --- a/backend/tests/test_agent_guidance_check.py +++ b/backend/tests/test_agent_guidance_check.py @@ -23,6 +23,7 @@ EXPECTED_GUIDANCE_PATHS = { "backend/packages/harness/deerflow/agents/memory/AGENTS.md", "backend/packages/harness/deerflow/agents/task_continuity/AGENTS.md", "backend/packages/harness/deerflow/community/brave/AGENTS.md", + "backend/packages/harness/deerflow/community/browser_automation/AGENTS.md", "backend/packages/harness/deerflow/community/groundroute/AGENTS.md", "backend/packages/harness/deerflow/community/image_search/AGENTS.md", "backend/packages/harness/deerflow/community/infoquest/AGENTS.md", diff --git a/backend/tests/test_browser_automation.py b/backend/tests/test_browser_automation.py index e9f3ddc617..6213e71a66 100644 --- a/backend/tests/test_browser_automation.py +++ b/backend/tests/test_browser_automation.py @@ -608,6 +608,146 @@ async def test_ensure_page_serializes_concurrent_rebuilds(): context.new_page.assert_awaited_once() +@pytest.fixture +def fake_playwright_api(monkeypatch): + """Lifecycle unit tests do not require the optional browser extra.""" + async_api = ModuleType("playwright.async_api") + async_api.async_playwright = MagicMock() + playwright = ModuleType("playwright") + playwright.async_api = async_api + monkeypatch.setitem(sys.modules, "playwright", playwright) + monkeypatch.setitem(sys.modules, "playwright.async_api", async_api) + + +@pytest.mark.parametrize("surviving_tab", [True, False]) +@pytest.mark.parametrize("cdp", [False, True]) +@pytest.mark.asyncio +async def test_ensure_page_reuses_context_after_active_tab_closes(surviving_tab, cdp, fake_playwright_api): + """Closing an OAuth popup or the final tab must retain session storage.""" + session = BrowserSession(MagicMock(), headless=True, timeout_ms=1000, viewport={"width": 1000, "height": 500}) + closed = MagicMock() + closed.is_closed.return_value = True + page = MagicMock() + page.is_closed.return_value = False + focused = False + + async def focus_page(): + nonlocal focused + assert session._page is closed + await asyncio.sleep(0) + focused = True + + page.bring_to_front = AsyncMock(side_effect=focus_page) + original_set_active_page = session._set_active_page + + def adopt_page(target): + assert focused is not cdp + original_set_active_page(target) + + session._set_active_page = MagicMock(side_effect=adopt_page) + context = MagicMock() + context.pages = [page] if surviving_tab else [] + creation_started = asyncio.Event() + release_creation = asyncio.Event() + + async def new_page(): + creation_started.set() + await release_creation.wait() + return page + + context.new_page = AsyncMock(side_effect=new_page) + browser = MagicMock() + browser.is_connected.return_value = True + browser.contexts = [context] + browser.new_context = AsyncMock(side_effect=AssertionError("discarded the existing context")) + session._browser = browser + session._connected_over_cdp = cdp + session._context = context + session._page = closed + session._request_guard_bound = True + session._page_listener_bound = True + session._playwright = MagicMock() + + if surviving_tab: + assert await session._ensure_page() is page + context.new_page.assert_not_awaited() + else: + first = asyncio.create_task(session._ensure_page()) + try: + # Await the operation too, so a broken implementation fails without + # stranding a caller while waiting for the creation signal. + signal = asyncio.create_task(creation_started.wait()) + await asyncio.wait({first, signal}, return_when=asyncio.FIRST_COMPLETED) + if first.done(): + await first + second = asyncio.create_task(session._ensure_page()) + release_creation.set() + assert await asyncio.gather(first, second) == [page, page] + finally: + release_creation.set() + signal.cancel() + await asyncio.gather(first, signal, return_exceptions=True) + context.new_page.assert_awaited_once() + + assert session._context is context + assert session._page is page + assert session._request_guard_bound is True + assert session._page_listener_bound is True + browser.new_context.assert_not_awaited() + context.on.assert_not_called() + session._set_active_page.assert_called_once_with(page) + if cdp: + page.bring_to_front.assert_not_awaited() + else: + page.bring_to_front.assert_awaited_once_with() + + +@pytest.mark.parametrize("cdp", [False, True]) +@pytest.mark.asyncio +async def test_ensure_page_rebinds_hooks_when_context_is_replaced(cdp, fake_playwright_api): + session = BrowserSession( + MagicMock(), + headless=True, + timeout_ms=1000, + viewport={"width": 1000, "height": 500}, + cdp_url="http://127.0.0.1:9222" if cdp else None, + url_guard=lambda _url: None, + ) + page = MagicMock() + page.is_closed.return_value = False + closed = MagicMock() + closed.is_closed.return_value = True + old_context = MagicMock() + context = MagicMock() + context.pages = [page] if cdp else [] + context.new_page = AsyncMock(return_value=page) + context.route = AsyncMock() + browser = MagicMock() + browser.is_connected.return_value = True + browser.contexts = [context] if cdp else [] + browser.new_context = AsyncMock(return_value=context) + session._browser = browser + session._context = old_context + session._page = closed + session._request_guard_bound = True + session._page_listener_bound = True + session._playwright = MagicMock() + + assert await session._ensure_page() is page + context.on.assert_called_once() + event, on_page = context.on.call_args.args + assert event == "page" + popup = MagicMock() + on_page(popup) + assert session._page is popup + if cdp: + context.route.assert_not_awaited() + assert session._request_guard_bound is False + else: + context.route.assert_awaited_once() + assert session._request_guard_bound is True + + @pytest.mark.asyncio async def test_tabs_report_active_page(): session = BrowserSession( @@ -1111,6 +1251,47 @@ async def test_real_playwright_navigate_click_type(): session_mod.reset_browser_session_manager() +@pytest.mark.asyncio +async def test_real_playwright_preserves_context_after_popup_closes(): + """Real popup/tab closure keeps the opener, login cookie and local storage.""" + pytest.importorskip("playwright.async_api") + session = BrowserSession(MagicMock(), headless=True, timeout_ms=5000, viewport={"width": 800, "height": 600}) + try: + opener = await session._ensure_page() + context = session._context + + async def app_page(route): + await route.fulfill(body="Authenticated app", content_type="text/html") + + await context.route("https://app.example/**", app_page) + await session._navigate("https://app.example/home") + await opener.evaluate("localStorage.setItem('account', 'alice')") + async with context.expect_page() as popup_event: + await opener.evaluate("window.open('/login')") + popup = await popup_event.value + await popup.wait_for_load_state("domcontentloaded") + assert session._page is popup + await popup.evaluate("document.cookie = 'logged_in=yes; path=/'") + await popup.close() + + tabs = await session._tabs() + assert [(tab.url, tab.active) for tab in tabs] == [("https://app.example/home", True)] + assert session._page is opener + assert session._context is context + assert len(session._browser.contexts) == 1 + assert await opener.evaluate("localStorage.getItem('account')") == "alice" + assert any(cookie["name"] == "logged_in" for cookie in await context.cookies()) + + await opener.close() + replacement = await session._ensure_page() + assert session._context is context + await replacement.goto("https://app.example/home") + assert await replacement.evaluate("localStorage.getItem('account')") == "alice" + assert any(cookie["name"] == "logged_in" for cookie in await context.cookies()) + finally: + await session._close() + + class _FakeRoute: def __init__(self, url: str): self.request = SimpleNamespace(url=url)