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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AdXho5tTkjFLeUXKbEjKBh
This commit is contained in:
26
PROGRESS.md
26
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
|
## 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.
|
Three reports in quick succession, all fair.
|
||||||
|
|||||||
38
src/main.rs
38
src/main.rs
@@ -200,7 +200,8 @@ async fn run(ctx: &Arc<Ctx>, cmd: Cmd) -> Result<()> {
|
|||||||
Cmd::Download { enclosure } => download_one(ctx, enclosure).await,
|
Cmd::Download { enclosure } => download_one(ctx, enclosure).await,
|
||||||
Cmd::Status => {
|
Cmd::Status => {
|
||||||
let (pending, downloaded) = ctx.db.counts()?;
|
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(())
|
Ok(())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -237,7 +238,10 @@ async fn daemon(
|
|||||||
// One command at a time: the queue is what keeps two scans from overlapping.
|
// 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));
|
let mut ticker = tokio::time::interval(std::time::Duration::from_secs(60));
|
||||||
ticker.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip);
|
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
|
// 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
|
// jobs. Racing `shutdown()` in the outer select only cancels branch selection: once
|
||||||
@@ -370,8 +374,9 @@ async fn add(
|
|||||||
keywords: Vec<String>,
|
keywords: Vec<String>,
|
||||||
) -> Result<()> {
|
) -> Result<()> {
|
||||||
let mut cfg = (*ctx.cfg()).clone();
|
let mut cfg = (*ctx.cfg()).clone();
|
||||||
if let Some((id, _)) = cfg.feeds.iter().find(|(_, f)| f.url == url) {
|
// Includes feeds derived from an OPML, or the same show could be added twice.
|
||||||
anyhow::bail!("already subscribed as {id:?}");
|
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?;
|
let id = add_one(ctx, &mut cfg, url, folder, keywords).await?;
|
||||||
cfg.save(config_path)?;
|
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<String, config::Feed> = subscriptions(ctx)?
|
||||||
|
.into_iter()
|
||||||
|
.map(|s| (s.id, s.cfg))
|
||||||
|
.collect();
|
||||||
|
let id = config::unique_slug(&title, &taken);
|
||||||
cfg.feeds.insert(id.clone(), probe);
|
cfg.feeds.insert(id.clone(), probe);
|
||||||
Ok(id)
|
Ok(id)
|
||||||
}
|
}
|
||||||
@@ -434,7 +445,11 @@ fn url_stem(url: &str) -> String {
|
|||||||
fn rm(ctx: &Ctx, config_path: &std::path::Path, feed: &str) -> Result<()> {
|
fn rm(ctx: &Ctx, config_path: &std::path::Path, feed: &str) -> Result<()> {
|
||||||
let mut cfg = (*ctx.cfg()).clone();
|
let mut cfg = (*ctx.cfg()).clone();
|
||||||
if cfg.feeds.remove(feed).is_none() {
|
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)?;
|
cfg.save(config_path)?;
|
||||||
// State and files stay: re-adding the feed should not re-download its back catalogue.
|
// 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<Ctx>, id: i64) -> Result<()> {
|
|||||||
if enc.path.is_some() {
|
if enc.path.is_some() {
|
||||||
return Ok(()); // Already here.
|
return Ok(()); // Already here.
|
||||||
}
|
}
|
||||||
let feed_cfg = cfg
|
// Must look through the derived feeds too: anything inside an OPML subscription has
|
||||||
.feeds
|
// no config entry, so a config-only lookup called every one of them "unsubscribed".
|
||||||
.get(&enc.feed_id)
|
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))?;
|
.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 title = ctx.db.feed_summary(&enc.feed_id)?.title;
|
||||||
let folder = download::folder_for(&cfg, &enc.feed_id, feed_cfg, title.as_deref());
|
let folder = download::folder_for(&cfg, &enc.feed_id, feed_cfg, title.as_deref());
|
||||||
|
|||||||
@@ -398,8 +398,12 @@ async fn add_feed(
|
|||||||
Json(body): Json<NewFeed>,
|
Json(body): Json<NewFeed>,
|
||||||
) -> Result<Json<serde_json::Value>, ApiError> {
|
) -> Result<Json<serde_json::Value>, ApiError> {
|
||||||
let mut cfg = (*state.ctx.cfg()).clone();
|
let mut cfg = (*state.ctx.cfg()).clone();
|
||||||
if let Some((id, _)) = cfg.feeds.iter().find(|(_, f)| f.url == body.url) {
|
// Derived feeds count as subscribed: adding one an OPML already lists would duplicate it.
|
||||||
return Ok(Json(serde_json::json!({ "id": id, "existing": true })));
|
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?;
|
let id = crate::add_one(&state.ctx, &mut cfg, &body.url, body.folder, body.keywords).await?;
|
||||||
cfg.save(&state.config_path)?;
|
cfg.save(&state.config_path)?;
|
||||||
|
|||||||
Reference in New Issue
Block a user