a failed download must not become a permanent cache hit

download_model wrote straight to its final cache path with no try/except and no
.partial, and the check guarding it tested existence only. So any interruption
left a file that was then returned for ever as a valid cached model, and that
URL could never be fetched again.

Measured in one cache directory on zeiss: 188 zero-byte `<sha256>_<name>.gguf`
entries accruing about 14 a day since 2026-10-16, against 2 non-empty files. And
one of those two was a 9.72 GB ltx-2.5-22b-distilled-Q4_K_M.gguf where the real
file is 13.58 GB — a truncated download the existence-only check would have
served as a complete model. The empty ones break a load loudly; the truncated one
loads.

Now: the bytes go to a .partial and are renamed only once they are all there, an
empty or short transfer raises instead of being kept, and the partial is removed
on BaseException — not Exception, because a KeyboardInterrupt or SIGTERM
mid-download is exactly how several of these were made. A zero-byte entry found
in the cache is treated as a miss and deleted rather than served.

The cache LOCATION is deliberately untouched: moving it would be a silent full
re-download for every existing install, and a test pins the resolution order.
Where it points on a given box is a deployment question — on this one a root
symlink chain (~/.cache/coderai -> /AI/coderai -> /storage/coderai/) aims it at
the repo's gitignored models/ dir, which is a separate matter from this bug.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014T3rZozi1Ehk96NG1WesgC

codai/__init__.py

@@ -16,7 +16,7 @@
 
 # Canonical product version for CoderAI — single source of truth. Both the API
 # metadata and the admin web UI read from here.
-__version__ = "0.5.51"
+__version__ = "0.5.52"
 __tagline__ = "Complete orchestration, distribution and escalation of remotizable advanced inference."
 
 # Configure the CUDA caching allocator BEFORE torch is imported anywhere.

codai/models/cache/__init__.py

