fix(scripts): regenerate-thumbnails resolves external sources through ensureThumbnail (#1148) (#1155)

Stable twin of #1151.

The script here carried the identical defect: it computed `storage/events/active/<photo.path>` and fs.access'd it, which does not exist for external or reference rows. #1129 already landed on this branch, so the route was fixed and the script was the remaining half.

Resolution goes through ensureThumbnail, which stable already exports with the external branch intact. Also carried: videos skipped on every marker, skip-vs-generate asked from isThumbnailValid, and a nonzero exit when a photo could not be built.

Not carried: the responsive-tier backfill — THUMBNAIL_WIDTHS and ensureThumbnailAtWidth are #1095/#1109 and do not exist on this branch.

Merged with admin privileges: the author cannot self-approve.

backend/__tests__/integration/regenerateThumbnailsScript.test.js

@@ -0,0 +1,259 @@
+/**
+ * scripts/regenerate-thumbnails.js against external photos (#1148).
+ *
+ * The same defect #1129 fixed in the admin route, still standing in the CLI
+ * fallback: the script resolved every source as
+ * `storage/events/active/<photo.path>` and fs.access'd it. External and
+ * reference rows do not live there — their originals sit under
+ * `events.external_path` — so every one failed the check and was counted as an
+ * error. On an install where all photos are external the script did nothing at
+ * all, while reporting one error per photo.
+ *
+ * Driven against a REAL file on a REAL external mount with the real
+ * imageProcessor, not a mock: the whole point is that the source resolves off
+ * the mount, and a mocked ensureThumbnail would assert nothing about that.
+ *
+ * Responsive tiers (#1095/#1109) do not exist on this branch, so the tier
+ * backfill in the main twin has nothing to port. Everything else does.
+ */
+
+const fs = require('fs');
+const path = require('path');
+const os = require('os');
+const sharp = require('sharp');
+const { execFile } = require('child_process');
+
+describe('regenerate-thumbnails script (#1148)', () => {
+  let tmpDir; let db; let cleanup; let regenerateThumbnails;
+  let eventId; let externalPhotoId; let videoPhotoId; let watcherVideoId; let repairPhotoId;
+  let vanishingPhotoId;
+  let externalRoot;
+
+  beforeAll(async () => {
+    tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'picpeak-regen-script-'));
+    process.env.NODE_ENV = 'test';
+    process.env.TEST_DATABASE_PATH = path.join(tmpDir, 'data', 'test.db');
+    process.env.STORAGE_PATH = path.join(tmpDir, 'storage');
+    // External sources are sandboxed under EXTERNAL_MEDIA_ROOT; event
+    // external_path is relative to it, exactly as on a real install.
+    process.env.EXTERNAL_MEDIA_ROOT = path.join(tmpDir, 'media');
+    externalRoot = path.join(process.env.EXTERNAL_MEDIA_ROOT, 'wedding');
+
+    await fs.promises.mkdir(path.dirname(process.env.TEST_DATABASE_PATH), { recursive: true });
+    await fs.promises.mkdir(process.env.STORAGE_PATH, { recursive: true });
+    await fs.promises.mkdir(externalRoot, { recursive: true });
+
+    jest.resetModules();
+    ({ db, cleanup } = await require('./helpers/crmDb').bootCrmDb());
+
+    // A real image on the external mount — never under events/active.
+    await sharp({
+      create: { width: 1200, height: 800, channels: 3, background: { r: 10, g: 90, b: 160 } },
+    }).jpeg().toFile(path.join(externalRoot, 'shot.jpg'));
+
+    const [ev] = await db('events').insert({
+      slug: 'regen-script-event',
+      event_type: 'wedding',
+      event_name: 'Regen Script',
+      event_date: '2026-08-01',
+      host_email: '[email protected]',
+      admin_email: '[email protected]',
+      password_hash: 'x',
+      share_link: '/gallery/regen-script-event/share',
+      expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(),
+      source_mode: 'reference',
+      external_path: 'wedding',
+      created_at: new Date().toISOString(),
+    }).returning('id');
+    eventId = typeof ev === 'object' ? ev.id : ev;
+
+    const [p] = await db('photos').insert({
+      event_id: eventId,
+      filename: 'shot.jpg',
+      // `path` is what the old script joined onto events/active. Left
+      // populated on purpose: the fix must ignore it for an external row.
+      path: 'regen-script-event/shot.jpg',
+      type: 'individual',
+      source_origin: 'external',
+      external_relpath: 'shot.jpg',
+      uploaded_at: new Date().toISOString(),
+    }).returning('id');
+    externalPhotoId = typeof p === 'object' ? p.id : p;
+
+    const [v] = await db('photos').insert({
+      event_id: eventId,
+      filename: 'clip.mp4',
+      path: 'regen-script-event/clip.mp4',
+      type: 'individual',
+      media_type: 'video',
+      mime_type: 'video/mp4',
+      source_origin: 'external',
+      external_relpath: 'clip.mp4',
+      uploaded_at: new Date().toISOString(),
+    }).returning('id');
+    videoPhotoId = typeof v === 'object' ? v.id : v;
+
+    // How fileWatcher.processNewPhoto actually writes a video: `type` and
+    // `mime_type` set, media_type left to its 'image' default. A media_type-only
+    // filter lets this through and hands the container to Sharp.
+    //
+    // The file has to EXIST, otherwise the row fails resolution and looks
+    // skipped for the wrong reason — the bug is Sharp being handed a video, not
+    // a missing source. Real MP4 header bytes, no image in sight.
+    await fs.promises.writeFile(
+      path.join(externalRoot, 'watched.mp4'),
+      Buffer.from('00000018667479706d70343200000000', 'hex')
+    );
+    const [wv] = await db('photos').insert({
+      event_id: eventId,
+      filename: 'watched.mp4',
+      path: 'regen-script-event/watched.mp4',
+      type: 'video',
+      mime_type: 'video/mp4',
+      source_origin: 'external',
+      external_relpath: 'watched.mp4',
+      uploaded_at: new Date().toISOString(),
+    }).returning('id');
+    watcherVideoId = typeof wv === 'object' ? wv.id : wv;
+    expect((await db('photos').where('id', watcherVideoId).first()).media_type).not.toBe('video');
+
+    // A photo whose thumbnail_path points at something that is no longer there.
+    await sharp({
+      create: { width: 900, height: 600, channels: 3, background: { r: 200, g: 40, b: 40 } },
+    }).jpeg().toFile(path.join(externalRoot, 'repair.jpg'));
+    const [rp] = await db('photos').insert({
+      event_id: eventId,
+      filename: 'repair.jpg',
+      path: 'regen-script-event/repair.jpg',
+      type: 'individual',
+      thumbnail_path: 'thumbnails/thumb_ext_missing_repair.jpg',
+      source_origin: 'external',
+      external_relpath: 'repair.jpg',
+      uploaded_at: new Date().toISOString(),
+    }).returning('id');
+    repairPhotoId = typeof rp === 'object' ? rp.id : rp;
+
+    // A photo whose source is not on the mount at all — an unavailable mount,
+    // which is the failure an operator most needs to hear about.
+    const [vp] = await db('photos').insert({
+      event_id: eventId,
+      filename: 'missing.jpg',
+      path: 'regen-script-event/missing.jpg',
+      type: 'individual',
+      source_origin: 'external',
+      external_relpath: 'missing.jpg',
+      uploaded_at: new Date().toISOString(),
+    }).returning('id');
+    vanishingPhotoId = typeof vp === 'object' ? vp.id : vp;
+
+    ({ regenerateThumbnails } = require('../../scripts/regenerate-thumbnails'));
+  }, 180000);
+
+  afterAll(async () => {
+    if (cleanup) await cleanup();
+    await fs.promises.rm(tmpDir, { recursive: true, force: true }).catch(() => {});
+  });
+
+  it('builds a thumbnail for an external photo instead of erroring on events/active', async () => {
+    // The location the old script computed and fs.access'd. Nothing is there,
+    // which is the whole defect — it is not where an external original lives.
+    // (The old script cannot be driven from a test directly: it had no export
+    // and ran on require, calling process.exit. Making it importable is part
+    // of this fix.)
+    const legacyPath = path.join(process.env.STORAGE_PATH, 'events/active', 'regen-script-event/shot.jpg');
+    expect(fs.existsSync(legacyPath)).toBe(false);
+
+    const result = await regenerateThumbnails(eventId);
+
+    // The old script reported an error for this photo and wrote nothing.
+    // The unresolvable row fails; the external photo and the repair row build.
+    expect(result.errorCount).toBe(1);
+    expect(result.successCount).toBe(2);
+
+    const row = await db('photos').where('id', externalPhotoId).first();
+    expect(row.thumbnail_path).toBeTruthy();
+    const onDisk = path.join(process.env.STORAGE_PATH, row.thumbnail_path);
+    expect(fs.existsSync(onDisk)).toBe(true);
+
+    // Named per-photo so two events referencing one NAS basename cannot
+    // clobber each other — the property ensureThumbnail owns and the reason
+    // the script must not build this name itself.
+    expect(path.basename(row.thumbnail_path)).toContain(`ext${externalPhotoId}_`);
+  });
+
+  it('leaves videos alone', async () => {
+    // A video thumbnail is a poster frame from videoProcessor; handing the
+    // container to Sharp produced one error per video row.
+    const row = await db('photos').where('id', videoPhotoId).first();
+    expect(row.thumbnail_path).toBeFalsy();
+  });
+
+  it('leaves a watcher-imported video alone, which carries no media_type', async () => {
+    // fileWatcher writes type + mime_type and lets media_type default to
+    // 'image', so filtering on media_type alone still fed these to Sharp. The
+    // signal is errorCount: the images are already done by now, so the only
+    // NEW thing that could fail this run is a video reaching Sharp. One error
+    // is the deliberately unresolvable row; two would be the video.
+    const result = await regenerateThumbnails(eventId);
+
+    expect(result.errorCount).toBe(1);
+    const row = await db('photos').where('id', watcherVideoId).first();
+    expect(row.thumbnail_path).toBeFalsy();
+  });
+
+  it('is idempotent — a second run skips instead of rebuilding', async () => {
+    const before = await db('photos').where('id', externalPhotoId).first();
+    const result = await regenerateThumbnails(eventId);
+
+    expect(result.errorCount).toBe(1);
+    expect(result.successCount).toBe(0);
+    expect(result.skipCount).toBe(2);
+
+    const after = await db('photos').where('id', externalPhotoId).first();
+    expect(after.thumbnail_path).toBe(before.thumbnail_path);
+  });
+
+  it('counts a repaired thumbnail as generated, not skipped', async () => {
+    // Both images are valid at this point. Destroy ONE thumbnail object while
+    // leaving thumbnail_path pointing at it — the corrupt/missing case.
+    const row = await db('photos').where('id', repairPhotoId).first();
+    const onDisk = path.join(process.env.STORAGE_PATH, row.thumbnail_path);
+    await fs.promises.rm(onDisk);
+
+    const result = await regenerateThumbnails(eventId);
+
+    // On local and external storage the rebuilt key is identical, so inferring
+    // "skipped" from an unchanged path reports this repair as already valid —
+    // the one number an operator running this is actually reading.
+    expect(result.successCount).toBe(1);
+    expect(result.skipCount).toBe(1);
+    expect(result.errorCount).toBe(1);
+    expect(fs.existsSync(onDisk)).toBe(true);
+  });
+
+  /** Run the CLI the way cron does, and hand back its exit status. */
+  const runCli = (args = []) => new Promise((resolve) => {
+    execFile(
+      process.execPath,
+      [path.join(__dirname, '..', '..', 'scripts', 'regenerate-thumbnails.js'), ...args],
+      { env: { ...process.env }, cwd: path.join(__dirname, '..', '..') },
+      (error, stdout, stderr) => resolve({ code: error?.code ?? 0, stdout, stderr })
+    );
+  });
+
+  it('exits nonzero when a photo could not be built', async () => {
+    // Exit status is the only thing a cron job reads, and `missing.jpg` has no
+    // source on the mount.
+    const failed = await runCli([String(eventId)]);
+    expect(failed.code).toBe(1);
+    expect(failed.stderr).toContain('completed with failures');
+  }, 120000);
+
+  it('exits zero when every photo resolves', async () => {
+    // Drop the unresolvable row: a clean run must not cry wolf at automation.
+    await db('photos').where('id', vanishingPhotoId).del();
+    const ok = await runCli([String(eventId)]);
+    expect(ok.code).toBe(0);
+    expect(ok.stdout).toContain('Script completed successfully');
+  }, 120000);
+});

