fix(browser): a file link that opens a new tab keeps the clicked page

changes/307.fixed.md

@@ -0,0 +1 @@
+**A file link that opens a new tab no longer looks like a blank page.** When Sentient clicked a download link that opens in a new tab, it reported a blank page instead of the page it was on. Now it stays on that page and reports the saved file.

docs/API.md

@@ -2339,6 +2339,9 @@ Every new tool declares a `Risk`; approvals behave as in section 1.
   - `browser_click(ref)` write; effective risk `send` (via `risk_fn`) when the element's text/aria/value looks like buy,
     pay, place order, checkout, send, post, publish, delete, remove, confirm, subscribe, transfer, or it submits a form
     with payment fields → `{ok, clicked, url, message, new_tab?, dialogs?}`. A link opening a new tab makes it active.
+    A new tab that only carried a download (a file link with `target=_blank`: a blank tab the download starts from,
+    which may close again) or is Edge's downloads hub is not made active, and `url` stays the clicked page (#307); a
+    blank new tab gets 3 s to show a page before it counts as one.
     `confirm()` dialogs are accepted only for clicks approved as `send`; alerts are accepted; `dialogs` lists them.
     Its approval `risk_label` is "Purchase" for buy, pay, order, checkout and the like. A subscribe, follow, sign up or
     "notify me" reads "Signs up" (normal approval rules) only when the element or the text around it (its card or

sentient/browser/service.py

@@ -65,6 +65,8 @@
 STORAGE_FLUSH_S = 1.5
 PAGE_CLOSE_TIMEOUT_S = 5.0
 DOWNLOAD_APPEAR_GRACE_S = 0.3
+# #307: how long a blank tab a click opened gets to turn into a page, a download or nothing
+DOWNLOAD_TAB_WAIT_S = 3.0
 DOWNLOAD_MAX_DURATION_S = 120.0
 DOWNLOAD_CANCEL_TIMEOUT_S = 5.0
 DOWNLOAD_TASK_RETENTION_S = 300.0
@@ -234,6 +236,9 @@ def __init__(self, app):
         self._attached = False
         self._download_pages: set[Any] = set()
         self._internal_pages: set[Any] = set()
+        # a new tab -> the tab that was active when it opened (its opener when known), to go back to when the new
+        # tab closes or turns out to be Edge's downloads hub (#307)
+        self._came_from: dict[Any, Any] = {}
         self._profile = DEFAULT_PROFILE  # the running profile, or the next one to start
         self._for_user = False  # the open window was shown for the user (Open to sign in): don't switch under them
         self._engine: str | None = None
@@ -724,6 +729,7 @@ async def _close_context(self) -> None:
             self._focused = None
             self._download_pages.clear()
             self._internal_pages.clear()
+            self._came_from.clear()
             if self._browser_cdp is not None:
                 with contextlib.suppress(Exception):
                     await self._browser_cdp.detach()
@@ -822,6 +828,7 @@ def _on_context_closed(self, context: Any) -> None:
         self._snap = None
         self._focused = None
         self._download_pages.clear()
+        self._came_from.clear()
         self._browser_cdp = None
         self._clear_attached_downloads()
         if not self._closing:  # the user closed the visible window (or their attached browser)
@@ -854,23 +861,27 @@ def _is_downloads_hub(url: Any) -> bool:
 
     def _mark_internal_page(self, page: Any) -> None:
         self._internal_pages.add(page)
+        back = self._came_from.pop(page, None)
         if self._active is page:
             context = self._context
             pages = [
                 candidate for candidate in (self._usable_pages(context) if context is not None else [])
                 if not self._attached or candidate in self._download_pages
             ]
-            self._active = pages[-1] if pages else None
+            self._active = back if back in pages else (pages[-1] if pages else None)
 
     def _on_page(self, page: Any) -> None:
         self._wire_page(page)
         if self._is_downloads_hub(page.url):
             self._mark_internal_page(page)
         elif not self._attached:  # a link opened a new tab: keep working in it
+            if self._active is not None and self._active is not page:
+                self._came_from.setdefault(page, self._active)
             self._active = page
         self._spawn(self._publish_status())
 
     def _on_popup(self, opener: Any, popup: Any) -> None:
+        self._came_from[popup] = opener
         if self._is_downloads_hub(popup.url):
             self._mark_internal_page(popup)
             return
@@ -882,6 +893,7 @@ def _on_popup(self, opener: Any, popup: Any) -> None:
     def _on_page_closed(self, page: Any) -> None:
         self._download_pages.discard(page)
         self._internal_pages.discard(page)
+        back = self._came_from.pop(page, None)
         if self._snap and self._snap.get("page") is page:
             self._snap = None
         ctx = self._context
@@ -890,7 +902,8 @@ def _on_page_closed(self, page: Any) -> None:
                 p for p in (self._usable_pages(ctx) if ctx else [])
                 if p is not page and (not self._attached or p in self._download_pages)
             ]
