fix(history): treat history_retention=0 as keep-all (#136)

apps/rustnzb/config.example.toml

@@ -9,7 +9,7 @@ speed_limit_bps = 0  # 0 = unlimited
 cache_size = 524288000  # 500 MB
 log_level = "info"
 # log_file = "data/rustnzb.log"
-# history_retention = 100  # Number of NZBs to keep in history (omit for keep all)
+# history_retention = 100  # Number of NZBs to keep in history (omit or 0 = keep all)
 # Independent post-processing jobs overlap downloads. Separate repair and
 # extraction gates allow those stages to overlap safely across different jobs.
 # These limits are applied at process start.

apps/rustnzb/src/handlers.rs

@@ -24,7 +24,9 @@ const MAX_FETCH_BODY_BYTES: usize = 100 * 1024 * 1024;
 
 #[cfg(feature = "webdav")]
 use nzb_web::nzb_core::config::DavConfig;
-use nzb_web::nzb_core::config::{CategoryConfig, RssFeedConfig, ServerConfig};
+use nzb_web::nzb_core::config::{
+    CategoryConfig, RssFeedConfig, ServerConfig, normalize_history_retention,
+};
 use nzb_web::nzb_core::models::*;
 use nzb_web::nzb_core::nzb_parser;
 use nzb_web::nzb_core::sabnzbd_import;
@@ -1111,10 +1113,13 @@ pub async fn h_history_retention_set(
     State(state): State<Arc<AppState>>,
     Json(body): Json<HistoryRetentionBody>,
 ) -> Result<Json<SimpleResponse>, ApiError> {
+    // 0 means "keep all" (GH #136); persist the normalized value so GET
+    // reports what is actually enforced.
+    let retention = normalize_history_retention(body.retention);
     let mut config = (*state.config()).clone();
-    config.general.history_retention = body.retention;
+    config.general.history_retention = retention;
     state.update_config(config).map_err(ApiError::from)?;
-    state.queue_manager.set_history_retention(body.retention);
+    state.queue_manager.set_history_retention(retention);
     Ok(Json(SimpleResponse { status: true }))
 }
 
@@ -1547,6 +1552,7 @@ pub async fn h_general_update(
         config.general.max_extract_workers = max.max(1);
     }
     if let Some(ret) = body.history_retention {
+        let ret = normalize_history_retention(ret);
         state.queue_manager.set_history_retention(ret);
         config.general.history_retention = ret;
     }

crates/nzb-core/src/config.rs

@@ -54,7 +54,8 @@ pub struct GeneralConfig {
     pub log_level: String,
     /// Log file path (None = stdout only)
     pub log_file: Option<PathBuf>,
-    /// History retention: how many NZBs to keep in history (None = keep all)
+    /// History retention: how many NZBs to keep in history.
+    /// `None` or `Some(0)` both mean keep all; see [`normalize_history_retention`].
     pub history_retention: Option<usize>,
     /// Max number of NZBs downloading simultaneously (default 1)
     pub max_active_downloads: usize,
@@ -141,6 +142,16 @@ fn default_article_timeout_secs() -> u64 {
     30
 }
 
+/// Normalize a history retention limit so that `0` means "keep all".
+///
+/// SABnzbd users (and this codebase's own `speed_limit_bps`) treat `0` as
+/// unlimited. Enforcing a literal limit of zero would delete every history
+/// row immediately after each completion (GH #136), so a zero is folded into
+/// `None` at every entry point before it reaches the database.
+pub fn normalize_history_retention(limit: Option<usize>) -> Option<usize> {
+    limit.filter(|max| *max > 0)
+}
+
 impl Default for GeneralConfig {
     fn default() -> Self {
         Self {
@@ -408,6 +419,13 @@ mod tests {
         assert_eq!(cfg.max_nested_archive_depth, 5);
     }
 
+    #[test]
+    fn zero_history_retention_normalizes_to_keep_all() {
+        assert_eq!(normalize_history_retention(Some(0)), None);
+        assert_eq!(normalize_history_retention(None), None);
+        assert_eq!(normalize_history_retention(Some(25)), Some(25));
+    }
+
     #[test]
     fn direct_unpack_can_be_explicitly_disabled() {
         let cfg: GeneralConfig = toml::from_str("direct_unpack = false").unwrap();

crates/nzb-core/src/db.rs

@@ -602,7 +602,15 @@ impl Database {
     }
 
     /// Enforce history retention limit by deleting oldest entries.
+    ///
+    /// A limit of `0` is treated as "keep all" and is a no-op: the
+    /// `LIMIT 0` subquery would otherwise match nothing and delete every
+    /// row (GH #136). Callers normalize zero away already; this is the last
+    /// line of defence.
     pub fn history_enforce_retention(&self, max_entries: usize) -> Result<(), NzbError> {
+        if max_entries == 0 {
+            return Ok(());
+        }
         self.conn.execute(
             "DELETE FROM history WHERE id NOT IN (
                 SELECT id FROM history ORDER BY completed_at DESC LIMIT ?1
@@ -1305,6 +1313,19 @@ mod tests {
         assert_eq!(db.history_count().unwrap(), 3);
     }
 
+    /// GH #136: a retention limit of zero must not wipe history.
+    #[test]
+    fn test_history_enforce_retention_zero_keeps_all() {
+        let db = Database::open_memory().unwrap();
+        for i in 0..3 {
+            db.history_insert(&make_history(&format!("ret0-{i}"), &format!("Job {i}")))
+                .unwrap();
+        }
+
+        db.history_enforce_retention(0).unwrap();
+        assert_eq!(db.history_count().unwrap(), 3);
+    }
+
     #[test]
     fn test_history_store_and_get_logs() {
         let db = Database::open_memory().unwrap();

crates/nzb-web/src/queue_manager.rs

@@ -15,7 +15,7 @@ use serde::{Deserialize, Serialize};
 use tokio::sync::{broadcast, mpsc};
 use tracing::{debug, error, info, warn};
 
-use crate::nzb_core::config::{CategoryConfig, ServerConfig};
+use crate::nzb_core::config::{CategoryConfig, ServerConfig, normalize_history_retention};
 use crate::nzb_core::db::Database;
 use crate::nzb_core::models::*;
 use crate::nzb_core::nzb_parser;
@@ -876,9 +876,10 @@ impl QueueManager {
         }
     }
 
-    /// Set history retention limit.
+    /// Set history retention limit. `Some(0)` is normalized to `None`
+    /// (keep all) so a zero can never wipe history on completion (GH #136).
     pub fn set_history_retention(&self, limit: Option<usize>) {
-        *self.history_retention.lock() = limit;
+        *self.history_retention.lock() = normalize_history_retention(limit);
     }
 
     /// Current generation of the SAB-compatible history view.
@@ -3924,6 +3925,50 @@ mod global_pause_tests {
         assert_eq!(manager.history_update(), 3);
     }
 
+    /// GH #136: a configured retention of 0 used to run `LIMIT 0` retention
+    /// right after the insert and silently delete the row just persisted.
+    #[tokio::test]
+    async fn zero_history_retention_keeps_completed_jobs() {
+        let (manager, tempdir) = manager();
+        manager.set_history_retention(Some(0));
+        assert_eq!(manager.get_history_retention(), None);
+
+        insert_job(
+            &manager,
+            job("zero-retention", JobStatus::Completed, tempdir.path()),
+        );
+        {
+            let mut jobs = manager.jobs.lock();
+            manager.move_to_history(jobs.get_mut("zero-retention").unwrap(), Vec::new());
+        }
+
+        let db = manager.db.lock();
+        let entry = db
+            .history_get("zero-retention")
+            .unwrap()
+            .expect("completed job must remain in history with retention 0");
+        assert_eq!(entry.status, JobStatus::Completed);
+        assert_eq!(db.history_count().unwrap(), 1);
+    }
+
+    /// A positive retention limit still prunes, oldest first.
+    #[tokio::test]
+    async fn positive_history_retention_prunes_after_completion() {
+        let (manager, tempdir) = manager();
+        manager.set_history_retention(Some(1));
+        assert_eq!(manager.get_history_retention(), Some(1));
+
+        for id in ["ret-first", "ret-second"] {
+            insert_job(&manager, job(id, JobStatus::Completed, tempdir.path()));
+            let mut jobs = manager.jobs.lock();
+            manager.move_to_history(jobs.get_mut(id).unwrap(), Vec::new());
+        }
+
+        let db = manager.db.lock();
+        assert_eq!(db.history_count().unwrap(), 1);
+        assert!(db.history_get("ret-second").unwrap().is_some());
+    }
+
     #[tokio::test]
     async fn failed_history_cleanup_removes_raw_work_directory_after_persistence() {
         let (manager, tempdir) = manager();

crates/nzb-web/src/startup.rs

@@ -172,10 +172,8 @@ pub async fn initialize(
         config.general.article_timeout_secs,
     );
 
-    // Set history retention
-    if let Some(retention) = config.general.history_retention {
-        queue_manager.set_history_retention(Some(retention));
-    }
+    // Set history retention (the setter folds 0 into "keep all").
+    queue_manager.set_history_retention(config.general.history_retention);
 
     // Restore any in-progress jobs from the database
     if let Err(e) = queue_manager.restore_from_db() {