From 892ed518807f517f26fa8c5d57afd924cd4b7d87 Mon Sep 17 00:00:00 2001 From: Michal Ondrej Date: Fri, 9 Oct 2026 12:06:42 +0200 Subject: [PATCH 1/8] Adding browser download handling --- ISSUE-87-FIX.md | 27 +++++++++++++ docs/API.md | 5 +-- sentient/browser/service.py | 65 ++++++++++++++++++++++++++++++ tests/browser/conftest.py | 2 + tests/browser/test_live_browser.py | 12 ++++++ 5 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 ISSUE-87-FIX.md diff --git a/ISSUE-87-FIX.md b/ISSUE-87-FIX.md new file mode 100644 index 00000000..618f2f46 --- /dev/null +++ b/ISSUE-87-FIX.md @@ -0,0 +1,27 @@ +## What the fix does: + +* **_on_download** - creates an asynchrone task that uses the function _save_download and appends the asynchrone event to a set in self._download_tasks + +--- + +* **_save_download** - handels the download name so there is no path traversal and changes inapropriate characters to _ + +* **_save_download** - function saves files to paths.files_dir() / "downloads", so the apps config folder is used, where at the end an relative path is returned for the saved file + +* **_save_download** - Prevents overwriting files with same name by checking the file names and trying to append a number to the file name until file name is unique + +* **_save_download** - Uses a lock (self._download_lock) for saving files, so there will be only one access to the downloads directory at a time, preventing corrupted files or direcotry + +--- + +* **_include_downloads** - is waiting for the downloads to finish. It is called before returning results from functions - open, click, type, select, press, back + +* **_include_downloads** - Because the browser activity is asynchronous, this function gets previous not finished download list and substracts the previouse/current asynchrone tasks to see, if new downloads appeared within the time, that is set in DOWNLOAD_APPEAR_GRACE_S + +* **_include_downloads** - If a new download appeared, it waits until it finishes with **await asyncio.gather(*tasks)** and then removes the tasks from the set + +--- + +* **test_download_is_saved_and_reported** - test uses offline site and lets the browser download data through finding link on the attribute on the site. +1. assert - It expects from the browser one download at the expected path +2. assert - checks the saved content is equal to the content on the site diff --git a/docs/API.md b/docs/API.md index 9a9b68d7..3988a627 100644 --- a/docs/API.md +++ b/docs/API.md @@ -1439,9 +1439,8 @@ Every new tool declares a `Risk`; approvals behave as in section 1. idle_minutes, allow_domains, block_domains, max_snapshot_chars, max_extract_chars, confirm_purchases, live_view, profiles`. Every safety rule below applies the same in every profile, attached ones included. - Tools (plugin `browser`). Failures return `{error}` with a message the model can act on. - - `browser_open(url, profile="")` read → same as `browser_snapshot` plus `profile` (http/https only; allow/block lists - apply, also after redirects). `profile` switches to that profile for this and the following calls of the run; - empty uses the run's profile, else `default`. + - `browser_open(url, profile="")` read → same as `browser_snapshot` plus `profile` (http/https only; allow/block lists apply, also after redirects). `profile` switches to that profile for this and the following calls of the run; empty uses the run's profile, else `default`. + Any download started by a browser action is saved under `downloads/` in the Files folder and included as a relative path in that action's optional `downloads` array. - `browser_snapshot()` read → `{url, title, text, truncated?}`. `text` is `Page:`/`URL:`/`Scroll:` header, interactive elements one per line (`[e12] button "Sign in"`, `[e4] textbox "Search" value=""`, `[e7] combobox "Country" value="India" options: India | Japan`, `[e3] link "Docs" -> /docs`, flags `checked`, `disabled`, `focused`) and the diff --git a/sentient/browser/service.py b/sentient/browser/service.py index bf885c6d..d5aa507c 100644 --- a/sentient/browser/service.py +++ b/sentient/browser/service.py @@ -64,6 +64,7 @@ STORAGE_FLAG = "--enable-aggressive-domstorage-flushing" STORAGE_FLUSH_S = 1.5 PAGE_CLOSE_TIMEOUT_S = 5.0 +DOWNLOAD_APPEAR_GRACE_S = 0.3 # models often pass the whole snapshot line ("[e4] button \"Place order\"") instead of just "e4" _REF_RE = re.compile(r"\b(e\d+)\b", re.IGNORECASE) @@ -226,6 +227,8 @@ def __init__(self, app): self._site_seen = False # a tab of the open browser showed a website (its storage may need writing out) self._last_tabs_sig: Any = None self.idle_check_s = 30.0 + self._download_tasks: set[asyncio.Task[str]] = set() + self._download_lock = asyncio.Lock() # ------------------------------------------------------------------ lifecycle async def start(self) -> None: @@ -538,6 +541,7 @@ async def _launch_persistent(self, prof: Any, headless: bool) -> Any: kwargs: dict[str, Any] = { "user_data_dir": str(folder), "headless": headless, + "accept_downloads": True, "timeout": 45_000, "args": ["--no-first-run", "--no-default-browser-check", "--hide-crash-restore-bubble", STORAGE_FLAG], } @@ -727,6 +731,7 @@ def _wire_page(self, page: Any) -> None: page.on("dialog", self._on_dialog) page.on("popup", lambda popup: self._on_popup(page, popup)) page.on("framenavigated", lambda frame: self._on_navigated(page, frame)) + page.on("download", self._on_download) def _on_navigated(self, page: Any, frame: Any) -> None: # any website shown, by the assistant or by the user in a visible window @@ -762,6 +767,54 @@ async def _on_dialog(self, dialog: Any) -> None: else: await dialog.dismiss() + def _on_download(self, download: Any) -> None: + task = asyncio.create_task(self._save_download(download), name="browser:download") + task.add_done_callback(lambda done: done.exception() if not done.cancelled() else None) + self._download_tasks.add(task) + + async def _save_download(self, download: Any) -> str: + raw_name = str(download.suggested_filename or "download") + name = raw_name.replace("\\", "/").rsplit("/", 1)[-1] + name = re.sub(r"[^\w.\- ()]+", "_", name).strip(" .")[:160] or "download" + + async with self._download_lock: + folder = paths.files_dir() / "downloads" + folder.mkdir(parents=True, exist_ok=True) + target = folder / name + stem = target.stem + suffix = target.suffix + index = 1 + while target.exists(): + target = folder / f"{stem}({index}){suffix}" + index += 1 + try: + await download.save_as(str(target)) + except Exception as exc: + raise BrowserError(f"Couldn't save downloaded file '{name}'") from exc + return target.relative_to(paths.files_dir()).as_posix() + + async def _include_downloads(self, result: dict, before: set[asyncio.Task[str]]) -> dict: + # Finished tasks from earlier actions (already in `before`) were never reported; drop them. + for task in before: + if task.done(): + self._download_tasks.discard(task) + + deadline = time.monotonic() + DOWNLOAD_APPEAR_GRACE_S + while time.monotonic() < deadline: + if self._download_tasks - before: + break + await asyncio.sleep(0.05) + + tasks = self._download_tasks - before + if not tasks: + return result + try: + names = await asyncio.gather(*tasks) + finally: + self._download_tasks.difference_update(tasks) + result["downloads"] = list(names) + return result + async def _idle_watch(self) -> None: while self._context is not None: await asyncio.sleep(self.idle_check_s) @@ -979,6 +1032,7 @@ async def open(self, ctx: Any, url: str, profile: str = "") -> dict: raise BrowserError(problem) self.use_profile(ctx, profile) async with self._lock: + downloads_before = self._download_tasks.copy() page = await self._page(ctx) await page.goto(target, wait_until="domcontentloaded") await self._settle(page) @@ -986,6 +1040,7 @@ async def open(self, ctx: Any, url: str, profile: str = "") -> dict: result = await self._snapshot_locked(page) result["profile"] = self._profile await self._after_action(ctx, page) + result = await self._include_downloads(result, downloads_before) return self._take_dialogs(result) async def snapshot(self, ctx: Any) -> dict: @@ -1013,6 +1068,7 @@ async def _snapshot_locked(self, page: Any) -> dict: async def click(self, ctx: Any, ref: str) -> dict: async with self._lock: + downloads_before = self._download_tasks.copy() page, loc, info = await self._locate(ctx, ref) if info.get("disabled"): raise BrowserError(f"{format_element(info)} is disabled. Something else on the page may need to be done first.") @@ -1048,10 +1104,12 @@ async def click(self, ctx: Any, ref: str) -> dict: 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) + out = await self._include_downloads(out, downloads_before) return self._take_dialogs(out) async def type(self, ctx: Any, ref: str, text: str, submit: bool = False) -> dict: async with self._lock: + downloads_before = self._download_tasks.copy() page, loc, info = await self._locate(ctx, ref) kind = safety.sensitive_field(info) or safety.sensitive_field(info.get("live")) if kind: @@ -1082,10 +1140,12 @@ async def type(self, ctx: Any, ref: str, text: str, submit: bool = False) -> dic described = format_element({k: v for k, v in info.items() if k != "value"}) out = {"ok": True, "typed_into": described, "submitted": bool(submit), "url": active.url} await self._after_action(ctx, active) + out = await self._include_downloads(out, downloads_before) return self._take_dialogs(out) async def select(self, ctx: Any, ref: str, option: str) -> dict: async with self._lock: + downloads_before = self._download_tasks.copy() page, loc, info = await self._locate(ctx, ref) if str(info.get("tag", "")).lower() != "select": raise BrowserError( @@ -1108,11 +1168,13 @@ async def select(self, ctx: Any, ref: str, option: str) -> dict: await self._settle(page, 1_000) out = {"ok": True, "selected": option, "in": format_element(info), "url": page.url} await self._after_action(ctx, page) + out = await self._include_downloads(out, downloads_before) return self._take_dialogs(out) async def press(self, ctx: Any, key: str) -> dict: name = normalize_key(key) async with self._lock: + downloads_before = self._download_tasks.copy() page = await self._page(ctx) focused = None with contextlib.suppress(Exception): @@ -1135,6 +1197,7 @@ async def press(self, ctx: Any, key: str) -> dict: await self._enforce_domains(active) out = {"ok": True, "pressed": name, "url": active.url} await self._after_action(ctx, active) + out = await self._include_downloads(out, downloads_before) return self._take_dialogs(out) async def scroll(self, ctx: Any, direction: str = "down") -> dict: @@ -1163,6 +1226,7 @@ async def scroll(self, ctx: Any, direction: str = "down") -> dict: async def back(self, ctx: Any) -> dict: async with self._lock: + downloads_before = self._download_tasks.copy() page = await self._page(ctx) resp = await page.go_back(wait_until="domcontentloaded") if resp is None and page.url in {"about:blank", ""}: @@ -1174,6 +1238,7 @@ async def back(self, ctx: Any) -> dict: title = await page.title() out = {"ok": True, "url": page.url, "title": title} await self._after_action(ctx, page) + out = await self._include_downloads(out, downloads_before) return out async def tabs(self, ctx: Any, profile: str = "") -> dict: diff --git a/tests/browser/conftest.py b/tests/browser/conftest.py index 343ee714..a676e0f7 100644 --- a/tests/browser/conftest.py +++ b/tests/browser/conftest.py @@ -47,6 +47,8 @@ "index.html": INDEX, "search.html": "Results

