fix: network-share readers, model unload off the loop, bad save requests (#620)

Every read-only SQLite open now uses one URI helper, so a library on a Windows
network share works in backups, restores, moves and history reads. Unloading a
model no longer blocks the server. A malformed /save body gets a 400, not a 500.

database/health.py

@@ -16,18 +16,17 @@ check, and none of them carries a server path.
 
 import datetime
 import logging
-import ntpath
 import os
 import sqlite3
-from pathlib import PurePosixPath, PureWindowsPath
-from urllib.parse import quote
 
 try:
     from ..utils.hashing import register_prompt_functions
     from .models import normalize_image_path
+    from .sqlite_uri import readonly_uri
 except ImportError:
     from utils.hashing import register_prompt_functions
     from database.models import normalize_image_path
+    from database.sqlite_uri import readonly_uri
 
 logger = logging.getLogger("prompt_manager.database.health")
 
@@ -89,25 +88,6 @@ def _counted(lines):
     return _result(count, detail)
 
 
-def readonly_uri(db_path, *, windows=None):
-    """The ``mode=ro`` SQLite URI of *db_path*, on this platform or the given one.
-
-    pathlib spells a UNC path ``\\\\server\\share\\x.db`` as
-    ``file://server/share/x.db``, a remote authority SQLite refuses to open.
-    With an empty authority (``file:////server/share/x.db``) SQLite hands
-    Windows the ``//server/share`` path, which it opens as the UNC path.
-    """
-    if windows is None:
-        windows = os.name == "nt"
-        db_path = os.path.abspath(db_path)
-    if not windows:
-        return PurePosixPath(db_path).as_uri() + "?mode=ro"
-    path = PureWindowsPath(db_path)
-    if ntpath.splitdrive(str(path))[0].startswith("\\\\"):
-        return "file:" + quote("//" + path.as_posix()) + "?mode=ro"
-    return path.as_uri() + "?mode=ro"
-
-
 def _integrity(conn, _roots):
     rows = [
         row[0] for row in conn.execute(f"PRAGMA integrity_check({INTEGRITY_LIMIT})")

database/library_move.py

@@ -47,11 +47,11 @@ import sqlite3
 import time
 import types
 import uuid
-from pathlib import Path
 from dataclasses import dataclass, field
 from typing import Callable, Dict, List, Optional, Tuple
 
 from .models import BUSY_TIMEOUT_SECONDS, PromptModel
+from .sqlite_uri import readonly_uri
 
 try:
     from ..utils import library_location as ll
@@ -322,7 +322,7 @@ def _verify_copy(temp: str, counts, documents) -> None:
             "verify-failed",
             f"{MESSAGES['verify-failed']} {detail}" if detail else None,
         )
-    uri = Path(os.path.abspath(temp)).as_uri() + "?mode=ro"
+    uri = readonly_uri(temp)
     try:
         with contextlib.closing(sqlite3.connect(uri, uri=True)) as conn:
             copied = _describe(conn)

database/models.py

@@ -14,6 +14,7 @@ from contextlib import closing
 from pathlib import Path
 from typing import Optional, Set, Tuple
 
+from .sqlite_uri import readonly_uri
 from .managed_connection import (
     ManagedConnection,
     await_closed,
@@ -1199,7 +1200,7 @@ class PromptModel:
         if header != SQLITE_HEADER:
             return False, "Not a SQLite database file"
 
-        uri = Path(os.path.abspath(path)).as_uri() + "?mode=ro"
+        uri = readonly_uri(path)
         try:
             conn = sqlite3.connect(uri, uri=True)
         except sqlite3.Error as e:
@@ -1306,9 +1307,7 @@ class PromptModel:
         replacement (for example, incompatible page sizes in WAL mode),
         propagate the error rather than copy files under other open owners.
         """
-        source = sqlite3.connect(
-            Path(src_path).resolve().as_uri() + "?mode=ro", uri=True
-        )
+        source = sqlite3.connect(readonly_uri(Path(src_path).resolve()), uri=True)
         try:
             target = self.get_connection()
             # The target is written under backup's step too: mark it in use

database/restore_stage.py

@@ -42,6 +42,7 @@ from typing import Dict, Optional
 from . import workflow_documents as wd
 from .execution_history_schema import rotate_history_epoch
 from .safety_backup import take_safety_backup
+from .sqlite_uri import readonly_uri
 
 logger = logging.getLogger("prompt_manager.database.restore_stage")
 
@@ -114,7 +115,7 @@ def file_digest(path: str) -> str:
 
 
 def _readonly(path: str) -> sqlite3.Connection:
-    return sqlite3.connect(Path(path).resolve().as_uri() + "?mode=ro", uri=True)
+    return sqlite3.connect(readonly_uri(Path(path).resolve()), uri=True)
 
 
 def _schema(path: str) -> list:

database/sqlite_uri.py

@@ -0,0 +1,25 @@
+"""The read-only SQLite URI of a path, safe for Windows network shares."""
+
+import ntpath
+import os
+from pathlib import PurePosixPath, PureWindowsPath
+from urllib.parse import quote
+
+
+def readonly_uri(db_path, *, windows=None):
+    """The ``mode=ro`` SQLite URI of *db_path*, on this platform or the given one.
+
+    pathlib spells a UNC path ``\\\\server\\share\\x.db`` as
+    ``file://server/share/x.db``, a remote authority SQLite refuses to open.
+    With an empty authority (``file:////server/share/x.db``) SQLite hands
+    Windows the ``//server/share`` path, which it opens as the UNC path.
+    """
+    if windows is None:
+        windows = os.name == "nt"
+        db_path = os.path.abspath(db_path)
+    if not windows:
+        return PurePosixPath(db_path).as_uri() + "?mode=ro"
+    path = PureWindowsPath(db_path)
+    if ntpath.splitdrive(str(path))[0].startswith("\\\\"):
+        return "file:" + quote("//" + path.as_posix()) + "?mode=ro"
+    return path.as_uri() + "?mode=ro"

py/api/admin.py

@@ -30,8 +30,10 @@ from .jobs import (
 )
 
 try:
+    from ...database.sqlite_uri import readonly_uri
     from ...utils.parallel import map_parallel
 except ImportError:
+    from database.sqlite_uri import readonly_uri
     from utils.parallel import map_parallel
 
 try:
@@ -399,7 +401,7 @@ def _inspect_restore_upload(path):
     Returns:
         None when acceptable, otherwise a human-readable reason (blocking).
     """
-    uri = Path(os.path.abspath(path)).as_uri() + "?mode=ro"
+    uri = readonly_uri(path)
     try:
         conn = sqlite3.connect(uri, uri=True)
     except sqlite3.Error as e:

py/api/autotag_routes.py

@@ -1411,7 +1411,8 @@ class AutotagRoutesMixin:
                     )
 
                 model_type = service.get_loaded_model_type()
-                service.unload_model()
+                # Off the loop; a dropped request keeps the gate until it ends.
+                await model_gate.run(owner, service.unload_model)
             finally:
                 model_gate.release(owner)
 

py/api/backup_restore.py

@@ -45,7 +45,6 @@ import os
 import sqlite3
 import tempfile
 from contextlib import closing
-from pathlib import Path
 
 from aiohttp import web
 
@@ -56,6 +55,7 @@ from .maintenance_plans import PlanExpired, PlanUsed
 from .restore_plans import RestorePlanStore
 
 try:
+    from ...database.sqlite_uri import readonly_uri
     from ...database import restore_stage
     from ...database.operations import PromptDatabase
     from ...database.safety_backup import (
@@ -66,6 +66,7 @@ try:
         safety_temp,
     )
 except ImportError:
+    from database.sqlite_uri import readonly_uri
     from database import restore_stage
     from database.operations import PromptDatabase
     from database.safety_backup import (
@@ -186,7 +187,7 @@ def snapshot_corruption(path):
         with open(path, "rb") as handle:
             if handle.read(len(SQLITE_HEADER)) != SQLITE_HEADER:
                 return "Not a SQLite database file"
-        uri = Path(os.path.abspath(path)).as_uri() + "?mode=ro"
+        uri = readonly_uri(path)
         with closing(sqlite3.connect(uri, uri=True)) as conn:
             row = conn.execute("PRAGMA integrity_check").fetchone()
     except (OSError, sqlite3.Error) as error:

py/api/prompts.py

@@ -727,7 +727,15 @@ class PromptRoutesMixin:
     async def save_prompt(self, request):
         """Save a new prompt with metadata and duplicate detection."""
         try:
-            data = await request.json()
+            data, error = await self._read_json_body(request)
+            if error is not None:
+                return error
+            for field in ("text", "category", "notes"):
+                if data.get(field) is not None and not isinstance(data[field], str):
+                    return web.json_response(
+                        {"success": False, "error": f"{field} must be text"},
+                        status=400,
+                    )
 
             text = (data.get("text") or "").strip()
             if not text:

tests/test_api_autotag.py

@@ -732,6 +732,32 @@ class TestAutotagErrorBodiesHideServerPaths(AutotagAPITestCase):
 
 class TestUnloadAutotag(AutotagAPITestCase):
 
+    async def test_unload_runs_off_the_loop_and_holds_the_gate_until_done(self):
+        started = threading.Event()
+        release = threading.Event()
+        self.addCleanup(release.set)
+        threads = []
+
+        def unload():
+            threads.append(threading.get_ident())
+            started.set()
+            release.wait(5)
+            self.service._loaded_type = None
+
+        self.service._loaded_type = "gguf"
+        self.service.unload_model = unload
+        first = asyncio.ensure_future(
+            self.client.request("POST", "/prompt_manager/autotag/unload")
+        )
+        self.assertTrue(await asyncio.to_thread(started.wait, 5))
+        self.assertNotEqual(threads, [threading.get_ident()])
+        second = await self.client.request("POST", "/prompt_manager/autotag/unload")
+        self.assertEqual(second.status, 409)
+        release.set()
+        data = await (await first).json()
+        self.assertEqual(data["message"], "GGUF model unloaded successfully")
+        self.assertIsNone(model_gate.holder())
+
     async def test_nothing_loaded(self):
         resp = await self.client.request("POST", "/prompt_manager/autotag/unload")
         data = await resp.json()

tests/test_api_prompts.py

@@ -684,6 +684,28 @@ class TestTagMutationRoutes(RouteCoverageCase):
 
 class TestSaveRoute(RouteCoverageCase):
 
+    async def test_a_malformed_body_is_refused_and_saves_nothing(self):
+        for body in (
+            [],
+            "text",
+            {"text": 123},
+            {"text": "fixture", "category": 123},
+            {"text": "fixture", "notes": 123},
+            {"text": "fixture", "tags": 123},
+            {"text": "fixture", "rating": "high"},
+        ):
+            with self.subTest(body=body):
+                status, data = await self._json(
+                    "POST", "/prompt_manager/save", json=body
+                )
+                self.assertEqual(status, 400)
+                self.assertFalse(data["success"])
+        status, data = await self._json(
+            "POST", "/prompt_manager/save", data=b"{not json"
+        )
+        self.assertEqual(status, 400)
+        self.assertEqual(self.api.db.search_prompts(text="fixture"), [])
+
     async def test_invalid_rating_is_400(self):
         status, data = await self._json(
             "POST", "/prompt_manager/save", json={"text": "ok", "rating": 99}

tests/test_sqlite_uri.py

@@ -0,0 +1,39 @@
+"""Every read-only SQLite open uses one URI helper (#620).
+
+pathlib writes a Windows network path as ``file://server/share/x.db``, which
+SQLite refuses. A reader that builds its own URI breaks backups, restores,
+library moves or history reads for a library on a network share.
+"""
+
+import re
+import sys
+import unittest
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parents[1]
+sys.path.insert(0, str(ROOT))
+
+from database.sqlite_uri import readonly_uri  # noqa: E402
+
+
+class TestReadonlyUri(unittest.TestCase):
+    def test_a_network_path_has_an_empty_authority(self):
+        self.assertEqual(
+            readonly_uri(r"\\server\share\Comfy UI\prompts.db", windows=True),
+            "file:////server/share/Comfy%20UI/prompts.db?mode=ro",
+        )
+
+    def test_no_reader_builds_its_own_uri(self):
+        offenders = []
+        for folder in ("database", "py", "utils"):
+            for path in (ROOT / folder).rglob("*.py"):
+                if path.name == "sqlite_uri.py":
+                    continue
+                text = path.read_text(encoding="utf-8")
+                if re.search(r"as_uri\(\)\s*\+\s*[\"']\?mode=ro", text):
+                    offenders.append(path.relative_to(ROOT).as_posix())
+        self.assertEqual(offenders, [])
+
+
+if __name__ == "__main__":
+    unittest.main()

utils/history_read_fence.py

@@ -1,7 +1,6 @@
 """Restore publication checks for snapshot-owned history reads."""
 
 import os
-from pathlib import Path
 import sqlite3
 import threading
 from typing import TYPE_CHECKING
@@ -10,11 +9,13 @@ if TYPE_CHECKING:
     from ..database.operations import PromptDatabase
 
 try:
+    from ..database.sqlite_uri import readonly_uri
     from ..database.execution_history import (
         HistoryEpochChanged,
         subscribe_history_restore,
     )
 except ImportError:
+    from database.sqlite_uri import readonly_uri
     from database.execution_history import (
         HistoryEpochChanged,
         subscribe_history_restore,
@@ -53,9 +54,7 @@ class HistoryReadFence:
             self._check(generation, epoch)
         # An API restore retires its previous model. Never reopen that owner,
         # and never reuse the projection transaction's view of the epoch.
-        conn = sqlite3.connect(
-            Path(self._path).as_uri() + "?mode=ro", uri=True, timeout=0.25
-        )
+        conn = sqlite3.connect(readonly_uri(self._path), uri=True, timeout=0.25)
         try:
             row = conn.execute(
                 "SELECT storage_epoch FROM execution_history_metadata "