fix(ai): use json_schema instead of json_object for broader provider compatibility (#1033)

LM Studio and some other OpenAI-compatible servers reject
response_format=json_object but accept json_schema. Switch to the
equivalent json_schema format and add a fallback that retries
without response_format when the provider rejects the format type.

AGENTS.md

@@ -137,7 +137,13 @@ npm run test:coverage       # Generate coverage report
 - Dual mode: ComfyUI plugin (folder_paths) vs standalone (settings.json)
 - Detection: `os.environ.get("LORA_MANAGER_STANDALONE", "0") == "1"`
 - Run `python scripts/sync_translation_keys.py` after adding UI strings to `locales/en.json`
-- Symlinks require normalized paths
+- Symlinks require normalized paths.
+  **Business paths vs real paths**: All stored paths and operation routing use the
+  original paths as they appear under configured model roots — symlinks are NOT
+  resolved. `os.path.realpath` is only for scanner dedup and the symlink cache.
+  Any path passed to `os.remove`/`os.rename`/`shutil.move` or validated by a
+  containment check MUST use the business path (i.e. `os.path.abspath`, not
+  `realpath`).
 
 ## Git / Commit Messages
 

py/services/download_manager.py

@@ -1392,8 +1392,8 @@ class DownloadManager:
                 base_save_dir = save_dir
                 save_dir = os.path.join(save_dir, relative_path)
                 # Security: validate path containment after joining
-                resolved_dir = os.path.realpath(os.path.normpath(save_dir))
+                resolved_dir = os.path.abspath(os.path.normpath(save_dir))
-                base_dir = os.path.realpath(os.path.normpath(base_save_dir))
+                base_dir = os.path.abspath(os.path.normpath(base_save_dir))
                 if not resolved_dir.startswith(base_dir + os.sep) and resolved_dir != base_dir:
                     logger.warning(
                         "Path traversal detected: %s escapes %s",

py/services/llm_service.py

@@ -566,18 +566,52 @@ class LLMService:
         if effective_max is None:
             effective_max = 4096
 
-        result = await self.chat_completion(
+        # Use json_schema (not json_object) for broader provider compatibility:
-            messages=messages,
+        # LM Studio and some other OpenAI-compatible servers reject
-            model=model,
+        # json_object but accept json_schema.  {"type": "object"} is
-            temperature=temperature,
+        # functionally equivalent — it accepts any JSON object without
-            response_format={"type": "json_object"},
+        # constraining specific fields.
-            max_tokens=effective_max,
+        response_format = {
-        )
+            "type": "json_schema",
+            "json_schema": {
+                "name": "metadata",
+                "schema": {"type": "object"},
+            },
+        }
+
+        try:
+            result = await self.chat_completion(
+                messages=messages,
+                model=model,
+                temperature=temperature,
+                response_format=response_format,
+                max_tokens=effective_max,
+            )
+        except LLMResponseError as e:
+            # Only fall back when the provider rejects the response_format
+            # type value (e.g. "'response_format.type' must be...").  Avoid
+            # catching unrelated 400 errors whose body happens to mention
+            # "response_format" (e.g. "model does not support
+            # response_format restrictions on this endpoint").
+            if "'response_format.type'" not in str(e).lower():
+                raise
+            logger.info(
+                "Provider rejected response_format, retrying without it. "
+                "Falling back to prompt-only JSON mode. Error: %s",
+                e,
+            )
+            result = await self.chat_completion(
+                messages=messages,
+                model=model,
+                temperature=temperature,
+                response_format=None,
+                max_tokens=effective_max,
+            )
 
         content = result.get("content", "") or ""
         if not content:
             raise LLMResponseError(
-                "LLM returned empty content in json_object mode. "
+                "LLM returned empty content. "
                 f"Raw response: {json.dumps(result)[:500]}"
             )
 

py/services/model_lifecycle_service.py

@@ -51,9 +51,10 @@ async def delete_model_artifacts(
 def _require_path_in_library_roots(file_path: str, scanner, *, label: str = "path") -> None:
     """Raise ``ValueError`` if *file_path* is not inside a configured model root.
 
-    Uses ``os.path.realpath()`` to resolve symlinks before comparing,
+    Uses ``os.path.abspath()`` (NOT ``realpath``) to resolve ``..`` and ``.``
-    so symlink-based escapes are also caught.  Skips when the scanner
+    while preserving symlinks — this keeps the check in business-path space.
-    does not expose ``get_model_roots`` or the list is empty.
+    Skips when the scanner does not expose ``get_model_roots`` or the list
+    is empty.
     """
 
     roots = None
@@ -65,10 +66,10 @@ def _require_path_in_library_roots(file_path: str, scanner, *, label: str = "pat
     if not roots:
         return
 
-    resolved = os.path.realpath(os.path.normpath(file_path))
+    resolved = os.path.abspath(os.path.normpath(file_path))
 
     for root in roots:
-        root_resolved = os.path.realpath(os.path.normpath(root))
+        root_resolved = os.path.abspath(os.path.normpath(root))
         if resolved == root_resolved or resolved.startswith(root_resolved + os.sep):
             return
 

tests/services/test_download_manager_basic.py

@@ -1248,6 +1248,50 @@ def test_relative_path_sanitizes_double_slashes():
     assert relative_path == "SDXL/no tags/Author"
 
 
+def test_download_containment_accepts_symlink_save_dir(tmp_path):
+    """Verify the download path containment check (download_manager.py:1395-1397)
+    accepts save directories reached through user-created symlinks inside the
+    library root — reproducing the symlink scenario from issue #1028."""
+    # Library root with a symlink subdirectory pointing to an external drive
+    lora_root = tmp_path / "loras"
+    lora_root.mkdir()
+
+    external_drive = tmp_path / "external" / "models"
+    external_drive.mkdir(parents=True)
+
+    symlink = lora_root / "Krea 2"
+    symlink.symlink_to(str(external_drive))
+
+    # Simulate a download: base_save_dir = library root,
+    # relative_path = "Krea 2/concept/NewModel"
+    base_save_dir = str(lora_root)
+    save_dir = os.path.join(base_save_dir, "Krea 2", "concept", "NewModel")
+
+    # Replicate the exact containment check from download_manager.py
+    resolved_dir = os.path.abspath(os.path.normpath(save_dir))
+    base_dir = os.path.abspath(os.path.normpath(base_save_dir))
+
+    # Must NOT be rejected — symlinks are legitimate business paths
+    assert resolved_dir.startswith(base_dir + os.sep)
+
+
+def test_download_containment_rejects_dot_dot_traversal(tmp_path):
+    """Verify the download path containment check still blocks ``..`` traversal
+    after the realpath → abspath change."""
+    lora_root = tmp_path / "loras"
+    lora_root.mkdir()
+
+    base_save_dir = str(lora_root)
+    save_dir = os.path.join(base_save_dir, "..", "..", "etc", "passwd")
+
+    resolved_dir = os.path.abspath(os.path.normpath(save_dir))
+    base_dir = os.path.abspath(os.path.normpath(base_save_dir))
+
+    # Must be rejected — dot-dot escapes the library root
+    assert not resolved_dir.startswith(base_dir + os.sep)
+    assert resolved_dir != base_dir
+
+
 def test_distribute_preview_to_entries_moves_and_copies(tmp_path):
     """Test that preview distribution moves file to first entry and copies to others."""
     manager = DownloadManager()

tests/services/test_llm_service.py

@@ -243,6 +243,56 @@ class TestLLMServiceChatCompletionJson:
 
         assert result == {"key": "value"}
 
+    @pytest.mark.asyncio
+    async def test_chat_completion_json_falls_back_on_response_format_rejection(
+        self, llm_service,
+    ):
+        """Retry without response_format when provider rejects it (HTTP 400)."""
+        error_response = MockResponse(
+            400,
+            text_data=(
+                '{"error":"\'response_format.type\' must be '
+                '\'json_schema\' or \'text\'"}'
+            ),
+        )
+        success_response = MockResponse(
+            200,
+            json_data={
+                "choices": [{"message": {"content": '{"key": "value"}'}}],
+                "usage": {},
+                "model": "local-model",
+            },
+        )
+
+        call_index = 0
+
+        class FallbackMockSession:
+            def __init__(self):
+                self.last_url = None
+                self.last_json = None
+
+            def post(self, url, json=None, headers=None):
+                nonlocal call_index
+                self.last_url = url
+                self.last_json = json
+                call_index += 1
+                return error_response if call_index == 1 else success_response
+
+            async def __aenter__(self):
+                return self
+
+            async def __aexit__(self, *args):
+                pass
+
+        with mock.patch("aiohttp.ClientSession", return_value=FallbackMockSession()):
+            result = await llm_service.chat_completion_json(
+                system_prompt="You are helpful.",
+                user_prompt="Return JSON.",
+            )
+
+        assert result == {"key": "value"}
+        assert call_index == 2
+
     @pytest.mark.asyncio
     async def test_chat_completion_json_raises_on_non_json(self, llm_service):
         # Non-JSON content raises LLMResponseError (salvage also fails)

tests/services/test_model_lifecycle_service.py

@@ -1,4 +1,5 @@
 import json
+import os
 from pathlib import Path
 
 import pytest
@@ -51,11 +52,12 @@ class TestRequirePathInLibraryRoots:
         scanner = ScannerWithRoots([str(root)])
         _require_path_in_library_roots(str(root), scanner)
 
-    def test_rejects_symlink_escape(self, tmp_path):
+    def test_accepts_symlink_within_root(self, tmp_path):
+        """Symlinks under a configured root are legitimate business paths
+        and should be accepted — containment works on business-path space,
+        not resolved physical paths."""
         root = tmp_path / "loras"
         root.mkdir()
-        model = root / "model.safetensors"
-        model.write_text("")
 
         outside_dir = tmp_path / "outside"
         outside_dir.mkdir()
@@ -65,9 +67,22 @@ class TestRequirePathInLibraryRoots:
         symlink = root / "link.safetensors"
         symlink.symlink_to(outside_file)
 
+        scanner = ScannerWithRoots([str(root)])
+        # Symlink path is under root in business-path space → accepted
+        _require_path_in_library_roots(str(symlink), scanner)
+
+    def test_rejects_dot_dot_traversal(self, tmp_path):
+        """Verify that ``..`` components are still resolved and blocked —
+        ``abspath`` normalises dot-dot but does not resolve symlinks."""
+        root = tmp_path / "loras"
+        root.mkdir()
+
+        # A path that traverses up out of the root via ..
+        escaped = os.path.join(str(root), "..", "..", "etc", "passwd")
+
         scanner = ScannerWithRoots([str(root)])
         with pytest.raises(ValueError, match="outside configured library"):
-            _require_path_in_library_roots(str(symlink), scanner)
+            _require_path_in_library_roots(escaped, scanner)
 
 
 class ScannerForDelete:

web/comfyui/loras_widget.js

@@ -751,7 +751,11 @@ export function addLorasWidget(node, name, opts, callback) {
         }
       }
 
-      renderLoras(widgetValue, widget);
+      // Skip DOM re-render during drag to preserve pointer capture and event listeners.
+      // The strength inputs are updated directly via the pointermove handler instead.
+      if (!widget.__dragActive) {
+        renderLoras(widgetValue, widget);
+      }
     },
     hideOnZoom: true,
     selectOn: ['click', 'focus']

web/comfyui/loras_widget_events.js

@@ -37,17 +37,18 @@ export function handleStrengthDrag(name, initialStrength, initialX, event, widge
       syncClipStrengthIfCollapsed(lorasData[loraIndex]);
     }
     
-    // Update the widget value only if updateWidget flag is true
+    // Always write back to widget.value to persist the mutation.
-    // This allows us to update inputs directly during drag without triggering re-render
+    // During drag (updateWidget=false), setValue skips renderLoras via __dragActive flag,
-    if (updateWidget) {
+    // so the DOM survives and pointer capture is preserved.
-      widget.value = formatLoraValue(lorasData);
+    widget.value = formatLoraValue(lorasData);
-    }
     
-    // Force re-render via callback only if updateWidget is true
+    // Only fire callback on the final commit, not during drag
     if (updateWidget && widget.callback) {
       widget.callback(widget.value);
     }
   }
+  
+  return newStrength;
 }
 
 // Function to handle proportional strength adjustment for all LoRAs via header dragging
