Skip to content

fix: bring in xbmc4lyfe batch 2 fixes - #167

Merged
thedancingdeveloper merged 17 commits into
mainfrom
upstream/xbmc-batch2
Oct 9, 2026
Merged

thedancingdeveloper merged 17 commits into
mainfrom
upstream/xbmc-batch2

Conversation

@thedancingdeveloper

@thedancingdeveloper thedancingdeveloper commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Cherry-picks the xbmc4lyfe batch 2 fixes onto our main, keeping original authorship (git cherry-pick -x). Head 13932cb.

Upstream was re-listed before opening: open issues are exactly #69 #71 #73 #75 #77 #79 #81 #83 #85 #87 #89 #91 #93 #95, and no newer fix PRs exist beyond #96. Older open PRs (#6 #8 #36 #49 #51 #56) were brought in by earlier batches.

Upstream map

Issue Upstream PR Our commit Notes
#69 addurl names jobs after the URL, nzbname ignored #70 30523ab Adapted: loopback tests use fetch_allowed_hosts instead of the ALLOW_LOOPBACK_FOR_TESTS seam
#71 concurrent config writes lose updates #72 102a3ab Adapted: write lock wraps our partial server merge, password masking and RSS URL validation; started_at kept
#73 watch folder ignores .zip / .nzb.bz2 #74 a593772 Clean
#75 StatPipeline leaves a broken connection Busy #76 6d05f7a Clean
#77 pp override on addfile/addurl ignored #78 bd2b41e Adapted: kept both nzbname plumbing and the pp override
#79 native API returns 500 for unknown ids #80 8a639b2 Adapted: contract tests use our login() helper
#81 watch folder retries bad NZBs, partial writes, duplicates #82 56dee53 Adapted: claim-before-enqueue wraps our archive-aware watcher
#83 addfile/addurl reject .nzb.gz/.nzb.bz2/.zip #84 87635fa Adapted: archive module and deps already present from #74; nzbname/pp threaded through unpack, only the first member of a multi-NZB archive keeps an explicit nzbname
#85 mode=server_stats wrong shape #86 5d505b2 Single SHA (branch stacked)
#87 mode=warnings returns Unknown mode #88 ebfb91c Single SHA
#89 pipelined article errors trip the circuit breaker #90 098bf57 Single SHA faebca6. Adapted: gating sits on top of our MAX_OUTAGE_TRIES_PER_SERVER cap; connect-time failures still always count
#91 speed-limit change leaves workers parked #92 03a1f85 Took upstream's notify-based acquire wholesale
#93 same-named jobs share a complete folder #94 b1536a2 Adapted: kept our post_processing_level and sanitize_job_name
#95 retained incomplete dirs never reclaimed #96 f8dac41 Adapted: sweep helpers coexist with our claim_unique_output_dir
— — 13932cb cargo fmt plus removal of an unused import left by the archive port

Already covered, not cherry-picked

Test plan

  • cargo fmt --all --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test --workspace (all green)
  • New harnesses: harness_article_errors, harness_speed_limit, harness_unique_output_dir, harness_incomplete_cleanup
  • sabnzbd_compat::tests (70), api_contracts (11), workflow_fixtures (7), nzb-dispatch lib (59)

Do not merge until the security review records "no blockers" against this head.

xbmc4lyfe and others added 15 commits October 9, 2026 04:54
mode=addurl&name=<URL> passed name both as the URL and as the job name,
so the job was named after the whole URL, which output_dir_for rejects
as not a single safe path component: every successful addurl fetch
failed at enqueue. The nzbname override was also ignored on addfile and
addurl over GET, form and multipart.

Read nzbname from the query string, a urlencoded body or a multipart
field. For addurl the job name is nzbname, else the Content-Disposition
filename, else the URL's last path segment (percent-decoded, query
excluded), each without .nzb; never the URL itself. addfile prefers
nzbname over the uploaded file name.

Unit tests reach the loopback fixture server through a cfg(test)-only
task-local seam in fetch_guard (ALLOW_LOOPBACK_FOR_TESTS), scoped to the
test's own future; the SSRF guard is unchanged in every other build.

