fix(versions): don't let a locked version file abort the delete cascade

backend/services/history.py

@@ -4,6 +4,7 @@ Generation history management module.
 
 from typing import List, Optional, Tuple
 from datetime import UTC, datetime
+import logging
 import uuid
 import shutil
 from pathlib import Path
@@ -14,6 +15,8 @@ from ..models import GenerationRequest, GenerationResponse, HistoryQuery, Histor
 from ..database import Generation as DBGeneration, GenerationVersion as DBGenerationVersion, StoryItem as DBStoryItem, VoiceProfile as DBVoiceProfile
 from .. import config
 
+logger = logging.getLogger(__name__)
+
 
 def _delete_generation_children(generation_id: str, db: Session) -> None:
     """Remove the rows that reference a generation, plus any version audio files.
@@ -334,7 +337,7 @@ async def delete_failed_generations(db: Session) -> int:
                 except OSError:
                     # Best-effort cleanup — don't abort the whole sweep
                     # if a single file can't be removed.
-                    pass
+                    logger.warning("Could not delete generation audio %s", audio_path)
 
         db.delete(generation)
         count += 1
@@ -372,7 +375,7 @@ async def delete_generations_by_profile(
             except OSError:
                 # A file locked by playback shouldn't abort the whole sweep
                 # and leave the profile half-deleted.
-                pass
+                logger.warning("Could not delete generation audio %s", audio_path)
 
         # Delete from database
         db.delete(generation)

backend/services/versions.py

@@ -8,6 +8,7 @@ version and any number of processed versions with different effects chains.
 from __future__ import annotations
 
 import json
+import logging
 import uuid
 from pathlib import Path
 from typing import List, Optional
@@ -21,6 +22,8 @@ from ..database import (
 from ..models import GenerationVersionResponse, EffectConfig
 from .. import config
 
+logger = logging.getLogger(__name__)
+
 
 def _version_response(v: DBGenerationVersion) -> GenerationVersionResponse:
     """Convert a DB version row to a Pydantic response."""
@@ -185,7 +188,15 @@ def delete_version(version_id: str, db: Session) -> bool:
 
 
 def delete_versions_for_generation(generation_id: str, db: Session) -> int:
-    """Delete all versions for a generation (used when deleting a generation)."""
+    """Delete all versions for a generation (used when deleting a generation).
+
+    This runs as part of a wider cascade — deleting one generation, sweeping
+    failed ones, or deleting a whole profile. A version file the OS won't let
+    us remove (locked by playback on Windows) must not abort that cascade and
+    strand the caller half-deleted, so the row goes regardless and the leaked
+    file is logged. ``delete_version`` keeps raising, because a single
+    user-initiated delete should fail loudly.
+    """
     versions = (
         db.query(DBGenerationVersion)
         .filter_by(generation_id=generation_id)
@@ -195,7 +206,12 @@ def delete_versions_for_generation(generation_id: str, db: Session) -> int:
     for v in versions:
         audio_path = config.resolve_storage_path(v.audio_path)
         if audio_path is not None and audio_path.exists():
+            try:
                 audio_path.unlink()
+            except OSError:
+                logger.warning(
+                    "Could not delete version audio %s; removing the row anyway", audio_path
+                )
         db.delete(v)
         count += 1
     if count > 0:

backend/tests/test_profile_delete_cascade.py

@@ -145,6 +145,18 @@ async def test_delete_profile_clears_references_to_it(db):
     assert db.query(CaptureSettings).filter_by(id=1).one().default_playback_voice_id is None
 
 
+def _lock(monkeypatch, locked: Path) -> None:
+    """Make ``locked`` refuse deletion, the way Windows does during playback."""
+    real_unlink = Path.unlink
+
+    def refuse_locked_file(self, *args, **kwargs):
+        if self == locked:
+            raise OSError("file is in use by another process")
+        return real_unlink(self, *args, **kwargs)
+
+    monkeypatch.setattr(Path, "unlink", refuse_locked_file)
+
+
 @pytest.mark.asyncio
 async def test_delete_profile_survives_unremovable_audio(db, monkeypatch):
     """A file locked by playback must not abort the delete half-way through."""
@@ -153,20 +165,64 @@ async def test_delete_profile_survives_unremovable_audio(db, monkeypatch):
     db.add(Generation(id="gen-1", profile_id="profile-1", text="hello", audio_path=gen_stored))
     db.commit()
 
-    real_unlink = Path.unlink
+    _lock(monkeypatch, gen_file)
-
-    def refuse_locked_file(self, *args, **kwargs):
-        if self == gen_file:
-            raise OSError("file is in use by another process")
-        return real_unlink(self, *args, **kwargs)
-
-    monkeypatch.setattr(Path, "unlink", refuse_locked_file)
 
     assert await profiles.delete_profile("profile-1", db) is True
     assert db.query(VoiceProfile).count() == 0
     assert db.query(Generation).count() == 0
 
 
+@pytest.mark.asyncio
+async def test_delete_profile_survives_unremovable_version_audio(db, monkeypatch):
+    """Same guarantee for version files, which are unlinked one call deeper."""
+    _make_profile(db)
+    _, gen_stored = _write_wav("gen.wav")
+    version_file, version_stored = _write_wav("gen-reverb.wav")
+    db.add(Generation(id="gen-1", profile_id="profile-1", text="hello", audio_path=gen_stored))
+    db.add(
+        GenerationVersion(
+            id="version-1",
+            generation_id="gen-1",
+            label="Reverb",
+            audio_path=version_stored,
+        )
+    )
+    db.commit()
+
+    _lock(monkeypatch, version_file)
+
+    assert await profiles.delete_profile("profile-1", db) is True
+    assert db.query(VoiceProfile).count() == 0
+    assert db.query(Generation).count() == 0
+    assert db.query(GenerationVersion).count() == 0
+
+
+@pytest.mark.asyncio
+async def test_one_locked_file_does_not_strand_the_rest(db, monkeypatch):
+    """The sweep commits per generation, so aborting mid-loop half-deletes."""
+    _make_profile(db)
+    for i in range(3):
+        _, gen_stored = _write_wav(f"gen{i}.wav")
+        _, version_stored = _write_wav(f"gen{i}-take2.wav")
+        db.add(Generation(id=f"gen-{i}", profile_id="profile-1", text="hi", audio_path=gen_stored))
+        db.add(
+            GenerationVersion(
+                id=f"version-{i}",
+                generation_id=f"gen-{i}",
+                label="take-2",
+                audio_path=version_stored,
+            )
+        )
+    db.commit()
+
+    _lock(monkeypatch, config.get_generations_dir() / "gen1-take2.wav")
+
+    assert await profiles.delete_profile("profile-1", db) is True
+    assert db.query(Generation).count() == 0
+    assert db.query(GenerationVersion).count() == 0
+    assert db.query(VoiceProfile).count() == 0
+
+
 @pytest.mark.asyncio
 async def test_delete_profile_leaves_other_profiles_alone(db):
     """Cleanup is scoped to the deleted profile."""