agent: lumotia — Phase B.3 unlink .part on ResumeUnsupported so retry can recover

Phase B.3 audit of commit 9f67ab2 (atomic model download + manifest —
Rev-1, Rev-5). Existing coverage is solid: the transcription-side
download_file has fixture tests for resume-and-verify, restart-on-200,
SHA-mismatch cleanup, 5xx rejection, Rev-1 preserve-existing-file, and
the Rev-5 manifest tmp+rename atomicity. The llm-side download_impl
has resume-and-verify and the Rev-1 preserve-existing-file regression.

One real residual found in crates/llm/src/model_manager.rs that the
original commit did not close.

When a stale .part exists (resume_from > 0) and the server returns a
200 full-body response to a Range request, download_impl returns
DownloadError::ResumeUnsupported without unlinking the .part. Every
subsequent download_model() call computes the same resume_from > 0,
sends the same Range request, gets the same 200, and fails the same
way — the download is wedged until the user manually invokes
delete_model(). That is itself a reversibility kill in the same
family as Rev-1: stale partial state stuck on disk, no automatic
recovery, the user has to discover an out-of-band command to escape.

The transcription-side download_file handles this case by treating
200-on-resume as a fresh-start (line 268: "Server ignored our Range
header — treat as fresh start"). The llm-side does not have an
analogous restart code path, but the simpler fix is sufficient: unlink
the .part before returning ResumeUnsupported. The next call sees
resume_from = 0, sends no Range header, the server returns 200, and
download_impl writes the new payload into a fresh .part and renames
atomically over dest. Single retry recovers.

Fix:
  * crates/llm/src/model_manager.rs:
      - download_impl: tokio::fs::remove_file(&tmp).await.ok() before
        returning ResumeUnsupported, with a comment that names this as
        a Phase B.3 audit residual and explains the wedge scenario.
      - New test resume_unsupported_unlinks_part_so_retry_starts_fresh
        — spins a server that ignores Range and returns 200, plants a
        sentinel .part, asserts ResumeUnsupported AND .part removed AND
        dest not written.

Verification:
  * cargo test -p lumotia-llm --lib model_manager
      → 5/5 pass including the new test.
  * cargo fmt --check → clean.
  * cargo clippy -p lumotia-llm --all-targets -- -D warnings → clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

crates/llm/src/model_manager.rs

@@ -364,6 +364,18 @@ where
         .await
         .map_err(|e| DownloadError::Http(e.to_string()))?;
     if resume_from > 0 && response.status() != reqwest::StatusCode::PARTIAL_CONTENT {
+        // Server downgraded from Range-aware to full-body 200 (typically a
+        // mirror / CDN that advertises `Accept-Ranges` but doesn't honour a
+        // mid-stream resume). The existing `.part` bytes are stale — they
+        // cannot be stitched onto a fresh 200 stream. Unlink them BEFORE
+        // returning so the next `download_model()` call starts from
+        // `resume_from = 0` and succeeds. Without this unlink the user is
+        // wedged: every retry sends the same Range header, the server
+        // returns 200 again, and `ResumeUnsupported` fires forever until
+        // the user manually calls `delete_model()`. That is itself a
+        // reversibility kill in the same family as Rev-1 (atomiser
+        // 2026-05-12); fixed in Phase B.3 audit.
+        tokio::fs::remove_file(&tmp).await.ok();
         return Err(DownloadError::ResumeUnsupported);
     }
     if !response.status().is_success() && response.status() != reqwest::StatusCode::PARTIAL_CONTENT
@@ -512,6 +524,76 @@ mod tests {
         server_task.await.unwrap();
     }
 
+    /// Phase B.3 audit residual (2026-05-14). The original Rev-1 fix
+    /// stopped the pre-emptive unlink of `dest` on SHA mismatch, but it
+    /// did NOT clean up `.part` when `download_impl` returned
+    /// `ResumeUnsupported`. That meant a transient mirror downgrade
+    /// (server returns 200 to a Range request) left a stale `.part` on
+    /// disk that every subsequent retry kept feeding back into the same
+    /// failing Range request — wedged until the user manually called
+    /// `delete_model()`. Same reversibility-kill family as Rev-1.
+    ///
+    /// We spin a server that ignores the Range header and returns 200
+    /// with full body. With a pre-existing `.part` the call must fail
+    /// with `ResumeUnsupported` AND the stale `.part` must be gone, so
+    /// a follow-up call would compute `resume_from = 0` and start
+    /// fresh.
+    #[tokio::test]
+    async fn resume_unsupported_unlinks_part_so_retry_starts_fresh() {
+        let body = b"fresh full body returned by server ignoring Range header".to_vec();
+        let expected_sha = format!("{:x}", Sha256::digest(&body));
+
+        let server = TcpListener::bind("127.0.0.1:0").await.unwrap();
+        let addr = server.local_addr().unwrap();
+        let content = body.clone();
+
+        let server_task = tokio::spawn(async move {
+            let (mut socket, _) = server.accept().await.unwrap();
+            let mut request = vec![0u8; 2048];
+            let _ = socket.read(&mut request).await.unwrap();
+            // Deliberately ignore Range header and return 200 with the
+            // full body — the case the downloader must recover from
+            // without leaving a stuck `.part`.
+            let response = format!(
+                "HTTP/1.1 200 OK\r\nContent-Length: {}\r\n\r\n",
+                content.len()
+            );
+            socket.write_all(response.as_bytes()).await.unwrap();
+            socket.write_all(&content).await.unwrap();
+        });
+
+        let dir = tempdir().unwrap();
+        let dest = dir.path().join("fixture.gguf");
+        let part = dest.with_extension("gguf.part");
+        // Pretend a previous interrupted attempt left 10 stale bytes.
+        tokio::fs::write(&part, b"STALEBYTES").await.unwrap();
+        assert!(part.exists());
+
+        let err = download_impl(
+            &format!("http://{addr}/fixture.gguf"),
+            &expected_sha,
+            &dest,
+            |_, _| {},
+        )
+        .await
+        .expect_err("server ignoring Range must surface ResumeUnsupported");
+
+        assert!(
+            matches!(err, DownloadError::ResumeUnsupported),
+            "expected ResumeUnsupported, got: {err:?}"
+        );
+        assert!(
+            !part.exists(),
+            "ResumeUnsupported must unlink .part so the next attempt starts fresh"
+        );
+        assert!(
+            !dest.exists(),
+            "dest must not have been written — only the unlink should run"
+        );
+
+        server_task.await.unwrap();
+    }
+
     /// Rev-1 regression (atomiser 2026-05-12). Before the fix the
     /// SHA-mismatch path in `download_model` deleted the existing
     /// file BEFORE the network call. A failing download then left