Fixes #69

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 9ed6598)
Adapted: tests admit the loopback fixture through an explicit fetch_allowed_hosts entry on the test state instead of the upstream cfg(test) ALLOW_LOOPBACK_FOR_TESTS seam, which our fetch guard (private-LAN allowlist plus explicit hosts) does not have.
Every /api/config/* handler cloned the current config, mutated the copy
and stored it with an unguarded ArcSwap::store. Two concurrent writers
started from the same snapshot, and the later store silently dropped
the earlier writer's change, in memory and in config.toml. A burst of
24 concurrent category/feed adds lost entries on every run.

Add AppState::update_config_with(|cfg| ...), which runs the closure on
the latest config and saves the TOML and publishes the result while
holding a config write mutex. An error from the closure writes nothing.
update_config takes the same lock and is documented as a blind
replacement. All 17 mutating handlers (SAB API key rotation, servers,
categories, history retention, max active downloads, speed limit, disk
guards, RSS feeds, general settings, SABnzbd import apply, DAV) now go
through it. Server handlers push the latest committed server list to
the queue manager, so concurrent writers converge on the same state.

Bumps nzb-web to 0.4.23.

Fixes #71

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit aec138d)
Adapted: update_config_with keeps our partial server merge, password masking and RSS feed URL validation inside the new write lock; started_at stays alongside config_write.
The watch folder only matched .nzb and .nzb.gz, so a .zip of NZBs (or
a .nzb.bz2) dropped there was skipped without a log line. The upload
API already unpacked .gz and .zip, but through extract_nzbs in the
binary crate, which the watcher in nzb-web cannot reach.

Move extract_nzbs (unchanged, with its size bounds and tests) into
nzb_web::nzb_archive and have both the upload handler and the watcher
call it. The helper also learns .bz2, using the bzip2 crate that was
already in the dependency tree via zip. The watcher now enqueues each
.nzb inside a zip, names jobs after the NZB without any archive
directory, and moves the archive to processed/ once at least one NZB
was enqueued. zip and flate2 drop out of the binary crate, which no
longer uses them directly.

Fixes #73

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 67a7070)
StatPipeline::execute set the connection Busy for each batch and then
propagated send, flush and read failures with a bare `?`, leaving the
connection Busy. After such a failure the stream is misaligned with
the outstanding STATs, so the connection must not be reused. All
three now set ConnectionState::Error first, matching fetch_article
and the ARTICLE pipeline.

Fixes #75

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit e20357d)
SABnzbd's pp parameter selects post-processing for a single job, but the
compat layer dropped it, so every job used its category's level.

Add NzbJob::pp_override (serde default; persisted in a new nullable
queue.pp_override column via schema migration v13, which tolerates
partial schemas without a queue table) and
QueueManager::post_processing_level, which prefers the override over the
category setting where post-processing chooses its level. addfile/addurl
read pp from the query string, a urlencoded body or a multipart field,
mapping SABnzbd's cumulative scale onto RustNZB's (2 and 3 -> 3;
-1/absent/out of range -> category default). script is accepted and
ignored, as before (unknown parameters are not rejected).

Fixes #77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit a3c7cd7)
Adapted: kept both the earlier nzbname plumbing and the pp override in the shared form/multipart parsers and in handle_addurl; existing SAB config/priority/diskspace tests kept alongside the new pp tests.
…nput

Client errors on the native API surfaced as 500 internal_error because
From<NzbError> for ApiError always used 500, and several handlers wrapped
"not found" and validation failures in anyhow!, which carries no status.

Central mapping (nzb-web error.rs):
- NzbError::JobNotFound/ServerNotFound/CategoryNotFound -> 404,
  ParseError/InvalidNzb -> 400, AdmissionConflict -> 409, rest 500.
- error_kind recognises the wrapped NzbError variants and otherwise
  classifies by status (bad_request / not_found / conflict /
  internal_error) instead of labelling every non-typed error
  internal_error.

Handlers:
- history retry and history logs: unknown id -> 404 (logs previously
  returned 200 with an empty list).
- queue priority: out-of-range value -> 400; unknown job -> 404 via the
  central mapping, as is queue move.
- GET /api/articles/{id}: NNTP 430 (ArticleNotFound) -> 404.
- Group routes extract ids with IdPath, which rejects a non-numeric id
  with a JSON 400 "Invalid id in request path" instead of axum's parser
  text that leaked Rust type names.
- RSS rules: invalid or overlong regex -> 400; PUT of an unknown rule
  -> 404 (DELETE stays idempotent); a rule naming feeds that are not
  configured is still saved, logged as a warning, and the response
  carries a "warnings" array.

Bumps nzb-web to 0.4.23.

Fixes #79

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 1e09a67)
Adapted: the new contract tests authenticate with the existing login() helper (this tree's first-run setup is already covered elsewhere) and sit alongside the existing contract tests.
…to settle

The watch folder could import one NZB twice, retry a bad one forever,
or read one mid-copy:

- A parse failure only logged a warning and left the file in place, so
  every restart retried it. Unreadable, unparseable or unenqueueable
  files now move to failed/.
- A fixed 500 ms sleep was the only guard against reading a file still
  being written. The watcher now waits until the size and modification
  time are unchanged across two checks 500 ms apart (capped at 10
  minutes), for both new events and files present at startup.
- The move to processed/ after add_job was best-effort, so if it failed
  the next restart enqueued a duplicate. The watcher now renames the
  file to <name>.processing before enqueueing. Claimed files are never
  imported again, and the claimed file then moves to processed/ or
  failed/. If that move fails, the file stays claimed, and startup logs
  a warning for leftover claimed files instead of re-importing them.

Fixes #81

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 5b8c138)
Adapted: the claim-before-enqueue and write-settle logic wraps our archive-aware watcher (zip/bz2/gz), so enqueue_claimed unpacks every NZB in the claimed file and any failure sends the whole file to failed/; both sets of watcher tests are kept.
SAB addfile/addurl passed compressed NZBs straight to the XML parser,
which rejects them, while the native add endpoint already unpacked .gz
and .zip. Move that extraction (and its decompression-bomb limits) from
the app crate into a shared nzb_web::nzb_archive module, used by both
the native handlers and the SAB layer, and add .bz2 support there
(bzip2 0.6 was already in the dependency tree via zip's default
features; no new crates are locked).

addfile unpacks the upload; a multi-NZB zip enqueues each NZB as its
own job and reports every nzo_id. addurl unpacks the fetched body,
naming the job after the inner NZB, and hands multi-NZB zips to
addfile.

The addurl test reaches its loopback fixture through the same
cfg(test)-only fetch_guard seam (ALLOW_LOOPBACK_FOR_TESTS) introduced on
the nzbname branch; the hunk is identical.

Fixes #83

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 066b77f)
mode=server_stats was aliased to fullstatus, so clients got the
{"status": {...}} dashboard document instead of SABnzbd's server_stats
shape. Add a dedicated handler that remaps the native per-server
statistics (QueueManager::server_stats_get_all, as served by
GET /api/config/servers/stats) into SABnzbd's layout: overall
total/month/week/day bytes and a per-server map keyed by server name
with total/month/week/day, daily, articles_tried and articles_success.

Fixes #85

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 205e70d)
mode=warnings is a real SABnzbd mode that clients poll, but RustNZB
rejected it as unknown. Answer with SABnzbd's {"warnings": [...]}
shape. RustNZB keeps no SABnzbd-style warnings list (fullstatus
already reports warnings: [] and have_warnings "0"), so the list is
empty and consistent with fullstatus; name=clear returns
{"status": true}.

Fixes #87

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 83938f6)
(cherry picked from commit faebca6)

Adapted: our pipeline error path already caps per-article outage retries
(server-wide vs article-outage vs fail-over). Only the circuit-breaker
accounting (record_failure / report_provider_outage) is gated by
counts_against_circuit_breaker; the outage-try cap is unchanged.
Connect-time failures still always count — they are provider-level.
acquire_download loaded the limiter once and awaited it for the whole
article in burst-sized chunks. At a very low limit (20 B/s) one ~750 KB
article parks a worker for hours, and set_download_bps only swapped the
ArcSwap: parked acquires kept waiting on the old limiter, so raising
the limit or clearing it to 0 had no effect. Because every worker
connection stayed parked, new jobs stalled too, and only a restart
recovered.

The limiter now signals a Notify on every reconfiguration. Each chunk
wait races the limiter against that notification, which is registered
before the limiter is loaded so a change cannot be missed. On a change
the acquire reloads the current limiter and continues with the bytes
still owed. With no limit configured it returns immediately. No lock is
held across an await.

Tests: unit tests in bandwidth.rs (20 B/s to unlimited and to 10 MB/s
release a parked acquire within 2 s; unlimited never waits), plus
harness_speed_limit.rs, which drives a real QueueManager against the
mock NNTP server: a limit set at startup, a limit set before a job is
added, and throttle/unthrottle mid-download.

Fixes #91

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 1050af7)
Two jobs with the same name derived the same output folder at add time
and both completed into it, so the second release overwrote or merged
into the first and both history rows reported the same storage path.

Reserve the complete folder when a job first writes output (direct
unpack, post-processing, or the final move) rather than at add time, so
queued jobs never hold a name. The reservation uses create_dir and
retries with `<name>.1`, `<name>.2`, ... on AlreadyExists, as SABnzbd
does, so two jobs finishing together cannot pick the same folder. The
chosen folder is recorded on the job and therefore in history, and in
the job checkpoint so an interrupted post-processing run resumes into
its own folder instead of reserving another. A failed job releases an
empty reservation, and a rename or category change drops it.

Fixes #93

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 2ef6207)

Adapted: kept our post_processing_level() helper (job override wins over
the category default) and our sanitize_job_name rename path; the output
dir claim and release calls from upstream are applied on top of both.
(cherry picked from commit 60eeb1e)

Adapted: the new incomplete-dir sweep helpers were merged alongside our
existing claim_unique_output_dir / restore_claimed_output_dir helpers;
neither replaced the other.
The batch 2 port bumped nzb-web to 0.4.23 and added bzip2 and zip to its
dependency list, which left desktop/src-tauri/Cargo.lock stale under
`cargo build --locked`. Only the desktop lockfile changes; no dependency
versions were upgraded.

@thedancingdeveloper thedancingdeveloper left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security review: 3 blockers (head 1b6009e)

I reviewed all 16 commits line by line. The upstream content was treated as untrusted. No malicious code was found. Each blocker below was confirmed with a local PoC test (not committed).

Local checks at this head:

  • cargo test --workspace: 925 passed, 0 failed.
  • cargo clippy --workspace --all-targets -D warnings: clean.
  • cargo fmt --check: clean.
  • ci/migration-policy.sh: passes.
  • CI is green.

Blockers

1. The zip size cap can be bypassed by under-declaring entry sizes (crates/nzb-web/src/nzb_archive.rs:45-60)

The running total sums each entry's declared entry.size(). The actual read is capped at 100 MiB per entry. zip 8.6.0 does not check decompressed output against the declared size: Deflate is a plain DeflateDecoder, and only the CRC is verified. Overlapping central-directory entries are also accepted unless has_overlapping_files() is called.

  • PoC: a 180 KB zip with three entries declaring uncompressed_size = 1 (CRCs left correct) returned 3 entries totalling 180 MiB against the 100 MiB cap.
  • Impact: at about 1000:1 deflate, a 100 MB addurl body (MAX_ADDURL_BODY_BYTES) can decompress to roughly 100 GB held in memory, so the process is OOM-killed.
  • New exposure: this logic was already in the native upload handler on main. This PR newly exposes it to SAB addurl (remote, indexer-controlled content), SAB addfile and the watch folder.
  • Fix: budget on bytes actually read, e.g. entry.take(MAX - total + 1) then total += buf.len(), and bail when total > MAX. Optionally cap the entry count, or reject archives where has_overlapping_files() is true.

2. The incomplete-dir reclaim fails open on DB errors and deletes active downloads (crates/nzb-web/src/queue_manager.rs:4217-4260, f8dac41)

referenced_work_dirs only logs a warning when db.queue_list() or db.history_list() fails, then carries on with whatever it collected. A per-row history_get_retry_data error is also just skipped (:4245). In those cases sweep_orphaned_work_dirs (run at startup) and remove_unreferenced_work_dir call remove_dir_all on directories that are still in use.

  • PoC: a queued job with a UUID work dir. With a healthy DB the sweep keeps the dir. After one column of that row is made undecodable (UPDATE queue SET total_bytes='x'), queue_list() returns Err and the same sweep deletes the dir.
  • Why it matters: queue_list collects into a Result, so one bad row, or SQLITE_BUSY from a backup tool or sqlite shell, wipes every partial download. restore_from_db then fails on the same queue_list()?, which startup.rs only logs. On main, such a failure was recoverable. With this PR the data is gone.
  • Fix: have referenced_work_dirs return Result, and skip all removal if any listing or retry-data read fails (fail closed).

3. clean_nzb_name panics on non-ASCII names (crates/nzb-web/src/sabnzbd_compat.rs:378, 30523ab)

base[base.len() - 4..] slices by bytes. This is the same class as the #158 parse_rar_volume bug.

  • PoC: "Amélie" and "Pokémon" panic, because byte len-4 falls inside the é.
  • Reach: nzbname on SAB addfile and addurl (Sonarr and Radarr send release titles here, and accented titles are common), the remote server's Content-Disposition filename, and zip entry names.
  • Impact: the handler task panics and the request is dropped. The process survives because nothing sets panic = "abort", but those adds fail every time.
  • Fix: base.get(base.len() - 4..).is_some_and(|s| s.eq_ignore_ascii_case(".nzb")), or use to_ascii_lowercase. Add a test with Amélie.

Areas checked, no issues

  • Dependencies:
    • zip moved from the app crate to nzb-web at the same 8.6.0, with default-features = false and deflate only.
    • bzip2 0.6.1 uses the pure-Rust libbz2-rs-sys backend, so there is no C build.
    • Both were already in the lockfile through nzb-postproc. No new crates, no checksum changes, and no build.rs, CI or script changes.
  • gz/bz2: output is bounded by take(MAX+1). The .gz/.bz2 suffix slicing is char-safe, because no non-ASCII character lowercases to ., g, z, b or 2.
  • Path traversal:
    • Zip entry names, nzbname and Content-Disposition names all pass through parse_nzb → sanitize_job_name.
    • add_job re-checks safe_component and that the output dir is under the complete root.
    • SAB paths use output_dir_for. The watcher's complete_dir().join(name) is unchanged from main and is still guarded by add_job.
  • Watch folder:
    • Each file is claimed by rename before enqueueing.
    • Symlinks are refused after the claim, via symlink_metadata.
    • Reads are capped at 100 MiB, with a settle wait capped at 600 s.
    • The watcher starts only after restore_from_db.
  • addurl: the SSRF guard and pinned client are unchanged, and the body is capped before decompression.
  • Unique output folders (b1536a2): reservation is atomic via create_dir. Release is remove_dir, which only removes an empty directory. A checkpointed folder is accepted only if it is a real (non-symlink) <base> or <base>.<n> folder next to the target.
  • Reclaim confinement (apart from blocker 2): removal is limited to direct, non-symlink children of the canonical incomplete root. The sweep only touches UUID-named directories.
  • Config read-modify-write lock (102a3ab): every mutating handler goes through update_config_with, and no blind update_config callers remain. The lock order is config, then queue manager, and QueueManager never takes the config lock, so there is no deadlock. Password masking and feed URL validation stay inside the lock.
  • Native API validation (8a639b2):
    • Status mapping only.
    • Regex length is bounded, and Rust regex is linear-time.
    • Priority must be 0–3.
    • IdPath returns a JSON 400 for bad path ids.
  • NNTP (6d05f7a, 098bf57): a STAT pipeline I/O failure now sets the connection to Error, so the misaligned stream is never reused. The circuit-breaker gating is consistent with #157's per-article outage cap.
  • Bandwidth (03a1f85): correct Notify pattern: the notification is enabled before the limiter reload.
  • SAB modes: pp maps to {0, 1, 3}. warnings always returns an empty list. server_stats returns only names and counters.
  • Migration v13: does not collide with existing migrations, and uses bound parameters only.

Should-fix (non-blocking)

  • Timeout no longer counts against the circuit breaker, so a server that black-holes reads is only limited by the per-article outage cap.
  • A multi-NZB addurl zip is decompressed twice: once in addurl, then again in addfile on the raw bytes.
  • The general-settings closure applies queue-manager runtime changes before the TOML save. If the save fails, runtime and disk diverge.

Nits

  • 1b6009e's message says "no dependency versions were upgraded". It also re-points existing lock edges down: windows-sys 0.61.2 → 0.59.0/0.45.0 for dirs-sys, errno, rustix, tempfile, quinn-udp, nu-ansi-term, os_pipe and winapi-util, and toml 1.1.2 → 0.9.12 for tauri-utils. All of those versions were already in the lock, and desktop CI is green. Consider cargo update -p nzb-web to keep the diff minimal.
  • Attribution:
    • xbmc4lyfe authorship and cherry picked from lines are kept.
    • Unlike batch 1, there are no Upstream-PR: / Fixes-upstream-issue: trailers.
    • The bare Fixes #69…#93 lines point at unrelated items in this repo. All are already closed, so nothing auto-closes, but the references are misleading.

This verdict covers only this head SHA. A new head needs a fresh review.

- Budget zip extraction on bytes actually read instead of the declared
  entry size, cap the number of NZB entries, and reject archives whose
  central-directory entries overlap.
- Make the incomplete-dir reclaim fail closed: an error from queue_list,
  history_list or a per-row history_get_retry_data skips every removal in
  both the startup sweep and single-directory removal.
- Slice the ".nzb" suffix in clean_nzb_name on a char boundary, so
  non-ASCII names no longer panic.

Each has a regression test. Timeouts now also count against the circuit
breaker: a server that accepts the connection and then black-holes the
read is as unhealthy as one that refuses it.

@thedancingdeveloper thedancingdeveloper left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security review: no blockers (head 54e49e0)

The follow-up commit 54e49e0 sits directly on 1b6009e, so the 16 commits reviewed in round 1 are unchanged. 54e49e0 touches only the three blocker sites, their tests, and the circuit-breaker gating. It adds no dependencies, and there are no lockfile, build, CI or script changes.

Round-1 blockers: all fixed

I reran my original PoCs unchanged against this head.

  1. Zip size cap (nzb_archive.rs)
    • Fix: each entry's read is limited to the remaining budget + 1 byte, and the running total counts bytes actually read. Archives whose central-directory entries overlap (has_overlapping_files()) are rejected, and NZB entries are capped at 10,000.
    • PoC: three 60 MiB entries declaring 1 byte each are now rejected ("exceeds the 100 MB limit"); round 1 accepted 180 MiB. The byte budget alone bounds overlapping entries, so the overlap check is defence in depth.
  2. Reclaim fails closed (queue_manager.rs)
    • Fix: referenced_work_dirs returns Result. queue_list, history_list and per-row history_get_retry_data errors are all propagated, and both the startup sweep and remove_unreferenced_work_dir skip all removal on error.
    • PoC: with an undecodable queue row, the queued job's work dir now survives both the sweep and single removal; round 1 deleted it.
  3. clean_nzb_name panic (sabnzbd_compat.rs)
    • Fix: the suffix is checked with base.get(len-4..), so &base[..len-4] is only taken on a char boundary.
    • PoC: Amélie → Amélie, Pokémon → Pokémon, Amélie.nzb → Amélie, é.NZB → é, and a€ → a€. None panic.

Each fix has a regression test, and all three pass.

Also in 54e49e0

  • NntpError::Timeout now counts against the circuit breaker. This resolves my round-1 should-fix about servers that accept the connection and then black-hole reads. Article-level answers still don't count, and #157's per-article outage cap is unchanged.

Checks at this head

  • cargo test --workspace: 928 passed, 0 failed.
  • cargo clippy --workspace --all-targets -- -D warnings: clean.
  • cargo fmt --check: clean.
  • ci/migration-policy.sh: passes.
  • All CI checks are green at this head.

Remaining non-blocking items

  • Should-fix:
    • A multi-NZB addurl zip is decompressed twice.
    • The general-settings closure applies runtime changes before the TOML save.
  • Nits:
    • startup_sweep_keeps_everything_when_a_queue_row_is_undecodable shells out to an external sqlite3 binary. It passes on CI, but will fail wherever sqlite3 isn't installed.
    • extract_nzbs_rejects_zip_entries_that_underdeclare_their_size builds two unused archives (let _ = (zip, second)).
    • Limit errors always say "100 MB", even when a smaller test cap applies.
    • From round 1: the desktop lockfile edges are re-pointed downward, there are no Upstream-PR: trailers, and the bare Fixes #69…#93 lines point at unrelated items that are already closed.
    • 54e49e0 has no Co-Authored-By trailer.

Not merged; that's for oversight. This verdict covers only this head SHA. A new head needs a fresh review.

@thedancingdeveloper
thedancingdeveloper merged commit 1ca5ee7 into main Oct 9, 2026
11 checks passed
@thedancingdeveloper
thedancingdeveloper deleted the upstream/xbmc-batch2 branch October 9, 2026 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants