fix(jellyfin): inline-validate the off-media config folder field (like Plex) (#275)

The off-media "Jellyfin config folder" settings field had no inline validation
— only the Setup Health tab checked it (#274). The Plex config-folder field
validates live via /api/settings/validate-plex-config-folder; this adds the
Jellyfin equivalent so the field shows valid/invalid as you type.

- New POST /api/settings/validate-jellyfin-config-folder (mirrors the Plex
  one): {exists, valid_jellyfin_structure, writable, detail, error}. Unlike
  Plex it isn't restricted to a fixed root (Jellyfin config is an arbitrary
  admin mount); admin-only, read-only stat, null-byte guarded.
- Extracted looks_like_jellyfin_config_dir()/jellyfin_config_data_marker()
  into output/jellyfin_trickplay.py and pointed BOTH the Setup Health probe
  and the new endpoint at them, so the field validator and the health check
  can never disagree.
- servers.js: the editJellyfinConfigFolder field is wired to the live
  validator (bound once; validates on input, on browse-pick, and on open when
  off-media is on) and routed to the new endpoint with a "Valid Jellyfin
  config folder" message. servers.html: added the invalid/valid feedback spans
  + has-validation to the field's input-group so messages render.

Tests: endpoint valid-via-data, valid-via-plugins, not-found, wrong-folder
(media dir) -> error, empty -> neutral, valid-but-read-only -> flagged.
Architecture Review run; the MED (read-only branch coverage) fixed before commit.


Claude-Session: https://claude.ai/code/session_01YC1Z1JFBVJ5xKi5YAqYKfc

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: stevezau

media_preview_generator/output/jellyfin_trickplay.py

@@ -79,6 +79,35 @@
 # — so a future Jellyfin path move is picked up automatically.
 _DEFAULT_TRICKPLAY_ROOT = "data/trickplay"
 
+# Markers that identify a directory as Jellyfin's config dir (vs a media folder
+# or unrelated path). The off-media write target's first segment (``data`` by
+# default) is the primary one; the rest cover both the official and linuxserver
+# images, before or after first boot. Any ONE present is enough.
+_JELLYFIN_CONFIG_MARKERS = ("plugins", "config", ".jellyfin-data", "system.xml")
+
+
+def jellyfin_config_data_marker(trickplay_root: str = _DEFAULT_TRICKPLAY_ROOT) -> str:
+    """First path segment of the off-media trickplay root — the ``data`` dir
+    off-media trickplay is written into (``<config>/<data_marker>/trickplay``).
+    """
+    return (trickplay_root or _DEFAULT_TRICKPLAY_ROOT).replace("\\", "/").split("/", 1)[0] or "data"
+
+
+def looks_like_jellyfin_config_dir(folder: str, trickplay_root: str = _DEFAULT_TRICKPLAY_ROOT) -> bool:
+    """Best-effort check that ``folder`` is Jellyfin's config directory.
+
+    Off-media trickplay writes into ``<folder>/<trickplay_root>``; a real
+    Jellyfin config dir (official or linuxserver image) holds that root's first
+    segment (``data`` by default) plus ``plugins``/``config``. Any one marker is
+    enough — the goal is to reject a media folder or unrelated path, not to be
+    exhaustive. Shared by the Setup Health probe and the inline field validator
+    so they can never disagree. Mirrors Plex's Media/localhost structural check.
+    """
+    if not folder or not os.path.isdir(folder):
+        return False
+    markers = (jellyfin_config_data_marker(trickplay_root), *_JELLYFIN_CONFIG_MARKERS)
+    return any(os.path.exists(os.path.join(folder, marker)) for marker in markers)
+
 
 def _normalize_item_guid(item_id: str) -> str:
     """Normalise a Jellyfin item id to its dashed-lowercase ``"D"`` form.

media_preview_generator/servers/jellyfin.py

@@ -1765,16 +1765,19 @@ def previews_readiness(self) -> dict[str, Any]:
                 # trickplay writes into (<config>/<trickplay_root>/…), plus
                 # plugins/ and config/. Pointing this at a media folder or an
                 # unrelated path is the common mistake — catch it here rather
-                # than silently writing tiles Jellyfin will never find. (It
-                # can't catch pointing one level too deep, e.g. <config>/data,
-                # since that still nests data/ + plugins/ — the markers below
-                # would still match; only a wrong/unrelated root is rejected.)
-                trickplay_root = (self.offmedia_trickplay_root or "data/trickplay").replace("\\", "/")
-                data_marker = trickplay_root.split("/", 1)[0] or "data"
-                jellyfin_markers = (data_marker, "plugins", "config", ".jellyfin-data", "system.xml")
-                looks_like_jellyfin = exists and any(
-                    os.path.exists(os.path.join(config_folder, marker)) for marker in jellyfin_markers
+                # than silently writing tiles Jellyfin will never find. Shared
+                # with the inline field validator via looks_like_jellyfin_config_dir
+                # so the two can never disagree. (It can't catch pointing one
+                # level too deep, e.g. <config>/data, since that still nests
+                # data/ + plugins/ — only a wrong/unrelated root is rejected.)
+                from ..output.jellyfin_trickplay import (
+                    jellyfin_config_data_marker,
+                    looks_like_jellyfin_config_dir,
                 )
+
+                _root = self.offmedia_trickplay_root or "data/trickplay"
+                data_marker = jellyfin_config_data_marker(_root)
+                looks_like_jellyfin = looks_like_jellyfin_config_dir(config_folder, _root)
                 if not exists:
                     folder_ok = False
                     folder_current = "missing"

media_preview_generator/web/routes/api_settings.py

@@ -915,6 +915,71 @@ def validate_plex_config_folder():
     )
 
 
+@api.route("/settings/validate-jellyfin-config-folder", methods=["POST"])
+@setup_or_auth_required
+def validate_jellyfin_config_folder():
+    """Inline check that a path looks like a real Jellyfin config folder.
+
+    The Jellyfin analog of :func:`validate_plex_config_folder` — used by the
+    Servers > Edit Jellyfin > General tab to give the user a confident "yes
+    this is the right folder" on the off-media config-folder field, instead of
+    a bare "directory exists". Shares the marker logic with the Setup Health
+    probe (``looks_like_jellyfin_config_dir``) so the two never disagree.
+
+    Unlike Plex (restricted to ``PLEX_DATA_ROOT``), the Jellyfin config dir is
+    an arbitrary admin-chosen bind mount (e.g. ``/jellyfin-config``), so there
+    is no fixed allowed root — we stat the typed path directly. Admin-only.
+
+    Request JSON: ``{"path": "/jellyfin-config"}``
+    Returns: ``{"exists": bool, "valid_jellyfin_structure": bool,
+                "writable": bool, "detail": str, "error": str|null}``
+    """
+    from ...output.jellyfin_trickplay import jellyfin_config_data_marker, looks_like_jellyfin_config_dir
+
+    def _resp(*, exists=False, valid=False, writable=False, detail="", error=None):
+        return jsonify(
+            {
+                "exists": exists,
+                "valid_jellyfin_structure": valid,
+                "writable": writable,
+                "detail": detail,
+                "error": error,
+            }
+        )
+
+    data = request.get_json() or {}
+    raw_path = (data.get("path") or "").strip()
+    if not raw_path:
+        return _resp()
+    if "\x00" in raw_path:
+        return _resp(error="Invalid path")
+
+    probe = os.path.realpath(os.path.normpath(raw_path))
+    if not os.path.isdir(probe):
+        return _resp(error="Folder not found in this container")
+
+    writable = os.access(probe, os.W_OK)
+    marker = jellyfin_config_data_marker()
+    if not looks_like_jellyfin_config_dir(probe):
+        return _resp(
+            exists=True,
+            writable=writable,
+            error=(
+                f"This doesn't look like Jellyfin's config dir — no '{marker}/', 'plugins/' or "
+                "'config/' inside. Point it at the folder mounted into Jellyfin as /config "
+                f"(containing '{marker}/'), not a media folder or unrelated path."
+            ),
+        )
+    if not writable:
+        return _resp(
+            exists=True,
+            valid=True,
+            writable=False,
+            error="Folder is not writable by this container — check the mount isn't :ro and PUID/PGID",
+        )
+    return _resp(exists=True, valid=True, writable=True, detail="valid Jellyfin config folder")
+
+
 # ============================================================================
 # Setup Wizard
 # ============================================================================

media_preview_generator/web/static/js/servers.js

@@ -1257,7 +1257,20 @@
         const configFolderGroup = document.getElementById('editJellyfinConfigFolderGroup');
         const configFolderInput = document.getElementById('editJellyfinConfigFolder');
         if (offMediaToggle) offMediaToggle.checked = saveOffMedia;
-        if (configFolderInput) configFolderInput.value = jfOutput.jellyfin_config_folder || '';
+        if (configFolderInput) {
+            configFolderInput.value = jfOutput.jellyfin_config_folder || '';
+            configFolderInput.classList.remove('is-valid', 'is-invalid');
+            // Bind the inline structural validator once (mirrors the Plex
+            // config-folder field) so the path is checked live as you type.
+            if (!configFolderInput.dataset.validatorBound) {
+                configFolderInput.addEventListener('input', _debouncedValidatePath(configFolderInput));
+                configFolderInput.dataset.validatorBound = '1';
+            }
+            // Only validate up-front when off-media is actually on (the field is
+            // visible) — avoids a fetch against a hidden field for a server that
+            // has a stale stored config folder but off-media disabled.
+            if (configFolderInput.value && saveOffMedia) _validateLocalPathInput(configFolderInput);
+        }
         if (configFolderGroup) configFolderGroup.classList.toggle('d-none', !(isJellyfin && saveOffMedia));
         // The "Webhook & Scanner" tab now shows for ALL server types.
         // Pre-fix it was hidden for non-Plex servers — closing the user's
@@ -1433,13 +1446,20 @@
         //   * wizardPlexConfigFolder     — /setup wizard step 3 (renamed to
         //                                   avoid colliding with the partial's
         //                                   hidden input on the same page)
-        const useStructuralCheck =
+        const isPlexCfg =
             input.id === 'editPlexConfigFolder' ||
             input.id === 'plexConfigFolder' ||
             input.id === 'wizardPlexConfigFolder';
-        const endpoint = useStructuralCheck
+        // Jellyfin off-media config folder gets the same deeper structural
+        // check (mirrors Plex) so the success message can confidently say
+        // "valid Jellyfin config folder" rather than just "path exists".
+        const isJellyfinCfg = input.id === 'editJellyfinConfigFolder';
+        const useStructuralCheck = isPlexCfg || isJellyfinCfg;
+        const endpoint = isPlexCfg
             ? '/api/settings/validate-plex-config-folder'
-            : '/api/settings/validate-local-path';
+            : isJellyfinCfg
+              ? '/api/settings/validate-jellyfin-config-folder'
+              : '/api/settings/validate-local-path';
         try {
             const resp = await fetch(endpoint, {
                 method: 'POST',
@@ -1464,8 +1484,10 @@
                 input.classList.add('is-valid');
                 if (success && useStructuralCheck && data.detail) {
                     success.textContent = `Looks like a ${data.detail}`;
-                } else if (success && useStructuralCheck) {
+                } else if (success && isPlexCfg) {
                     success.textContent = 'Valid Plex config folder';
+                } else if (success && isJellyfinCfg) {
+                    success.textContent = 'Valid Jellyfin config folder';
                 } else if (success) {
                     success.textContent = 'Path exists';
                 }
@@ -3361,6 +3383,7 @@
                 const start = ((cfgInput && cfgInput.value) || '').trim() || '/';
                 window.openFolderPicker(start, (picked) => {
                     cfgInput.value = picked;
+                    _validateLocalPathInput(cfgInput);
                 });
             });
         }

media_preview_generator/web/templates/servers.html

@@ -173,11 +173,13 @@ <h5 class="modal-title d-flex align-items-center gap-2 flex-grow-1" style="min-w
                                     <button type="button" class="info-icon ms-1" tabindex="0" data-bs-toggle="tooltip" data-bs-placement="top"
                                             title="Path inside THIS container where Jellyfin's config dir is bind-mounted read-write. This app writes trickplay into its data/trickplay subfolder. Must be mounted :rw, not :ro."><i class="bi bi-info-circle"></i></button>
                                 </label>
-                                <div class="input-group">
+                                <div class="input-group has-validation">
                                     <input type="text" id="editJellyfinConfigFolder" class="form-control" placeholder="/jellyfin-config">
                                     <button type="button" class="btn btn-outline-secondary" id="editJellyfinConfigBrowseBtn" title="Browse folders">
                                         <i class="bi bi-folder2-open"></i>
                                     </button>
+                                    <div class="invalid-feedback small"></div>
+                                    <div class="valid-feedback small">Valid Jellyfin config folder</div>
                                 </div>
                                 <div class="alert alert-warning small mt-2 mb-0" id="editJellyfinOffMediaWarning">
                                     <i class="bi bi-exclamation-triangle me-1"></i>With this on, the app writes to Jellyfin's config dir (not your media). Jellyfin's <code>SaveTrickplayWithMedia</code> must be <strong>off</strong> &mdash; the <strong>Setup Health</strong> tab guides you and flags the plugin + read-write mount.

tests/test_routes.py

@@ -5895,6 +5895,75 @@ def test_outside_root_nonexistent_folder_suggests_mount(self, client, tmp_path,
         assert "-v" in body["error"]
 
 
+class TestValidateJellyfinConfigFolder:
+    """Inline structural check used by Servers > Edit Jellyfin (off-media field).
+    Mirrors the Plex validator; shares looks_like_jellyfin_config_dir."""
+
+    def _post(self, client, path):
+        return client.post(
+            "/api/settings/validate-jellyfin-config-folder",
+            headers=_api_headers(),
+            json={"path": path},
+        )
+
+    def test_valid_jellyfin_config_dir(self, client, tmp_path):
+        cfg = tmp_path / "jellyfin-config"
+        (cfg / "data").mkdir(parents=True)  # the data/ marker
+        body = self._post(client, str(cfg)).get_json()
+        assert body["exists"] is True
+        assert body["valid_jellyfin_structure"] is True
+        assert body["writable"] is True
+        assert body["error"] is None
+        assert "Jellyfin config folder" in body["detail"]
+
+    def test_valid_via_plugins_marker_only(self, client, tmp_path):
+        cfg = tmp_path / "jf-cfg"
+        (cfg / "plugins").mkdir(parents=True)  # leniency: plugins/ alone is enough
+        body = self._post(client, str(cfg)).get_json()
+        assert body["valid_jellyfin_structure"] is True
+
+    def test_missing_folder_reports_not_found(self, client, tmp_path):
+        body = self._post(client, str(tmp_path / "nope")).get_json()
+        assert body["exists"] is False
+        assert body["valid_jellyfin_structure"] is False
+        assert "not found" in body["error"].lower()
+
+    def test_wrong_folder_rejected_with_clear_error(self, client, tmp_path):
+        """A real, writable dir that's a media folder (no Jellyfin markers) must
+        be flagged — the exact mistake the user hit."""
+        media = tmp_path / "Movies"
+        media.mkdir()
+        (media / "Movie.mkv").write_bytes(b"\x00")
+        body = self._post(client, str(media)).get_json()
+        assert body["exists"] is True
+        assert body["valid_jellyfin_structure"] is False
+        assert "doesn't look like Jellyfin's config dir" in body["error"]
+
+    def test_empty_path_is_neutral(self, client):
+        body = self._post(client, "").get_json()
+        assert body["exists"] is False
+        assert body["error"] is None  # neutral — no nag on an empty field
+
+    @pytest.mark.skipif(
+        hasattr(os, "geteuid") and os.geteuid() == 0,
+        reason="root bypasses W_OK, so a read-only dir still reports writable",
+    )
+    def test_valid_structure_but_read_only_is_flagged(self, client, tmp_path):
+        """The '/config mounted :ro' mistake: structure is right but not writable.
+        Must report valid_structure=True, writable=False with a clear error."""
+        cfg = tmp_path / "jf-ro"
+        (cfg / "data").mkdir(parents=True)
+        os.chmod(cfg, 0o555)
+        try:
+            body = self._post(client, str(cfg)).get_json()
+        finally:
+            os.chmod(cfg, 0o755)  # restore so tmp cleanup can remove it
+        assert body["exists"] is True
+        assert body["valid_jellyfin_structure"] is True
+        assert body["writable"] is False
+        assert "not writable" in body["error"].lower()
+
+
 # ---------------------------------------------------------------------------
 # Phase I5 — Per-server Plex webhook endpoints
 # ---------------------------------------------------------------------------