From afd95f06df784346999a9ba52f6438b29f96f691 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20F=C3=B6rtsch?= Date: Tue, 14 Apr 2026 18:29:00 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=93=E2=9C=93=20is=20now=20jellyfin-corrob?= =?UTF-8?q?orated,=20not=20a=20self-confirming=20ffprobe?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit user reported ad astra got the double checkmark instantly after transcode — correct, and correct to flag: the post-execute verifyDesiredState ran ffprobe on the file we had just written, so it tautologically matched the plan every time. not a second opinion. replaced the flow with the semantics we actually wanted: 1. refreshItem now returns { refreshed: boolean } — true when jellyfin's DateLastRefreshed actually advanced within the timeout, false when it didn't. callers can tell 'jellyfin really re-probed' apart from 'we timed out waiting'. 2. handOffToJellyfin post-job: refresh → (only if refreshed=true) fetch fresh streams → upsertJellyfinItem(source='webhook'). the rescan SQL sets verified=1 exactly when the fresh analysis sees is_noop=1, so ✓✓ now means 'jellyfin independently re-probed the file we wrote and agrees it matches the plan'. if jellyfin sees a drifted layout the plan flips back to pending so the user notices instead of the job silently rubber-stamping a bad output. 3. dropped the post-execute ffprobe block. the preflight-skipped branch no longer self-awards verified=1 either; it now does the same hand- off so jellyfin's re-probe drives the ✓✓ in that branch too. refreshItem's other two callers (review /rescan, subtitles /rescan) ignore the return value — their semantics haven't changed. Co-Authored-By: Claude Opus 4.6 (1M context) --- package.json | 2 +- server/api/execute.ts | 109 +++++++++++++++++++++++++----------- server/services/jellyfin.ts | 19 ++++++- 3 files changed, 92 insertions(+), 38 deletions(-) diff --git a/package.json b/package.json index 4aca19b..d9afa32 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "netfelix-audio-fix", - "version": "2026.04.14.19", + "version": "2026.04.14.20", "scripts": { "dev:server": "NODE_ENV=development bun --hot server/index.tsx", "dev:client": "vite", diff --git a/server/api/execute.ts b/server/api/execute.ts index c32550d..07b4a88 100644 --- a/server/api/execute.ts +++ b/server/api/execute.ts @@ -4,7 +4,9 @@ import { stream } from "hono/streaming"; import { getAllConfig, getDb } from "../db/index"; import { log, error as logError, warn } from "../lib/log"; import { predictExtractedFiles } from "../services/ffmpeg"; -import { refreshItem } from "../services/jellyfin"; +import { getItem, refreshItem } from "../services/jellyfin"; +import { loadLibrary as loadRadarrLibrary, isUsable as radarrUsable } from "../services/radarr"; +import { type RescanConfig, upsertJellyfinItem } from "../services/rescan"; import { getScheduleConfig, isInProcessWindow, @@ -13,13 +15,25 @@ import { sleepBetweenJobs, waitForProcessWindow, } from "../services/scheduler"; +import { loadLibrary as loadSonarrLibrary, isUsable as sonarrUsable } from "../services/sonarr"; import { verifyDesiredState } from "../services/verify"; import type { Job, MediaItem, MediaStream } from "../types"; /** - * Fire-and-forget hand-off to Jellyfin after a successful job: ask Jellyfin - * to re-scan the file and return immediately. The MQTT webhook subscriber - * closes the loop once Jellyfin finishes its rescan and publishes an event. + * Post-job hand-off to Jellyfin. Three phases: + * 1. refreshItem — ask Jellyfin to re-probe the file on disk and wait + * until its DateLastRefreshed advances (or 15s timeout). + * 2. getItem — pull back the freshly-probed metadata. + * 3. upsertJellyfinItem(source='webhook') — re-run the analyzer against + * Jellyfin's view. If it matches the plan (is_noop=1), sets verified=1 + * — the ✓✓ in the Done column. If Jellyfin sees a different layout + * (is_noop=0) the plan flips back to 'pending' so the user notices. + * + * This closes the previously-dangling "we asked Jellyfin to refresh but + * never checked what it saw" gap. Earlier attempt used our own ffprobe of + * the output, but that was tautological — ffmpeg just wrote the file to + * match the plan, so the check always passed immediately. Jellyfin is the + * independent observer that matters. */ async function handOffToJellyfin(itemId: number): Promise { const db = getDb(); @@ -32,10 +46,50 @@ async function handOffToJellyfin(itemId: number): Promise { const jellyfinCfg = { url: cfg.jellyfin_url, apiKey: cfg.jellyfin_api_key, userId: cfg.jellyfin_user_id }; if (!jellyfinCfg.url || !jellyfinCfg.apiKey) return; + let refreshResult: { refreshed: boolean }; try { - await refreshItem(jellyfinCfg, row.jellyfin_id); + refreshResult = await refreshItem(jellyfinCfg, row.jellyfin_id); } catch (err) { - warn(`Jellyfin refresh for item ${itemId} failed: ${String(err)}`); + warn(`Jellyfin refresh for item ${itemId} failed: ${String(err)} — skipping verification`); + return; + } + if (!refreshResult.refreshed) { + // DateLastRefreshed never advanced within the timeout — Jellyfin may + // still be probing asynchronously. We can't trust the item data we'd + // fetch right now, so skip the verify step; the plan stays verified=0 + // (single ✓) rather than risk flipping it based on stale metadata. + warn(`Jellyfin refresh for item ${itemId} timed out — leaving plan unverified`); + return; + } + + try { + const fresh = await getItem(jellyfinCfg, row.jellyfin_id); + if (!fresh) { + warn(`Jellyfin returned no item for ${row.jellyfin_id} during verification`); + return; + } + + const radarrCfg = { url: cfg.radarr_url, apiKey: cfg.radarr_api_key }; + const sonarrCfg = { url: cfg.sonarr_url, apiKey: cfg.sonarr_api_key }; + const radarrEnabled = cfg.radarr_enabled === "1" && radarrUsable(radarrCfg); + const sonarrEnabled = cfg.sonarr_enabled === "1" && sonarrUsable(sonarrCfg); + const [radarrLibrary, sonarrLibrary] = await Promise.all([ + radarrEnabled ? loadRadarrLibrary(radarrCfg) : Promise.resolve(null), + sonarrEnabled ? loadSonarrLibrary(sonarrCfg) : Promise.resolve(null), + ]); + const audioLanguages = JSON.parse(cfg.audio_languages || "[]") as string[]; + const rescanCfg: RescanConfig = { + audioLanguages, + radarr: radarrEnabled ? radarrCfg : null, + sonarr: sonarrEnabled ? sonarrCfg : null, + radarrLibrary, + sonarrLibrary, + }; + + const result = await upsertJellyfinItem(db, fresh, rescanCfg, { source: "webhook" }); + log(`Post-job verify for item ${itemId}: is_noop=${result.isNoop}`); + } catch (err) { + warn(`Post-job verification for item ${itemId} failed: ${String(err)}`); } } @@ -377,11 +431,17 @@ async function runJob(job: Job): Promise { "UPDATE jobs SET status = 'done', exit_code = 0, output = ?, completed_at = datetime('now') WHERE id = ?", ) .run(msg, job.id); - // Preflight matched → file is already correct, so the plan is - // both done AND independently verified. ✓✓ in the Done column. - db.prepare("UPDATE review_plans SET status = 'done', verified = 1 WHERE item_id = ?").run(job.item_id); + // Preflight matched → file is already correct per our own ffprobe. + // We still hand off to Jellyfin below so its independent re-probe + // drives the ✓✓ verified flag, rather than trusting our check of + // our own output. + db.prepare("UPDATE review_plans SET status = 'done' WHERE item_id = ?").run(job.item_id); })(); emitJobUpdate(job.id, "done", msg); + // Hand off so Jellyfin re-probes and can corroborate the ✓✓. + handOffToJellyfin(job.item_id).catch((err) => + warn(`Jellyfin hand-off for item ${job.item_id} failed: ${String(err)}`), + ); return; } log(`Job ${job.id} preflight: ${verify.reason} — running FFmpeg`); @@ -490,31 +550,12 @@ async function runJob(job: Job): Promise { log(`Job ${job.id} completed successfully`); emitJobUpdate(job.id, "done", fullOutput); - // Post-execute verification: ffprobe the output file and compare to - // the plan. Independent check that ffmpeg actually produced what we - // asked for — ffmpeg can exit 0 while having dropped a stream or - // muxed into an unexpected layout. Sets verified=1 (the ✓✓ signal) - // on match. If the probe disagrees, we leave verified=0 and log the - // reason; the plan is still 'done' (the job technically succeeded) - // but the UI will surface it as unverified so the user notices. - if (item) { - verifyDesiredState(db, job.item_id, item.file_path) - .then((v) => { - if (v.matches) { - db.prepare("UPDATE review_plans SET verified = 1 WHERE item_id = ?").run(job.item_id); - log(`Job ${job.id} post-verify: ${v.reason}`); - } else { - warn(`Job ${job.id} post-verify FAILED: ${v.reason}`); - } - }) - .catch((err) => warn(`Job ${job.id} post-verify errored: ${String(err)}`)); - } - - // Fire-and-forget: tell Jellyfin to rescan the file. The MQTT subscriber - // will pick up Jellyfin's resulting Library event and re-analyze the - // item — flipping the plan back to 'pending' if the on-disk streams - // don't actually match the plan. We don't await that; the job queue - // moves on. + // Fire-and-forget hand-off. Jellyfin re-probes the file we just wrote, + // we wait for DateLastRefreshed to advance, then re-analyze its fresh + // view. Setting verified=1 only happens when Jellyfin's independent + // probe confirms is_noop=1. If its view disagrees the plan flips back + // to 'pending' so the user notices — better than silently rubber- + // stamping a bad output as ✓✓. handOffToJellyfin(job.item_id).catch((err) => warn(`Jellyfin hand-off for item ${job.item_id} failed: ${String(err)}`), ); diff --git a/server/services/jellyfin.ts b/server/services/jellyfin.ts index 9e5b477..8ca1bec 100644 --- a/server/services/jellyfin.ts +++ b/server/services/jellyfin.ts @@ -139,7 +139,18 @@ export async function getItem(cfg: JellyfinConfig, jellyfinId: string): Promise< * Trigger a Jellyfin metadata refresh for a single item and wait until it completes. * Polls DateLastRefreshed until it changes (or timeout is reached). */ -export async function refreshItem(cfg: JellyfinConfig, jellyfinId: string, timeoutMs = 15000): Promise { +/** + * Trigger a Jellyfin metadata refresh and poll until the item's + * `DateLastRefreshed` advances. Returns true when the probe actually ran; + * false on timeout (caller decides whether to trust the item's current + * metadata or treat it as unverified — verification paths should NOT + * proceed on false, since a stale snapshot would give a bogus verdict). + */ +export async function refreshItem( + cfg: JellyfinConfig, + jellyfinId: string, + timeoutMs = 15000, +): Promise<{ refreshed: boolean }> { const itemUrl = `${cfg.url}/Items/${jellyfinId}`; // 1. Snapshot current DateLastRefreshed @@ -164,9 +175,11 @@ export async function refreshItem(cfg: JellyfinConfig, jellyfinId: string, timeo const checkRes = await fetch(itemUrl, { headers: headers(cfg.apiKey) }); if (!checkRes.ok) continue; const check = (await checkRes.json()) as { DateLastRefreshed?: string }; - if (check.DateLastRefreshed && check.DateLastRefreshed !== beforeDate) return; + if (check.DateLastRefreshed && check.DateLastRefreshed !== beforeDate) { + return { refreshed: true }; + } } - // Timeout reached — proceed anyway (refresh may still complete in background) + return { refreshed: false }; } /** Case-insensitive hints that a track is a dub / commentary, not the original. */