fix(models): reject false-complete downloads that wrote no data (#1548) (#1562)

* fix(download-manager): validate output before marking a download complete

Both the torrent and HTTP paths could mark a download complete without
checking that anything was actually written to dest. The torrent path
trusted torrent.download() returning cleanly; the HTTP path only
checked SHA256 when one was supplied, so an empty or truncated body
still passed. Add a shared validation step (file exists, size > 0, size
matches the known total when available, SHA256 matches when provided)
and run it before marking either path complete. On failure the task is
marked error and the bad file is removed, matching the existing
SHA-mismatch behavior.

Fixes the backend half of #1548.

* fix(downloads): fold review findings on the false-complete validation

- MAJOR: do not treat Content-Length as the expected on-disk size for
  content-encoded (gzip/br/deflate/zstd) responses. httpx auto-decompresses,
  so the written file is larger than Content-Length and the size check would
  delete a valid download. Leave total_bytes unknown and rely on the SHA.
- normalize SHA256 comparison to lowercase so an uppercase expected hash is
  not a false mismatch
- torrent path: torrent.download() already SHA-verifies internally, so skip
  the redundant full-file re-hash (avoids a multi-GB blocking read)

* fix(downloads): offload the SHA fallback read to a thread so it can't block the loop

The computed_sha256-is-None fallback re-reads the whole file to hash it;
for a multi-GB model that would stall the event loop. Move _validate_download
to async and run the read+hash via asyncio.to_thread.

Author: jaylfc

tests/test_download_manager.py

@@ -314,6 +314,23 @@ async def test_download_creates_parent_dirs(self, dm, tmp_path):
         assert dest.exists()
         assert dest.read_bytes() == data
 
+    @pytest.mark.asyncio
+    async def test_download_empty_body_marks_error_not_complete(self, dm, tmp_path):
+        """A 0-byte response body (e.g. an error page served with a 200,
+        or a stream that closes immediately) must not be reported as a
+        complete download."""
+        dest = tmp_path / "out.bin"
+        task = DownloadTask(id="dl", url="http://example.com/f.bin", dest=dest)
+        mock_resp = self._make_async_context_manager_mock(b"", None)
+        mock_client = self._make_mock_client(mock_resp)
+
+        with patch("tinyagentos.download_manager.httpx.AsyncClient", return_value=mock_client):
+            await dm._download(task, expected_sha256=None)
+
+        assert task.status == "error"
+        assert task.error == "download produced no data"
+        assert not dest.exists()
+
     @pytest.mark.asyncio
     async def test_download_no_content_length(self, dm, tmp_path):
         data = b"no content length"
@@ -391,13 +408,15 @@ async def test_falls_through_when_license_disallows(self, dm, tmp_path):
     async def test_torrent_success_path(self, dm, tmp_path):
         dest = tmp_path / "model.bin"
         task = DownloadTask(id="dl", url="http://example.com/m.bin", dest=dest)
+        data = b"z" * 999
 
         fake_torrent = AsyncMock()
         fake_progress_task = MagicMock()
         fake_progress_task.total_bytes = 999
         fake_progress_task.downloaded_bytes = 999
 
         async def mock_download(task_id, magnet_or_torrent, dest, expected_sha256, progress_cb):
+            dest.write_bytes(data)
             progress_cb(fake_progress_task)
 
         fake_torrent.download = mock_download
@@ -414,6 +433,62 @@ async def mock_download(task_id, magnet_or_torrent, dest, expected_sha256, progr
         assert task.total_bytes == 999
         assert task.downloaded_bytes == 999
         assert task.completed_at > 0
+        assert dest.read_bytes() == data
+
+    @pytest.mark.asyncio
+    async def test_torrent_success_reported_but_no_file_marks_error(self, dm, tmp_path):
+        """Guards against the false-complete bug: the torrent swarm can
+        report a clean finish via progress_cb while writing nothing (or
+        an empty file) to dest. That must not be reported as complete."""
+        dest = tmp_path / "model.bin"
+        task = DownloadTask(id="dl", url="http://example.com/m.bin", dest=dest)
+
+        fake_torrent = AsyncMock()
+        fake_progress_task = MagicMock()
+        fake_progress_task.total_bytes = 999
+        fake_progress_task.downloaded_bytes = 999
+
+        async def mock_download(task_id, magnet_or_torrent, dest, expected_sha256, progress_cb):
+            # never writes dest, just reports progress as if it finished
+            progress_cb(fake_progress_task)
+
+        fake_torrent.download = mock_download
+
+        with patch.object(dm, "_get_torrent_downloader", return_value=fake_torrent):
+            await dm._download_with_fallback(
+                task,
+                expected_sha256=None,
+                magnet="magnet:?xt=urn:btih:abc",
+                license_allows_redistribution=True,
+            )
+
+        assert task.status == "error"
+        assert task.error == "download produced no data"
+        assert not dest.exists()
+
+    @pytest.mark.asyncio
+    async def test_torrent_success_reported_but_empty_file_marks_error(self, dm, tmp_path):
+        dest = tmp_path / "model.bin"
+        task = DownloadTask(id="dl", url="http://example.com/m.bin", dest=dest)
+
+        fake_torrent = AsyncMock()
+
+        async def mock_download(task_id, magnet_or_torrent, dest, expected_sha256, progress_cb):
+            dest.write_bytes(b"")
+
+        fake_torrent.download = mock_download
+
+        with patch.object(dm, "_get_torrent_downloader", return_value=fake_torrent):
+            await dm._download_with_fallback(
+                task,
+                expected_sha256=None,
+                magnet="magnet:?xt=urn:btih:abc",
+                license_allows_redistribution=True,
+            )
+
+        assert task.status == "error"
+        assert task.error == "download produced no data"
+        assert not dest.exists()
 
     @pytest.mark.asyncio
     async def test_torrent_failure_falls_back_to_http(self, dm, tmp_path):

tinyagentos/download_manager.py

@@ -97,6 +97,38 @@ def list_active(self) -> list[DownloadTask]:
     def list_all(self) -> list[DownloadTask]:
         return list(self._tasks.values())
 
+    async def _validate_download(
+        self,
+        task: DownloadTask,
+        expected_sha256: str | None = None,
+        computed_sha256: str | None = None,
+    ) -> str | None:
+        """Check a finished download before it is marked complete.
+
+        Returns None if the download is valid, or an error message
+        describing why it isn't. Applies to both the torrent and HTTP
+        paths so neither can mark a task complete when nothing (or the
+        wrong thing) was actually written to disk.
+        """
+        if not task.dest.exists() or task.dest.stat().st_size == 0:
+            return "download produced no data"
+        if task.total_bytes and task.dest.stat().st_size != task.total_bytes:
+            return "size mismatch"
+        if expected_sha256:
+            digest = computed_sha256
+            if digest is None:
+                # Fallback for a caller that did not stream the hash. Reading a
+                # potentially multi-GB model is offloaded to a thread so it
+                # never blocks the event loop.
+                digest = await asyncio.to_thread(
+                    lambda: hashlib.sha256(task.dest.read_bytes()).hexdigest()
+                )
+            # Hex digests are case-insensitive; a caller passing an uppercase
+            # expected value must not be treated as a mismatch.
+            if digest.lower() != expected_sha256.lower():
+                return "SHA256 mismatch"
+        return None
+
     async def _download_with_fallback(
         self,
         task: DownloadTask,
@@ -133,6 +165,21 @@ def _progress(t):
                     expected_sha256=expected_sha256,
                     progress_cb=_progress,
                 )
+                # torrent.download() already SHA-verified the file internally
+                # (and raised on mismatch), so re-hashing here would just re-read
+                # a multi-GB file to no benefit. Only the cheap non-empty / size
+                # floor is needed on this path.
+                error = await self._validate_download(task)
+                if error:
+                    task.dest.unlink(missing_ok=True)
+                    task.status = "error"
+                    task.error = error
+                    logger.error(
+                        "Torrent download for %s produced an invalid result (%s)",
+                        task.id,
+                        error,
+                    )
+                    return
                 task.status = "complete"
                 task.completed_at = time.time()
                 logger.info("Downloaded %s via torrent swarm", task.id)
@@ -160,17 +207,29 @@ async def _download(self, task: DownloadTask, expected_sha256: str | None = None
             async with httpx.AsyncClient(timeout=None, follow_redirects=True) as client:
                 async with client.stream("GET", task.url) as resp:
                     resp.raise_for_status()
+                    # Content-Length is the size of the ON-THE-WIRE body. When
+                    # the response is content-encoded (gzip/br/deflate/zstd),
+                    # httpx's aiter_bytes() transparently decompresses, so the
+                    # bytes we write to disk are LARGER than Content-Length.
+                    # Treating that as the expected on-disk size would make
+                    # _validate_download flag a perfectly good download as a
+                    # "size mismatch" and delete it, so leave total_bytes at 0
+                    # (unknown) for encoded responses and rely on the SHA check.
                     total = resp.headers.get("content-length")
-                    task.total_bytes = int(total) if total else 0
+                    if total and not resp.headers.get("content-encoding"):
+                        task.total_bytes = int(total)
+                    else:
+                        task.total_bytes = 0
                     with open(task.dest, "wb") as f:
                         async for chunk in resp.aiter_bytes(chunk_size=65536):
                             f.write(chunk)
                             sha.update(chunk)
                             task.downloaded_bytes += len(chunk)
-            if expected_sha256 and sha.hexdigest() != expected_sha256:
+            error = await self._validate_download(task, expected_sha256, computed_sha256=sha.hexdigest())
+            if error:
                 task.dest.unlink(missing_ok=True)
                 task.status = "error"
-                task.error = "SHA256 mismatch"
+                task.error = error
             else:
                 task.status = "complete"
                 task.completed_at = time.time()