The two were ~90% identical (lock/skip, job_runs create, subprocess exec,
timeout/kill, finalize) and had already drifted once during the use_lock
change. Extract _execute(capture=bool) plus _record_run/_finalize_run helpers;
run_job and run_job_capture become thin wrappers. run_job_capture also gains
the use_lock parameter for free. Behavior is unchanged.
Covered by tests/test_pipeline_runner.py, now including run_job_capture output
capture + persistence and its skipped-lock case (42 tests green).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Navidrome password is now passed to curl via stdin (--data-urlencode "p@-")
in navidrome-scan.sh and pipeline-status.sh, so it no longer appears in
ps/proc. Verified the query sent is identical and a live scan still triggers.
- MIN_ARTIST_DIRS (the share-health gate) is now a setting, threaded through to
the pipeline env, so a user with a small library can lower it instead of the
scan/sync being permanently blocked by the hardcoded 500.
- /auth/logout is now POST-only (with a nav form + aria-label), so a drive-by
GET can't log the user out; enforced allowed_email already landed separately.
- view_log now confirms the run's log_path resolves under the logs dir before
serving it (defense in depth).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Root cause of a missed 9am notification: the read-only pipeline_status_report
runs at 09:30, but enrich_buy_url (09:10, --apply) held the single global
pipeline lock long past 09:30, so the status report recorded skipped_lock and
never sent its Telegram digest. build_fingerprint_index (09:25) was starved the
same way.
- run_job/run_lib_script gain use_lock; the status report runs with
use_lock=False since it only reads logs, pings Navidrome, and sends Telegram.
It can no longer be blocked by a long write job, and running concurrently is
safe.
- _lib was silently dropping its timeout argument, so maintenance jobs had no
timeout and a hung one held the lock until the next container restart. Thread
timeout through _lib -> run_lib_script -> run_job, cap enrich_buy_url at 30m
and build_fingerprint_index at 60m, and give the status report a 5m cap.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dedup-library.sh: additive --json (one NDJSON line per candidate deletion
to stdout, alongside the unchanged human log) and --only-paths FILE (in
--apply mode, only actually delete entries whose path is in FILE; without
it, --apply deletes everything as before -- existing direct callers are
unaffected). Without --only-paths the safety re-verification is free:
--apply --only-paths re-runs all 4 passes from scratch on every invocation,
so if a group's ranking changed since a scan (e.g. the old keep_path is
gone), the fresh pass assigns the previously-"delete" path the KEEP role
instead and the only-paths allowlist naming it is simply never consulted --
no duplicate ranking logic needed in the review service.
spotify-genre.py: additive --json emitting one JSON line per genre change
(dry-run or --apply) for genre_review_service to persist.
pipeline_runner.run_job_capture(): like run_job() but captures stdout as
text (still under the same shared lock, still writes a job_runs row) for
callers that need to parse structured output rather than just log it.
dedup_review_service.scan() persists dry-run candidates into
dedup_runs/dedup_candidates. confirm_and_apply() re-checks confirmed
candidates still exist before invoking --apply --only-paths, so nothing is
ever deleted without an explicit confirm -- matches the false-negative-
biased dedup preference. scheduler_service's maintenance:dedup job now
goes through this (still dry-run only, every day).
genre_review_service.run() wraps spotify-genre.py for both dry-run preview
and the real scheduled --apply run, persisting every run's diff into
genre_runs/genre_candidates either way -- genre writes keep their current
auto-apply behavior (low-risk, reversible, GENRE_LOCK-protected) but are
now reviewable after the fact. lock_artist_genre() gives a one-click revert
path when a run gets something wrong.
Added minimal routers+templates for /dedup (scan, review, confirm-and-
delete) and /genres (preview, review, lock-old-genre).
Verified end-to-end against REAL duplicate files (not mocked): built an
actual FLAC+MP3 duplicate pair in a real beets library, ran dedup-library.sh
--json and confirmed correct JSON output, verified --apply --only-paths
with an empty confirm list deletes nothing and with the real confirmed path
deletes exactly that file (DB + disk) while preserving the FLAC, and ran
the full dedup_review_service scan->confirm->apply flow through the same
fixture. genre_review_service and spotify-genre.py --json verified against
mocked/direct output (spotify-genre.py's own artist-genre lookup needs a
live Spotify API call, out of reach in this sandbox). Confirmed the full
app boots with all five routers registered.
pipeline_runner: subprocess execution wrapper replacing flock -n with a
single asyncio.Lock (deliberately one global lock, not per-resource --
matches the old flock's all-jobs-share-one-lock behavior rather than
over-engineering it). Records one job_runs row per invocation
(started/finished/status/exit_code/summary/log_path/triggered_by).
If the lock is already held, records status='skipped_lock' immediately
instead of silently dropping the run.
Found and fixed a real concurrency bug via testing: the obvious
asyncio.wait_for(lock.acquire(), timeout=0) idiom for a non-blocking
try-acquire is broken in asyncio -- the wrapping Task's first iteration and
the timeout-0 callback race with no guaranteed ordering, so it timed out on
literally every call, including the very first uncontended one. Fixed using
lock.locked() + acquire(), relying on acquire()'s fast path never
suspending when uncontended.
scheduler_service: AsyncIOScheduler with the default in-memory job store
(NOT SQLAlchemyJobStore -- it pickles job functions to persist them, and
the _lib()/_beet() factory closures here aren't picklable; MemoryJobStore
avoids this since register_all_jobs() rebuilds the whole schedule from code
+ DB on every startup anyway). All 19 maintenance jobs ported 1:1 from
/etc/cron.d/sldl-maintenance with their exact cron expressions; dedup
deliberately has no --apply (dry-run only on schedule, per the
false-negative-biased dedup preference). Playlist jobs are synced live from
the playlists table (sync_playlist_jobs), so add/remove-in-the-UI takes
effect with no redeploy -- wired into playlist_service's create/update/
delete. Maintenance job enable/disable persists to the scheduled_jobs table
so a paused job stays paused across a restart despite the in-memory job
store. trigger_now() supports "Run now" by invoking a job's function
immediately with triggered_by='manual', bypassing its schedule.
Wired into main.py's lifespan (start on boot, shutdown on exit).
Verified via integration tests: 36 jobs register correctly (17 seeded
playlists + 19 maintenance) with correct cron triggers; live playlist
create/delete correctly adds/removes its scheduler job; maintenance
enable/disable persists to the DB and takes effect live; trigger_now
correctly bypasses the schedule; pipeline_runner correctly handles
success/failure/timeout/concurrent-lock-contention with real subprocesses;
and the full app boots with the scheduler running and shuts down cleanly.
andrew