Quarantine: manage a quarantined file from the download modal (Listen / Accept / Search)

Clicking a quarantined track's status used to open the generic search modal,
identical to a plain failure — no way to review or recover the file. It now
opens a chooser:
- Listen: streams the file in-app via a new /api/quarantine/<id>/stream
  endpoint (range-supported; the real audio Content-Type is recovered from the
  sidecar since the on-disk file ends in .quarantined).
- Accept & Import: existing /approve (restore + re-import, gates bypassed).
- Search for a different result: the existing candidates modal (old behavior).

Non-quarantine failures (not_found / failed / cancelled) are unchanged — a
single click listener routes by dataset set at render time, so a task that
fails then later quarantines can't end up double-bound.

Also fixes the Accept failure on Windows: the Listen stream holds an open file
handle, so the subsequent restore move hit WinError 32 ('file in use') and the
endpoint mislabeled it 'thin sidecar'. Accept now releases the audio handle
before approving, and approve/recover moves retry briefly on transient OS locks
(_move_with_retry). Accept also auto-falls-back to Recover-to-Staging for
genuinely thin/orphaned sidecars.

Tests: stream-info resolution (sidecar + filename-fallback + missing), and
_move_with_retry success/give-up.
This commit is contained in:
BoulderBadgeDad 2026-05-31 15:41:04 -07:00
parent ec8c8d939c
commit 3060678f29
5 changed files with 354 additions and 10 deletions

View file

