From 9dc4c1ddfa5fda878fb2b8454a7189fbbef40966 Mon Sep 17 00:00:00 2001 From: rays Date: Thu, 10 Sep 2026 02:25:38 +0000 Subject: [PATCH] Add feed check scheduling, global with per-feed override general.schedule and feeds..schedule take "every 30m", "4h", "1d" or bare minutes. The legacy interval_mins is still read. An explicit per-feed schedule wins over the publisher's ttl; without one, ttl still raises the interval when they ask to be polled less often. Fixes two bugs found while testing it: null never cleared a field. serde maps JSON null onto the outer None of an Option>, so "clear this" was indistinguishable from "not supplied" and every clear silently no-opped with a 204. The daemon ignored SIGTERM while working. select! races branches only at selection time, so a signal queued behind an in-flight download and the process had to be SIGKILLed. The stop signal now cancels work in progress: SIGTERM mid-download exits in 1s. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RPyeapneuXrCdojsaiXGbe --- PROGRESS.md | 44 +++++++++++++++++++ src/config.rs | 94 +++++++++++++++++++++++++++++++++++++--- src/main.rs | 72 ++++++++++++++++++++++++++----- src/web.rs | 114 ++++++++++++++++++++++++++++++++++++++++++++++++- web/index.html | 11 ++++- 5 files changed, 316 insertions(+), 19 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index 73d71a6..9f6d19e 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -56,6 +56,50 @@ and until now nothing set them. --- +## 2026-09-10 — Scheduling, and two bugs it uncovered + +**Scheduling.** The original engine had none — it only skipped feeds on ``; the schedule lived +in the Cocoa GUI that was never open-sourced, so there was no original behaviour to match. Scope +agreed with Ray: interval only, global default plus per-feed override, no quiet hours. + +`general.schedule` takes "every 30m", "every 4h", "1d", or a bare number of minutes; +`feeds..schedule` overrides it. The legacy `interval_mins` is still read so existing configs +work, and saving from the UI migrates it. A malformed value in the file warns and falls back rather +than stopping the daemon; a malformed value over the API is a 400. + +Precedence, which is a judgement call: an explicit per-feed schedule wins outright, including over +the publisher's ``. With no per-feed schedule, `` still raises the interval when the +publisher asks to be polled less often. Visible in practice — TWiT resolves to 720 min from its own +ttl against a 60 min global. + +UI: a gear opens global Settings (schedule, quota, max age); each feed's Settings gains a schedule +field; the feed header shows "every 4 hours · next in 37m". + +**Bug 1: `null` never cleared anything.** For `Option>`, serde maps JSON `null` onto the +*outer* `None`, which the handler reads as "field absent, leave alone" — so `Some(None)` was +unreachable and every clear was a no-op returning 204. That silently affected `folder`, `schedule` +and `max_new_per_check`; unsetting any of them from the UI did nothing and looked like it worked. +Fixed with a `double_option` deserializer, and pinned by a test covering absent / null / value. + +**Bug 2: the daemon ignored SIGTERM while working.** `tokio::select!` races its branches only at +selection time; once inside `run(...).await` the shutdown future was not polled at all, so a signal +queued behind a 90 MB download. Under systemd every restart would have hit `TimeoutStopSec` and been +SIGKILLed mid-transfer. + +This one cost real time: `pkill` appeared to work but didn't, a stale daemon kept port 8099, and +because the restart also `rm -f`'d the socket it defeated the "already listening" guard and a second +daemon started. Every PATCH "verified" after that went to the *old* binary — so bug 1 was reported +fixed while completely untested. Lesson: confirm the process actually died and the socket guard is +doing its job, rather than trusting the restart. + +The stop signal now lives in a `watch` channel raced *inside* each job. Measured: SIGTERM mid- +download exits in 1s (was: blocked until the transfer finished). An abandoned download leaves a +partial in `.ipx-incomplete`, discarded on the next attempt. + +`cargo test` 37/37. + +--- + ## 2026-09-10 — App name, and a transparent icon Title is `iPodderX` in the browser tab and the sidebar. The heading's `text-transform: uppercase` diff --git a/src/config.rs b/src/config.rs index 1833208..00e3c9f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -23,8 +23,12 @@ pub struct Config { pub struct General { pub download_dir: PathBuf, pub socket: PathBuf, - /// Default poll interval; a feed's own wins when it is longer. - pub interval_mins: u64, + /// How often to re-check feeds: "every 30m", "every 4h", "90" (minutes), "1d". + /// A feed's own `schedule` overrides this. + pub schedule: String, + /// Superseded by `schedule`. Still read so existing configs keep working. + #[serde(skip_serializing_if = "Option::is_none")] + pub interval_mins: Option, pub organize: Organize, /// 0 = unlimited. pub max_total_gb: f64, @@ -93,6 +97,9 @@ pub struct Feed { pub allow_explicit: bool, #[serde(default = "yes")] pub auto_download: bool, + /// Overrides the global schedule for this feed. Same forms: "every 6h", "2d". + #[serde(default, skip_serializing_if = "Option::is_none")] + pub schedule: Option, /// Cap on new downloads per scan. None = unlimited. #[serde(default, skip_serializing_if = "Option::is_none")] pub max_new_per_check: Option, @@ -114,7 +121,8 @@ impl Default for General { Self { download_dir: home().join("Podcasts"), socket: default_socket(), - interval_mins: 60, + schedule: "every 60m".into(), + interval_mins: None, organize: Organize::Feed, max_total_gb: 0.0, max_age_days: 0, @@ -134,6 +142,47 @@ impl Default for Torrent { } } +impl General { + /// Minutes between checks. Falls back to the legacy `interval_mins`, then to an hour. + /// A malformed value warns rather than stopping the daemon. + pub fn interval(&self) -> u64 { + if let Some(n) = parse_interval(&self.schedule) { + return n; + } + if !self.schedule.trim().is_empty() { + tracing::warn!(schedule = %self.schedule, "unrecognised schedule; using the default"); + } + self.interval_mins.filter(|n| *n > 0).unwrap_or(60) + } +} + +/// Parses a check interval into minutes. +/// +/// Accepts "every 30m", "30m", "4h", "1d", "every 4 hours", or a bare number of minutes. +/// Returns None for anything it cannot read, or for zero. +pub fn parse_interval(s: &str) -> Option { + let s = s.trim().to_lowercase(); + let s = s.strip_prefix("every").unwrap_or(&s).trim(); + if s.is_empty() { + return None; + } + + let digits: String = s.chars().take_while(|c| c.is_ascii_digit()).collect(); + if digits.is_empty() { + return None; + } + let n: u64 = digits.parse().ok()?; + let unit = s[digits.len()..].trim(); + + let mins = match unit { + "" | "m" | "min" | "mins" | "minute" | "minutes" => n, + "h" | "hr" | "hrs" | "hour" | "hours" => n.checked_mul(60)?, + "d" | "day" | "days" => n.checked_mul(1440)?, + _ => return None, + }; + (mins > 0).then_some(mins) +} + impl Torrent { /// Inclusive listen port range. Falls back to the BitTorrent default on garbage input. pub fn ports(&self) -> (u16, u16) { @@ -273,7 +322,7 @@ mod tests { .unwrap(); assert_eq!(cfg.general.download_dir, PathBuf::from("/tmp/pods")); - assert_eq!(cfg.general.interval_mins, 60); + assert_eq!(cfg.general.interval(), 60); assert_eq!(cfg.general.organize, Organize::Feed); assert!(cfg.torrent.enabled); @@ -284,6 +333,40 @@ mod tests { assert_eq!(feed.keywords, vec!["deep dive"]); } + #[test] + fn intervals_parse_from_the_forms_people_actually_type() { + for (input, want) in [ + ("every 30m", 30), ("30m", 30), ("30", 30), ("every 30 minutes", 30), + ("every 4h", 240), ("4h", 240), ("4 hours", 240), ("EVERY 4H", 240), + ("1d", 1440), ("every 2 days", 2880), (" every 90m ", 90), + ] { + assert_eq!(parse_interval(input), Some(want), "{input:?}"); + } + for bad in ["", " ", "every", "soon", "-5m", "0", "0h", "every 0 minutes", "5 fortnights"] { + assert_eq!(parse_interval(bad), None, "{bad:?} should not parse"); + } + } + + #[test] + fn interval_falls_back_through_legacy_then_default() { + let mut g = General::default(); + assert_eq!(g.interval(), 60, "the default schedule"); + + g.schedule = "every 15m".into(); + assert_eq!(g.interval(), 15); + + // A config written before `schedule` existed still works. + g.schedule = String::new(); + g.interval_mins = Some(45); + assert_eq!(g.interval(), 45); + + // Garbage must not stop the daemon. + g.schedule = "whenever".into(); + assert_eq!(g.interval(), 45); + g.interval_mins = None; + assert_eq!(g.interval(), 60); + } + #[test] fn port_range_falls_back_when_malformed() { let mut t = Torrent::default(); @@ -307,7 +390,7 @@ mod tests { let mut taken = BTreeMap::new(); taken.insert("the-daily".to_string(), Feed { - url: "u".into(), folder: None, keywords: vec![], allow_explicit: false, + url: "u".into(), folder: None, schedule: None, keywords: vec![], allow_explicit: false, auto_download: true, max_new_per_check: None, username: None, password: None, password_env: None, }); @@ -319,6 +402,7 @@ mod tests { let mut f = Feed { url: "https://x/y".into(), folder: None, + schedule: None, keywords: vec![], allow_explicit: false, auto_download: true, diff --git a/src/main.rs b/src/main.rs index ccfc769..835ac89 100644 --- a/src/main.rs +++ b/src/main.rs @@ -12,7 +12,8 @@ use clap::{Parser, Subcommand}; use ipc::{Command as Cmd, Emitter, Event}; use std::path::PathBuf; use std::sync::Arc; -use tokio::sync::{broadcast, mpsc}; +use std::future::Future; +use tokio::sync::{broadcast, mpsc, watch}; #[derive(Parser)] #[command(name = "ipx", version, about = "A headless podcatcher")] @@ -202,21 +203,57 @@ async fn daemon(ctx: Ctx, config_path: PathBuf, web_addr: Option) -> Res ticker.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); tracing::info!(feeds = ctx.cfg().feeds.len(), "daemon started"); - loop { + // 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 + // inside a long download the daemon stopped listening and had to be SIGKILLed. + let (tx_stop, rx_stop) = tokio::sync::watch::channel(false); + tokio::spawn(async move { + shutdown().await; + let _ = tx_stop.send(true); + }); + + // Runs one job, abandoning it if a signal arrives. Returns false to end the loop. + async fn until_stopped( + ctx: &Ctx, + rx: &watch::Receiver, + job: impl Future>, + ) -> bool { + let mut stop = rx.clone(); tokio::select! { + _ = stop.changed() => { + tracing::info!("signal received; abandoning the job in progress"); + false + } + result = job => { + if let Err(e) = result { + ctx.out.emit(Event::Error { msg: format!("{e:#}") }); + } + true + } + } + } + + let mut stop = rx_stop.clone(); + loop { + if *stop.borrow() { + break; + } + tokio::select! { + biased; + _ = stop.changed() => break, Some(cmd) = rx_cmd.recv() => { tracing::info!(?cmd, "command from a client"); - if let Err(e) = run(&ctx, cmd).await { - ctx.out.emit(Event::Error { msg: format!("{e:#}") }); + if !until_stopped(&ctx, &rx_stop, run(&ctx, cmd)).await { + break; } } _ = ticker.tick() => { - // Per-feed TTL decides what actually gets polled. - if let Err(e) = run(&ctx, Cmd::Fetch { feed: None, force: false }).await { - ctx.out.emit(Event::Error { msg: format!("{e:#}") }); + // Per-feed schedule and TTL decide what actually gets polled. + let job = run(&ctx, Cmd::Fetch { feed: None, force: false }); + if !until_stopped(&ctx, &rx_stop, job).await { + break; } } - _ = shutdown() => break, } } @@ -318,6 +355,7 @@ pub async fn add_one( let probe = config::Feed { url: url.to_owned(), folder: folder.clone(), + schedule: None, keywords: keywords.clone(), allow_explicit: false, auto_download: true, @@ -383,6 +421,7 @@ async fn import(ctx: Ctx, config_path: &std::path::Path, file: &std::path::Path) config::Feed { url, folder: None, + schedule: None, keywords: vec![], allow_explicit: false, auto_download: true, @@ -487,10 +526,8 @@ async fn fetch(ctx: &Ctx, only: Option<&str>, force: bool) -> Result<()> { { let state = ctx.db.http_state(id)?; - // TTL: the feed's own wins when it is longer than our poll interval. if !force && let Some(last) = state.last_checked { - let wait = state.ttl_mins.unwrap_or(0).max(cfg.general.interval_mins) * 60; - let due = last + wait as i64; + let due = last + due_after(&cfg, feed_cfg, state.ttl_mins) as i64; if due > db::now() { ctx.out.emit(Event::FeedSkip { feed: id.clone(), @@ -526,6 +563,19 @@ async fn fetch(ctx: &Ctx, only: Option<&str>, force: bool) -> Result<()> { Ok(()) } +/// Seconds to wait before re-checking a feed. +/// +/// A per-feed schedule is an explicit instruction and wins outright. Without one, the +/// global schedule applies, but the feed's own raises it when the publisher asks to +/// be polled less often. +pub fn due_after(cfg: &config::Config, feed: &config::Feed, ttl_mins: Option) -> u64 { + let mins = match feed.schedule.as_deref().and_then(config::parse_interval) { + Some(explicit) => explicit, + None => ttl_mins.unwrap_or(0).max(cfg.general.interval()), + }; + mins * 60 +} + #[derive(Default)] struct Scan { new_entries: usize, diff --git a/src/web.rs b/src/web.rs index df389e1..8846b49 100644 --- a/src/web.rs +++ b/src/web.rs @@ -71,6 +71,7 @@ pub fn router(state: WebState) -> Router { .route("/api/enclosures/{id}", delete(delete_file)) .route("/api/fetch", post(fetch_now)) .route("/api/opml", get(export_opml).post(import_opml)) + .route("/api/settings", get(get_settings).patch(patch_settings)) .route("/api/events", get(events)) .route("/media/{id}", get(media)) .layer(middleware::from_fn_with_state(state.clone(), auth)) @@ -152,7 +153,11 @@ struct FeedRow { allow_explicit: bool, auto_download: bool, max_new_per_check: Option, + schedule: Option, + /// Effective schedule in minutes, after the global default and the feed's . + every_mins: u64, last_checked: Option, + next_check: Option, last_error: Option, entries: i64, downloaded: i64, @@ -164,6 +169,7 @@ async fn feeds(State(state): State) -> Result>, ApiE let mut out = Vec::with_capacity(cfg.feeds.len()); for (id, feed) in &cfg.feeds { let s = state.ctx.db.feed_summary(id)?; + let st = state.ctx.db.http_state(id)?; out.push(FeedRow { id: id.clone(), url: feed.url.clone(), @@ -174,7 +180,12 @@ async fn feeds(State(state): State) -> Result>, ApiE allow_explicit: feed.allow_explicit, auto_download: feed.auto_download, max_new_per_check: feed.max_new_per_check, + schedule: feed.schedule.clone(), + every_mins: crate::due_after(&cfg, feed, st.ttl_mins) / 60, last_checked: s.last_checked, + next_check: s + .last_checked + .map(|t| t + crate::due_after(&cfg, feed, st.ttl_mins) as i64), last_error: s.last_error, entries: s.entries, downloaded: s.downloaded, @@ -252,12 +263,29 @@ mod tests { fn feed(url: &str) -> crate::config::Feed { crate::config::Feed { - url: url.into(), folder: None, keywords: vec![], allow_explicit: false, + url: url.into(), folder: None, schedule: None, keywords: vec![], allow_explicit: false, auto_download: true, max_new_per_check: None, username: None, password: None, password_env: None, } } + #[test] + fn null_clears_a_field_while_absent_leaves_it_alone() { + // serde maps null onto the outer None for a bare Option>, which made + // "clear this field" indistinguishable from "field not supplied". + let absent: FeedPatch = serde_json::from_str("{}").unwrap(); + assert!(absent.schedule.is_none() && absent.folder.is_none()); + + let cleared: FeedPatch = + serde_json::from_str(r#"{"schedule":null,"folder":null,"max_new_per_check":null}"#).unwrap(); + assert_eq!(cleared.schedule, Some(None), "null must mean clear"); + assert_eq!(cleared.folder, Some(None)); + assert_eq!(cleared.max_new_per_check, Some(None)); + + let set: FeedPatch = serde_json::from_str(r#"{"schedule":"every 6h"}"#).unwrap(); + assert_eq!(set.schedule, Some(Some("every 6h".into()))); + } + #[test] fn feed_urls_are_validated_before_being_saved() { let mut feeds = std::collections::BTreeMap::new(); @@ -359,17 +387,33 @@ async fn add_feed( Ok(Json(serde_json::json!({ "id": id, "existing": false }))) } -/// Only the fields that are present are changed. +/// Absent means "leave alone"; JSON `null` means "clear this". +/// +/// That distinction needs `double_option`: serde maps `null` onto the *outer* `None` for a +/// plain `Option>`, making the clear case unreachable and silently turning +/// "unset the folder" into a no-op. #[derive(Deserialize)] struct FeedPatch { url: Option, + #[serde(default, deserialize_with = "double_option")] + schedule: Option>, + #[serde(default, deserialize_with = "double_option")] folder: Option>, keywords: Option>, allow_explicit: Option, auto_download: Option, + #[serde(default, deserialize_with = "double_option")] max_new_per_check: Option>, } +fn double_option<'de, T, D>(de: D) -> Result>, D::Error> +where + T: Deserialize<'de>, + D: serde::Deserializer<'de>, +{ + Deserialize::deserialize(de).map(Some) +} + async fn patch_feed( State(state): State, Path(id): Path, @@ -393,6 +437,17 @@ async fn patch_feed( feed.url = url; } + if let Some(v) = body.schedule { + let v = v.map(|x| x.trim().to_owned()).filter(|x| !x.is_empty()); + if let Some(text) = &v + && crate::config::parse_interval(text).is_none() + { + return Err(ApiError::bad_request(format!( + "{text:?} is not a schedule — try \"every 30m\", \"every 4h\" or \"1d\"" + ))); + } + feed.schedule = v; + } if let Some(v) = body.folder { feed.folder = v.filter(|s| !s.trim().is_empty()); } @@ -659,6 +714,7 @@ async fn import_opml( crate::config::Feed { url, folder: None, + schedule: None, keywords: vec![], allow_explicit: false, auto_download: true, @@ -674,3 +730,57 @@ async fn import_opml( state.ctx.reload_cfg(&state.config_path)?; Ok(Json(serde_json::json!({ "added": added }))) } + +#[derive(Serialize)] +struct Settings { + schedule: String, + every_mins: u64, + download_dir: String, + max_total_gb: f64, + max_age_days: u64, +} + +async fn get_settings(State(state): State) -> Json { + let cfg = state.ctx.cfg(); + Json(Settings { + schedule: cfg.general.schedule.clone(), + every_mins: cfg.general.interval(), + download_dir: cfg.general.download_dir.display().to_string(), + max_total_gb: cfg.general.max_total_gb, + max_age_days: cfg.general.max_age_days, + }) +} + +#[derive(Deserialize)] +struct SettingsPatch { + schedule: Option, + max_total_gb: Option, + max_age_days: Option, +} + +async fn patch_settings( + State(state): State, + Json(body): Json, +) -> Result { + let mut cfg = (*state.ctx.cfg()).clone(); + if let Some(sched) = body.schedule { + let sched = sched.trim().to_owned(); + if crate::config::parse_interval(&sched).is_none() { + return Err(ApiError::bad_request(format!( + "{sched:?} is not a schedule — try \"every 30m\", \"every 4h\" or \"1d\"" + ))); + } + cfg.general.schedule = sched; + // The legacy key would otherwise keep shadowing intent in the file. + cfg.general.interval_mins = None; + } + if let Some(v) = body.max_total_gb { + cfg.general.max_total_gb = v.max(0.0); + } + if let Some(v) = body.max_age_days { + cfg.general.max_age_days = v; + } + cfg.save(&state.config_path)?; + state.ctx.reload_cfg(&state.config_path)?; + Ok(StatusCode::NO_CONTENT) +} diff --git a/web/index.html b/web/index.html index 0d8a71f..a120e33 100644 --- a/web/index.html +++ b/web/index.html @@ -253,6 +253,7 @@ input[type=range]::-moz-range-thumb{width:12px;height:12px;border:0;border-radiu