From 4429f1119c9631595029994bffe36572f2b8139a Mon Sep 17 00:00:00 2001 From: rays Date: Thu, 10 Sep 2026 16:55:54 +0000 Subject: [PATCH] Look up derived feeds everywhere, not just in config Moving OPML feeds into the database left several call sites still searching config.toml only, so anything inside a subscription looked unsubscribed: Download failed outright, status and the startup line counted 3 feeds instead of 85, add could duplicate or collide with a derived feed, and rm could not remove one. The first grep for this missed the failing call because the method chain spans lines; searching with newlines collapsed found all of them. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AdXho5tTkjFLeUXKbEjKBh --- PROGRESS.md | 26 ++++++++++++++++++++++++++ src/main.rs | 38 +++++++++++++++++++++++++++++--------- src/web.rs | 8 ++++++-- 3 files changed, 61 insertions(+), 11 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index 2bde1b7..b5c9504 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -56,6 +56,32 @@ and until now nothing set them. --- +## 2026-09-10 — Regression: derived feeds looked "unsubscribed" to half the code + +Reported as `error: enclosure 235 belongs to unsubscribed feed "abort-retry-fail"`. Moving OPML +feeds out of config.toml meant a config-only lookup no longer finds them, and `download_one` still +did exactly that — so Download on any episode from an OPML subscription failed outright. + +Worse than the one bug: **my first grep for the pattern gave false confidence.** `grep -n +'cfg\.feeds\.get'` returned three hits, all legitimate, so it looked clean — but the failing call +was split across lines, `cfg` then `.feeds` then `.get`, and never matched. Re-searching with +newlines collapsed found sixteen `.feeds` accesses, several of them wrong: + +- `download_one` — the reported failure. +- `Status` and the daemon's startup line counted config feeds only: 3 reported where there are 85. +- `add` deduped and slugged against config only, so adding a URL an OPML already lists would have + duplicated it, and a new feed could collide with a derived feed's id. +- `rm` on a derived feed said "no feed with id". +- The web add endpoint had the same duplicate hole. + +All now go through `subscriptions()`. Verified against the exact failure: enclosure 235 went +`pending` -> `done`. + +Lesson recorded because it will recur: when a lookup moves, a single-line grep is not a survey. +Collapse newlines before searching Rust method chains. + +--- + ## 2026-09-10 — OPML feeds out of the config, a real cap, and daemon output in the log Three reports in quick succession, all fair. diff --git a/src/main.rs b/src/main.rs index 852571d..97ea866 100644 --- a/src/main.rs +++ b/src/main.rs @@ -200,7 +200,8 @@ async fn run(ctx: &Arc, cmd: Cmd) -> Result<()> { 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 }); + let feeds = subscriptions(ctx).map(|s| s.len()).unwrap_or(0); + ctx.out.emit(Event::Status { feeds, pending, downloaded }); Ok(()) } } @@ -237,7 +238,10 @@ async fn daemon( // One command at a time: the queue is what keeps two scans from overlapping. let mut ticker = tokio::time::interval(std::time::Duration::from_secs(60)); ticker.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); - tracing::info!(feeds = ctx.cfg().feeds.len(), "daemon started"); + tracing::info!( + feeds = subscriptions(&ctx).map(|s| s.len()).unwrap_or(0), + "daemon started" + ); // A signal has to be able to interrupt work in progress, not just the wait between // jobs. Racing `shutdown()` in the outer select only cancels branch selection: once @@ -370,8 +374,9 @@ async fn add( keywords: Vec, ) -> Result<()> { let mut cfg = (*ctx.cfg()).clone(); - if let Some((id, _)) = cfg.feeds.iter().find(|(_, f)| f.url == url) { - anyhow::bail!("already subscribed as {id:?}"); + // Includes feeds derived from an OPML, or the same show could be added twice. + if let Some(existing) = subscriptions(ctx)?.iter().find(|s| s.cfg.url == url) { + anyhow::bail!("already subscribed as {:?}", existing.id); } let id = add_one(ctx, &mut cfg, url, folder, keywords).await?; cfg.save(config_path)?; @@ -418,7 +423,13 @@ pub async fn add_one( } }; - let id = config::unique_slug(&title, &cfg.feeds); + // Slugs must be unique across derived feeds too, or a new feed can collide with one + // an OPML already introduced. + let taken: std::collections::BTreeMap = subscriptions(ctx)? + .into_iter() + .map(|s| (s.id, s.cfg)) + .collect(); + let id = config::unique_slug(&title, &taken); cfg.feeds.insert(id.clone(), probe); Ok(id) } @@ -434,7 +445,11 @@ fn url_stem(url: &str) -> String { fn rm(ctx: &Ctx, config_path: &std::path::Path, feed: &str) -> Result<()> { let mut cfg = (*ctx.cfg()).clone(); if cfg.feeds.remove(feed).is_none() { - anyhow::bail!("no feed with id {feed:?}"); + // Derived from an OPML: drop it here, though the subscription will list it again + // on the next read unless the OPML itself goes. + ctx.db.drop_managed(feed)?; + println!("removed {feed}; it came from an OPML subscription and may return on the next read"); + return Ok(()); } cfg.save(config_path)?; // State and files stay: re-adding the feed should not re-download its back catalogue. @@ -1072,10 +1087,15 @@ async fn download_one(ctx: &Arc, id: i64) -> Result<()> { if enc.path.is_some() { return Ok(()); // Already here. } - let feed_cfg = cfg - .feeds - .get(&enc.feed_id) + // Must look through the derived feeds too: anything inside an OPML subscription has + // no config entry, so a config-only lookup called every one of them "unsubscribed". + let subs = subscriptions(ctx)?; + let feed_cfg = subs + .iter() + .find(|s| s.id == enc.feed_id) + .map(|s| s.cfg.clone()) .ok_or_else(|| anyhow::anyhow!("enclosure {id} belongs to unsubscribed feed {:?}", enc.feed_id))?; + let feed_cfg = &feed_cfg; let title = ctx.db.feed_summary(&enc.feed_id)?.title; let folder = download::folder_for(&cfg, &enc.feed_id, feed_cfg, title.as_deref()); diff --git a/src/web.rs b/src/web.rs index df22fca..214e86b 100644 --- a/src/web.rs +++ b/src/web.rs @@ -398,8 +398,12 @@ async fn add_feed( Json(body): Json, ) -> Result, ApiError> { let mut cfg = (*state.ctx.cfg()).clone(); - if let Some((id, _)) = cfg.feeds.iter().find(|(_, f)| f.url == body.url) { - return Ok(Json(serde_json::json!({ "id": id, "existing": true }))); + // Derived feeds count as subscribed: adding one an OPML already lists would duplicate it. + if let Some(existing) = crate::subscriptions(&state.ctx)? + .into_iter() + .find(|s| s.cfg.url == body.url) + { + return Ok(Json(serde_json::json!({ "id": existing.id, "existing": true }))); } let id = crate::add_one(&state.ctx, &mut cfg, &body.url, body.folder, body.keywords).await?; cfg.save(&state.config_path)?;