@@ -90,12 +91,11 @@ export function handleAllStrengthsDrag(initialStrengths, initialX, event, widget
     lorasData[index].clipStrength = Number(newClipStrength);
   });
   
-  // Update widget value only if updateWidget flag is true
+  // Always write back to widget.value to persist mutations.
-  if (updateWidget) {
+  // During drag (updateWidget=false), setValue skips renderLoras via __dragActive flag.
-    widget.value = formatLoraValue(lorasData);
+  widget.value = formatLoraValue(lorasData);
-  }
   
-  // Force re-render via callback only if updateWidget is true
+  // Only fire callback on the final commit, not during drag
   if (updateWidget && widget.callback) {
     widget.callback(widget.value);
   }
@@ -149,6 +149,13 @@ export function initDrag(
     activePointerId = e.pointerId;
     currentDragElement = e.currentTarget;
 
+    // Suppress renderLoras in setValue during drag so the DOM survives.
+    // The getter creates a new array on every read, so mutations to a
+    // parsed copy are lost unless we write back through widget.value.
+    // Writing back would normally trigger a full DOM re-render via setValue,
+    // destroying pointer capture. __dragActive tells setValue to skip the render.
+    widget.__dragActive = true;
+
     // Capture pointer to receive all subsequent events regardless of stopPropagation
     const target = e.currentTarget;
     target.setPointerCapture(e.pointerId);
@@ -181,17 +188,12 @@ export function initDrag(
     }
 
     // Call the strength adjustment function without updating widget.value during drag
-    handleStrengthDrag(name, initialStrength, initialX, e, widget, isClipStrength, false);
+    const newStrength = handleStrengthDrag(name, initialStrength, initialX, e, widget, isClipStrength, false);
     
     // Update strength input directly instead of re-rendering to avoid losing event listeners
     const strengthInput = currentDragElement.querySelector('.lm-lora-strength-input');
-    if (strengthInput) {
+    if (strengthInput && typeof newStrength === 'number') {
-      const lorasData = parseLoraValue(widget.value);
+      strengthInput.value = newStrength.toFixed(2);
-      const loraData = lorasData.find(l => l.name === name);
-      if (loraData) {
-        const strengthValue = isClipStrength ? loraData.clipStrength : loraData.strength;
-        strengthInput.value = Number(strengthValue).toFixed(2);
-      }
     }
     
     // Prevent showing the preview tooltip during drag
@@ -226,23 +228,30 @@ export function initDrag(
     // Remove the class to restore normal cursor behavior
     document.body.classList.remove('lm-lora-strength-dragging');
 
-    // Only call onDragEnd and re-render if we actually dragged
+    // Only call onDragEnd and re-render if we actually dragged.
-    if (wasDragging) {
+    // try-finally guarantees __dragActive is always cleared, preventing a
-      if (typeof onDragEnd === 'function') {
+    // permanent UI freeze if onDragEnd or setValue throws during cleanup.
-        onDragEnd();
+    try {
-      }
+      if (wasDragging) {
+        if (typeof onDragEnd === 'function') {
+          onDragEnd();
+        }
 
-      // Commit final value through options.setValue so external observers are notified.
+        // Re-enable renderLoras in setValue and flush final value through setter.
-      // During drag, handleStrengthDrag mutates widgetValue in-place (updateWidget=false),
+        // The last handleStrengthDrag call already wrote the final strength to
-      // bypassing widget.value setter and options.setValue entirely. This assignment
+        // widgetValue via setValue (with render suppressed). widget.value = widget.value
-      // flushes the in-place mutation through the setter so any setValue wrappers fire.
+        // triggers setValue again, which now calls renderLoras since __dragActive is false.
-      widget.value = widget.value;
+        widget.__dragActive = false;
-      if (typeof widget.callback === 'function') {
+        widget.value = widget.value;
-        widget.callback(widget.value);
+        if (typeof widget.callback === 'function') {
+          widget.callback(widget.value);
+        }
       }
+    } finally {
+      widget.__dragActive = false;
     }
   };
-  
+
   dragEl.addEventListener('pointerup', endDrag);
   dragEl.addEventListener('pointercancel', endDrag);
 }
@@ -285,6 +294,9 @@ export function initHeaderDrag(headerEl, widget, renderFunction) {
     activePointerId = e.pointerId;
     currentHeaderElement = e.currentTarget;
 
+    // Suppress renderLoras in setValue during drag (see initDrag for rationale)
+    widget.__dragActive = true;
+
     // Capture pointer to receive all subsequent events regardless of stopPropagation
     const target = e.currentTarget;
     target.setPointerCapture(e.pointerId);
@@ -352,13 +364,20 @@ export function initHeaderDrag(headerEl, widget, renderFunction) {
     // Remove the class to restore normal cursor behavior
     document.body.classList.remove('lm-lora-strength-dragging');
 
-    // Only re-render if we actually dragged
+    // Only re-render if we actually dragged.
-    if (wasDragging) {
+    // try-finally guarantees __dragActive is always cleared, preventing a
-      // Commit final value through options.setValue so external observers are notified.
+    // permanent UI freeze if setValue throws during cleanup.
-      widget.value = widget.value;
+    try {
-      if (typeof widget.callback === 'function') {
+      if (wasDragging) {
-        widget.callback(widget.value);
+        // Re-enable renderLoras in setValue and flush final value through setter
+        widget.__dragActive = false;
+        widget.value = widget.value;
+        if (typeof widget.callback === 'function') {
+          widget.callback(widget.value);
+        }
       }
+    } finally {
+      widget.__dragActive = false;
     }
   };