From f9f50fcf07331acff8eee1ff3fefe0f706107cfe Mon Sep 17 00:00:00 2001 From: rays Date: Sat, 14 Nov 2026 03:59:02 +0000 Subject: [PATCH] reap --dry-run says what would go, not that it went (#461) A dry run printed "deleted (N MB)" for every file and "deleted N old file(s)" at the end, word for word a real reap, of files it kept: Reaped and ReapDone carried no flag to say otherwise. Both now carry dry_run (serde default false, so an older message still reads), and the words are "would delete". Found writing the browser suite's first test of retention, which ran on the daemon's own clock and nowhere in the suite: over a tiny quota, a dry reap lists the downloads and leaves them, and a reap with no limits deletes nothing and only tidies. Co-Authored-By: Claude Opus 5.5 --- crates/ipx/CHANGELOG.md | 1 + crates/ipx/src/ipc.rs | 23 +++++++++++++++++------ crates/ipx/src/retention.rs | 4 ++-- crates/ipx/src/web.rs | 6 +++++- tests/ui/app.spec.ts | 25 +++++++++++++++++++++++++ 5 files changed, 50 insertions(+), 9 deletions(-) diff --git a/crates/ipx/CHANGELOG.md b/crates/ipx/CHANGELOG.md index 66c82b3..b1d7d92 100644 --- a/crates/ipx/CHANGELOG.md +++ b/crates/ipx/CHANGELOG.md @@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `ipx reap --dry-run` says "would delete" of each file and of the total, rather than "deleted", word for word a real reap, of files it kept (#461). - Subscribing to a feed someone here already reads puts its recent items to your Signals at once, rather than at the judge's next pass, up to five minutes later (#415). - A download a restart stopped, a deploy's for one, is taken up again as the daemon starts, rather than waiting for its feed's next check, an hour or a day away (#410). - A YouTube channel no longer misses most days' videos: YouTube answers some feed requests with a 404 or 500 it does not mean, and ipx now asks again, twice, a second apart, before calling the feed failing (#382), and then every hour for a day where it would have waited a day, since YouTube's failures run for hours; and its YouTube feeds are read one at a time, two seconds apart, since YouTube refuses many asked together (#386). diff --git a/crates/ipx/src/ipc.rs b/crates/ipx/src/ipc.rs index 4e664ac..8bc7a04 100644 --- a/crates/ipx/src/ipc.rs +++ b/crates/ipx/src/ipc.rs @@ -83,6 +83,9 @@ pub enum Event { Reaped { path: String, bytes: u64, + /// Only what would go: `reap --dry-run` said "deleted" of files it kept (#461). + #[serde(default)] + dry_run: bool, }, /// Terminal: a client that asked for work stops reading here. ScanDone { @@ -97,6 +100,8 @@ pub enum Event { ReapDone { files: usize, bytes: u64, + #[serde(default)] + dry_run: bool, }, Status { feeds: usize, @@ -165,11 +170,11 @@ impl Event { }, Event::DownloadDone { path, .. } => format!(" saved {path}"), Event::DownloadError { url, msg, .. } => format!(" failed {url}: {msg}"), - Event::Reaped { path, bytes } => { - format!("deleted {path} ({:.1} MB)", *bytes as f64 / 1_048_576.0) + Event::Reaped { path, bytes, dry_run } => { + format!("{} {path} ({:.1} MB)", deleted(*dry_run), *bytes as f64 / 1_048_576.0) } - Event::ReapDone { files, bytes } => { - format!("deleted {files} old file(s), {:.1} MB", *bytes as f64 / 1_048_576.0) + Event::ReapDone { files, bytes, dry_run } => { + format!("{} {files} old file(s), {:.1} MB", deleted(*dry_run), *bytes as f64 / 1_048_576.0) } Event::Status { feeds, pending, downloaded } => { format!("{feeds} feeds, {pending} queued to download, {downloaded} downloaded") @@ -288,6 +293,10 @@ impl Event { } } +const fn deleted(dry_run: bool) -> &'static str { + if dry_run { "would delete" } else { "deleted" } +} + /// An event, logged once (#91): its words as the message, and `ev`, `feed`, `new` and the rest as /// fields, so Loki reads them without parsing the message. A failure also gets `error.type` and, /// from an HTTP error, `http.response.status_code`, so failures group by kind without a regex. @@ -516,7 +525,9 @@ mod tests { assert!(Event::ScanDone { feeds: 3, only: None, forced: false }.is_for(&me, &mine)); assert!(Event::SignalHits { user: 1, hits: 2 }.is_for(&me, &mine)); assert!(!Event::SignalHits { user: 3, hits: 2 }.is_for(&me, &mine)); - assert!(!Event::Reaped { path: "/downloads/Theirs/ep.mp3".into(), bytes: 1 }.is_for(&me, &mine)); + assert!( + !Event::Reaped { path: "/downloads/Theirs/ep.mp3".into(), bytes: 1, dry_run: false }.is_for(&me, &mine) + ); } #[test] @@ -627,7 +638,7 @@ mod tests { #[test] fn only_completion_events_end_a_client_session() { assert!(Event::ScanDone { feeds: 1, only: None, forced: false }.is_terminal()); - assert!(Event::ReapDone { files: 0, bytes: 0 }.is_terminal()); + assert!(Event::ReapDone { files: 0, bytes: 0, dry_run: false }.is_terminal()); assert!(!Event::FeedDone { feed: "a".into(), new: 0, downloaded: 0, failed: 0, torrents: 0 }.is_terminal()); } diff --git a/crates/ipx/src/retention.rs b/crates/ipx/src/retention.rs index 73e97d2..b3b16b1 100644 --- a/crates/ipx/src/retention.rs +++ b/crates/ipx/src/retention.rs @@ -142,10 +142,10 @@ pub(crate) async fn reap(ctx: &Ctx, dry_run: bool, standalone: bool) -> Result<( } let removed = r.aged_out.iter().chain(r.over_quota.iter()); for c in removed { - ctx.out.emit(Event::Reaped { path: c.path.clone(), bytes: c.bytes.max(0) as u64 }); + ctx.out.emit(Event::Reaped { path: c.path.clone(), bytes: c.bytes.max(0) as u64, dry_run }); } if standalone { - ctx.out.emit(Event::ReapDone { files: r.aged_out.len() + r.over_quota.len(), bytes: r.bytes_freed }); + ctx.out.emit(Event::ReapDone { files: r.aged_out.len() + r.over_quota.len(), bytes: r.bytes_freed, dry_run }); } Ok(()) } diff --git a/crates/ipx/src/web.rs b/crates/ipx/src/web.rs index d03c720..6a657dd 100644 --- a/crates/ipx/src/web.rs +++ b/crates/ipx/src/web.rs @@ -2518,7 +2518,11 @@ async fn delete_file( state.ctx.db.mark_reaped(id).await?; state .events - .send(Event::Reaped { path: enc.path.unwrap_or_default(), bytes: enc.length.unwrap_or(0).max(0) as u64 }) + .send(Event::Reaped { + path: enc.path.unwrap_or_default(), + bytes: enc.length.unwrap_or(0).max(0) as u64, + dry_run: false, + }) .ok(); Ok(StatusCode::NO_CONTENT) } diff --git a/tests/ui/app.spec.ts b/tests/ui/app.spec.ts index 2c55ce8..56c31f6 100644 --- a/tests/ui/app.spec.ts +++ b/tests/ui/app.spec.ts @@ -2915,3 +2915,28 @@ test('each service serves its metrics as Prometheus reads them, the requests mad expect(play).toMatch(/^play_build_info\{version="[^"]+"\} 1$/m); expect(play).toMatch(/^play_uptime_seconds [0-9]/m); }); + +test('a reap over the quota says what it would delete and, dry, deletes nothing', async ({ page }) => { + // Retention (retention.rs) runs on the daemon's own clock in production, once a day; nothing in + // the suite ran it. Dry, so the files the tests after this one play are still there. + const env = { ...process.env, IPX_CONFIG: `${setup.root}/config/config.toml`, IPX_DATA_DIR: `${setup.root}/data` }; + const ipx = (args: string[]) => execFileSync(setup.IPX_BIN, args, { env, encoding: 'utf8', timeout: 30_000 }); + const files = () => fs.readdirSync(`${setup.root}/downloads`, { recursive: true, withFileTypes: true }).filter(f => f.isFile()).length; + const had = (await (await page.request.get('/api/settings')).json()).max_total_gb; + const before = files(); + expect(before, 'the suite has downloaded files by now').toBeGreaterThan(0); + expect((await page.request.patch('/api/settings', { data: { max_total_gb: 0.000001 } })).status()).toBe(204); + try { + const out = ipx(['reap', '--dry-run']); + // Said as what would go, not as gone (#461). + expect(out).toMatch(/^would delete [1-9]\d* old file\(s\)/m); + expect(out).toMatch(/^would delete .+\/downloads\/.+ \(\d+\.\d MB\)$/m); + expect(out).not.toMatch(/^deleted /m); + expect(files(), 'a dry run deletes nothing').toBe(before); + } finally { + await page.request.patch('/api/settings', { data: { max_total_gb: had } }); + } + // No limit: a real reap only tidies (the directory, where unread lists start, the artwork kept). + expect(ipx(['reap'])).toContain('deleted 0 old file(s), 0.0 MB'); + expect(files()).toBe(before); +});