✓✓ is now jellyfin-corroborated, not a self-confirming ffprobe
All checks were successful
Build and Push Docker Image / build (push) Successful in 1m11s
All checks were successful
Build and Push Docker Image / build (push) Successful in 1m11s
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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<void> {
|
||||
const db = getDb();
|
||||
@@ -32,10 +46,50 @@ async function handOffToJellyfin(itemId: number): Promise<void> {
|
||||
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<void> {
|
||||
"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<void> {
|
||||
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)}`),
|
||||
);
|
||||
|
||||
@@ -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<void> {
|
||||
/**
|
||||
* 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. */
|
||||
|
||||
Reference in New Issue
Block a user