Merge pull request 'External-attach orphan fix (#859) + image-less post display' (#108) from dev into main

backend/app/api/admin.py

@@ -348,3 +348,16 @@ async def trigger_reextract_archives():
 
     async_result = reextract_archive_attachments_task.delay()
     return jsonify({"task_id": async_result.id, "status": "queued"}), 202
+
+
+@admin_bp.route("/maintenance/prune-missing-files", methods=["POST"])
+async def trigger_prune_missing_files():
+    """Operator-triggered orphan repair (#859): delete ImageRecords whose backing
+    file is gone from disk (e.g. left by the external-attach unlink bug), so they
+    stop 404-ing on playback. The task aborts WITHOUT deleting if a large fraction
+    of files look missing (a filesystem/NFS stall). Maintenance queue;
+    operator-triggered only — never an unattended sweep."""
+    from ..tasks.admin import prune_missing_file_records_task
+
+    async_result = prune_missing_file_records_task.delay()
+    return jsonify({"task_id": async_result.id, "status": "queued"}), 202

backend/app/tasks/admin.py

@@ -14,9 +14,11 @@ from __future__ import annotations
 import logging
 from pathlib import Path
 
+from sqlalchemy import delete, select
 from sqlalchemy.exc import DBAPIError, OperationalError
 
 from ..celery_app import celery
+from ..models import ImageRecord
 from ..services import cleanup_service
 from ._sync_engine import sync_session_factory as _sync_session_factory
 
@@ -41,6 +43,85 @@ def delete_artist_cascade_task(self, *, artist_id: int) -> dict:
         )
 
 