backend/scripts/regenerate-thumbnails.js

@@ -1,141 +1,147 @@
 #!/usr/bin/env node
 
 /**
- * Script to regenerate missing thumbnails for photos in the database
+ * Fill in missing thumbnails for photos already in the database.
- * Usage: node scripts/regenerate-thumbnails.js [eventId]
+ *
+ * The CLI fallback for when the admin UI is not reachable. It is deliberately
+ * "missing only": ensureThumbnail short-circuits on a thumbnail that is
+ * already present and valid, so re-running this is cheap and safe. To REBUILD
+ * everything after a settings change, use POST /api/admin/thumbnails/regenerate
+ * — that path drops the existing renditions first, which this one must not do.
+ *
+ * Resolution goes through ensureThumbnail rather than a hand-built path
+ * (#1148, same defect as #1129). This script used to compute
+ * `storage/events/active/<photo.path>` and fs.access it, a location that does
+ * not exist for `external` or `reference` rows — their originals live under
+ * the mount in events.external_path. Every such photo failed the check and was
+ * counted as an error, so on an external-media install the script was inert
+ * while reporting one error per photo.
+ *
+ * ensureThumbnail already branches on source_origin, resolves both kinds via
+ * photoResolver, uses the per-photo `ext<id>_` output name so two events
+ * referencing one NAS basename cannot clobber each other, and writes
+ * thumbnail_path back itself. Sharing it is what stops the script and the
+ * route drifting apart again.
+ *
+ * Usage:
+ *   node scripts/regenerate-thumbnails.js [eventId]
  */
 