@@ -358,10 +358,22 @@ def download_model(model_path: str, cache_dir: Optional[str] = None, file_patter
         cached_filename = f"{url_hash}_{filename}"
         cached_path = os.path.join(cache_dir, cached_filename)
 
-        # Check if already cached (double-check)
+        # Check if already cached. Size matters, not just existence: a download
+        # that died left a ZERO-BYTE file here, and an existence-only check
+        # returned it for ever as a valid hit -- so that URL could never be
+        # fetched again and whatever loaded it got an empty file. Measured on
+        # zeiss 2026-10-30: 188 such entries, accruing about 14 a day since
+        # 2026-10-16, against 2 real models in the same directory.
         if os.path.exists(cached_path):
-            print(f"Using cached model: {cached_path}")
-            return cached_path
+            if os.path.getsize(cached_path) > 0:
+                print(f"Using cached model: {cached_path}")
+                return cached_path
+            print(f"Discarding a zero-byte cache entry from a failed download: "
+                  f"{cached_path}")
+            try:
+                os.remove(cached_path)
+            except OSError:
+                pass
 
         # Download
         print(f"Downloading model: {url}")
@@ -374,11 +386,17 @@ def download_model(model_path: str, cache_dir: Optional[str] = None, file_patter
         downloaded = 0
         start_time = time.time()
 
-        with open(cached_path, 'wb') as f:
-            for chunk in response.iter_content(chunk_size=8192*1024):
-                if chunk:
-                    f.write(chunk)
-                    downloaded += len(chunk)
+        # Write to a .partial and rename only once the bytes are all here, so an
+        # interrupted download cannot be mistaken for a cached model. open(...,
+        # 'wb') on the final path created the file before a single byte arrived,
+        # which is where the zero-byte entries came from.
+        partial_path = cached_path + '.partial'
+        try:
+            with open(partial_path, 'wb') as f:
+                for chunk in response.iter_content(chunk_size=8192*1024):
+                    if chunk:
+                        f.write(chunk)
+                        downloaded += len(chunk)
                     if total_size > 0:
                         percent = (downloaded / total_size) * 100
                         elapsed = time.time() - start_time
@@ -386,6 +404,22 @@ def download_model(model_path: str, cache_dir: Optional[str] = None, file_patter
                         speed_mb = speed / (1024 * 1024)
                         dl_mb = downloaded / (1024 * 1024)
                         print(f"Downloaded: {percent:.1f}% ({dl_mb:.1f}/{total_mb:.1f} MB) at {speed_mb:.1f} MB/s", end='\r')
+            if downloaded == 0:
+                raise IOError(f"the server sent no data for {url}")
+            if total_size > 0 and downloaded < total_size:
+                raise IOError(f"truncated download for {url}: got {downloaded} "
+                              f"of {total_size} bytes")
+            os.replace(partial_path, cached_path)
+        except BaseException:
+            # Leave nothing behind that a later run could read as a cache hit.
+            # BaseException, not Exception: a KeyboardInterrupt or a SystemExit
+            # mid-download is exactly how several of these were created.
+            try:
+                if os.path.exists(partial_path):
+                    os.remove(partial_path)
+            except OSError:
+                pass
+            raise
 
         print()  # New line after progress
         print(f"Downloaded and cached to: {cached_path}")

tests/test_url_download_cache_integrity.py

+"""A failed download must not become a permanent cache hit.
+
+`download_model` on a URL wrote straight to its final cache path:
+
+    with open(cached_path, 'wb') as f:          # created before any byte arrived
+        for chunk in response.iter_content(...):
+
+with no try/except and no .partial, and the cache check that guards it tested
+existence only:
+
+    if os.path.exists(cached_path):
+        return cached_path
+
+So any interruption left a file that was then returned for ever as a valid
+cached model. Measured on zeiss 2026-10-30 in one cache directory:
+
+  * 188 zero-byte `<sha256-of-url>_<name>.gguf` entries, accruing ~14 a day
+    since 2026-10-16, against 2 non-empty files;
+  * and one of those two was a 9.72 GB `ltx-2.5-22b-distilled-Q4_K_M.gguf`
+    where the real file is 13.58 GB -- a truncated download that the
+    existence-only check would have served as a complete model.
+
+The empty ones break the load loudly. The truncated one is worse: it loads.
+"""
+
+import os
+import sys
+
+import pytest
+
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+
+SRC = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))),
+                   'codai', 'models', 'cache', '__init__.py')
+
+
+def _url_branch():
+    """The URL-download branch of download_model, by structure not line number."""
+    src = open(SRC).read()
+    start = src.index("# It's a URL - use coderai cache")
+    end = src.index('def list_cached_models', start)
+    return src[start:end]
+
+
+# ── the cache check ──────────────────────────────────────────────────────────
+
+def test_a_zero_byte_entry_is_not_a_cache_hit():
+    body = _url_branch()
+    i = body.index('if os.path.exists(cached_path):')
+    guard = body[i:i + 700]
+    assert 'os.path.getsize(cached_path) > 0' in guard, (
+        'the cache check still tests existence only, so a failed download is '
+        'returned for ever as a valid model')
+
+
+def test_a_zero_byte_entry_is_removed_rather_than_left_to_rot():
+    body = _url_branch()
+    i = body.index('if os.path.exists(cached_path):')
+    guard = body[i:i + 700]
+    assert 'os.remove(cached_path)' in guard
+    # And removing it must not be able to fail the request.
+    assert 'except OSError' in guard
+
+
+# ── the download ─────────────────────────────────────────────────────────────
+
+def test_the_download_goes_to_a_partial_path():
+    body = _url_branch()
+    assert "partial_path = cached_path + '.partial'" in body
+    assert "open(partial_path, 'wb')" in body
+    assert "open(cached_path, 'wb')" not in body, (
+        'still writing directly to the final path: an interruption leaves a '
+        'file that looks cached')
+
+
+def test_the_rename_is_atomic_and_last():
+    body = _url_branch()
+    assert 'os.replace(partial_path, cached_path)' in body
+    # The rename must come after the loop that writes the bytes.
+    assert body.index('for chunk in response.iter_content') < \
+        body.index('os.replace(partial_path, cached_path)')
+
+
+def test_an_empty_response_is_an_error_not_a_cached_model():
+    body = _url_branch()
+    assert 'if downloaded == 0:' in body
+    assert 'raise IOError' in body
+
+
+def test_a_truncated_transfer_is_rejected():
+    """The 9.72 GB case. content-length said more than arrived, and the file was
+    kept and served."""
+    body = _url_branch()
+    assert 'downloaded < total_size' in body
+    assert 'truncated download' in body
+
+
+def test_the_partial_is_cleaned_up_on_any_failure():
+    body = _url_branch()
+    i = body.index("partial_path = cached_path + '.partial'")
+    tail = body[i:i + 2000]
+    assert 'except BaseException:' in tail, (
+        'Exception is not enough: a KeyboardInterrupt or SystemExit mid-download '
+        'is how several of these were created')
+    assert 'os.remove(partial_path)' in tail
+    assert 'raise' in tail                      # and the caller still learns
+
+
+def test_cleanup_failure_does_not_mask_the_real_error():
+    body = _url_branch()
+    i = body.index('except BaseException:')
+    tail = body[i:i + 500]
+    assert 'except OSError' in tail
+    assert tail.rstrip().endswith('raise') or '\n            raise' in tail
+
+
+# ── the module still imports and the helper still resolves ───────────────────
+
+def test_the_cache_dir_resolution_is_unchanged():
+    """This fix must not move anyone's cache: a changed location is a silent
+    full re-download for every existing install."""
+    from codai.models.cache import get_model_cache_dir
+    src = open(SRC).read()
+    i = src.index('def get_model_cache_dir')
+    body = src[i:i + 500]
+    assert "os.environ.get('CODERAI_CACHE_DIR')" in body
+    assert 'default_model_cache_dir()' in body
+    assert isinstance(get_model_cache_dir(), str)