fix(gallery): remove image file only after the delete commit succeeds (#2196)

delete_gallery_image() deleted the on-disk file before setting
is_active=False and committing. If that commit failed and rolled back,
the record stayed active but its file was already gone — a broken,
unviewable image (data loss).

Soft-delete and commit first, then remove the file best-effort, so a
missing or locked file can no longer 500 a delete that already succeeded
logically.

Adds tests/test_gallery_delete_file_ordering.py covering the
commit-failure (file kept) and success (file removed) paths.

routes/gallery_routes.py

@@ -931,15 +931,23 @@ def setup_gallery_routes() -> APIRouter:
                 raise HTTPException(404, "Image not found")
 
             img_filename = img.filename
-            # Remove the file from disk
-            img_path = _gallery_image_path(img_filename)
-            if img_path.exists():
-                img_path.unlink()
-
-            # Soft-delete the record
+            # Soft-delete the record first; the DB is the source of truth.
             img.is_active = False
             db.commit()
 
+            # Only after the soft-delete commit succeeds do we remove the file.
+            # If the file were deleted first and the commit then failed/rolled
+            # back, the still-active record would point at a missing file.
+            # Best-effort so a missing or locked file can't 500 a delete that
+            # already succeeded logically. Uses the path-confined resolver so a
+            # malformed stored filename can't escape generated_images.
+            try:
+                img_path = _gallery_image_path(img_filename)
+                if img_path.exists():
+                    img_path.unlink()
+            except Exception as e:
+                logger.warning(f"Could not remove gallery image file for {img_filename}: {e}")
+
             # Strip stale chat-history references so the image bubble
             # (and its prompt caption) doesn't come back after a server
             # reboot replays the session. We remove the matching tool

tests/test_gallery_delete_file_ordering.py

@@ -0,0 +1,83 @@
+"""Regression: deleting a gallery image must not remove the file before the DB
+commit succeeds.
+
+delete_gallery_image() removed the on-disk file first and only then set
+is_active=False and committed. If that commit failed and rolled back, the record
+stayed active but its file was already gone — a broken, unviewable image (data
+loss). The file is now removed only after the soft-delete commit succeeds, and
+best-effort so a missing/locked file can't fail an otherwise-successful delete.
+"""
+import asyncio
+
+import pytest
+from fastapi import HTTPException, Request
+from sqlalchemy import create_engine
+from sqlalchemy.orm import sessionmaker
+
+from core.database import Base, GalleryImage
+import routes.gallery_routes as gallery_routes
+
+
+def _delete_endpoint():
+    router = gallery_routes.setup_gallery_routes()
+    for route in router.routes:
+        if getattr(route, "path", "") == "/api/gallery/{image_id}" and "DELETE" in getattr(route, "methods", set()):
+            return route.endpoint
+    raise AssertionError("DELETE /api/gallery/{image_id} endpoint not found")
+
+
+def _seed(tmp_path):
+    engine = create_engine("sqlite:///:memory:")
+    Base.metadata.create_all(bind=engine)
+    SessionLocal = sessionmaker(bind=engine)
+    db = SessionLocal()
+    db.add(GalleryImage(id="img-1", filename="x.png", owner="alice", is_active=True))
+    db.commit()
+    db.close()
+    img_dir = tmp_path / "data" / "generated_images"
+    img_dir.mkdir(parents=True)
+    (img_dir / "x.png").write_bytes(b"image-bytes")
+    return SessionLocal
+
+
+def test_file_kept_when_commit_fails(tmp_path, monkeypatch):
+    monkeypatch.chdir(tmp_path)
+    SessionLocal = _seed(tmp_path)
+    monkeypatch.setattr(gallery_routes, "get_current_user", lambda r: "alice")
+
+    # A session whose commit always fails, to simulate a DB error mid-delete.
+    sess = SessionLocal()
+
+    def _boom():
+        raise RuntimeError("commit failed")
+
+    monkeypatch.setattr(sess, "commit", _boom)
+    monkeypatch.setattr(gallery_routes, "SessionLocal", lambda: sess)
+
+    delete = _delete_endpoint()
+    with pytest.raises(HTTPException):
+        asyncio.run(delete(Request(scope={"type": "http"}), "img-1"))
+
+    # File must survive a failed commit — the record is still active after rollback.
+    assert (tmp_path / "data" / "generated_images" / "x.png").exists()
+    check = SessionLocal()
+    row = check.query(GalleryImage).filter(GalleryImage.id == "img-1").first()
+    assert row.is_active is True
+    check.close()
+
+
+def test_file_removed_on_successful_delete(tmp_path, monkeypatch):
+    monkeypatch.chdir(tmp_path)
+    SessionLocal = _seed(tmp_path)
+    monkeypatch.setattr(gallery_routes, "get_current_user", lambda r: "alice")
+    monkeypatch.setattr(gallery_routes, "SessionLocal", SessionLocal)
+
+    delete = _delete_endpoint()
+    result = asyncio.run(delete(Request(scope={"type": "http"}), "img-1"))
+
+    assert result["status"] == "deleted"
+    assert not (tmp_path / "data" / "generated_images" / "x.png").exists()
+    check = SessionLocal()
+    row = check.query(GalleryImage).filter(GalleryImage.id == "img-1").first()
+    assert row.is_active is False
+    check.close()