-const path = require('path');
-const fs = require('fs').promises;
-const sharp = require('sharp');
 const { db } = require('../src/database/db');
-
+const { ensureThumbnail, isThumbnailValid } = require('../src/services/imageProcessor');
-// Configuration
-const THUMBNAIL_SIZE = 300;
-const STORAGE_PATH = process.env.STORAGE_PATH || path.join(__dirname, '../../storage');
-const THUMBNAILS_DIR = path.join(STORAGE_PATH, 'thumbnails');
-
-async function ensureDirectoryExists(dirPath) {
-  try {
-    await fs.access(dirPath);
-  } catch {
-    await fs.mkdir(dirPath, { recursive: true });
-    console.log(`Created directory: ${dirPath}`);
-  }
-}
-
-async function generateThumbnail(photoPath, thumbnailPath) {
-  try {
-    await sharp(photoPath)
-      .resize(THUMBNAIL_SIZE, THUMBNAIL_SIZE, {
-        fit: 'cover',
-        position: 'center'
-      })
-      .jpeg({ quality: 80 })
-      .toFile(thumbnailPath);
-    
-    return true;
-  } catch (error) {
-    console.error(`Failed to generate thumbnail for ${photoPath}:`, error.message);
-    return false;
-  }
-}
 
 async function regenerateThumbnails(eventId = null) {
-  try {
+  console.log('Starting thumbnail regeneration...');
-    console.log('Starting thumbnail regeneration...');
+
-    console.log(`Storage path: ${STORAGE_PATH}`);
+  // These columns are what ensureThumbnail branches on to resolve a source and
-    console.log(`Thumbnails directory: ${THUMBNAILS_DIR}`);
+  // name its output. Selecting a subset that misses
-    
+  // source_origin/external_relpath is how the old path bug would come back —
-    // Ensure thumbnails directory exists
+  // an external row would look managed and resolve under events/active.
-    await ensureDirectoryExists(THUMBNAILS_DIR);
+  let query = db('photos').select(
-    
+    'id', 'event_id', 'path', 'filename', 'thumbnail_path',
-    // Build query
+    'type', 'media_type', 'mime_type', 'source_origin', 'external_relpath'
-    let query = db('photos')
+  );
-      .join('events', 'photos.event_id', 'events.id')
+
-      .select(
+  if (eventId) {
-        'photos.id',
+    query = query.where('event_id', eventId);
-        'photos.filename',
+    console.log(`Filtering for event ID: ${eventId}`);
-        'photos.path',
-        'photos.thumbnail_path',
-        'events.slug as event_slug'
-      );
-    
-    if (eventId) {
-      query = query.where('photos.event_id', eventId);
-      console.log(`Filtering for event ID: ${eventId}`);
-    }
-    
-    const photos = await query;
-    console.log(`Found ${photos.length} photos to process`);
-    
-    let successCount = 0;
-    let skipCount = 0;
-    let errorCount = 0;
-    
-    for (const photo of photos) {
-      const photoPath = path.join(STORAGE_PATH, 'events/active', photo.path);
-      const thumbnailFilename = `thumb_${photo.filename}`;
-      const thumbnailPath = path.join(THUMBNAILS_DIR, thumbnailFilename);
-      
-      try {
-        // Check if photo file exists
-        await fs.access(photoPath);
-        
-        // Check if thumbnail already exists
-        try {
-          await fs.access(thumbnailPath);
-          console.log(`Thumbnail already exists for ${photo.filename}, skipping...`);
-          skipCount++;
-          continue;
-        } catch {
-          // Thumbnail doesn't exist, generate it
-        }
-        
-        console.log(`Generating thumbnail for ${photo.filename}...`);
-        const success = await generateThumbnail(photoPath, thumbnailPath);
-        
-        if (success) {
-          // Update database with thumbnail path
-          await db('photos')
-            .where('id', photo.id)
-            .update({
-              thumbnail_path: `thumbnails/${thumbnailFilename}`
-            });
-          
-          successCount++;
-          console.log(`✓ Generated thumbnail for ${photo.filename}`);
-        } else {
-          errorCount++;
-        }
-      } catch (error) {
-        console.error(`✗ Photo file not found: ${photoPath}`);
-        errorCount++;
-      }
-    }
-    
-    console.log('\nThumbnail regeneration complete!');
-    console.log(`- Successfully generated: ${successCount}`);
-    console.log(`- Skipped (already exist): ${skipCount}`);
-    console.log(`- Errors: ${errorCount}`);
-    console.log(`- Total processed: ${photos.length}`);
-    
-  } catch (error) {
-    console.error('Error during thumbnail regeneration:', error);
-    process.exit(1);
-  } finally {
-    await db.destroy();
   }
+
+  // Skip videos. A video's thumbnail is a poster frame produced by
+  // videoProcessor, not a resize of the stored file, so handing the container
+  // to Sharp here only ever produced one error per row.
+  //
+  // Tested on every marker a video row can carry, not media_type alone:
+  // fileWatcher.processNewPhoto writes `type` and `mime_type` but never
+  // media_type, which defaults to 'image' — so an auto-imported video passes a
+  // media_type-only filter. Each clause is null-safe on its own so a row that
+  // simply has no mime_type is not swept up with them.
+  query = query
+    .where(function () {
+      this.whereNull('media_type').orWhere('media_type', '!=', 'video');
+    })
+    .where(function () {
+      this.whereNull('type').orWhere('type', '!=', 'video');
+    })
+    .where(function () {
+      this.whereNull('mime_type').orWhereNot('mime_type', 'like', 'video/%');
+    });
+
+  const photos = await query;
+  console.log(`Found ${photos.length} photos to process`);
+
+  let successCount = 0;
+  let skipCount = 0;
+  let errorCount = 0;
+
+  for (const photo of photos) {
+    const label = photo.filename || `photo ${photo.id}`;
+    try {
+      const existing = photo.thumbnail_path;
+      // Asked BEFORE the call, not inferred from the returned path afterwards.
+      // On local and external storage the key is deterministic, so repairing a
+      // missing or corrupt thumbnail hands back the identical string — and
+      // comparing paths would report that repair as "already valid", which is
+      // the one number an operator running this is actually reading.
+      const wasValid = existing ? await isThumbnailValid(existing) : false;
+      const thumbnailPath = await ensureThumbnail(photo);
+
+      if (!thumbnailPath) {
+        console.error(`✗ Could not generate thumbnail for ${label}`);
+        errorCount++;
+        continue;
+      }
+
+      if (wasValid && thumbnailPath === existing) {
+        skipCount++;
+      } else {
+        successCount++;
+        console.log(`✓ Generated thumbnail for ${label}`);
+      }
+    } catch (error) {
+      console.error(`✗ Failed for ${label}: ${error.message}`);
+      errorCount++;
+    }
+  }
+
+  console.log('\nThumbnail regeneration complete!');
+  console.log(`- Generated: ${successCount}`);
+  console.log(`- Skipped (already valid): ${skipCount}`);
+  console.log(`- Errors: ${errorCount}`);
+  console.log(`- Total processed: ${photos.length}`);
+
+  return { successCount, skipCount, errorCount };
 }
 
-// Parse command line arguments
+if (require.main === module) {
-const eventId = process.argv[2] ? parseInt(process.argv[2]) : null;
+  const args = process.argv.slice(2);
+  const eventArg = args.find((a) => !a.startsWith('--'));
+  const eventId = eventArg ? parseInt(eventArg, 10) : null;
 
-// Run the script
+  if (eventArg && !Number.isInteger(eventId)) {
-regenerateThumbnails(eventId).then(() => {
+    console.error(`Not an event id: ${eventArg}`);
-  console.log('Script completed successfully');
+    process.exit(1);
-  process.exit(0);
+  }
-}).catch(error => {
+
-  console.error('Script failed:', error);
+  regenerateThumbnails(eventId)
-  process.exit(1);
+    .then(async (result) => {
-});
+      await db.destroy();
+      // Exit status is the only thing a cron job reads. Resolving with a
+      // nonzero errorCount and still exiting 0 told automation the backfill
+      // was done when it had failed — which is how an unavailable mount stays
+      // unnoticed until someone opens a gallery.
+      if (result.errorCount) {
+        console.error(`Script completed with failures: ${result.errorCount} photo(s)`);
+        process.exit(1);
+      }
+      console.log('Script completed successfully');
+      process.exit(0);
+    })
+    .catch(async (error) => {
+      console.error('Script failed:', error);
+      await db.destroy().catch(() => {});
+      process.exit(1);
+    });
+}
+
+module.exports = { regenerateThumbnails };