From 93b4815d8432e186d4da6e0a4b8462086a259044 Mon Sep 17 00:00:00 2001 From: rays Date: Thu, 10 Sep 2026 01:11:12 +0000 Subject: [PATCH] Make the UI's Download button download the episode you clicked POST /api/enclosures/{id}/download requeued the row and asked for a normal scan, but a scan takes the lowest-id pending rows up to max_new_per_check. With a large backlog and a small cap the requested row was never a candidate, so other episodes downloaded while it stayed pending. A queue expresses what is outstanding, not what was asked for. Download is now its own command that fetches one specific enclosure immediately, ignoring queue order and the per-scan cap, still via the single worker. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RPyeapneuXrCdojsaiXGbe --- PROGRESS.md | 24 ++++++++++++++++++++++ src/ipc.rs | 11 ++++++++++ src/main.rs | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++ src/web.rs | 12 ++++++----- 4 files changed, 100 insertions(+), 5 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index c5ce04e..1ad4bc2 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -56,6 +56,30 @@ and until now nothing set them. --- +## 2026-09-10 — Fixed: the UI's Download button downloaded the wrong episodes + +Reported from the running instance: clicking Download on *Music from a Darkened Room | Session Zero* +showed progress, then left the episode at `pending`. + +**Cause, and it was a design error in step 11, not a glitch.** `POST /api/enclosures/{id}/download` +requeued the row to `pending` and asked for an ordinary scan, justified at the time as "needed no +new machinery — the queue is the table". But a scan means *take the lowest-id pending rows, up to +`max_new_per_check`*. Against a 125-item backlog with a cap of 2, that is ids 7 and 8; the requested +row 11 was never a candidate. The progress on screen was two other episodes downloading past it. + +The queue can express what is outstanding but not what was *asked for*. `Command::Download { +enclosure }` is now its own command: it fetches that one row immediately, ignoring both queue order +and the per-scan cap, still through the single worker so it cannot overlap a scan. Torrent +enclosures route to `torrent_one` the same way a scan would. + +Verified live: enclosure 11 went `pending` -> `done`, 90.3 MB on disk, and nothing else was pulled +in its place. `cargo test` 30/30, with the new command's wire form pinned. + +Worth remembering: **the daemon scans immediately on its first tick**, so every restart costs +`max_new_per_check` episodes on the normal path. + +--- + ## 2026-09-10 — Phase 2: web front end (steps 9-13) axum in the daemon process, plain HTML/JS in `web/index.html` (embedded with `include_str!`), no diff --git a/src/ipc.rs b/src/ipc.rs index b2c69c8..d66922a 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -90,6 +90,12 @@ pub enum Command { #[serde(default)] dry_run: bool, }, + /// Fetch one specific enclosure now, ignoring max_new_per_check and the queue order. + /// A scan cannot express "this one, now": it takes the lowest-id pending rows up to + /// the per-scan cap, so an explicit request has to bypass both. + Download { + enclosure: i64, + }, Status, } @@ -234,6 +240,11 @@ mod tests { let got: Command = serde_json::from_str(r#"{"cmd":"reap","dry_run":true}"#).unwrap(); assert!(matches!(got, Command::Reap { dry_run: true })); + // "Download this one now" is its own command precisely because a scan cannot + // express it: a scan takes the lowest-id pending rows up to max_new_per_check. + let got: Command = serde_json::from_str(r#"{"cmd":"download","enclosure":11}"#).unwrap(); + assert!(matches!(got, Command::Download { enclosure: 11 })); + assert!(serde_json::from_str::(r#"{"cmd":"nope"}"#).is_err()); } diff --git a/src/main.rs b/src/main.rs index a0c9a88..e7c86d8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -175,6 +175,7 @@ async fn run(ctx: &Ctx, cmd: Cmd) -> Result<()> { fetch(ctx, feed.as_deref(), force).await } Cmd::Reap { dry_run } => reap(ctx, dry_run, true), + Cmd::Download { enclosure } => download_one(ctx, enclosure).await, Cmd::Status => { let (pending, downloaded) = ctx.db.counts()?; ctx.out.emit(Event::Status { feeds: ctx.cfg().feeds.len(), pending, downloaded }); @@ -716,6 +717,63 @@ async fn fetch_one( Ok((path, got.bytes)) } +/// Downloads one specific enclosure immediately, whatever the per-scan cap says and +/// wherever it sits in the queue. +async fn download_one(ctx: &Ctx, id: i64) -> Result<()> { + let cfg = ctx.cfg(); + let enc = ctx + .db + .enclosure(id)? + .ok_or_else(|| anyhow::anyhow!("no enclosure {id}"))?; + if enc.path.is_some() { + return Ok(()); // Already here. + } + let feed_cfg = cfg + .feeds + .get(&enc.feed_id) + .ok_or_else(|| anyhow::anyhow!("enclosure {id} belongs to unsubscribed feed {:?}", enc.feed_id))?; + + let title = ctx.db.feed_summary(&enc.feed_id)?.title; + let folder = download::folder_for(&cfg, &enc.feed_id, feed_cfg, title.as_deref()); + let dest_dir = cfg.general.download_dir.join(&folder); + + ctx.out.emit(Event::FeedStart { feed: enc.feed_id.clone() }); + let is_torrent = download::looks_like_torrent(&enc.url, enc.mime.as_deref()); + let result = if is_torrent { + if !cfg.torrent.enabled { + Err(anyhow::anyhow!("torrents are disabled")) + } else { + torrent_one(ctx, &enc.feed_id, &enc.url, &dest_dir).await + } + } else { + fetch_one(ctx, &enc.feed_id, feed_cfg, &enc.url, &dest_dir).await + }; + + match result { + Ok((path, bytes)) => { + ctx.db.mark_downloaded(&enc.url, &path, bytes)?; + ctx.out.emit(Event::DownloadDone { + feed: enc.feed_id.clone(), + url: enc.url.clone(), + path: path.display().to_string(), + bytes, + }); + } + Err(e) => { + let msg = format!("{e:#}"); + ctx.db.mark_enclosure(&enc.url, "error", Some(&msg))?; + ctx.out.emit(Event::DownloadError { + feed: enc.feed_id.clone(), + url: enc.url.clone(), + msg, + }); + } + } + // Terminal, so a UI waiting on this request stops here. + ctx.out.emit(Event::ScanDone { feeds: 1 }); + Ok(()) +} + /// Torrent progress is reported the same way an HTTP download's is, throttled to whole /// percents so a UI is not flooded. async fn torrent_one( diff --git a/src/web.rs b/src/web.rs index 2dae41e..b191fe6 100644 --- a/src/web.rs +++ b/src/web.rs @@ -343,8 +343,9 @@ async fn set_flags( Ok(StatusCode::NO_CONTENT) } -/// Puts one enclosure back in the queue and kicks a scan of its feed. The queue is the -/// table, so this is all it takes -- including for something a filter once skipped. +/// Downloads one enclosure now. This cannot be "requeue and scan": a scan takes the +/// lowest-id pending rows up to max_new_per_check, so with a big backlog it would download +/// other episodes and leave the requested one pending. async fn download_now( State(state): State, Path(id): Path, @@ -358,10 +359,11 @@ async fn download_now( return Ok(StatusCode::NO_CONTENT); // Already here. } state.ctx.db.requeue(id)?; - let _ = state + state .cmds - .send(Command::Fetch { feed: Some(enc.feed_id), force: true }) - .await; + .send(Command::Download { enclosure: id }) + .await + .map_err(|_| anyhow::anyhow!("the daemon is not accepting commands"))?; Ok(StatusCode::ACCEPTED) }