@ -14,6 +14,7 @@ from __future__ import annotations
import json
import os
import shutil
import time
from pathlib import Path
from typing import Any, Dict, List, Optional, Tuple
@ -289,6 +290,56 @@ def _restore_filename(quarantined_filename: str, sidecar_original: Optional[str]
return base
def get_quarantine_entry_stream_info(
quarantine_dir: str, entry_id: str
) -> Optional[Tuple[str, str]]:
"""Resolve a quarantined entry to ``(file_path, original_extension)`` for
in-app playback.
The on-disk file carries a ``.quarantined`` suffix, so its own extension is
useless for picking an audio MIME type. Recover the real extension from the
sidecar's ``original_filename`` when present, else from the quarantine
filename convention. Returns None when the entry's file can't be found.
"""
file_path, sidecar_path = _resolve_entry_paths(quarantine_dir, entry_id)
if not file_path or not os.path.isfile(file_path):
return None
sidecar_original: Optional[str] = None
if sidecar_path:
try:
with open(sidecar_path, encoding="utf-8") as f:
sidecar_original = json.load(f).get("original_filename")
except Exception as exc:
logger.debug("stream-info: sidecar read failed for %s: %s", entry_id, exc)
original_name = _restore_filename(os.path.basename(file_path), sidecar_original)
extension = os.path.splitext(original_name)[1].lower()
return file_path, extension
def _move_with_retry(src: str, dst: str, attempts: int = 4, delay: float = 0.4) -> bool:
"""Move a file, retrying briefly on transient OS locks.
On Windows a still-open read handle (e.g. the in-app quarantine preview
player that just streamed the file) makes shutil.move raise
PermissionError / WinError 32 "file in use". The handle is released a beat
after playback stops, so a few short retries clear the common case without
failing the whole Approve/Recover. Returns True on success.
"""
last_exc: Optional[BaseException] = None
for i in range(attempts):
try:
shutil.move(src, dst)
return True
except OSError as exc:
last_exc = exc
if i < attempts - 1:
time.sleep(delay)
logger.error("move failed after %d attempts: %s -> %s: %s", attempts, src, dst, last_exc)
return False
def approve_quarantine_entry(
quarantine_dir: str,
entry_id: str,
@ -334,10 +385,9 @@ def approve_quarantine_entry(
restored_path = os.path.join(restore_dir, original_name)
restored_path = _ensure_unique_path(restored_path)
try:
shutil.move(file_path, restored_path)
except OSError as exc:
logger.error("approve: failed to restore %s -> %s: %s", file_path, restored_path, exc)
if not _move_with_retry(file_path, restored_path):
logger.error("approve: failed to restore %s -> %s (file may still be in use)",
file_path, restored_path)
return None
try:
@ -377,10 +427,8 @@ def recover_to_staging(
os.makedirs(staging_dir, exist_ok=True)
target = _ensure_unique_path(os.path.join(staging_dir, restored_name))
try:
shutil.move(file_path, target)
except OSError as exc:
logger.error("recover: failed to move %s -> %s: %s", file_path, target, exc)
if not _move_with_retry(file_path, target):
logger.error("recover: failed to move %s -> %s (file may still be in use)", file_path, target)
return None
if sidecar_path and os.path.isfile(sidecar_path):

View file

@ -5,6 +5,7 @@ from core.imports.quarantine import (
approve_quarantine_entry,
delete_quarantine_entry,
entry_id_from_quarantined_filename,
get_quarantine_entry_stream_info,
get_quarantined_source_keys,
list_quarantine_entries,
recover_to_staging,
@ -125,6 +126,41 @@ def test_list_handles_orphan_quarantined_file_without_sidecar(tmp_path):
assert entries[0]["has_full_context"] is False
# ──────────────────────────────────────────────────────────────────────
# get_quarantine_entry_stream_info — in-app "Listen" support
# ──────────────────────────────────────────────────────────────────────
def test_stream_info_resolves_path_and_extension_from_sidecar(tmp_path):
qfile, _ = _write_entry(tmp_path, "20260514_120000", "song.flac", with_context=True)
entry_id = entry_id_from_quarantined_filename(qfile.name)
info = get_quarantine_entry_stream_info(str(tmp_path), entry_id)
assert info is not None
file_path, ext = info
assert file_path == str(qfile)
assert ext == ".flac" # real audio ext, NOT ".quarantined"
def test_stream_info_recovers_extension_without_sidecar(tmp_path):
# Orphan .quarantined with no sidecar — extension comes from the filename
# convention so playback still gets a correct Content-Type.
qfile = tmp_path / "20260514_120000_orphan.mp3.quarantined"
qfile.write_bytes(b"X" * 100)
entry_id = entry_id_from_quarantined_filename(qfile.name)
info = get_quarantine_entry_stream_info(str(tmp_path), entry_id)
assert info is not None
file_path, ext = info
assert file_path == str(qfile)
assert ext == ".mp3"
def test_stream_info_returns_none_for_missing_entry(tmp_path):
assert get_quarantine_entry_stream_info(str(tmp_path), "does_not_exist") is None
def test_list_skips_orphan_sidecars_without_file(tmp_path):
sidecar = tmp_path / "20260514_120000_only.json"
sidecar.write_text(json.dumps({"original_filename": "only.flac", "quarantine_reason": "x"}))
@ -398,3 +434,23 @@ def test_source_keys_dedup_repeated_sources(tmp_path):
keys = get_quarantined_source_keys(str(tmp_path))
assert keys == {("peer", "dupe.flac")}
# ──────────────────────────────────────────────────────────────────────
# _move_with_retry — resilient move (Windows file-lock case)
# ──────────────────────────────────────────────────────────────────────
def test_move_with_retry_succeeds(tmp_path):
from core.imports.quarantine import _move_with_retry
src = tmp_path / "a.flac"; src.write_bytes(b"x" * 10)
dst = tmp_path / "out" / "a.flac"
(tmp_path / "out").mkdir()
assert _move_with_retry(str(src), str(dst)) is True
assert dst.exists() and not src.exists()
def test_move_with_retry_returns_false_on_missing_source(tmp_path):
from core.imports.quarantine import _move_with_retry
# attempts=1 keeps the test fast (no retry sleeps)
assert _move_with_retry(str(tmp_path / "nope.flac"), str(tmp_path / "dst.flac"),
attempts=1, delay=0) is False

View file

@ -7131,6 +7131,25 @@ def approve_quarantine_item(entry_id):
return jsonify({"success": False, "error": str(e)}), 500
@app.route('/api/quarantine/<entry_id>/stream', methods=['GET'])
def stream_quarantine_item(entry_id):
"""Stream a quarantined audio file in-app (range-supported) so the user can
listen before deciding to approve, search again, or delete it. The file
lives in the quarantine dir with a `.quarantined` suffix, so the real audio
extension (and thus Content-Type) is recovered from the sidecar."""
try:
from core.imports.quarantine import get_quarantine_entry_stream_info
info = get_quarantine_entry_stream_info(_get_quarantine_dir(), entry_id)
if info is None:
return jsonify({"error": "Quarantined file not found"}), 404
file_path, extension = info
mimetype = _AUDIO_MIME_TYPES.get(extension, 'audio/mpeg')
return _serve_audio_file_with_range(file_path, mimetype_override=mimetype)
except Exception as e:
logger.error(f"[Quarantine] Error streaming {entry_id}: {e}")
return jsonify({"error": str(e)}), 500
@app.route('/api/quarantine/<entry_id>/recover', methods=['POST'])
def recover_quarantine_item(entry_id):
"""Fallback for legacy thin sidecars: move file into Staging so the user
@ -11877,17 +11896,22 @@ _AUDIO_MIME_TYPES = {
}
def _serve_audio_file_with_range(file_path):
def _serve_audio_file_with_range(file_path, mimetype_override=None):
"""Serve an on-disk audio file with HTTP range support (HTML5 seeking).
Shared by /stream/audio (current track) and /stream/library-audio (the
crossfade pre-loader, which plays the NEXT track on a second <audio>).
``mimetype_override`` lets callers supply the Content-Type explicitly when
the on-disk extension isn't the audio extension — e.g. the quarantine
streamer, whose files end in ``.quarantined`` (real type recovered from the
sidecar). When None, the type is inferred from the file extension as before.
"""
if not os.path.exists(file_path):
return jsonify({"error": "Audio file not found"}), 404
file_ext = os.path.splitext(file_path)[1].lower()
mimetype = _AUDIO_MIME_TYPES.get(file_ext, 'audio/mpeg')
mimetype = mimetype_override or _AUDIO_MIME_TYPES.get(file_ext, 'audio/mpeg')
file_size = os.path.getsize(file_path)
range_header = request.headers.get('Range', None)

View file

@ -2982,6 +2982,19 @@ function _ensureCandidatesClickListener(statusEl) {
statusEl.addEventListener('click', function (e) {
e.stopPropagation();
_hideErrorTooltip();
// Single click handler routes by dataset set at render time. A
// quarantined task opens the chooser; everything else opens the search
// modal. Deciding at click-time (not bind-time) means a task that fails
// first and only quarantines on a later retry can't end up double-bound.
if (this.dataset.quarantineEntryId) {
showQuarantineChooser({
entryId: this.dataset.quarantineEntryId,
taskId: this.dataset.taskId,
reason: this.dataset.quarantineReason || '',
trackName: this.dataset.quarantineTrack || 'this track',
});
return;
}
const taskId = this.dataset.taskId;
if (taskId) showCandidatesModal(taskId);
});
@ -3377,6 +3390,122 @@ async function approveQuarantineFromDownloadRow(button) {
}
}
// ── Quarantine chooser (download-modal status click) ──────────────────────
// Clicking a 🛡️ Quarantined status opens this instead of the search modal:
// listen to the file, accept & import it (bypassing the failed check), or fall
// back to searching for a different result. (Routing is handled inside
// _ensureCandidatesClickListener so a single listener covers both cases.)
// Stop and unload the chooser's <audio> so the server closes its read handle
// on the quarantined file. On Windows an open handle blocks the subsequent
// Approve/Recover move (WinError 32 "file in use") — listening then accepting
// would otherwise fail. Returns a short promise so callers can let the
// streaming connection actually tear down before moving the file.
function _qcReleaseAudio() {
const aud = document.getElementById('qc-audio');
if (!aud) return Promise.resolve();
try {
aud.pause();
aud.removeAttribute('src');
aud.load(); // forces the browser to drop the current stream connection
} catch (e) { /* ignore */ }
return new Promise((r) => setTimeout(r, 400));
}
function closeQuarantineChooser() {
const o = document.getElementById('quarantine-chooser-overlay');
if (o) o.remove();
}
function showQuarantineChooser({ entryId, taskId, reason, trackName }) {
closeQuarantineChooser();
const streamUrl = entryId ? `/api/quarantine/${encodeURIComponent(entryId)}/stream` : '';
const overlay = document.createElement('div');
overlay.id = 'quarantine-chooser-overlay';
overlay.className = 'candidates-modal-overlay';
overlay.addEventListener('mousedown', (e) => { if (e.target === overlay) closeQuarantineChooser(); });
overlay.innerHTML = `
<div class="candidates-modal quarantine-chooser-modal">
<div class="candidates-modal-header">
<div>
<h2>🛡 Quarantined File</h2>
<div class="quarantine-chooser-track">${escapeHtml(trackName)}</div>
</div>
<button class="candidates-modal-close" onclick="closeQuarantineChooser()">×</button>
</div>
<div class="quarantine-chooser-body">
<div class="quarantine-chooser-reason"><strong>Why it was quarantined:</strong><br>${escapeHtml(reason) || 'Unknown reason'}</div>
<div class="quarantine-chooser-listen">
<label>Listen before deciding</label>
${streamUrl ? `<audio id="qc-audio" controls preload="none" src="${streamUrl}"></audio>` : '<span class="quarantine-chooser-muted">Playback unavailable for this entry.</span>'}
</div>
<div class="quarantine-chooser-actions">
<button class="quarantine-chooser-btn quarantine-chooser-accept" id="qc-accept-btn"> Accept &amp; Import</button>
<button class="quarantine-chooser-btn quarantine-chooser-search" id="qc-search-btn">🔍 Search for a different result</button>
</div>
</div>
</div>`;
document.body.appendChild(overlay);
requestAnimationFrame(() => overlay.classList.add('visible'));
const acceptBtn = overlay.querySelector('#qc-accept-btn');
if (acceptBtn) acceptBtn.addEventListener('click', () => acceptQuarantineFromChooser(acceptBtn, entryId));
const searchBtn = overlay.querySelector('#qc-search-btn');
if (searchBtn) searchBtn.addEventListener('click', () => {
closeQuarantineChooser();
if (taskId) showCandidatesModal(taskId);
});
}
async function acceptQuarantineFromChooser(button, entryId) {
if (!entryId) { showToast('Cannot accept — missing quarantine id.', 'error'); return; }
const confirmed = await showConfirmDialog({
title: 'Accept Quarantined File',
message: 'Import this file and skip the quarantine checks for this approved pass?',
confirmText: 'Accept & Import',
cancelText: 'Cancel',
});
if (!confirmed) return;
const original = button.textContent;
button.disabled = true;
button.textContent = 'Importing…';
const restore = () => { button.disabled = false; button.textContent = original; };
// Release the preview stream first so the file isn't locked during the move.
await _qcReleaseAudio();
try {
const resp = await fetch(`/api/quarantine/${encodeURIComponent(entryId)}/approve`, { method: 'POST' });
const data = await resp.json();
if (data.success) {
showToast('Accepted. Re-running post-processing.', 'success');
closeQuarantineChooser();
return;
}
// One-click approve needs the saved import context. Older / orphaned
// quarantine entries (thin or missing sidecar) can't be re-imported in
// place — fall back to moving the file into Staging so the user can
// finish via the Import flow, same as the Quarantine manager does.
const needsRecover = /thin sidecar|recover to staging|embedded context|missing file or sidecar/i.test(data.error || '');
if (needsRecover) {
button.textContent = 'Recovering…';
const rec = await fetch(`/api/quarantine/${encodeURIComponent(entryId)}/recover`, { method: 'POST' });
const recData = await rec.json();
if (recData.success) {
showToast('Older entry — moved to Staging. Finish it from the Import page.', 'success');
closeQuarantineChooser();
return;
}
showToast(`Recover failed: ${recData.error || 'Unknown error'}`, 'error');
restore();
return;
}
showToast(`Accept failed: ${data.error || 'Unknown error'}`, 'error');
restore();
} catch (err) {
showToast(`Accept failed: ${err.message}`, 'error');
restore();
}
}
function closeCandidatesModal() {
const overlay = document.getElementById('candidates-modal-overlay');
if (overlay) {
@ -3655,6 +3784,19 @@ function processModalStatusUpdate(playlistId, data) {
if (task.status === 'not_found' || task.status === 'failed' || task.status === 'cancelled') {
statusEl.classList.add('has-candidates');
statusEl.dataset.taskId = task.task_id;
// Quarantined files route to the chooser (Listen / Accept /
// Search); everything else to the search modal. Set or CLEAR
// the quarantine flags every render so a row that flips
// between failure kinds always reflects its current state.
if (isQuarantinedTask && task.quarantine_entry_id) {
statusEl.dataset.quarantineEntryId = task.quarantine_entry_id;
statusEl.dataset.quarantineReason = task.error_message || '';
statusEl.dataset.quarantineTrack = (task.track_info && task.track_info.name) || '';
} else {
delete statusEl.dataset.quarantineEntryId;
delete statusEl.dataset.quarantineReason;
delete statusEl.dataset.quarantineTrack;
}
_ensureCandidatesClickListener(statusEl);
}
console.debug(`✅ [Status Update] Updated track ${task.track_index} to: ${statusText}${isV2Task ? ' (V2)' : ''}`);

View file

@ -42394,6 +42394,80 @@ div.artist-hero-badge {
color: #fff;
}
/* ── Quarantine chooser (download-modal status click) ───────────────────── */
.quarantine-chooser-modal {
max-width: 540px;
}
.quarantine-chooser-track {
color: rgba(255, 255, 255, 0.65);
font-size: 13px;
margin-top: 4px;
}
.quarantine-chooser-body {
padding: 22px 28px 26px;
display: flex;
flex-direction: column;
gap: 18px;
}
.quarantine-chooser-reason {
background: rgba(250, 204, 21, 0.08);
border: 1px solid rgba(250, 204, 21, 0.25);
border-radius: 10px;
padding: 12px 14px;
color: rgba(255, 255, 255, 0.85);
font-size: 13px;
line-height: 1.5;
}
.quarantine-chooser-listen {
display: flex;
flex-direction: column;
gap: 8px;
}
.quarantine-chooser-listen label {
color: rgba(255, 255, 255, 0.7);
font-size: 12px;
text-transform: uppercase;
letter-spacing: 0.04em;
}
.quarantine-chooser-listen audio {
width: 100%;
}
.quarantine-chooser-muted {
color: rgba(255, 255, 255, 0.45);
font-size: 13px;
font-style: italic;
}
.quarantine-chooser-actions {
display: flex;
flex-direction: column;
gap: 10px;
}
.quarantine-chooser-btn {
padding: 12px 16px;
border-radius: 10px;
border: 1px solid rgba(255, 255, 255, 0.12);
background: rgba(255, 255, 255, 0.05);
color: #fff;
font-size: 14px;
font-weight: 600;
cursor: pointer;
transition: background 0.15s ease, border-color 0.15s ease;
}
.quarantine-chooser-btn:hover:not(:disabled) {
background: rgba(255, 255, 255, 0.12);
}
.quarantine-chooser-btn:disabled {
opacity: 0.6;
cursor: default;
}
.quarantine-chooser-accept {
border-color: rgba(var(--accent-rgb), 0.5);
background: rgba(var(--accent-rgb), 0.15);
}
.quarantine-chooser-accept:hover:not(:disabled) {
background: rgba(var(--accent-rgb), 0.28);
}
.candidates-modal-body {
padding: 20px 28px;
overflow-y: auto;