+# Orphan repair (#859). Safety guard: an NFS/filesystem stall makes EVERY file
+# look missing — never delete records en masse on that basis. If a non-trivial
+# sample comes back mostly missing, ABORT without deleting (assume the FS is
+# unhealthy, not that the library evaporated). Operator-triggered ONLY — NOT a
+# periodic sweep, precisely to avoid an unattended run firing during an NFS blip.
+_ORPHAN_MIN_SAMPLE = 50
+_ORPHAN_MAX_MISSING_FRAC = 0.10
+
+
+@celery.task(
+    name="backend.app.tasks.admin.prune_missing_file_records_task",
+    bind=True,
+    autoretry_for=(OperationalError, DBAPIError),
+    retry_backoff=15, retry_backoff_max=180, max_retries=1,
+    soft_time_limit=1800, time_limit=2400,  # 30 min / 40 min
+)
+def prune_missing_file_records_task(self) -> dict:
+    """Delete ImageRecords whose backing file is gone from disk (orphans — e.g.
+    the external-attach unlink bug #859). Every FK to image_record is CASCADE /
+    SET NULL, so a Core DELETE cleans provenance, series pages, predictions and
+    tag links; leftover thumbnails are unlinked best-effort. Returns a summary.
+
+    Aborts WITHOUT deleting if a non-trivial sample is mostly missing (a
+    filesystem/NFS stall, not real orphans) — see the guard constants above.
+    """
+    SessionLocal = _sync_session_factory()
+    checked = 0
+    missing_ids: list[int] = []
+    thumbs: list[str] = []
+    last_id = 0
+    with SessionLocal() as session:
+        while True:
+            rows = session.execute(
+                select(ImageRecord.id, ImageRecord.path, ImageRecord.thumbnail_path)
+                .where(ImageRecord.id > last_id)
+                .order_by(ImageRecord.id)
+                .limit(1000)
+            ).all()
+            if not rows:
+                break
+            for rid, path, thumb in rows:
+                last_id = rid
+                checked += 1
+                if not (IMAGES_ROOT / path).exists():
+                    missing_ids.append(rid)
+                    if thumb:
+                        thumbs.append(thumb)
+
+        if not missing_ids:
+            return {"checked": checked, "missing": 0, "deleted": 0}
+
+        frac = len(missing_ids) / checked if checked else 0.0
+        if checked >= _ORPHAN_MIN_SAMPLE and frac > _ORPHAN_MAX_MISSING_FRAC:
+            log.warning(
+                "orphan-repair ABORTED: %d/%d (%.0f%%) records missing on disk — "
+                "likely a filesystem/NFS problem, not real orphans. No deletions.",
+                len(missing_ids), checked, frac * 100,
+            )
+            return {
+                "checked": checked, "missing": len(missing_ids), "deleted": 0,
+                "aborted": "too many missing — filesystem problem suspected",
+            }
+
+        deleted = 0
+        for i in range(0, len(missing_ids), 500):  # keep well under psycopg's param ceiling
+            chunk = missing_ids[i:i + 500]
+            session.execute(delete(ImageRecord).where(ImageRecord.id.in_(chunk)))
+            deleted += len(chunk)
+        session.commit()
+
+    for t in thumbs:  # cosmetic — outside the txn, never fail the repair on these
+        try:
+            (IMAGES_ROOT / t).unlink(missing_ok=True)
+        except OSError:
+            pass
+    log.info("orphan-repair: checked=%d missing=%d deleted=%d", checked, len(missing_ids), deleted)
+    return {"checked": checked, "missing": len(missing_ids), "deleted": deleted}
+
+
 @celery.task(
     name="backend.app.tasks.admin.bulk_delete_images_task",
     bind=True,

backend/app/tasks/external.py

@@ -28,7 +28,7 @@ from sqlalchemy import delete, select, update
 
 from ..celery_app import celery
 from ..config import get_config
-from ..models import Artist, ExternalLink, ImportSettings, Post, Source
+from ..models import Artist, ExternalLink, ImageRecord, ImportSettings, Post, Source
 from ..services.external_fetch import fetch_external
 from ..services.importer import Importer
 from ..services.thumbnailer import Thumbnailer
@@ -167,7 +167,15 @@ def fetch_external_link(self, link_id: int, _serialize_waits: int = 0) -> dict:
         return {"link_id": link_id, "requeued": "host busy"}
 
     started = time.monotonic()
-    post_dir = IMAGES_ROOT / artist.slug / platform / "external" / str(post.external_post_id)
+    # Per-LINK staging dir (not just per-post): two links on the same post (e.g.
+    # the same film from mega + gdrive) can emit the same filename — sharing one
+    # external/<post_id>/ dir let the second overwrite the first's file, then the
+    # dedup-skip unlink orphaned a record → 404 on playback. Isolating per link
+    # keeps each link's file at its own path. (#859)
+    post_dir = (
+        IMAGES_ROOT / artist.slug / platform / "external"
+        / str(post.external_post_id) / str(link_id)
+    )
     try:
         result = fetch_external(host, url, post_dir, timeout=_FETCH_TIMEOUT)
         with SessionLocal() as session:
@@ -292,7 +300,23 @@ def _route_files(session, files, post, platform, artist, source) -> list[int]:
                 "extdl skipped: post=%s file=%s reason=%s", post.id, f.name, reason,
             )
             if reason in ("duplicate_hash", "duplicate_phash"):
