fix(history): treat history_retention=0 as keep-all (#136) - #137
Merged
Conversation
A configured history retention of 0 ran the retention pass with LIMIT 0 right after each history insert. The subquery matched nothing, so the DELETE removed every row silently: the job logged "Moving job to history", no error appeared, and the history table stayed empty. Fold Some(0) into None at every entry point (queue manager setter, startup, both config handlers) via normalize_history_retention(), and make Database::history_enforce_retention(0) a no-op as a last line of defence. Add regression tests at the db, config and queue-manager level; the queue-manager zero-retention test fails on v1.4.7 and passes here. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MPJczaZMiRkfwK3giJDVM5
thedancingdeveloper
added a commit
that referenced
this pull request
Sep 10, 2026
) Sets the base version the app reports (RUSTNZB_BUILD_VERSION = CARGO_PKG_VERSION + build ref) ahead of cutting v1.5.0-beta.1. Main is ahead of v1.4.7 by the history_retention fix (#137) and the compat + hardening feature backlog (#138: TAR extraction, RSS age rules, configurable cleanup, queue sorting, post-processing script contract), which is a minor release. Refreshes both lockfiles. Claude-Session: https://claude.ai/code/session_01G4d3N8VRucC7WVwNw8gwfV Co-authored-by: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #136.
Root cause
Database::history_enforce_retentionrunsDELETE FROM history WHERE id NOT IN (SELECT id ... LIMIT ?1). Withhistory_retention = 0(fromconfig.tomlor the Settings page field, which accepts0), the subquery matches nothing and theDELETEremoves every row immediately after the insert. Both statements succeed, so nothing is logged: the job reportsMoving job to history ... final_status=Completed, files land in the output directory, and the history table stays empty.SABnzbd and this codebase's own
speed_limit_bpsboth treat0as unlimited, so a0here is a reasonable user input. Nothing in v1.4.6 → v1.4.7 touched this path; it has been latent since retention was added.Changes
normalize_history_retention()in nzb-core config foldsSome(0)intoNone.QueueManager::set_history_retention, startup wiring,PUT /api/config/history-retentionandPUT /api/config/general, so the persisted config and the GET endpoint report what is enforced.Database::history_enforce_retention(0)is a no-op as a last line of defence.config.example.tomldocuments0as keep all.Tests
db::tests::test_history_enforce_retention_zero_keeps_allconfig::tests::zero_history_retention_normalizes_to_keep_allqueue_managerzero_history_retention_keeps_completed_jobs(fails on v1.4.7, passes here) andpositive_history_retention_prunes_after_completionWhy existing tests missed it: the only retention test used a limit of 3, and no test completed a job with retention configured.
Verified locally:
cargo test --workspace --all-targets --locked,cargo clippy --workspace --all-targets -- -D warnings,cargo fmt --all --check.🤖 Generated with Claude Code
https://claude.ai/code/session_01MPJczaZMiRkfwK3giJDVM5