-            self._active = remaining[-1] if remaining else None
+            # back to the tab it came from (a file link's blank tab closes once the download starts, #307)
+            self._active = back if back in remaining else (remaining[-1] if remaining else None)
         if ctx is not None and not self._closing:
             self._spawn(self._publish_status())
 
@@ -1621,7 +1634,8 @@ async def click(self, ctx: Any, ref: str) -> dict:
             refusal = self._guard("click", info["ref"], live_risk)
             if refusal:
                 return {"error": refusal}
-            pages_before = len(self._usable_pages(self._context))
+            tabs_before = list(self._usable_pages(self._context))
+            pages_before = len(tabs_before)
             self._accept_confirm = live_risk >= Risk.send or self.app.config.tools.approvals.mode == "off"
             try:
                 await loc.click(timeout=10_000)
@@ -1641,6 +1655,23 @@ async def click(self, ctx: Any, ref: str) -> dict:
             if self._context is None:
                 raise BrowserError("The browser window was closed. Call the tool again and it will reopen.")
             active = self._active or page
+            # an older tab took over because the tab the click opened is gone again
+            fell_back = active in tabs_before and not any(
+                p not in tabs_before for p in self._usable_pages(self._context)
+            )
+            if (
+                active is not page
+                and not page.is_closed()
+                and (fell_back or await self._download_only_tab(active, downloads_before))
+            ):
+                # #307: the tab the click opened only carried a download (or is Edge's downloads hub): the page that
+                # was clicked is still the one to work in, and the one to report. A real page that opened meanwhile
+                # in yet another new tab is kept.
+                now = self._active
+                if now is None or now is active or now.is_closed() or now in tabs_before or now in self._internal_pages:
+                    active = self._active = page
+                else:
+                    active = now
             await self._settle(active)
             await self._enforce_domains(active)
             out = {
@@ -1649,7 +1680,7 @@ async def click(self, ctx: Any, ref: str) -> dict:
                 "url": active.url,
                 "message": "Clicked. Call browser_snapshot to see the page now.",
             }
-            if len(self._usable_pages(self._context)) > pages_before:
+            if active is not page and len(self._usable_pages(self._context)) > pages_before:
                 out["new_tab"] = True
                 out["message"] = "Clicked; it opened a new tab, which is now active. Call browser_snapshot to see it."
             await self._after_action(ctx, active)
@@ -1658,6 +1689,22 @@ async def click(self, ctx: Any, ref: str) -> dict:
             )
             return self._take_dialogs(out)
 
+    async def _download_only_tab(self, tab: Any, downloads_before: set[asyncio.Task[str]]) -> bool:
+        """True when ``tab``, made active by a click, holds no page: Edge's downloads hub, or a blank tab that a
+        download started from (a link to a file with ``target=_blank``) or that closed again. A blank tab gets
+        ``DOWNLOAD_TAB_WAIT_S`` to show a page instead."""
+        deadline = time.monotonic() + DOWNLOAD_TAB_WAIT_S
+        while True:
+            if tab.is_closed() or tab in self._internal_pages or self._is_downloads_hub(tab.url):
+                return True
+            if str(tab.url or "") not in ("", "about:blank"):
+                return False
+            if self._download_tasks - downloads_before:
+                return True
+            if time.monotonic() >= deadline:
+                return False
+            await asyncio.sleep(0.05)
+
     async def type(self, ctx: Any, ref: str, text: str, submit: bool = False) -> dict:
         async with self._lock:
             downloads_before = self._download_tasks.copy()

tests/browser/conftest.py

@@ -48,6 +48,9 @@
     "search.html": "<!doctype html><title>Results</title><main><h1>Results page</h1><p>Search results here.</p></main>",
     "help.html": "<!doctype html><title>Help</title><main><h1>Help center</h1></main>",
     "download.html": '<!doctype html><title>Downloads</title><a href="/report.txt" download>Download report</a>',