-                f.unlink(missing_ok=True)  # canonical copy already in the library
+                # Path-safe unlink: external files are imported IN PLACE, so `f`
+                # can BE the canonical record's backing file (same content
+                # re-fetched / a colliding name). Only delete `f` when it is NOT
+                # the existing record's file — otherwise we orphan the record and
+                # playback 404s. (#859)
+                canonical = None
+                if result.image_id is not None:
+                    rec = session.get(ImageRecord, result.image_id)
+                    if rec is not None:
+                        canonical = (IMAGES_ROOT / rec.path).resolve()
+                if canonical != f.resolve():
+                    f.unlink(missing_ok=True)  # a redundant copy — safe to drop
+                else:
+                    log.info(
+                        "extdl keep: %s IS record %s's canonical file — not unlinking",
+                        f.name, result.image_id,
+                    )
         elif result.status == "failed":
             log.warning(
                 "extdl import FAILED: post=%s file=%s error=%s",

frontend/src/components/posts/PostCard.vue

@@ -25,36 +25,36 @@
     </div>
 
     <div class="fc-post-card__body">
-      <div class="fc-post-card__media">
+      <!-- Media column only when the post HAS images. Text-only posts (the bulk
-        <template v-if="images.length">
+           of Patreon WIP/announcement/poll posts) let the body span the full
-          <!-- Images open the post-scoped image modal (look bigger + arrow
+           card width instead of padding a dead "no images" placeholder. -->
-               through ALL the post's images) — the card never expands. -->
+      <div v-if="images.length" class="fc-post-card__media">
+        <!-- Images open the post-scoped image modal (look bigger + arrow
+             through ALL the post's images) — the card never expands. -->
+        <button
+          type="button" class="fc-post-card__hero"
+          aria-label="Open images" @click="openModal(hero.image_id)"
+        >
+          <img :src="hero.thumbnail_url" alt="hero thumbnail" loading="lazy" />
+        </button>
+        <div
+          v-if="rail.length || moreCount" class="fc-post-card__rail"
+          :style="{ '--fc-rail-cols': railCols }"
+        >
           <button
-            type="button" class="fc-post-card__hero"
+            v-for="t in rail" :key="t.image_id" type="button"
-            aria-label="Open images" @click="openModal(hero.image_id)"
+            class="fc-post-card__rail-cell"
+            aria-label="Open image" @click="openModal(t.image_id)"
           >
-            <img :src="hero.thumbnail_url" alt="hero thumbnail" loading="lazy" />
+            <img :src="t.thumbnail_url" alt="thumbnail" loading="lazy" />
           </button>
-          <div
+          <button
-            v-if="rail.length || moreCount" class="fc-post-card__rail"
+            v-if="moreCount > 0" type="button"
-            :style="{ '--fc-rail-cols': railCols }"
+            class="fc-post-card__rail-more"
-          >
+            :aria-label="`Open ${moreCount} more images`"
-            <button
+            @click="openModalAtMore"
-              v-for="t in rail" :key="t.image_id" type="button"
+          >+{{ moreCount }}</button>
-              class="fc-post-card__rail-cell"
+        </div>
-              aria-label="Open image" @click="openModal(t.image_id)"
-            >
-              <img :src="t.thumbnail_url" alt="thumbnail" loading="lazy" />
-            </button>
-            <button
-              v-if="moreCount > 0" type="button"
-              class="fc-post-card__rail-more"
-              :aria-label="`Open ${moreCount} more images`"
-              @click="openModalAtMore"
-            >+{{ moreCount }}</button>
-          </div>
-        </template>
-        <PostEmptyThumbs v-else />
       </div>
 
       <div class="fc-post-card__text">
@@ -128,7 +128,6 @@ import { RouterLink } from 'vue-router'
 import { useModalStore } from '../../stores/modal.js'
 import { usePostsStore } from '../../stores/posts.js'
 import { toPlainText } from '../../utils/htmlSanitize.js'
-import PostEmptyThumbs from './PostEmptyThumbs.vue'
 import PostSeriesMenu from './PostSeriesMenu.vue'
 
 const props = defineProps({

frontend/src/components/posts/PostEmptyThumbs.vue

@@ -1,35 +0,0 @@
-<template>
-  <div class="fc-post-empty">
-    <v-icon size="48" class="fc-post-empty__icon">mdi-image-off-outline</v-icon>
-    <div class="fc-post-empty__text">No images attached to this post</div>
-  </div>
-</template>
-
-<script setup>
-// No props — pure presentational placeholder.
-</script>
-
-<style scoped>
-.fc-post-empty {
-  display: flex;
-  flex-direction: column;
-  align-items: center;
-  justify-content: center;
-  gap: 12px;
-  padding: 32px 16px;
-  border: 1px dashed rgb(var(--v-theme-on-surface-variant));
-  border-radius: 8px;
-  background: rgba(20, 23, 26, 0.3);
-  width: 100%;
-  height: 100%;
-  min-height: 200px;
-}
-.fc-post-empty__icon {
-  color: rgb(var(--v-theme-on-surface-variant));
-  opacity: 0.6;
-}
-.fc-post-empty__text {
-  color: rgb(var(--v-theme-on-surface-variant));
-  font-size: 0.9rem;
-}
-</style>

frontend/src/components/settings/MaintenancePanel.vue

@@ -16,6 +16,7 @@
     <AliasTable class="mt-4" />
     <DbMaintenanceCard class="mt-6" />
     <ArchiveReextractCard class="mt-6" />
+    <MissingFileRepairCard class="mt-6" />
     <BackupCard class="mt-6" />
     <!-- TagMaintenanceCard moved to Cleanup tab (v26.05.25.7) — it
          operates on the existing library which fits the Cleanup-tab
@@ -35,6 +36,7 @@ import AllowlistTable from './AllowlistTable.vue'
 import AliasTable from './AliasTable.vue'
 import DbMaintenanceCard from './DbMaintenanceCard.vue'
 import ArchiveReextractCard from './ArchiveReextractCard.vue'
+import MissingFileRepairCard from './MissingFileRepairCard.vue'
 import BackupCard from './BackupCard.vue'
 import { useSystemActivityStore } from '../../stores/systemActivity.js'
 

frontend/src/components/settings/MissingFileRepairCard.vue

@@ -0,0 +1,47 @@
+<template>
+  <!-- #859: delete ImageRecords whose backing file is gone from disk (orphans
+       left by the external-attach unlink bug) so they stop 404-ing on playback. -->
+  <v-card>
+    <v-card-title>Repair missing-file records</v-card-title>
+    <v-card-text>
+      <p class="text-body-2 mb-3">
+        Scans every image/video record and removes any whose underlying file is
+        gone from disk — orphaned records that otherwise 404 when you open the
+        post. Safe: it <strong>aborts without deleting</strong> if a large share of
+        files look missing (which means a storage/NFS hiccup, not real orphans).
+        Run it only when storage is healthy.
+      </p>
+      <v-btn color="primary" rounded="pill" :loading="busy" @click="run">
+        <v-icon start>mdi-file-remove-outline</v-icon> Repair missing-file records
+      </v-btn>
+      <span v-if="queued" class="ml-3 text-caption text-success">Queued ✓</span>
+      <QueueStatusBar queue="maintenance" queue-label="Maintenance" />
+    </v-card-text>
+  </v-card>
+</template>
+
+<script setup>
+import { ref } from 'vue'
+
+import { useApi } from '../../composables/useApi.js'
+import { toast } from '../../utils/toast.js'
+import QueueStatusBar from './QueueStatusBar.vue'
+
+const api = useApi()
+const busy = ref(false)
+const queued = ref(false)
+
+async function run () {
+  busy.value = true
+  queued.value = false
+  try {
+    await api.post('/api/admin/maintenance/prune-missing-files')
+    queued.value = true
+    toast({ text: 'Missing-file repair queued', type: 'success' })
+  } catch (e) {
+    toast({ text: e?.body?.detail || e?.message || 'Failed to queue', type: 'error' })
+  } finally {
+    busy.value = false
+  }
+}
+</script>

tests/test_external_worker.py

@@ -5,13 +5,14 @@ import zipfile
 
 import pytest
 from PIL import Image
-from sqlalchemy import func, select
+from sqlalchemy import func, select, update
 
 import backend.app.tasks.external as ext
 from backend.app.models import (
     Artist,
     ExternalLink,
     ImageProvenance,
+    ImageRecord,
     Post,
     PostAttachment,
     Source,
@@ -102,6 +103,55 @@ def test_fetch_external_link_downloads_and_attaches(db_sync, tmp_path, monkeypat
     assert len(atts) == 1
 
 
+def test_refetch_same_link_keeps_canonical_file(db_sync, tmp_path, monkeypatch):
+    """#859: re-fetching the SAME link (identical bytes into the same per-link
+    staging dir) must NOT unlink the canonical file out from under its record.
+    The first fetch imports the image in place; the second dedups on sha256, and
+    the path-safe unlink recognises the file IS the record's backing file and
+    keeps it — so the post never 404s on playback."""
+    _, link = _seed(db_sync, host="pixeldrain")
+    link_id = link.id
+
+    monkeypatch.setattr(ext, "IMAGES_ROOT", tmp_path)
+    monkeypatch.setattr(ext, "_redis", lambda: _FakeRedis())
+
+    def fake_fetch(host, url, dest_dir, *, timeout, should_stop=lambda: False):
+        dest_dir.mkdir(parents=True, exist_ok=True)
+        f = dest_dir / "clip.jpg"  # art → ImageRecord (imported in place)
+        f.write_bytes(_structured_jpeg_bytes())
+        return FetchResult(files=[f], bytes=f.stat().st_size)
+
+    monkeypatch.setattr(ext, "fetch_external", fake_fetch)
+
+    import backend.app.tasks.ml as ml_mod
+    import backend.app.tasks.thumbnail as thumb_mod
+    monkeypatch.setattr(ml_mod.tag_and_embed, "delay", lambda i: None)
+    monkeypatch.setattr(thumb_mod.generate_thumbnail, "delay", lambda i: None)
+
+    out = ext.fetch_external_link(link_id)
+    assert out["images"] == 1
+
+    db_sync.expire_all()
+    rec = db_sync.execute(select(ImageRecord)).scalars().one()
+    canonical = tmp_path / rec.path  # absolute path collapses the join
+    assert canonical.exists()
+
+    # Re-arm the SAME link and fetch again — same staging dir, identical bytes.
+    db_sync.execute(
+        update(ExternalLink).where(ExternalLink.id == link_id)
+        .values(status="pending", attempts=0, completed_at=None)
+    )
+    db_sync.commit()
+
+    out2 = ext.fetch_external_link(link_id)
+    assert out2["images"] == 0  # deduped, no new record
+
+    db_sync.expire_all()
+    # Exactly one record still, and its file survived the dedup unlink (the bug).
+    assert db_sync.execute(select(func.count(ImageRecord.id))).scalar() == 1
+    assert canonical.exists()
+
+
 def test_fetch_external_link_records_failure(db_sync, tmp_path, monkeypatch):
     _, link = _seed(db_sync)
     link_id = link.id

tests/test_tasks_admin.py

@@ -141,6 +141,80 @@ async def test_bulk_delete_images_task_removes_listed_images(
     assert surviving == 0
 
 
+# --- prune_missing_file_records_task (#859 orphan repair) -----------
+
+
+def _img(db_sync, artist, *, path, sha):
+    img = ImageRecord(
+        artist_id=artist.id, path=str(path),
+        sha256=sha, size_bytes=10, mime="image/jpeg",
+        origin="downloaded",
+    )
+    db_sync.add(img)
+    db_sync.flush()
+    return img.id
+
+
+@pytest.mark.asyncio
+async def test_prune_missing_file_records_deletes_only_orphans(db_sync, tmp_path):
+    """Records whose backing file is gone get deleted; records whose file is
+    present survive. Below the abort sample size, fraction is irrelevant."""
+    from backend.app.tasks.admin import prune_missing_file_records_task
+
+    a = Artist(name="Orph", slug="orph")
+    db_sync.add(a)
+    db_sync.flush()
+
+    present = []
+    for i in range(3):
+        f = tmp_path / f"present{i}.jpg"
+        f.write_bytes(b"real")
+        present.append(_img(db_sync, a, path=f, sha=f"{i:064x}"))
+    missing = [
+        _img(db_sync, a, path=tmp_path / f"gone{i}.jpg", sha=f"f{i:063x}")
+        for i in range(2)
+    ]
+    db_sync.commit()
+
+    result = prune_missing_file_records_task.delay().get()
+    assert result["checked"] == 5
+    assert result["missing"] == 2
+    assert result["deleted"] == 2
+
+    db_sync.expire_all()
+    surviving = db_sync.execute(
+        select(ImageRecord.id).where(ImageRecord.id.in_(present + missing))
+    ).scalars().all()
+    assert set(surviving) == set(present)
+
+
+@pytest.mark.asyncio
+async def test_prune_missing_file_records_aborts_when_mostly_missing(db_sync, tmp_path):
+    """NFS-stall guard: a large sample that's mostly missing means the
+    filesystem is unhealthy, not that the library evaporated — abort, delete
+    nothing."""
+    from backend.app.tasks.admin import prune_missing_file_records_task
+
+    a = Artist(name="Stall", slug="stall")
+    db_sync.add(a)
+    db_sync.flush()
+    ids = [
+        _img(db_sync, a, path=tmp_path / f"phantom{i}.jpg", sha=f"{i:064x}")
+        for i in range(55)  # >= _ORPHAN_MIN_SAMPLE, all missing -> frac 1.0
+    ]
+    db_sync.commit()
+
+    result = prune_missing_file_records_task.delay().get()
+    assert result["deleted"] == 0
+    assert "aborted" in result
+
+    db_sync.expire_all()
+    surviving = db_sync.execute(
+        select(func.count(ImageRecord.id)).where(ImageRecord.id.in_(ids))
+    ).scalar_one()
+    assert surviving == 55
+
+
 @pytest.mark.asyncio
 async def test_bulk_delete_images_task_idempotent_on_empty_list(db_sync):
     from backend.app.tasks.admin import bulk_delete_images_task