Results page

Search results here.

", "help.html": "Help

Help center

", + "download.html": 'DownloadsDownload report', + "report.txt": "Sentient browser download test.\n", } diff --git a/tests/browser/test_live_browser.py b/tests/browser/test_live_browser.py index 978f778a..9aeab255 100644 --- a/tests/browser/test_live_browser.py +++ b/tests/browser/test_live_browser.py @@ -57,6 +57,18 @@ async def test_open_type_click_extract_screenshot(browser, site): assert jpeg and jpeg[:2] == b"\xff\xd8" +async def test_download_is_saved_and_reported(browser, site): + ctx = make_ctx(browser.app) + snap = await bt.browser_open.call(ctx, {"url": f"{site}/download.html"}) + link = ref_of(snap["text"], r'link "Download report"') + result = await bt.browser_click.call(ctx, {"ref": link}) + assert result["downloads"] == ["downloads/report.txt"] + saved = paths.files_dir() / result["downloads"][0] + print(saved) + print(saved.exists()) + assert saved.read_text(encoding="utf-8") == "Sentient browser download test.\n" + + async def test_safety_in_real_pages(browser, site): app = browser.app app.config.tools.approvals.mode = "ask" From bd255cf7555fe7c035b2897bfd4c8e606981d754 Mon Sep 17 00:00:00 2001 From: Michal Ondrej Date: Fri, 9 Oct 2026 14:09:37 +0200 Subject: [PATCH 2/8] fixing one unnecessary assignment that ruff revealed --- sentient/browser/service.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sentient/browser/service.py b/sentient/browser/service.py index d5aa507c..8f761379 100644 --- a/sentient/browser/service.py +++ b/sentient/browser/service.py @@ -1238,8 +1238,7 @@ async def back(self, ctx: Any) -> dict: title = await page.title() out = {"ok": True, "url": page.url, "title": title} await self._after_action(ctx, page) - out = await self._include_downloads(out, downloads_before) - return out + return await self._include_downloads(out, downloads_before) async def tabs(self, ctx: Any, profile: str = "") -> dict: self.use_profile(ctx, profile) From 4ee734b46d3d2a50798b9184d625921fbd94ed5b Mon Sep 17 00:00:00 2001 From: Michal Ondrej Date: Sat, 10 Oct 2026 12:38:45 +0200 Subject: [PATCH 3/8] Fix browser download handling --- ISSUE-87-FIX.md | 27 ---------------------- docs/API.md | 8 +++++-- sentient/browser/service.py | 42 ++++++++++++++++++++++------------ tests/browser/test_profiles.py | 13 +++++++++++ 4 files changed, 46 insertions(+), 44 deletions(-) delete mode 100644 ISSUE-87-FIX.md diff --git a/ISSUE-87-FIX.md b/ISSUE-87-FIX.md deleted file mode 100644 index 618f2f46..00000000 --- a/ISSUE-87-FIX.md +++ /dev/null @@ -1,27 +0,0 @@ -## What the fix does: - -* **_on_download** - creates an asynchrone task that uses the function _save_download and appends the asynchrone event to a set in self._download_tasks - ---- - -* **_save_download** - handels the download name so there is no path traversal and changes inapropriate characters to _ - -* **_save_download** - function saves files to paths.files_dir() / "downloads", so the apps config folder is used, where at the end an relative path is returned for the saved file - -* **_save_download** - Prevents overwriting files with same name by checking the file names and trying to append a number to the file name until file name is unique - -* **_save_download** - Uses a lock (self._download_lock) for saving files, so there will be only one access to the downloads directory at a time, preventing corrupted files or direcotry - ---- - -* **_include_downloads** - is waiting for the downloads to finish. It is called before returning results from functions - open, click, type, select, press, back - -* **_include_downloads** - Because the browser activity is asynchronous, this function gets previous not finished download list and substracts the previouse/current asynchrone tasks to see, if new downloads appeared within the time, that is set in DOWNLOAD_APPEAR_GRACE_S - -* **_include_downloads** - If a new download appeared, it waits until it finishes with **await asyncio.gather(*tasks)** and then removes the tasks from the set - ---- - -* **test_download_is_saved_and_reported** - test uses offline site and lets the browser download data through finding link on the attribute on the site. -1. assert - It expects from the browser one download at the expected path -2. assert - checks the saved content is equal to the content on the site diff --git a/docs/API.md b/docs/API.md index 3988a627..12f4f9e4 100644 --- a/docs/API.md +++ b/docs/API.md @@ -1439,8 +1439,12 @@ Every new tool declares a `Risk`; approvals behave as in section 1. idle_minutes, allow_domains, block_domains, max_snapshot_chars, max_extract_chars, confirm_purchases, live_view, profiles`. Every safety rule below applies the same in every profile, attached ones included. - Tools (plugin `browser`). Failures return `{error}` with a message the model can act on. - - `browser_open(url, profile="")` read → same as `browser_snapshot` plus `profile` (http/https only; allow/block lists apply, also after redirects). `profile` switches to that profile for this and the following calls of the run; empty uses the run's profile, else `default`. - Any download started by a browser action is saved under `downloads/` in the Files folder and included as a relative path in that action's optional `downloads` array. + - Downloads from launched profiles are accepted by Playwright; downloads from pages Sentient opens in an attached + profile are handled the same way. Every download is saved under `downloads/` in Sentient's Files folder. Browser + tool results (including `browser_scroll`) may include an optional `downloads` array of relative paths such as + `["downloads/report.pdf"]`. Results wait up to 0.3 s for a download to start during the action; if it starts later, + it remains tracked and is reported on a later browser result after saving finishes. + - `browser_open(url, profile="")` read → same as `browser_snapshot` plus `profile` (http/https only; allow/block lists apply, also after redirects). `profile` switches to that profile for this and the following calls of the run; empty uses the run's profile, else `default`. - `browser_snapshot()` read → `{url, title, text, truncated?}`. `text` is `Page:`/`URL:`/`Scroll:` header, interactive elements one per line (`[e12] button "Sign in"`, `[e4] textbox "Search" value=""`, `[e7] combobox "Country" value="India" options: India | Japan`, `[e3] link "Docs" -> /docs`, flags `checked`, `disabled`, `focused`) and the diff --git a/sentient/browser/service.py b/sentient/browser/service.py index 8f761379..09f40b8d 100644 --- a/sentient/browser/service.py +++ b/sentient/browser/service.py @@ -793,19 +793,19 @@ async def _save_download(self, download: Any) -> str: raise BrowserError(f"Couldn't save downloaded file '{name}'") from exc return target.relative_to(paths.files_dir()).as_posix() - async def _include_downloads(self, result: dict, before: set[asyncio.Task[str]]) -> dict: - # Finished tasks from earlier actions (already in `before`) were never reported; drop them. - for task in before: - if task.done(): - self._download_tasks.discard(task) - + async def _include_downloads( + self, + result: dict, + before: set[asyncio.Task[str]], + ) -> dict: deadline = time.monotonic() + DOWNLOAD_APPEAR_GRACE_S while time.monotonic() < deadline: - if self._download_tasks - before: + if self._download_tasks - before or any(task.done() for task in self._download_tasks): break await asyncio.sleep(0.05) - tasks = self._download_tasks - before + # Wait for downloads started during this action, and include older downloads only once finished. + tasks = (self._download_tasks - before) | {task for task in self._download_tasks if task.done()} if not tasks: return result try: @@ -1045,9 +1045,12 @@ async def open(self, ctx: Any, url: str, profile: str = "") -> dict: async def snapshot(self, ctx: Any) -> dict: async with self._lock: + downloads_before = self._download_tasks.copy() page = await self._page(ctx) self._last_used = time.monotonic() - return self._take_dialogs(await self._snapshot_locked(page)) + result = await self._snapshot_locked(page) + result = await self._include_downloads(result, downloads_before) + return self._take_dialogs(result) async def _snapshot_locked(self, page: Any) -> dict: cfg = self.app.config.browser @@ -1213,6 +1216,7 @@ async def scroll(self, ctx: Any, direction: str = "down") -> dict: if d not in scripts: raise BrowserError("direction must be one of: down, up, top, bottom, left, right.") async with self._lock: + downloads_before = self._download_tasks.copy() page = await self._page(ctx) pos = await page.evaluate( "() => { " + scripts[d] + "; return [Math.round(scrollY), Math.round(document.documentElement.scrollHeight" @@ -1222,7 +1226,7 @@ async def scroll(self, ctx: Any, direction: str = "down") -> dict: out = {"ok": True, "scrolled": d, "from_top": pos[0], "more_below": max(0, pos[1]), "message": "Call browser_snapshot to see what is visible now."} await self._after_action(ctx, page) - return out + return await self._include_downloads(out, downloads_before) async def back(self, ctx: Any) -> dict: async with self._lock: @@ -1243,12 +1247,15 @@ async def back(self, ctx: Any) -> dict: async def tabs(self, ctx: Any, profile: str = "") -> dict: self.use_profile(ctx, profile) async with self._lock: + downloads_before = self._download_tasks.copy() await self._ensure(ctx) self._last_used = time.monotonic() - return {"profile": self._profile, "tabs": await self._tabs()} + result = {"profile": self._profile, "tabs": await self._tabs()} + return await self._include_downloads(result, downloads_before) async def switch_tab(self, ctx: Any, index: int) -> dict: async with self._lock: + downloads_before = self._download_tasks.copy() c = await self._ensure(ctx) pages = list(c.pages) if not 0 <= index < len(pages): @@ -1266,11 +1273,12 @@ async def switch_tab(self, ctx: Any, index: int) -> dict: out = {"ok": True, "index": index, "url": self._active.url, "title": title, "message": "Switched. Call browser_snapshot to see this tab."} await self._after_action(ctx, self._active) - return out + return await self._include_downloads(out, downloads_before) async def extract(self, ctx: Any, question: str = "") -> dict: cfg = self.app.config.browser async with self._lock: + downloads_before = self._download_tasks.copy() page = await self._page(ctx) self._last_used = time.monotonic() data = await page.evaluate(EXTRACT_JS, {"maxText": cfg.max_extract_chars * 5}) @@ -1280,10 +1288,11 @@ async def extract(self, ctx: Any, question: str = "") -> dict: out["question"] = question if truncated: out["truncated"] = True - return out + return await self._include_downloads(out, downloads_before) async def screenshot(self, ctx: Any) -> dict: async with self._lock: + downloads_before = self._download_tasks.copy() page = await self._page(ctx) self._last_used = time.monotonic() folder = paths.files_dir() / "outputs" / "browser" @@ -1293,13 +1302,16 @@ async def screenshot(self, ctx: Any) -> dict: title = "" with contextlib.suppress(Exception): title = await page.title() - return {"file": f"outputs/browser/{name}", "url": page.url, "title": title} + result = {"file": f"outputs/browser/{name}", "url": page.url, "title": title} + return await self._include_downloads(result, downloads_before) async def close_tool(self, ctx: Any) -> dict: + downloads_before = self._download_tasks.copy() was_running = self._context is not None async with self._lock: await self._shutdown() - return {"ok": True, "message": "Browser closed." if was_running else "The browser wasn't open."} + result = {"ok": True, "message": "Browser closed." if was_running else "The browser wasn't open."} + return await self._include_downloads(result, downloads_before) def _clean_ref(ref: Any) -> str | None: diff --git a/tests/browser/test_profiles.py b/tests/browser/test_profiles.py index 15d58d6b..48ef0f43 100644 --- a/tests/browser/test_profiles.py +++ b/tests/browser/test_profiles.py @@ -406,3 +406,16 @@ async def launch_persistent_context(self, **kwargs): with pytest.raises(BrowserError): await svc._launch_persistent(BrowserProfileConfig(), headless=True) assert browser_service.STORAGE_FLAG in seen["args"] + assert seen["accept_downloads"] is True + + +async def test_finished_download_is_reported_on_the_next_result(): + svc = BrowserService(make_app()) + task = asyncio.create_task(asyncio.sleep(0, result="downloads/late-report.txt")) + svc._download_tasks.add(task) + await task + + result = await svc._include_downloads({"ok": True}, {task}) + + assert result["downloads"] == ["downloads/late-report.txt"] + assert task not in svc._download_tasks From cf3feb354a7b3e34621094dde56ed671a31eddc5 Mon Sep 17 00:00:00 2001 From: Michal Ondrej Date: Sat, 10 Oct 2026 15:40:54 +0200 Subject: [PATCH 4/8] Fix browser download lacks and reporting across profiles --- docs/API.md | 9 +- sentient/browser/service.py | 126 ++++++++++++++++++++++------ tests/browser/test_profiles.py | 114 +++++++++++++++++++++++++ tests/browser/test_profiles_live.py | 21 +++++ 4 files changed, 244 insertions(+), 26 deletions(-) diff --git a/docs/API.md b/docs/API.md index 12f4f9e4..d3cf6a73 100644 --- a/docs/API.md +++ b/docs/API.md @@ -1443,7 +1443,14 @@ Every new tool declares a `Risk`; approvals behave as in section 1. profile are handled the same way. Every download is saved under `downloads/` in Sentient's Files folder. Browser tool results (including `browser_scroll`) may include an optional `downloads` array of relative paths such as `["downloads/report.pdf"]`. Results wait up to 0.3 s for a download to start during the action; if it starts later, - it remains tracked and is reported on a later browser result after saving finishes. + it remains tracked and is reported on a later browser result after saving finishes. Actions do not wait for downloads + to finish; unfinished downloads are listed by filename in an optional `downloads_in_progress` array and reported + under `downloads` when a later result observes that they have finished. Each save is limited to 120 s total, + including waiting for another save to finish; a save that exceeds this limit is cancelled. + Failed saves are reported in an optional `download_errors` array; they don't discard the browser action result or + successful downloads from the same batch. Completed downloads that no browser result consumes become eligible for + cleanup after 5 minutes; one cleanup task checks every 30 seconds, and an active browser action gets the chance to + report them before cleanup. - `browser_open(url, profile="")` read → same as `browser_snapshot` plus `profile` (http/https only; allow/block lists apply, also after redirects). `profile` switches to that profile for this and the following calls of the run; empty uses the run's profile, else `default`. - `browser_snapshot()` read → `{url, title, text, truncated?}`. `text` is `Page:`/`URL:`/`Scroll:` header, interactive elements one per line (`[e12] button "Sign in"`, `[e4] textbox "Search" value=""`, `[e7] combobox "Country" diff --git a/sentient/browser/service.py b/sentient/browser/service.py index 09f40b8d..f23c3d7d 100644 --- a/sentient/browser/service.py +++ b/sentient/browser/service.py @@ -65,6 +65,9 @@ STORAGE_FLUSH_S = 1.5 PAGE_CLOSE_TIMEOUT_S = 5.0 DOWNLOAD_APPEAR_GRACE_S = 0.3 +DOWNLOAD_MAX_DURATION_S = 120.0 +DOWNLOAD_TASK_RETENTION_S = 300.0 +DOWNLOAD_TASK_CLEANUP_INTERVAL_S = 30.0 # models often pass the whole snapshot line ("[e4] button \"Place order\"") instead of just "e4" _REF_RE = re.compile(r"\b(e\d+)\b", re.IGNORECASE) @@ -228,6 +231,9 @@ def __init__(self, app): self._last_tabs_sig: Any = None self.idle_check_s = 30.0 self._download_tasks: set[asyncio.Task[str]] = set() + self._download_names: dict[asyncio.Task[str], str] = {} + self._download_completed_at: dict[asyncio.Task[str], float] = {} + self._download_cleanup_task: asyncio.Task | None = None self._download_lock = asyncio.Lock() # ------------------------------------------------------------------ lifecycle @@ -769,28 +775,78 @@ async def _on_dialog(self, dialog: Any) -> None: def _on_download(self, download: Any) -> None: task = asyncio.create_task(self._save_download(download), name="browser:download") - task.add_done_callback(lambda done: done.exception() if not done.cancelled() else None) self._download_tasks.add(task) + self._download_names[task] = self._download_filename(download) + task.add_done_callback(self._on_download_task_done) - async def _save_download(self, download: Any) -> str: - raw_name = str(download.suggested_filename or "download") + def _on_download_task_done(self, task: asyncio.Task[str]) -> None: + if not task.cancelled(): + with contextlib.suppress(Exception): + task.exception() + if task in self._download_tasks: + self._download_completed_at[task] = time.monotonic() + if self._download_cleanup_task is None or self._download_cleanup_task.done(): + self._download_cleanup_task = asyncio.create_task( + self._cleanup_download_tasks(), name="browser:download-cleanup" + ) + self._bg.add(self._download_cleanup_task) + self._download_cleanup_task.add_done_callback(self._bg.discard) + + async def _cleanup_download_tasks(self) -> None: + try: + while self._download_completed_at: + await asyncio.sleep(DOWNLOAD_TASK_CLEANUP_INTERVAL_S) + expired_before = time.monotonic() - DOWNLOAD_TASK_RETENTION_S + async with self._lock: + expired = [ + task + for task, completed_at in self._download_completed_at.items() + if completed_at <= expired_before and task.done() + ] + for task in expired: + self._download_tasks.discard(task) + self._download_names.pop(task, None) + self._download_completed_at.pop(task, None) + finally: + self._download_cleanup_task = None + + @staticmethod + def _download_filename(download: Any) -> str: + raw_name = str(getattr(download, "suggested_filename", None) or "download") name = raw_name.replace("\\", "/").rsplit("/", 1)[-1] - name = re.sub(r"[^\w.\- ()]+", "_", name).strip(" .")[:160] or "download" + return re.sub(r"[^\w.\- ()]+", "_", name).strip(" .")[:160] or "download" - async with self._download_lock: - folder = paths.files_dir() / "downloads" - folder.mkdir(parents=True, exist_ok=True) - target = folder / name - stem = target.stem - suffix = target.suffix - index = 1 - while target.exists(): - target = folder / f"{stem}({index}){suffix}" - index += 1 - try: + async def _save_download(self, download: Any) -> str: + name = self._download_filename(download) + + def reject_symlinked_folder(folder: Path) -> None: + if folder.is_symlink(): + raise BrowserError("The downloads folder must not be a symbolic link.") + + timeout = asyncio.timeout(DOWNLOAD_MAX_DURATION_S) + try: + async with timeout, self._download_lock: + folder = paths.files_dir() / "downloads" + reject_symlinked_folder(folder) + folder.mkdir(parents=True, exist_ok=True) + target = folder / name + stem = target.stem + suffix = target.suffix + index = 1 + while target.exists(): + target = folder / f"{stem}({index}){suffix}" + index += 1 await download.save_as(str(target)) - except Exception as exc: - raise BrowserError(f"Couldn't save downloaded file '{name}'") from exc + except TimeoutError as exc: + if timeout.expired(): + raise BrowserError( + f"Download '{name}' did not finish within {DOWNLOAD_MAX_DURATION_S:g} seconds." + ) from exc + raise BrowserError(f"Couldn't save downloaded file '{name}'") from exc + except Exception as exc: + if isinstance(exc, BrowserError): + raise + raise BrowserError(f"Couldn't save downloaded file '{name}'") from exc return target.relative_to(paths.files_dir()).as_posix() async def _include_downloads( @@ -804,15 +860,35 @@ async def _include_downloads( break await asyncio.sleep(0.05) - # Wait for downloads started during this action, and include older downloads only once finished. + # Collect finished downloads, but never wait for a save to complete inside a browser action. tasks = (self._download_tasks - before) | {task for task in self._download_tasks if task.done()} - if not tasks: - return result - try: - names = await asyncio.gather(*tasks) - finally: - self._download_tasks.difference_update(tasks) - result["downloads"] = list(names) + done = {task for task in tasks if task.done()} + if done: + try: + outcomes = await asyncio.gather(*done, return_exceptions=True) + finally: + self._download_tasks.difference_update(done) + for task in done: + self._download_names.pop(task, None) + self._download_completed_at.pop(task, None) + names = [] + errors = [] + for outcome in outcomes: + if isinstance(outcome, BaseException): + message = str(outcome).strip() or f"{type(outcome).__name__} while saving download" + errors.append(message) + else: + names.append(outcome) + if names: + result["downloads"] = names + if errors: + result["download_errors"] = errors + + pending = [task for task in self._download_tasks if not task.done()] + if pending: + result["downloads_in_progress"] = [ + self._download_names.get(task, "download") for task in pending + ] return result async def _idle_watch(self) -> None: diff --git a/tests/browser/test_profiles.py b/tests/browser/test_profiles.py index 48ef0f43..97707a57 100644 --- a/tests/browser/test_profiles.py +++ b/tests/browser/test_profiles.py @@ -419,3 +419,117 @@ async def test_finished_download_is_reported_on_the_next_result(): assert result["downloads"] == ["downloads/late-report.txt"] assert task not in svc._download_tasks + + +async def test_unfinished_download_stays_tracked_until_a_later_result(monkeypatch): + svc = BrowserService(make_app()) + completion = asyncio.get_running_loop().create_future() + + class SlowDownload: + suggested_filename = "slow-report.pdf" + + async def save_download(download): + return await completion + + monkeypatch.setattr(svc, "_save_download", save_download) + svc._on_download(SlowDownload()) + task = next(iter(svc._download_tasks)) + + result = await asyncio.wait_for(svc._include_downloads({"ok": True}, set()), timeout=0.5) + + assert result == {"ok": True, "downloads_in_progress": ["slow-report.pdf"]} + assert task in svc._download_tasks + assert not task.done() + + completion.set_result("downloads/slow-report.pdf") + await task + result = await svc._include_downloads({"ok": True}, set()) + + assert result["downloads"] == ["downloads/slow-report.pdf"] + assert task not in svc._download_tasks + assert task not in svc._download_names + if svc._download_cleanup_task: + svc._download_cleanup_task.cancel() + await asyncio.gather(svc._download_cleanup_task, return_exceptions=True) + + +async def test_failed_download_does_not_discard_action_or_successful_download(): + svc = BrowserService(make_app()) + + async def fail_download(): + raise BrowserError("Couldn't save downloaded file 'broken.pdf'") + + successful = asyncio.create_task(asyncio.sleep(0, result="downloads/report.pdf")) + failed = asyncio.create_task(fail_download()) + svc._download_tasks.update((successful, failed)) + await asyncio.sleep(0) + + result = await svc._include_downloads({"ok": True, "clicked": "Download"}, set()) + + assert result == { + "ok": True, + "clicked": "Download", + "downloads": ["downloads/report.pdf"], + "download_errors": ["Couldn't save downloaded file 'broken.pdf'"], + } + assert successful not in svc._download_tasks + assert failed not in svc._download_tasks + + +async def test_stalled_download_is_timed_out_and_reported(tmp_path, monkeypatch): + svc = BrowserService(make_app()) + + class StalledDownload: + suggested_filename = "stalled.pdf" + + async def save_as(self, target): + await asyncio.Future() + + monkeypatch.setattr(browser_service.paths, "files_dir", lambda: tmp_path) + monkeypatch.setattr(browser_service, "DOWNLOAD_MAX_DURATION_S", 0.01) + task = asyncio.create_task(svc._save_download(StalledDownload())) + svc._download_tasks.add(task) + + with pytest.raises(BrowserError, match="did not finish within"): + await task + result = await svc._include_downloads({"ok": True}, set()) + + assert result == { + "ok": True, + "download_errors": ["Download 'stalled.pdf' did not finish within 0.01 seconds."], + } + assert task not in svc._download_tasks + + +async def test_unreported_completed_download_expires_after_reporting_window(monkeypatch): + svc = BrowserService(make_app()) + monkeypatch.setattr(browser_service, "DOWNLOAD_TASK_RETENTION_S", 0.01) + monkeypatch.setattr(browser_service, "DOWNLOAD_TASK_CLEANUP_INTERVAL_S", 0.01) + + async def save_download(download): + return f"downloads/{download}.pdf" + + monkeypatch.setattr(svc, "_save_download", save_download) + await svc._lock.acquire() + try: + downloads = list(range(100)) + for download in downloads: + svc._on_download(download) + tasks = set(svc._download_tasks) + await asyncio.gather(*tasks) + await asyncio.sleep(0) + assert svc._download_cleanup_task is not None + assert svc._download_cleanup_task.get_name() == "browser:download-cleanup" + assert len(svc._download_tasks) == len(downloads) + await asyncio.sleep(0.02) + assert len(svc._download_tasks) == len(downloads) + finally: + svc._lock.release() + + for _ in range(20): + if not svc._download_tasks: + break + await asyncio.sleep(0.01) + assert not svc._download_tasks + assert not svc._download_completed_at + assert not svc._download_names diff --git a/tests/browser/test_profiles_live.py b/tests/browser/test_profiles_live.py index 0f40ef86..2afe3e1f 100644 --- a/tests/browser/test_profiles_live.py +++ b/tests/browser/test_profiles_live.py @@ -11,6 +11,7 @@ import httpx import pytest +from sentient import paths from sentient.browser import tools as bt from sentient.browser.service import _engine_paths, profile_dir from sentient.config.schema import BrowserProfileConfig @@ -164,7 +165,27 @@ async def test_attach_to_a_running_browser_and_detach_leaves_it_running(browser, assert status["attached"] and status["profile"] == "mine" and status["engine"] is None assert len(status["tabs"]) >= 2 # the user's own tab plus the one Sentient opened for itself + download_page = await bt.browser_open.call(ctx, {"url": f"{site}/download.html", "profile": "mine"}) + download_ref = next( + line.split("]")[0][1:] + for line in download_page["text"].splitlines() + if 'link "Download report"' in line + ) + download_result = await bt.browser_click.call(ctx, {"ref": download_ref}) + download_paths = download_result.get("downloads", []) + if not download_paths: + assert download_result["downloads_in_progress"] == ["report.txt"] + pending = [task for task in browser._download_tasks if not task.done()] + assert pending + await asyncio.wait_for(asyncio.gather(*pending), timeout=10) + later_result = await bt.browser_snapshot.call(ctx, {}) + download_paths = later_result["downloads"] + assert download_paths == ["downloads/report.txt"] + saved = paths.files_dir() / download_paths[0] + assert saved.read_text(encoding="utf-8") == "Sentient browser download test.\n" + # the same safety applies inside the user's browser + snap = await bt.browser_open.call(ctx, {"url": f"{site}/index.html", "profile": "mine"}) app.config.tools.approvals.mode = "ask" text = snap["text"] pw = next(line.split("]")[0][1:] for line in text.splitlines() if 'textbox "Password"' in line) From a6959cdfef08d5985e862b4854d21d08040b7ec7 Mon Sep 17 00:00:00 2001 From: Michal Ondrej Date: Sat, 10 Oct 2026 15:55:51 +0200 Subject: [PATCH 5/8] fixing failed test --- tests/browser/test_profiles.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/browser/test_profiles.py b/tests/browser/test_profiles.py index 97707a57..95365a7d 100644 --- a/tests/browser/test_profiles.py +++ b/tests/browser/test_profiles.py @@ -462,7 +462,7 @@ async def fail_download(): successful = asyncio.create_task(asyncio.sleep(0, result="downloads/report.pdf")) failed = asyncio.create_task(fail_download()) svc._download_tasks.update((successful, failed)) - await asyncio.sleep(0) + await asyncio.gather(successful, failed, return_exceptions=True) result = await svc._include_downloads({"ok": True, "clicked": "Download"}, set()) From c1b75f3a604fc56f5f22746f29cd989d6dc74a20 Mon Sep 17 00:00:00 2001 From: Michal Ondrej Date: Sat, 10 Oct 2026 17:05:21 +0200 Subject: [PATCH 6/8] implementing final steps and fixing browser download issues --- CHANGELOG.md | 4 + docs/API.md | 33 ++-- sentient/browser/service.py | 233 ++++++++++++++++++++----- tests/browser/conftest.py | 12 ++ tests/browser/test_live_browser.py | 10 +- tests/browser/test_profiles.py | 259 +++++++++++++++++++++++++++- tests/browser/test_profiles_live.py | 10 ++ 7 files changed, 501 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef8bea6b..ebcc9b09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -222,6 +222,7 @@ and versions follow [Semantic Versioning](https://semver.org/). dropped connection when the computer is busy. - What Sentient learns about you reads your messages in the order you sent them, even when two were saved at the same instant. +<<<<<<< HEAD - The model check-up no longer warns about the graphics card on an 8 GB card running the setup Sentient recommends for it (qwen3:8b reading 8,192 tokens at a time). About 90% of the model fits there, which is expected; the only way to fit all of it is a context below 8,192 tokens, which cuts off long tasks. A bigger spill, a longer context or a @@ -234,6 +235,9 @@ and versions follow [Semantic Versioning](https://semver.org/). - Everyday phrases like "If you don't know, say so" no longer make Sentient suggest a Never rule for unrelated tools. A suggested rule for a single tool now needs your words to name an action, the app or the tool, so "Never write files for me" still suggests blocking file writing. +======= +- Edge's downloads hub no longer replaces the active page or appears as a browser tab when a download starts. +>>>>>>> d6f3231d (implementing final steps and fixing browser download issues) ### Security - Once Sentient has read an email, a web page, a message or anything else other people wrote, it asks before diff --git a/docs/API.md b/docs/API.md index d3cf6a73..b913d0a0 100644 --- a/docs/API.md +++ b/docs/API.md @@ -1439,19 +1439,26 @@ Every new tool declares a `Risk`; approvals behave as in section 1. idle_minutes, allow_domains, block_domains, max_snapshot_chars, max_extract_chars, confirm_purchases, live_view, profiles`. Every safety rule below applies the same in every profile, attached ones included. - Tools (plugin `browser`). Failures return `{error}` with a message the model can act on. - - Downloads from launched profiles are accepted by Playwright; downloads from pages Sentient opens in an attached - profile are handled the same way. Every download is saved under `downloads/` in Sentient's Files folder. Browser - tool results (including `browser_scroll`) may include an optional `downloads` array of relative paths such as - `["downloads/report.pdf"]`. Results wait up to 0.3 s for a download to start during the action; if it starts later, - it remains tracked and is reported on a later browser result after saving finishes. Actions do not wait for downloads - to finish; unfinished downloads are listed by filename in an optional `downloads_in_progress` array and reported - under `downloads` when a later result observes that they have finished. Each save is limited to 120 s total, - including waiting for another save to finish; a save that exceeds this limit is cancelled. - Failed saves are reported in an optional `download_errors` array; they don't discard the browser action result or - successful downloads from the same batch. Completed downloads that no browser result consumes become eligible for - cleanup after 5 minutes; one cleanup task checks every 30 seconds, and an active browser action gets the chance to - report them before cleanup. - - `browser_open(url, profile="")` read → same as `browser_snapshot` plus `profile` (http/https only; allow/block lists apply, also after redirects). `profile` switches to that profile for this and the following calls of the run; empty uses the run's profile, else `default`. + - Downloads from launched profiles are accepted by Playwright; in an attached profile, only downloads from the tab + Sentient opens and popups opened from Sentient-owned tabs are handled (not the user's existing tabs). Every handled + download is saved under `downloads/` in Sentient's Files folder. Browser tool results (including `browser_scroll`) + may include an optional `downloads` array of relative paths such as `["downloads/report.pdf"]`. + `browser_open`, `browser_click`, `browser_type`, `browser_press`, `browser_select` and `browser_back` wait up to + 0.3 s for a download to start during the action. Opening a URL that directly starts a download returns a normal + browser result with its download status rather than a navigation error. Edge's internal downloads hub is excluded + from browser tabs and does not replace the active page. If a download starts later, it remains tracked and is + reported on a later browser result after saving finishes. Other browser results collect finished downloads + immediately without that start wait. Actions do not wait for downloads to finish; unfinished downloads are listed + by filename in an optional `downloads_in_progress` array and reported under `downloads` when a later result observes + that they have finished. Saves reserve unique filenames under a short lock, then transfer concurrently. Each save + is limited to 120 s total; when the limit is reached, Sentient asks the browser to cancel it, with a 5 s limit for + that cancellation request. A cancellation failure is included in `download_errors`. + Failed saves are reported in an optional `download_errors` array; they don't discard the browser action result or + successful downloads from the same batch. Completed downloads that no browser result consumes become eligible for + cleanup after 5 minutes, and active browser actions get the chance to report them before cleanup. + - `browser_open(url, profile="")` read → same as `browser_snapshot` plus `profile` (http/https only; allow/block lists + apply, also after redirects). `profile` switches to that profile for this and the following calls of the run; empty + uses the run's profile, else `default`. - `browser_snapshot()` read → `{url, title, text, truncated?}`. `text` is `Page:`/`URL:`/`Scroll:` header, interactive elements one per line (`[e12] button "Sign in"`, `[e4] textbox "Search" value=""`, `[e7] combobox "Country" value="India" options: India | Japan`, `[e3] link "Docs" -> /docs`, flags `checked`, `disabled`, `focused`) and the diff --git a/sentient/browser/service.py b/sentient/browser/service.py index f23c3d7d..dad1750c 100644 --- a/sentient/browser/service.py +++ b/sentient/browser/service.py @@ -66,6 +66,7 @@ PAGE_CLOSE_TIMEOUT_S = 5.0 DOWNLOAD_APPEAR_GRACE_S = 0.3 DOWNLOAD_MAX_DURATION_S = 120.0 +DOWNLOAD_CANCEL_TIMEOUT_S = 5.0 DOWNLOAD_TASK_RETENTION_S = 300.0 DOWNLOAD_TASK_CLEANUP_INTERVAL_S = 30.0 # models often pass the whole snapshot line ("[e4] button \"Place order\"") instead of just "e4" @@ -208,6 +209,8 @@ def __init__(self, app): self._context: Any = None self._browser: Any = None # attach profiles: the user's browser we are connected to self._attached = False + self._download_pages: set[Any] = set() + self._internal_pages: set[Any] = set() 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 @@ -307,13 +310,19 @@ async def _tabs(self) -> list[dict]: if ctx is None: return [] out = [] - for i, page in enumerate(list(ctx.pages)): + for i, page in enumerate(self._usable_pages(ctx)): title = "" with contextlib.suppress(Exception): title = await asyncio.wait_for(page.title(), timeout=2) out.append({"index": i, "url": page.url, "title": title, "active": page is self._active}) return out + def _usable_pages(self, context: Any) -> list[Any]: + return [ + page for page in list(context.pages) + if not page.is_closed() and page not in self._internal_pages + ] + async def status(self) -> dict: running = self._context is not None name = self._profile if running else DEFAULT_PROFILE @@ -529,10 +538,13 @@ async def _launch(self, headless: bool) -> None: context.on("page", self._on_page) for page in context.pages: self._wire_page(page) + if self._is_downloads_hub(page.url): + self._internal_pages.add(page) if self._attached: # work in a tab of our own, never in one the user is using - self._active = await context.new_page() + self._active = await self._new_page(context) else: - self._active = context.pages[0] if context.pages else await context.new_page() + pages = self._usable_pages(context) + self._active = pages[0] if pages else await context.new_page() self._last_used = time.monotonic() if self._idle_task is None or self._idle_task.done(): self._idle_task = asyncio.create_task(self._idle_watch(), name="browser:idle") @@ -611,10 +623,16 @@ async def _ensure(self, ctx: Any = None) -> Any: async def _page(self, ctx: Any = None) -> Any: c = await self._ensure(ctx) if self._active is None or self._active.is_closed(): - pages = [p for p in c.pages if not p.is_closed()] - self._active = pages[-1] if pages and not self._attached else await c.new_page() + pages = self._usable_pages(c) + self._active = pages[-1] if pages and not self._attached else await self._new_page(c) return self._active + async def _new_page(self, context: Any) -> Any: + page = await context.new_page() + if self._attached: + self._enable_attached_page_downloads(page) + return page + async def _close_context(self) -> None: """Close the running profile. An attached browser is only disconnected, never closed. Hold ``_life_lock``.""" ctx, browser, attached = self._context, self._browser, self._attached @@ -637,6 +655,8 @@ async def _close_context(self) -> None: self._active = None self._snap = None self._focused = None + self._download_pages.clear() + self._internal_pages.clear() async def _flush_storage(self, ctx: Any) -> None: """Let a launched browser write site storage to disk before it closes: close its tabs with their unload @@ -729,6 +749,7 @@ def _on_context_closed(self, context: Any) -> None: self._active = None self._snap = None self._focused = None + self._download_pages.clear() if not self._closing: # the user closed the visible window (or their attached browser) self._spawn(self._publish_status()) @@ -737,31 +758,62 @@ def _wire_page(self, page: Any) -> None: page.on("dialog", self._on_dialog) page.on("popup", lambda popup: self._on_popup(page, popup)) page.on("framenavigated", lambda frame: self._on_navigated(page, frame)) - page.on("download", self._on_download) + if not self._attached: + page.on("download", self._on_download) + + def _enable_attached_page_downloads(self, page: Any) -> None: + if page not in self._download_pages: + self._download_pages.add(page) + page.on("download", self._on_download) def _on_navigated(self, page: Any, frame: Any) -> None: + if frame is page.main_frame and self._is_downloads_hub(frame.url): + self._mark_internal_page(page) + return # any website shown, by the assistant or by the user in a visible window if frame is page.main_frame and str(frame.url or "").startswith(("http://", "https://")): self._site_seen = True + @staticmethod + def _is_downloads_hub(url: Any) -> bool: + return str(url or "").lower().startswith("edge://downloads-hub") + + def _mark_internal_page(self, page: Any) -> None: + self._internal_pages.add(page) + if self._active is page: + context = self._context + pages = self._usable_pages(context) if context is not None else [] + self._active = pages[-1] if pages else None + def _on_page(self, page: Any) -> None: self._wire_page(page) - if not self._attached: # a link opened a new tab: keep working in it + 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 self._active = page self._spawn(self._publish_status()) def _on_popup(self, opener: Any, popup: Any) -> None: - # attached: follow only tabs our own tab opened, never ones the user opens - if self._attached and self._active is opener: - self._active = popup + if self._is_downloads_hub(popup.url): + self._mark_internal_page(popup) + return + if self._attached and opener in self._download_pages: + self._enable_attached_page_downloads(popup) + if self._active is opener: + self._active = popup def _on_page_closed(self, page: Any) -> None: + self._download_pages.discard(page) + self._internal_pages.discard(page) if self._snap and self._snap.get("page") is page: self._snap = None ctx = self._context if self._active is page: - remaining = [p for p in (ctx.pages if ctx else []) if p is not page and not p.is_closed()] - self._active = remaining[-1] if remaining and not self._attached else None + remaining = [ + 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 if ctx is not None and not self._closing: self._spawn(self._publish_status()) @@ -823,42 +875,112 @@ def reject_symlinked_folder(folder: Path) -> None: if folder.is_symlink(): raise BrowserError("The downloads folder must not be a symbolic link.") - timeout = asyncio.timeout(DOWNLOAD_MAX_DURATION_S) - try: - async with timeout, self._download_lock: + async def reserve_target() -> Path: + async with self._download_lock: folder = paths.files_dir() / "downloads" reject_symlinked_folder(folder) folder.mkdir(parents=True, exist_ok=True) - target = folder / name - stem = target.stem - suffix = target.suffix - index = 1 - while target.exists(): - target = folder / f"{stem}({index}){suffix}" - index += 1 + stem = Path(name).stem + suffix = Path(name).suffix + index = 0 + while True: + candidate_name = f"{stem}({index}){suffix}" if index else name + candidate = folder / candidate_name + try: + with candidate.open("xb"): + pass + except FileExistsError: + index += 1 + continue + return candidate + + async def request_download_cancel() -> str | None: + cancel_task = asyncio.create_task(download.cancel()) + done, _ = await asyncio.wait({cancel_task}, timeout=DOWNLOAD_CANCEL_TIMEOUT_S) + if not done: + cancel_task.cancel() + cancel_task.add_done_callback( + lambda task: task.exception() if not task.cancelled() else None + ) + return f"timed out after {DOWNLOAD_CANCEL_TIMEOUT_S:g} seconds" + try: + cancel_task.result() + except asyncio.CancelledError: + return "the cancellation request was cancelled" + except Exception as cancel_exc: + return str(cancel_exc).strip() or type(cancel_exc).__name__ + return None + + def save_failure(exc: BaseException) -> str: + reason = str(exc).strip().splitlines()[0][:300] or type(exc).__name__ + return f"Couldn't save downloaded file '{name}': {reason}" + + timeout = asyncio.timeout(DOWNLOAD_MAX_DURATION_S) + target: Path | None = None + try: + async with timeout: + target = await reserve_target() await download.save_as(str(target)) + except asyncio.CancelledError as exc: + try: + cancel_error = await request_download_cancel() + finally: + cleanup_error = self._remove_download_reservation(target) + if cancel_error: + exc.add_note(f"Browser cancellation failed: {cancel_error}") + if cleanup_error: + exc.add_note(f"Couldn't remove the reserved file: {cleanup_error}") + raise except TimeoutError as exc: if timeout.expired(): - raise BrowserError( - f"Download '{name}' did not finish within {DOWNLOAD_MAX_DURATION_S:g} seconds." - ) from exc - raise BrowserError(f"Couldn't save downloaded file '{name}'") from exc + cancel_error = await request_download_cancel() + cleanup_error = self._remove_download_reservation(target) + message = f"Download '{name}' did not finish within {DOWNLOAD_MAX_DURATION_S:g} seconds." + if cancel_error: + message += f" Browser cancellation failed: {cancel_error}" + if cleanup_error: + message += f" Couldn't remove the reserved file: {cleanup_error}" + raise BrowserError(message) from exc + cleanup_error = self._remove_download_reservation(target) + message = save_failure(exc) + if cleanup_error: + message += f"; couldn't remove the reserved file: {cleanup_error}" + raise BrowserError(message) from exc except Exception as exc: + cleanup_error = self._remove_download_reservation(target) if isinstance(exc, BrowserError): + if cleanup_error: + raise BrowserError(f"{exc}; couldn't remove the reserved file: {cleanup_error}") from exc raise - raise BrowserError(f"Couldn't save downloaded file '{name}'") from exc + message = save_failure(exc) + if cleanup_error: + message += f"; couldn't remove the reserved file: {cleanup_error}" + raise BrowserError(message) from exc + assert target is not None return target.relative_to(paths.files_dir()).as_posix() + @staticmethod + def _remove_download_reservation(target: Path | None) -> str | None: + if target is None: + return None + try: + target.unlink(missing_ok=True) + except OSError as exc: + return str(exc).strip() or type(exc).__name__ + return None + async def _include_downloads( self, result: dict, before: set[asyncio.Task[str]], + wait_for_download_start: bool = False, ) -> dict: - deadline = time.monotonic() + DOWNLOAD_APPEAR_GRACE_S - while time.monotonic() < deadline: - if self._download_tasks - before or any(task.done() for task in self._download_tasks): - break - await asyncio.sleep(0.05) + if wait_for_download_start: + deadline = time.monotonic() + DOWNLOAD_APPEAR_GRACE_S + while time.monotonic() < deadline: + if self._download_tasks - before or any(task.done() for task in self._download_tasks): + break + await asyncio.sleep(0.05) # Collect finished downloads, but never wait for a save to complete inside a browser action. tasks = (self._download_tasks - before) | {task for task in self._download_tasks if task.done()} @@ -1110,13 +1232,22 @@ async def open(self, ctx: Any, url: str, profile: str = "") -> dict: async with self._lock: downloads_before = self._download_tasks.copy() page = await self._page(ctx) - await page.goto(target, wait_until="domcontentloaded") - await self._settle(page) + download_started = False + try: + await page.goto(target, wait_until="domcontentloaded") + except Exception as exc: + if "Download is starting" not in str(exc): + raise + download_started = True + if not download_started: + await self._settle(page) await self._enforce_domains(page) result = await self._snapshot_locked(page) result["profile"] = self._profile await self._after_action(ctx, page) - result = await self._include_downloads(result, downloads_before) + result = await self._include_downloads( + result, downloads_before, wait_for_download_start=True + ) return self._take_dialogs(result) async def snapshot(self, ctx: Any) -> dict: @@ -1155,7 +1286,7 @@ 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._context.pages) + pages_before = len(self._usable_pages(self._context)) self._accept_confirm = live_risk >= Risk.send or self.app.config.tools.approvals.mode == "off" try: await loc.click(timeout=10_000) @@ -1165,7 +1296,11 @@ async def click(self, ctx: Any, ref: str) -> dict: # a new tab arrives as a separate event; links with target=_blank get longer to show up limit = 3.0 if str(info.get("target", "")).lower() == "_blank" else 0.3 waited = 0.0 - while self._context is not None and len(self._context.pages) <= pages_before and waited < limit: + while ( + self._context is not None + and len(self._usable_pages(self._context)) <= pages_before + and waited < limit + ): await asyncio.sleep(0.05) waited += 0.05 if self._context is None: @@ -1179,11 +1314,13 @@ 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._context.pages) > pages_before: + if 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) - out = await self._include_downloads(out, downloads_before) + out = await self._include_downloads( + out, downloads_before, wait_for_download_start=True + ) return self._take_dialogs(out) async def type(self, ctx: Any, ref: str, text: str, submit: bool = False) -> dict: @@ -1219,7 +1356,9 @@ async def type(self, ctx: Any, ref: str, text: str, submit: bool = False) -> dic described = format_element({k: v for k, v in info.items() if k != "value"}) out = {"ok": True, "typed_into": described, "submitted": bool(submit), "url": active.url} await self._after_action(ctx, active) - out = await self._include_downloads(out, downloads_before) + out = await self._include_downloads( + out, downloads_before, wait_for_download_start=True + ) return self._take_dialogs(out) async def select(self, ctx: Any, ref: str, option: str) -> dict: @@ -1247,7 +1386,9 @@ async def select(self, ctx: Any, ref: str, option: str) -> dict: await self._settle(page, 1_000) out = {"ok": True, "selected": option, "in": format_element(info), "url": page.url} await self._after_action(ctx, page) - out = await self._include_downloads(out, downloads_before) + out = await self._include_downloads( + out, downloads_before, wait_for_download_start=True + ) return self._take_dialogs(out) async def press(self, ctx: Any, key: str) -> dict: @@ -1276,7 +1417,9 @@ async def press(self, ctx: Any, key: str) -> dict: await self._enforce_domains(active) out = {"ok": True, "pressed": name, "url": active.url} await self._after_action(ctx, active) - out = await self._include_downloads(out, downloads_before) + out = await self._include_downloads( + out, downloads_before, wait_for_download_start=True + ) return self._take_dialogs(out) async def scroll(self, ctx: Any, direction: str = "down") -> dict: @@ -1318,7 +1461,9 @@ async def back(self, ctx: Any) -> dict: title = await page.title() out = {"ok": True, "url": page.url, "title": title} await self._after_action(ctx, page) - return await self._include_downloads(out, downloads_before) + return await self._include_downloads( + out, downloads_before, wait_for_download_start=True + ) async def tabs(self, ctx: Any, profile: str = "") -> dict: self.use_profile(ctx, profile) @@ -1333,7 +1478,7 @@ async def switch_tab(self, ctx: Any, index: int) -> dict: async with self._lock: downloads_before = self._download_tasks.copy() c = await self._ensure(ctx) - pages = list(c.pages) + pages = self._usable_pages(c) if not 0 <= index < len(pages): raise BrowserError(f"There is no tab {index}. Open tabs are numbered 0 to {len(pages) - 1}.") url = pages[index].url or "" diff --git a/tests/browser/conftest.py b/tests/browser/conftest.py index a676e0f7..604fb9c3 100644 --- a/tests/browser/conftest.py +++ b/tests/browser/conftest.py @@ -56,6 +56,18 @@ class _Quiet(http.server.SimpleHTTPRequestHandler): def log_message(self, *args): # keep test output clean return + def do_GET(self): + if self.path == "/direct-download": + body = b"Direct browser-open download test.\n" + self.send_response(200) + self.send_header("Content-Type", "application/octet-stream") + self.send_header("Content-Disposition", 'attachment; filename="direct-report.txt"') + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + return + super().do_GET() + @pytest.fixture(scope="session") def site(tmp_path_factory): diff --git a/tests/browser/test_live_browser.py b/tests/browser/test_live_browser.py index 9aeab255..b135ff97 100644 --- a/tests/browser/test_live_browser.py +++ b/tests/browser/test_live_browser.py @@ -64,11 +64,17 @@ async def test_download_is_saved_and_reported(browser, site): result = await bt.browser_click.call(ctx, {"ref": link}) assert result["downloads"] == ["downloads/report.txt"] saved = paths.files_dir() / result["downloads"][0] - print(saved) - print(saved.exists()) assert saved.read_text(encoding="utf-8") == "Sentient browser download test.\n" +async def test_open_direct_download_is_reported_as_normal_result(browser, site): + result = await bt.browser_open.call(make_ctx(browser.app), {"url": f"{site}/direct-download"}) + + assert result.get("downloads") == ["downloads/direct-report.txt"] + saved = paths.files_dir() / result["downloads"][0] + assert saved.read_text(encoding="utf-8") == "Direct browser-open download test.\n" + + async def test_safety_in_real_pages(browser, site): app = browser.app app.config.tools.approvals.mode = "ask" diff --git a/tests/browser/test_profiles.py b/tests/browser/test_profiles.py index 95365a7d..4b7e0eaa 100644 --- a/tests/browser/test_profiles.py +++ b/tests/browser/test_profiles.py @@ -6,6 +6,7 @@ import http.server import json import threading +from pathlib import Path import pytest from fastapi.testclient import TestClient @@ -421,6 +422,27 @@ async def test_finished_download_is_reported_on_the_next_result(): assert task not in svc._download_tasks +async def test_download_collection_without_start_wait_returns_immediately(monkeypatch): + svc = BrowserService(make_app()) + monkeypatch.setattr(browser_service, "DOWNLOAD_APPEAR_GRACE_S", 0.01) + original_sleep = asyncio.sleep + sleeps = [] + + async def unexpected_sleep(delay): + sleeps.append(delay) + await original_sleep(delay) + + monkeypatch.setattr(asyncio, "sleep", unexpected_sleep) + + assert await svc._include_downloads({"ok": True}, set()) == {"ok": True} + assert sleeps == [] + + assert await svc._include_downloads( + {"ok": True}, set(), wait_for_download_start=True + ) == {"ok": True} + assert sleeps + + async def test_unfinished_download_stays_tracked_until_a_later_result(monkeypatch): svc = BrowserService(make_app()) completion = asyncio.get_running_loop().create_future() @@ -453,6 +475,115 @@ async def save_download(download): await asyncio.gather(svc._download_cleanup_task, return_exceptions=True) +async def test_completed_download_is_reported_while_another_download_is_stuck(monkeypatch): + svc = BrowserService(make_app()) + stalled_started = asyncio.Event() + finish_stalled = asyncio.Event() + + class Download: + def __init__(self, suggested_filename): + self.suggested_filename = suggested_filename + + async def save_download(download): + if download.suggested_filename == "stalled.pdf": + stalled_started.set() + await finish_stalled.wait() + return "downloads/stalled.pdf" + return "downloads/ready.pdf" + + monkeypatch.setattr(svc, "_save_download", save_download) + svc._on_download(Download("stalled.pdf")) + stalled_task = next(iter(svc._download_tasks)) + await stalled_started.wait() + svc._on_download(Download("ready.pdf")) + ready_task = next(task for task in svc._download_tasks if task is not stalled_task) + await ready_task + + result = await asyncio.wait_for(svc._include_downloads({"ok": True}, set()), timeout=0.5) + + assert result == { + "ok": True, + "downloads": ["downloads/ready.pdf"], + "downloads_in_progress": ["stalled.pdf"], + } + assert stalled_task in svc._download_tasks + + finish_stalled.set() + await stalled_task + later_result = await svc._include_downloads({"ok": True}, set()) + assert later_result == {"ok": True, "downloads": ["downloads/stalled.pdf"]} + if svc._download_cleanup_task: + svc._download_cleanup_task.cancel() + await asyncio.gather(svc._download_cleanup_task, return_exceptions=True) + + +async def test_attached_downloads_are_wired_only_for_sentient_pages_and_their_popups(): + svc = BrowserService(make_app()) + svc._attached = True + + class Page: + def __init__(self): + self.url = "about:blank" + self.handlers = {} + + def on(self, event, handler): + self.handlers.setdefault(event, []).append(handler) + + class Context: + async def new_page(self): + return sentient_page + + user_page = Page() + sentient_page = Page() + user_popup = Page() + sentient_popup = Page() + nested_popup = Page() + + svc._wire_page(user_page) + svc._wire_page(sentient_page) + svc._active = await svc._new_page(Context()) + svc._wire_page(user_popup) + user_page.handlers["popup"][0](user_popup) + svc._wire_page(sentient_popup) + sentient_page.handlers["popup"][0](sentient_popup) + assert svc._active is sentient_popup + svc._wire_page(nested_popup) + sentient_popup.handlers["popup"][0](nested_popup) + + assert "download" not in user_page.handlers + assert "download" not in user_popup.handlers + assert sentient_page.handlers["download"] == [svc._on_download] + assert sentient_popup.handlers["download"] == [svc._on_download] + assert nested_popup.handlers["download"] == [svc._on_download] + assert nested_popup in svc._download_pages + + +def test_edge_downloads_hub_is_ignored_and_active_page_is_restored(): + 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 + + original = Page("https://example.com/") + downloads_hub = Page("edge://downloads-hub/") + context = type("Context", (), {"pages": [original, downloads_hub]})() + svc._context = context + svc._active = downloads_hub + + svc._on_navigated(downloads_hub, downloads_hub) + + assert svc._active is original + assert svc._usable_pages(context) == [original] + + async def test_failed_download_does_not_discard_action_or_successful_download(): svc = BrowserService(make_app()) @@ -481,17 +612,26 @@ async def test_stalled_download_is_timed_out_and_reported(tmp_path, monkeypatch) class StalledDownload: suggested_filename = "stalled.pdf" + cancelled = False async def save_as(self, target): + Path(target).write_text("partial", encoding="utf-8") await asyncio.Future() + async def cancel(self): + self.cancelled = True + + download = StalledDownload() + monkeypatch.setattr(browser_service.paths, "files_dir", lambda: tmp_path) monkeypatch.setattr(browser_service, "DOWNLOAD_MAX_DURATION_S", 0.01) - task = asyncio.create_task(svc._save_download(StalledDownload())) + task = asyncio.create_task(svc._save_download(download)) svc._download_tasks.add(task) with pytest.raises(BrowserError, match="did not finish within"): await task + assert download.cancelled + assert not (tmp_path / "downloads" / "stalled.pdf").exists() result = await svc._include_downloads({"ok": True}, set()) assert result == { @@ -501,6 +641,123 @@ async def save_as(self, target): assert task not in svc._download_tasks +async def test_download_save_error_includes_underlying_reason(tmp_path, monkeypatch): + svc = BrowserService(make_app()) + + class FailedDownload: + suggested_filename = "broken.pdf" + + async def save_as(self, target): + raise OSError("The connection was lost") + + async def cancel(self): + raise AssertionError("a failed save should not be cancelled") + + monkeypatch.setattr(browser_service.paths, "files_dir", lambda: tmp_path) + + with pytest.raises( + BrowserError, + match=r"Couldn't save downloaded file 'broken\.pdf': The connection was lost", + ): + await svc._save_download(FailedDownload()) + + assert not (tmp_path / "downloads" / "broken.pdf").exists() + + +async def test_stalled_download_cancel_is_bounded_and_partial_file_is_removed(tmp_path, monkeypatch): + svc = BrowserService(make_app()) + cancel_started = asyncio.Event() + + class StalledDownload: + suggested_filename = "stalled.pdf" + + async def save_as(self, target): + Path(target).write_text("partial", encoding="utf-8") + await asyncio.Future() + + async def cancel(self): + cancel_started.set() + await asyncio.Future() + + monkeypatch.setattr(browser_service.paths, "files_dir", lambda: tmp_path) + monkeypatch.setattr(browser_service, "DOWNLOAD_MAX_DURATION_S", 0.01) + monkeypatch.setattr(browser_service, "DOWNLOAD_CANCEL_TIMEOUT_S", 0.01) + + with pytest.raises(BrowserError, match=r"Browser cancellation failed: timed out after 0.01 seconds"): + await asyncio.wait_for(svc._save_download(StalledDownload()), timeout=0.5) + + assert cancel_started.is_set() + assert not (tmp_path / "downloads" / "stalled.pdf").exists() + + +async def test_cancelled_save_cancels_download_and_removes_partial_file(tmp_path, monkeypatch): + svc = BrowserService(make_app()) + save_started = asyncio.Event() + + class StalledDownload: + suggested_filename = "cancelled.pdf" + cancelled = False + + async def save_as(self, target): + Path(target).write_text("partial", encoding="utf-8") + save_started.set() + await asyncio.Future() + + async def cancel(self): + self.cancelled = True + + download = StalledDownload() + monkeypatch.setattr(browser_service.paths, "files_dir", lambda: tmp_path) + task = asyncio.create_task(svc._save_download(download)) + await save_started.wait() + task.cancel() + + with pytest.raises(asyncio.CancelledError): + await task + + assert download.cancelled + assert not (tmp_path / "downloads" / "cancelled.pdf").exists() + + +async def test_stalled_download_does_not_block_another_save(tmp_path, monkeypatch): + svc = BrowserService(make_app()) + stalled_started = asyncio.Event() + finish_stalled = asyncio.Event() + + class StalledDownload: + suggested_filename = "report.txt" + + async def save_as(self, target): + stalled_started.set() + await finish_stalled.wait() + Path(target).write_text("first", encoding="utf-8") + + async def cancel(self): + finish_stalled.set() + + class QuickDownload: + suggested_filename = "report.txt" + + async def save_as(self, target): + Path(target).write_text("second", encoding="utf-8") + + async def cancel(self): + raise AssertionError("a completed download should not be cancelled") + + monkeypatch.setattr(browser_service.paths, "files_dir", lambda: tmp_path) + monkeypatch.setattr(browser_service, "DOWNLOAD_MAX_DURATION_S", 1) + + stalled = asyncio.create_task(svc._save_download(StalledDownload())) + await stalled_started.wait() + quick = asyncio.create_task(svc._save_download(QuickDownload())) + + assert await asyncio.wait_for(quick, timeout=0.2) == "downloads/report(1).txt" + finish_stalled.set() + assert await stalled == "downloads/report.txt" + assert (tmp_path / "downloads" / "report.txt").read_text(encoding="utf-8") == "first" + assert (tmp_path / "downloads" / "report(1).txt").read_text(encoding="utf-8") == "second" + + async def test_unreported_completed_download_expires_after_reporting_window(monkeypatch): svc = BrowserService(make_app()) monkeypatch.setattr(browser_service, "DOWNLOAD_TASK_RETENTION_S", 0.01) diff --git a/tests/browser/test_profiles_live.py b/tests/browser/test_profiles_live.py index 2afe3e1f..6a6da6ae 100644 --- a/tests/browser/test_profiles_live.py +++ b/tests/browser/test_profiles_live.py @@ -165,6 +165,16 @@ async def test_attach_to_a_running_browser_and_detach_leaves_it_running(browser, assert status["attached"] and status["profile"] == "mine" and status["engine"] is None assert len(status["tabs"]) >= 2 # the user's own tab plus the one Sentient opened for itself + user_page = next(page for page in browser._context.pages if page is not browser._active) + await user_page.goto(f"{site}/download.html") + downloads_before = browser._download_tasks.copy() + async with user_page.expect_download(timeout=10_000) as download_info: + await user_page.get_by_role("link", name="Download report").click() + user_download = await download_info.value + assert user_download.suggested_filename == "report.txt" + assert browser._download_tasks == downloads_before + await user_download.cancel() + download_page = await bt.browser_open.call(ctx, {"url": f"{site}/download.html", "profile": "mine"}) download_ref = next( line.split("]")[0][1:] From a09be634c4b0bb943fcb4ab22555a71899df5551 Mon Sep 17 00:00:00 2001 From: Michal Ondrej Date: Sat, 10 Oct 2026 17:18:52 +0200 Subject: [PATCH 7/8] applying recommanded fixes --- CHANGELOG.md | 6 +++--- sentient/browser/service.py | 5 ++++- tests/browser/test_profiles.py | 28 ++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebcc9b09..7e9b7ea6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,9 @@ and versions follow [Semantic Versioning](https://semver.org/). approvals and rules, and Stop everything stops it. It only answers your chats: tasks, suggestions and other background work keep using your other models, and it can't be the memory search model. Anthropic may change how this is counted or allowed. +- **Browser downloads:** files Sentient downloads while using a website are saved to `downloads/` in your Files + folder, and the browser result says where. In an attached browser, only downloads from Sentient's own tabs are + saved. - **Push to talk and dictation into any app:** hold Ctrl+Alt+Shift+T (Cmd+Option+Shift+T on a Mac) anywhere, speak, and let go: what you said goes to Sentient as a chat message, and the answer is read aloud. Press Ctrl+Alt+Shift+D, speak, and press it again (or just pause): your words are typed where your cursor is, in any app. A small bar at the @@ -222,7 +225,6 @@ and versions follow [Semantic Versioning](https://semver.org/). dropped connection when the computer is busy. - What Sentient learns about you reads your messages in the order you sent them, even when two were saved at the same instant. -<<<<<<< HEAD - The model check-up no longer warns about the graphics card on an 8 GB card running the setup Sentient recommends for it (qwen3:8b reading 8,192 tokens at a time). About 90% of the model fits there, which is expected; the only way to fit all of it is a context below 8,192 tokens, which cuts off long tasks. A bigger spill, a longer context or a @@ -235,9 +237,7 @@ and versions follow [Semantic Versioning](https://semver.org/). - Everyday phrases like "If you don't know, say so" no longer make Sentient suggest a Never rule for unrelated tools. A suggested rule for a single tool now needs your words to name an action, the app or the tool, so "Never write files for me" still suggests blocking file writing. -======= - Edge's downloads hub no longer replaces the active page or appears as a browser tab when a download starts. ->>>>>>> d6f3231d (implementing final steps and fixing browser download issues) ### Security - Once Sentient has read an email, a web page, a message or anything else other people wrote, it asks before diff --git a/sentient/browser/service.py b/sentient/browser/service.py index dad1750c..5bcc0df8 100644 --- a/sentient/browser/service.py +++ b/sentient/browser/service.py @@ -782,7 +782,10 @@ def _mark_internal_page(self, page: Any) -> None: self._internal_pages.add(page) if self._active is page: context = self._context - pages = self._usable_pages(context) if context is not None else [] + 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 def _on_page(self, page: Any) -> None: diff --git a/tests/browser/test_profiles.py b/tests/browser/test_profiles.py index 4b7e0eaa..58dbe94e 100644 --- a/tests/browser/test_profiles.py +++ b/tests/browser/test_profiles.py @@ -584,6 +584,34 @@ def is_closed(self): assert svc._usable_pages(context) == [original] +def test_attached_downloads_hub_fallback_stays_on_sentient_owned_tab(): + svc = BrowserService(make_app()) + svc._attached = True + + class Page: + def __init__(self, url): + self.url = url + + @property + def main_frame(self): + return self + + def is_closed(self): + return False + + sentient_page = Page("https://sentient.example/") + user_page = Page("https://user.example/") + downloads_hub = Page("edge://downloads-hub/") + context = type("Context", (), {"pages": [sentient_page, user_page, downloads_hub]})() + svc._context = context + svc._active = downloads_hub + svc._download_pages.update((sentient_page, downloads_hub)) + + svc._on_navigated(downloads_hub, downloads_hub) + + assert svc._active is sentient_page + + async def test_failed_download_does_not_discard_action_or_successful_download(): svc = BrowserService(make_app()) From 11e8f69b4e7b4c6c914bf74dd373014d4cdfa394 Mon Sep 17 00:00:00 2001 From: Michal Ondrej Date: Sat, 10 Oct 2026 19:05:12 +0200 Subject: [PATCH 8/8] adding fixes and improvements --- sentient/browser/service.py | 41 ++++++++++++-- tests/browser/test_live_browser.py | 29 +++++++--- tests/browser/test_profiles.py | 87 ++++++++++++++++++++++++++++++ 3 files changed, 148 insertions(+), 9 deletions(-) diff --git a/sentient/browser/service.py b/sentient/browser/service.py index 5bcc0df8..73909a89 100644 --- a/sentient/browser/service.py +++ b/sentient/browser/service.py @@ -1530,12 +1530,47 @@ async def screenshot(self, ctx: Any) -> dict: return await self._include_downloads(result, downloads_before) async def close_tool(self, ctx: Any) -> dict: - downloads_before = self._download_tasks.copy() - was_running = self._context is not None async with self._lock: + downloads_before = self._download_tasks.copy() + was_running = self._context is not None + shutdown_errors = [] + download_deadline = time.monotonic() + DOWNLOAD_MAX_DURATION_S + shutdown_deadline = download_deadline + DOWNLOAD_CANCEL_TIMEOUT_S + while pending := {task for task in self._download_tasks if not task.done()}: + _, pending = await asyncio.wait( + pending, + timeout=max(0.0, download_deadline - time.monotonic()), + ) + if not pending: + continue + + interrupted_names = { + task: self._download_names.get(task, "download") for task in pending + } + for task in pending: + task.cancel() + _, still_pending = await asyncio.wait( + pending, timeout=max(0.0, shutdown_deadline - time.monotonic()) + ) + for task, name in interrupted_names.items(): + if task.cancelled(): + self._download_tasks.discard(task) + self._download_names.pop(task, None) + self._download_completed_at.pop(task, None) + shutdown_errors.append( + f"Download '{name}' was cancelled because the browser was closing." + ) + elif task in still_pending: + shutdown_errors.append( + f"Download '{name}' did not stop before browser shutdown." + ) + break await self._shutdown() result = {"ok": True, "message": "Browser closed." if was_running else "The browser wasn't open."} - return await self._include_downloads(result, downloads_before) + result = await self._include_downloads(result, downloads_before) + if shutdown_errors: + result.setdefault("download_errors", []).extend(shutdown_errors) + return result def _clean_ref(ref: Any) -> str | None: diff --git a/tests/browser/test_live_browser.py b/tests/browser/test_live_browser.py index b135ff97..fa0da27b 100644 --- a/tests/browser/test_live_browser.py +++ b/tests/browser/test_live_browser.py @@ -62,16 +62,33 @@ async def test_download_is_saved_and_reported(browser, site): snap = await bt.browser_open.call(ctx, {"url": f"{site}/download.html"}) link = ref_of(snap["text"], r'link "Download report"') result = await bt.browser_click.call(ctx, {"ref": link}) - assert result["downloads"] == ["downloads/report.txt"] - saved = paths.files_dir() / result["downloads"][0] + download_paths = result.get("downloads", []) + if not download_paths: + assert result["downloads_in_progress"] == ["report.txt"] + pending = [task for task in browser._download_tasks if not task.done()] + assert pending + await asyncio.wait_for(asyncio.gather(*pending), timeout=10) + later_result = await bt.browser_snapshot.call(ctx, {}) + download_paths = later_result["downloads"] + assert download_paths == ["downloads/report.txt"] + saved = paths.files_dir() / download_paths[0] assert saved.read_text(encoding="utf-8") == "Sentient browser download test.\n" async def test_open_direct_download_is_reported_as_normal_result(browser, site): - result = await bt.browser_open.call(make_ctx(browser.app), {"url": f"{site}/direct-download"}) - - assert result.get("downloads") == ["downloads/direct-report.txt"] - saved = paths.files_dir() / result["downloads"][0] + ctx = make_ctx(browser.app) + result = await bt.browser_open.call(ctx, {"url": f"{site}/direct-download"}) + + download_paths = result.get("downloads", []) + if not download_paths: + assert result["downloads_in_progress"] == ["direct-report.txt"] + pending = [task for task in browser._download_tasks if not task.done()] + assert pending + await asyncio.wait_for(asyncio.gather(*pending), timeout=10) + later_result = await bt.browser_snapshot.call(ctx, {}) + download_paths = later_result["downloads"] + assert download_paths == ["downloads/direct-report.txt"] + saved = paths.files_dir() / download_paths[0] assert saved.read_text(encoding="utf-8") == "Direct browser-open download test.\n" diff --git a/tests/browser/test_profiles.py b/tests/browser/test_profiles.py index 58dbe94e..3bfbb52f 100644 --- a/tests/browser/test_profiles.py +++ b/tests/browser/test_profiles.py @@ -475,6 +475,93 @@ async def save_download(download): await asyncio.gather(svc._download_cleanup_task, return_exceptions=True) +async def test_close_tool_waits_for_unfinished_download_before_shutdown(monkeypatch): + svc = BrowserService(make_app()) + svc._context = object() + started = asyncio.Event() + completion = asyncio.get_running_loop().create_future() + + async def save_download(): + started.set() + return await completion + + task = asyncio.create_task(save_download()) + svc._download_tasks.add(task) + svc._download_names[task] = "stalled.pdf" + await started.wait() + + shutdown_started = asyncio.Event() + + async def shutdown(): + shutdown_started.set() + svc._context = None + + monkeypatch.setattr(svc, "_shutdown", shutdown) + + close_task = asyncio.create_task(svc.close_tool(None)) + await asyncio.sleep(0) + assert not close_task.done() + assert not shutdown_started.is_set() + + completion.set_result("downloads/stalled.pdf") + result = await close_task + + assert shutdown_started.is_set() + assert result["downloads"] == ["downloads/stalled.pdf"] + assert task not in svc._download_tasks + + +async def test_close_tool_bounds_wait_for_download_that_ignores_cancellation(monkeypatch): + svc = BrowserService(make_app()) + svc._context = object() + started = asyncio.Event() + cancellation_seen = asyncio.Event() + release = asyncio.Event() + + async def stubborn_download(): + started.set() + try: + await asyncio.Future() + except asyncio.CancelledError: + cancellation_seen.set() + await release.wait() + return "downloads/stalled.pdf" + + task = asyncio.create_task(stubborn_download()) + svc._download_tasks.add(task) + svc._download_names[task] = "stalled.pdf" + await started.wait() + + shutdown_started = asyncio.Event() + + async def shutdown(): + shutdown_started.set() + svc._context = None + + monkeypatch.setattr(svc, "_shutdown", shutdown) + monkeypatch.setattr(browser_service, "DOWNLOAD_MAX_DURATION_S", 0.01) + monkeypatch.setattr(browser_service, "DOWNLOAD_CANCEL_TIMEOUT_S", 0.01) + + result = await asyncio.wait_for(svc.close_tool(None), timeout=0.5) + + assert cancellation_seen.is_set() + assert shutdown_started.is_set() + assert result["download_errors"] == [ + "Download 'stalled.pdf' did not stop before browser shutdown." + ] + assert result["downloads_in_progress"] == ["stalled.pdf"] + assert task in svc._download_tasks and not task.done() + + release.set() + await task + later_result = await svc._include_downloads({"ok": True}, set()) + assert later_result["downloads"] == ["downloads/stalled.pdf"] + assert task not in svc._download_tasks + if svc._download_cleanup_task: + svc._download_cleanup_task.cancel() + await asyncio.gather(svc._download_cleanup_task, return_exceptions=True) + + async def test_completed_download_is_reported_while_another_download_is_stuck(monkeypatch): svc = BrowserService(make_app()) stalled_started = asyncio.Event()