files download [WIP]#70
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 5 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| break | ||
| if deadline is not None and asyncio.get_event_loop().time() >= deadline: | ||
| return None | ||
| await asyncio.sleep(0.5) |
There was a problem hiding this comment.
Completed downloads never returned
Medium Severity
wait_for_download only targets entries in inProgress state. If a download already finished before this call, it is skipped and the method polls until timeout, returning None despite completed data in self._downloads. This makes CloudBrowserWindow.wait_for_download unreliable for normal post-action usage.
|
|
||
| async def transfer_all_downloads( | ||
| self, | ||
| local_dir: str | Path = "/Users/Volodymyr/projects/narada/remote_downloads", |
There was a problem hiding this comment.
Default download directory is machine-specific
Medium Severity
transfer_all_downloads defaults local_dir to an absolute developer path (/Users/Volodymyr/...). On most environments this path is invalid or not writable, so calling transfer_all_downloads() with defaults can fail before transfer starts.
| # Disconnect Playwright from the browser (does not stop the remote session). | ||
| if self._browser is not None: | ||
| try: | ||
| self._browser.close() |
There was a problem hiding this comment.
Browser close coroutine not awaited
Medium Severity
CloudBrowserWindow.close calls self._browser.close() without await even though this is the Playwright async API. The coroutine may never run, leaving the CDP connection open and causing resource leakage or cleanup races during session shutdown.
| """ | ||
| if not self._downloads: | ||
| print("[cloud_downloads] No downloads were captured") | ||
| return [] |
There was a problem hiding this comment.
Early return misses in-flight download events
Medium Severity
wait_for_all returns immediately when self._downloads is empty, so transfer_all_downloads can miss downloads that start moments later. This creates a race where post-action transfers report no files even though a download was triggered and completes shortly after.
Additional Locations (1)
| loop = asyncio.get_running_loop() | ||
| loop.run_in_executor( | ||
| None, | ||
| lambda: self._on_download_complete( | ||
| self._session_id, guid, filename | ||
| ), | ||
| ) |
There was a problem hiding this comment.
if _on_download_complete is async
asyncio.create_task(_on_download_complete(...)) to create a background task
| return local_path | ||
|
|
||
| except Exception as exc: | ||
| logger.exception("Transfer failed: %s", exc) |
There was a problem hiding this comment.
| logger.exception("Transfer failed: %s", exc) | |
| logger.exception("Transfer failed") |
current exception is automatically captured
| extension_id: str = "bhioaidlggjdkheaajakomifblpjmokn" | ||
| interactive: bool = True | ||
| proxy: ProxyConfig | None = None | ||
| on_download_complete: Callable[[str, str, str], None] | None = None |
There was a problem hiding this comment.
can/should this be async? see question above about why we are alternating between blocking and async functions
| logger.warning("Browser reference not available -- cannot transfer file") | ||
| return None | ||
|
|
||
| from narada.cloud_downloads import download_remote_file_to_local |
There was a problem hiding this comment.
why are we importing in the function not at the top of the file
| def cloud_browser_session_id(self) -> str: | ||
| return self._session_id | ||
|
|
||
| async def wait_for_download( |
| return None | ||
| return await self._download_handler.wait_for_download(timeout=timeout) | ||
|
|
||
| async def transfer_download( |
| self._browser, download.remote_path, local_path | ||
| ) | ||
|
|
||
| async def transfer_all_downloads( |
| local_download_dir: str | Path | None = None, | ||
| download_wait_timeout: float = 60.0, |
2 participants
Note
Medium Risk
Introduces new CDP event handling and streaming file transfer for cloud browser sessions; failures could leak resources (contexts/sessions) or miss/incorrectly transfer downloads, but changes are scoped to cloud-browser flow.
Overview
Adds first-class download support for cloud browser sessions by wiring a browser-level
CDPDownloadHandlerintoopen_and_initialize_cloud_browser_window, capturing downloads across tabs and optionally invoking a user-providedBrowserConfig.on_download_completecallback.Introduces
narada.cloud_downloadswithDownloadInfo, download tracking (wait_for_download/wait_for_all), and chunked remote-to-local streaming via CDPFetch+IO.read, and exposes this onCloudBrowserWindow(has_pending_downloads, transfer helpers). Also exportsDownloadInfofrom the package and ensuresCloudBrowserWindow.close()disconnects the Playwright browser connection before stopping the cloud session.Written by Cursor Bugbot for commit 89a6126. This will update automatically on new commits. Configure here.