mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-11-08 12:00:36 +00:00
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
This commit is contained in:
parent
62de7f3425
commit
15433dda2a
@ -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
|
||||
|
||||
@ -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`)
|
||||
|
||||
|
||||
@ -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.
|
||||
@ -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
|
||||
|
||||
@ -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",
|
||||
|
||||
@ -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="<title>Authenticated app</title><button>Continue</button>", 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)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user