Abraham Lincoln crashed with exit 234 because the file had 14 dvd_subtitle
streams: our extraction dict only keyed on the long form (dvd_subtitle)
while jellyfin stores the short form (dvdsub), so the lookup fell back
to .srt, ffmpeg picked the srt muxer, and srt can't encode image-based
subs. textbook silent dict miss.
replaced the extension dict with an EXTRACTABLE map that pairs codec →
{ext, codecArg} and explicitly enumerates every codec we can route to a
single-file sidecar. everything else (dvd_subtitle/dvdsub, dvb_subtitle/
dvbsub, unknown codecs) is now skipped at command-build time. the plan
picks up a note like '14 subtitle(s) dropped: dvdsub (eng, est, ind,
kor, jpn, lav, lit, may, chi, chi, tha, vie, rus, ukr) — not extractable
to sidecar' so the user sees exactly what didn't make it.
also added extractErrorSummary in execute.ts: when a job errors, scan
the last 60 stderr lines for fatal keywords (Error:, Conversion failed!,
Unsupported, Invalid argument, Permission denied, No space left, …),
dedupe, prepend the summary to the job's stored output. the review_plan
notes get the same summary — surfaces the real cause next to the plan
instead of burying it under ffmpeg's 200-line banner.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
worked through AUDIT.md. triage:
- finding 2 (subtitle rescan wipes decisions): confirmed. /:id/rescan now
snapshots custom_titles and calls reanalyze() after the stream delete/
insert, mirroring the review rescan flow. exported reanalyze + titleKey
from review.ts so both routes share the logic.
- finding 3 (scan limit accepts NaN/negatives): confirmed. extracted
parseScanLimit into a pure helper, added unit tests covering NaN,
negatives, floats, infinity, numeric strings. invalid input 400s and
releases the scan_running lock.
- finding 4 (parseId lenient): confirmed. tightened the regex to /^\d+$/
so "42abc", "abc42", "+42", "42.0" all return null. rewrote the test
that codified the old lossy behaviour.
- finding 5 (setup_complete set before jellyfin test passes): confirmed.
the /jellyfin endpoint still persists url+key unconditionally, but now
only flips setup_complete=1 on a successful connection test.
- finding 6 (swallowed errors): partial. the mqtt restart and version-
fetch swallows are intentional best-effort with downstream surfaces
(getMqttStatus, UI fallback). only the scan.ts db-update swallow was
a real visibility gap — logs via logError now.
- finding 1 (auth): left as-is. redacting secrets on GET without auth
on POST is security theater; real fix is an auth layer, which is a
design decision not a bugfix. audit removed from the tree.
- lint fail on ffmpeg.test.ts: formatted.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
root cause: all five job-insert sites in review.ts blindly inserted a
'pending' row, so a double-click on approve (or an overlap between
/approve-all and individual /approve) wrote N jobs for the same item.
job 1 stripped subtitles + reordered audio; jobs 2..N then ran the
same stale stream-index command against the already-processed file
and ffmpeg bailed with 'Stream map matches no streams'.
fix: funnel every insert through enqueueAudioJob(), which only writes
when no pending job already exists for that item. covers approve,
retry, approve-all, season approve-all, series approve-all.