+    # #307: a link that opens its file in a new tab
+    "download-tab.html": ('<!doctype html><title>Downloads</title><a href="/direct-download" target="_blank">Get it in a new tab</a> '
+                          '<a href="/help.html" target="_blank">Open help in new tab</a>'),
     "report.txt": "Sentient browser download test.\n",
     "signup.html": """<!doctype html><meta charset="utf-8"><title>Newsletter</title><main>
   <section><h2>Weekly notes</h2><p>Free, no spam.</p><button>Subscribe</button></section>

tests/browser/test_live_browser.py

@@ -187,3 +187,38 @@ async def closed_event() -> dict:
             break
         await asyncio.sleep(0.05)
     assert not browser.running
+
+
+async def test_download_from_a_new_tab_link_keeps_the_page(browser, site):
+    """#307: a file link with target=_blank opens a blank tab the download starts from. The click reports the page
+    that was clicked, not the blank tab, and the file is saved and reported."""
+    ctx = make_ctx(browser.app)
+    snap = await bt.browser_open.call(ctx, {"url": f"{site}/download-tab.html"})
+    result = await bt.browser_click.call(ctx, {"ref": ref_of(snap["text"], r'link "Get it in a new tab"')})
+    assert result["url"] == f"{site}/download-tab.html"
+    assert "new_tab" not in result
+    download_paths = result.get("downloads", [])
+    if not download_paths:
+        pending = [task for task in browser._download_tasks if not task.done()]
+        await asyncio.wait_for(asyncio.gather(*pending), timeout=10)
+        download_paths = (await bt.browser_snapshot.call(ctx, {}))["downloads"]
+    assert download_paths == ["downloads/direct-report.txt"]
+    assert (paths.files_dir() / download_paths[0]).read_text(encoding="utf-8") == "Direct browser-open download test.\n"
+    later = await bt.browser_snapshot.call(ctx, {})
+    assert later["url"] == f"{site}/download-tab.html" and later["title"] == "Downloads"
+
+
+async def test_download_tab_link_with_another_tab_open_keeps_the_clicked_page(browser, site):
+    """#307: when the blank download tab closes, the clicked tab stays active, not the last other tab."""
+    ctx = make_ctx(browser.app)
+    text = (await bt.browser_open.call(ctx, {"url": f"{site}/download-tab.html"}))["text"]
+    assert (await bt.browser_click.call(ctx, {"ref": ref_of(text, r'link "Open help in new tab"')})).get("new_tab")
+    await bt.browser_switch_tab.call(ctx, {"index": 0})
+    text = (await bt.browser_snapshot.call(ctx, {}))["text"]
+    result = await bt.browser_click.call(ctx, {"ref": ref_of(text, r'link "Get it in a new tab"')})
+    assert result["url"] == f"{site}/download-tab.html" and "new_tab" not in result
+    later = await bt.browser_snapshot.call(ctx, {})
+    assert later["url"] == f"{site}/download-tab.html"
+    pending = [task for task in browser._download_tasks if not task.done()]
+    if pending:
+        await asyncio.wait_for(asyncio.gather(*pending), timeout=10)

tests/browser/test_profiles.py

@@ -2002,3 +2002,73 @@ async def new_cdp_session(self, page):
 
     assert rel == "downloads/scoped.txt"
     assert queried_pages == [dl_page]
+
+
+async def test_a_click_that_only_opened_a_download_tab_stays_on_the_page(monkeypatch):
+    """#307: the blank tab a file link opens, or Edge's downloads hub, is not a page to keep working in."""
+    import sentient.browser.service as service
+
+    monkeypatch.setattr(service, "DOWNLOAD_TAB_WAIT_S", 0.3)
+    svc = BrowserService(make_app())
+
+    class Page:
+        def __init__(self, url, closed=False):
+            self.url, self.closed = url, closed
+
+        def is_closed(self):
+            return self.closed
+
+    before: set = set()
+    assert await svc._download_only_tab(Page("", closed=True), before)  # the blank tab closed once the file came
+    assert await svc._download_only_tab(Page("edge://downloads-hub/"), before)
+    assert not await svc._download_only_tab(Page("https://example.com/help"), before)  # a real new page
+    assert not await svc._download_only_tab(Page("about:blank"), before)  # stays blank, no download: kept
+
+    async def save():
+        return "downloads/x.txt"
+
+    task = asyncio.create_task(save())
+    svc._download_tasks.add(task)
+    assert await svc._download_only_tab(Page("about:blank"), before)  # a download started from it
+    await task
+
+
+def test_a_closed_or_hub_tab_goes_back_to_the_tab_it_came_from():
+    """#307: with several tabs open, a blank download tab that closes, or a new tab that turns out to be Edge's
+    downloads hub, hands back to the tab that was active when it opened, not to the last tab."""
+    svc = BrowserService(make_app())
+
+    class Page:
+        def __init__(self, url):
+            self.url = url
+
+        @property
+        def main_frame(self):
+            return self
+
+        def is_closed(self):
+            return False
+
+        def on(self, *_):
+            return None
+
+    clicked, other = Page("https://example.com/reports"), Page("https://example.com/help")
+    context = type("Context", (), {"pages": [clicked, other]})()
+    svc._context = context
+    svc._closing = True  # no status publishing in this unit test
+    svc._active = clicked
+
+    blank = Page("")
+    context.pages.append(blank)
+    svc._on_page(blank)
+    assert svc._active is blank
+    context.pages.remove(blank)
+    svc._on_page_closed(blank)
+    assert svc._active is clicked
+
+    hub = Page("")
+    context.pages.append(hub)
+    svc._on_page(hub)
+    hub.url = "edge://downloads-hub/"
+    svc._on_navigated(hub, hub)
+    assert svc._active is clicked