Skip to content

fix(website-browser): ensure download ended_at is set when hashing fails - #521

Open
saiyam-jain-dev wants to merge 2 commits into
scaleapi:mainfrom
saiyam-jain-dev:fix/browser-download-ended-at
Open

saiyam-jain-dev wants to merge 2 commits into
scaleapi:mainfrom
saiyam-jain-dev:fix/browser-download-ended-at

Conversation

@saiyam-jain-dev

@saiyam-jain-dev saiyam-jain-dev commented Nov 6, 2026 •

Copy link
Copy Markdown

What and why

In BrowserControl._hashed(), computing the size and sha256 digest for a completed download could fail if the downloaded file was removed, moved, or inaccessible. Previously, an exception during hashing skipped download.update(ended_at=...), leaving the completed download without an ended_at timestamp.

This change:

  • Wraps hashing in a try/finally block so ended_at is always set once a download has completed, even if computing size or digest raises an error.
  • Adds a unit test in test_website_browser.py asserting that ended_at is recorded when download file hashing fails.

How it was tested

  • Ran pytest on packages/agentenv-protocol/tests/test_website_browser.py for control download record tests.
  • Ran ruff check on control.py and test_website_browser.py.
  • Ran check_plugin_api.py against origin/main (no plugin surface changes).

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge.

Summary

The PR ensures completed downloads receive ended_at even when hashing fails.

  • Completed downloads get an end time even when their files cannot be hashed.

Reviews (2) · Last reviewed commit: "docs(website-browser): clarify that size..." · Reviewed by Greptile

@saiyam-jain-dev
saiyam-jain-dev requested a review from a team as a code owner November 6, 2026 22:25

@andrewpark-scaleai andrewpark-scaleai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I ran test_website_browser.py (56 passed), and the new test fails against main. The finally still re-raises, so _finished keeps logging the hashing error, and Download already has size/sha256 as optional.

Nit: the Download docstring in shapes.py says a completed download comes with its size and sha256. That's no longer guaranteed, so it's worth a word there.

@saiyam-jain-dev

Copy link
Copy Markdown
Author

Updated the Download docstring in shapes.py to clarify that size and sha256 are included when available.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants