Local models: wait out a held finished download instead of copying it and failing

Symptom: a 22 GB catalog download ended with "[WinError 5] Access is denied:
'...\models\<model>.part'" even though the complete .gguf was on disk, and a
retry then found it and finished in seconds.

Root cause: download_file published the finished .part with shutil.move. On
Windows the just-closed file is commonly still open to an antivirus or
indexing scan, so os.rename fails with a permission error; shutil.move then
silently falls back to copy2 + unlink, which duplicates the whole file
(many seconds of disk churn and double the space) and finally reports the
leftover's failed delete as the download's failure. The except-path cleanup
could also replace the real error with its own unlink failure.

Fix: binaries.replace_when_released renames with os.replace and retries a
PermissionError with backoff through a bounded window (60 s), raising a
plain-language error if the hold outlasts it; there is no copy fallback.
Both the model download and the runtime-archive download publish through it,
and both cleanups suppress a failed leftover removal so the original error is
what the user sees. The job narrates the wait as "Finishing" so the bar does
not sit dead at 100%.

Tests: helper retries a transient refusal and lands the file; helper gives up
with the plain-language message chained to the OS error; route-level download
survives two refused renames with no .part left behind; a stuck leftover does
not mask the rename failure.

hermes_cli/local_runtime/binaries.py

@@ -10,8 +10,10 @@ import platform
 import shutil
 import subprocess
 import tempfile
+import time
 import urllib.request
 import zipfile
+from contextlib import suppress
 from dataclasses import dataclass, field
 from pathlib import Path
 from typing import Callable
@@ -182,6 +184,36 @@ def _sha256(path: Path) -> str:
     return h.hexdigest()
 
 
+# How long a finished download may wait for another process to let go of it before the
+# download is reported as failed.
+_RELEASE_WAIT_SECONDS = 60.0
+
+
+def replace_when_released(tmp: Path, dest: Path, *, timeout: float = _RELEASE_WAIT_SECONDS) -> None:
+    """Rename a finished download into place, waiting out a transient hold on the file.
+
+    On Windows a file whose last write handle just closed is often still open to an antivirus
+    or indexing scan, and renaming it fails with a permission error until the scan lets go —
+    for a multi-gigabyte model that can take many seconds. ``os.replace`` is retried through
+    that window; it never falls back to copying (``shutil.move`` does, which duplicates the whole
+    file and then reports the leftover's failed delete as the download's failure). A hold that
+    outlasts the window raises a plain-language error.
+    """
+    deadline = time.monotonic() + timeout
+    delay = 0.1
+    while True:
+        try:
+            os.replace(tmp, dest)
+            return
+        except PermissionError as exc:
+            if time.monotonic() >= deadline:
+                raise RuntimeError(
+                    f"The download finished, but another program (usually an antivirus scan) kept "
+                    f"{tmp.name} open and it could not be renamed into place. Please try again.") from exc
+            time.sleep(delay)
+            delay = min(delay * 2, 2.0)
+
+
 def _download(url: str, dest: Path,
               progress: "Callable[[int, int], None] | None" = None) -> None:
     """Stream url -> dest. ``progress(done_bytes, total_bytes)`` ticks per chunk (total 0 when
@@ -207,9 +239,11 @@ def _download(url: str, dest: Path,
             if length is not None and done != total:
                 raise BinaryResolutionError(
                     f"incomplete download for {dest.name}: expected {total} bytes, got {done}")
-        tmp.replace(dest)
+        replace_when_released(tmp, dest)
     finally:
-        tmp.unlink(missing_ok=True)
+        # Best effort: a leftover that cannot be removed must not hide the error that left it.
+        with suppress(OSError):
+            tmp.unlink(missing_ok=True)
 
 
 def _extract(archive: Path, dest: Path,

hermes_cli/web_routers/local_models.py

@@ -380,9 +380,12 @@ def download_file(url: str, dest: Path, job: Dict[str, Any], *, base_done: int =
             if length and file_done[0] != length:
                 raise RuntimeError(f"Download ended at {file_done[0]:,} bytes but the server "
                                    f"said {length:,} — connection dropped? Removed; try again")
-        shutil.move(str(tmp), str(dest))
+        job["detail"] = "Finishing"
+        binaries.replace_when_released(tmp, dest)
     except Exception:
-        tmp.unlink(missing_ok=True)
+        # Best effort: a leftover that cannot be removed must not hide the error that left it.
+        with contextlib.suppress(OSError):
+            tmp.unlink(missing_ok=True)
         raise
 
 

tests/hermes_cli/test_local_models_routes.py

@@ -8,6 +8,7 @@ from __future__ import annotations
 
 import io
 import json
+import os
 import time
 from pathlib import Path
 
@@ -358,3 +359,97 @@ def test_download_tolerates_stale_catalog_size(client, monkeypatch):
             break
         time.sleep(0.05)
     assert status is not None and status["status"] == "done", status.get("error")
+
+
+def test_download_survives_a_held_finished_file(client, monkeypatch):
+    """The finished .part is often still open to an antivirus scan when the rename runs (Windows),
+    which refuses it with a permission error. The job must wait the hold out and land the file —
+    not copy it, and not report a complete download as failed."""
+
+    body = b"x" * 48
+
+    class FakeResponse(io.BytesIO):
+        headers = {"Content-Length": str(len(body))}
+
+        def __enter__(self):
+            return self
+
+        def __exit__(self, *a):
+            return False
+
+    monkeypatch.setattr("urllib.request.urlopen", lambda *a, **k: FakeResponse(body))
+
+    real_replace = os.replace
+    refusals = []
+
+    def held_at_first(src, dst):
+        if str(src).endswith(".part") and len(refusals) < 2:
+            refusals.append(src)
+            raise PermissionError(13, "Access is denied")
+        real_replace(src, dst)
+
+    monkeypatch.setattr(os, "replace", held_at_first)
+
+    from hermes_cli.local_runtime.estimator import HardwareBudget
+
+    budget = HardwareBudget(usable_vram_bytes=64 << 30, total_device_bytes=64 << 30,
+                            ram_available_bytes=64 << 30)
+    monkeypatch.setattr("hermes_cli.local_runtime.hardware.probe_budget", lambda **kw: budget)
+    monkeypatch.setattr("hermes_cli.local_runtime.bootstrap.refresh_local_runtime", lambda: False)
+
+    from hermes_cli.local_runtime.bootstrap import models_dir
+    from hermes_cli.local_runtime.catalog import CATALOG
+
+    entry_id = CATALOG[0].id
+    r = client.post("/api/local-models/download", json={"model_id": entry_id})
+    assert r.status_code == 200
+    job_id = r.json()["job_id"]
+
+    deadline = time.time() + 10
+    status = None
+    while time.time() < deadline:
+        status = client.get(f"/api/local-models/jobs/{job_id}").json()
+        if status["status"] in ("done", "error"):
+            break
+        time.sleep(0.05)
+    assert status is not None and status["status"] == "done", status.get("error")
+    assert len(refusals) == 2
+    assert not list(models_dir().glob("*.part"))
+    assert any(p.read_bytes() == body for p in models_dir().glob("*.gguf"))
+
+
+def test_download_failure_is_not_masked_by_a_stuck_leftover(tmp_path, monkeypatch):
+    """When the rename gives up, the user must see that message — not the error from the
+    cleanup that could not remove the still-held .part either."""
+    from hermes_cli.web_routers import local_models
+
+    body = b"x" * 48
+
+    class FakeResponse(io.BytesIO):
+        headers = {"Content-Length": str(len(body))}
+
+        def __enter__(self):
+            return self
+
+        def __exit__(self, *a):
+            return False
+
+    monkeypatch.setattr("urllib.request.urlopen", lambda *a, **k: FakeResponse(body))
+
+    def still_held(tmp, dest, **kw):
+        raise RuntimeError("could not be renamed into place")
+
+    monkeypatch.setattr("hermes_cli.local_runtime.binaries.replace_when_released", still_held)
+    real_unlink = Path.unlink
+
+    def stuck_part(self, missing_ok=False):
+        if self.suffix == ".part":
+            raise PermissionError(13, "Access is denied")
+        return real_unlink(self, missing_ok=missing_ok)
+
+    monkeypatch.setattr(Path, "unlink", stuck_part)
+
+    dest = tmp_path / "models" / "model.gguf"
+    with pytest.raises(RuntimeError, match="could not be renamed"):
+        local_models.download_file("http://example.invalid/model.gguf", dest, {})
+    assert not dest.exists()

tests/hermes_cli/test_local_runtime_downloads.py

@@ -1,5 +1,6 @@
 """Runtime downloads must not turn a transient transfer failure into a poisoned cache."""
 
+import os
 import threading
 from concurrent.futures import ThreadPoolExecutor
 from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
@@ -94,3 +95,43 @@ def test_failed_request_preserves_another_active_download(tmp_path, asset_server
     assert dest.read_bytes() == payload
     assert not list(tmp_path.glob("*.part"))
     assert len(requests) == 2
+
+
+def test_replace_waits_out_a_transient_hold(tmp_path, monkeypatch):
+    """A rename refused while another process still holds the finished file (antivirus and
+    indexing scans on Windows) must be retried, not degraded to a copy or reported as a
+    failed download."""
+    tmp = tmp_path / "model.part"
+    dest = tmp_path / "model.gguf"
+    tmp.write_bytes(b"weights")
+    real_replace = os.replace
+    refusals = []
+
+    def held_twice(src, dst):
+        if len(refusals) < 2:
+            refusals.append(src)
+            raise PermissionError(13, "Access is denied")
+        real_replace(src, dst)
+
+    monkeypatch.setattr(os, "replace", held_twice)
+    binaries.replace_when_released(tmp, dest, timeout=5)
+    assert len(refusals) == 2
+    assert dest.read_bytes() == b"weights"
+    assert not tmp.exists()
+
+
+def test_replace_gives_up_with_a_plain_language_error(tmp_path, monkeypatch):
+    tmp = tmp_path / "model.part"
+    dest = tmp_path / "model.gguf"
+    tmp.write_bytes(b"weights")
+
+    def always_held(src, dst):
+        raise PermissionError(13, "Access is denied")
+
+    monkeypatch.setattr(os, "replace", always_held)
+    with pytest.raises(RuntimeError) as failed:
+        binaries.replace_when_released(tmp, dest, timeout=0.3)
+    assert "model.part" in str(failed.value)
+    assert "try again" in str(failed.value).lower()
+    assert isinstance(failed.value.__cause__, PermissionError)
+    assert not dest.exists()