Draft until #1571 merges: this is stacked on it, so the first two
commits here are that PR's and only the last one is new. I will rebase
once #1571 is in.
Shelfmark strips credentials and links out of text in five places,
written in different PRs with different patterns: the log formatter,
`_safe_url` and `_redact_urls_in_text` in `torrent_utils.py`,
`redact_status_message` in `viewer_redaction.py`, the `sed` rules in
`genDebug.sh`, and the password handling in the settings registry.
Between them there were three regexes that find a URL in text, two
separate userinfo patterns and no shared list of what counts as a
secret. #1571 made the gap visible, because every pattern added to the
logger had to be remembered somewhere else too.
This moves the text and URL part into `shelfmark/core/redaction.py`,
which imports nothing from the app so the logger can depend on it:
- `redact_secrets` (mask in place, for logs) moved from
`core/logger.py`, patterns unchanged. The logger imports it.
- `safe_url` and `redact_urls_in_text` (keep scheme, host and path, drop
the query) moved from `torrent_utils.py` and are public now, so
`alldebrid.py` no longer imports a private name from another module.
- `strip_links` (remove links entirely) is what `redact_status_message`
now calls. It and `redact_urls_in_text` share one link pattern, the one
the viewer redaction already used.
Two small behaviour changes come from sharing that pattern. A closing
bracket or quote right after a URL is no longer swallowed into it when
scrubbing an exception message. And the scheme-less path that requests
prints after `url: ` now keeps only what is before its query in
`redact_urls_in_text`, where before it passed through as it was (the log
formatter masked the key, but the path and query stayed in the message).
Also here: the two `sed` rules in `genDebug.sh` that mask userinfo now
stop at `?` and `#` like the logger pattern does, with a test that fails
on the old script.
Not in this PR: the debug bundle zips the live log directory as it is,
so log lines written before #1571 are not redacted in it. Fixing that
means copying the logs to a staging folder and running them through this
module, which I would do as a separate change.
Tests: a new file covers `safe_url`, `redact_urls_in_text` and
`strip_links` directly, and two checks that the viewer and the logger
really use these. Full suite passes.
Code scanning alert 106 (stack trace exposure on the release search 503)
turned out to be worth a look. Jackett sends its API key as `?apikey=`
in the query string, and when Jackett is unreachable the source builds
its "unavailable" message from the requests exception, whose text
includes the full request URL. `/api/releases` returns that message to
whoever ran the search, so any signed-in user could read the Jackett key
while Jackett was down. I reproduced it with Jackett pointed at a closed
port: the 503 body had `apikey=<the key>` in it.
Jackett was added in #1521 and isn't in a release yet, so nothing
shipped with this.
`SourceUnavailableError` now masks credentials in its message with the
same `redact_secrets` the log formatter uses (query parameters like
`apikey=`, `user:pass@` in URLs and so on). I did it in the exception
rather than at the two Jackett sites that build the text, since its
message is written for the user and any other source that ends up with a
key in a URL would hit the same thing. The rest of the message stays
readable, for example "Max retries exceeded with url:
...?apikey=(removed)&t=book".
Prowlarr and slskd send their keys in headers so they didn't leak a key,
but their messages do include the internal host and port. I left that
alone here.
Tests: two Jackett ones (a connection error on the search and on the
indexer lookup, with the key in the exception text) and three on the
exception itself. The four that check masking fail on current main. The
full suite, ruff and basedpyright are clean.
This is the UI side of #1581. A non-admin looking at their own
notification routes reads the same help text the admin does, including
"or mailtos://...?to={Email} in the admin default". They can't set that
default, a personal route can't use mailtos (personal routes are limited
to ntfy, Gotify, Discord, Slack, Telegram, Pushover and PushBullet), and
nothing on the page says whether an admin default exists. The default's
URL is hidden from them on purpose since it can carry credentials, so
the page just shows an empty All row, the same as when nothing is set
up.
What changes, on the user's own page only (the admin's view of a user
keeps the full text):
- The help text drops the admin default and mailtos part and lists the
services a personal route can use.
- The edit-context response now carries `adminDefaultRoutesSet`, a
boolean that is true when an admin default route with a URL exists. The
URL itself is still not sent, and the existing test that checks nothing
from the admin's values leaks still passes.
- When that is true, the help text adds a line saying a default applies
until the user adds their own, and once they have, that theirs replace
it.
How routes are resolved is unchanged. A user's own routes still replace
the default and don't merge with it. I didn't try to add mailtos for
personal routes, since keeping SMTP credentials in a user's own URL is
what the admin default avoids.
Tests: two route tests (flag true with a URL in the admin default, false
with an empty row), a vitest file for the description text, and the full
frontend suite, oxlint, oxfmt, tsc, ruff and basedpyright are clean. I
haven't looked at the page in a browser.
Fixes#1584. Thanks to @joeRob2468 for the write-up, which pinned this
down to the line.
Hardcover cuts a title like "Mistborn: The Final Empire" down to
"Mistborn" for searching when it can't tell the series prefix from a
title prefix, and search results don't carry the series. The library
check then matched the shelf title "Mistborn: The Final Empire" against
"Mistborn", saw "final empire" as extra words and took it for a longer,
different work (the "Dune" vs "Dune Messiah" rule), so the book got no
In library badge.
`match_entries` now tries the search title first as before and, if that
finds nothing and the full title is different, tries the full title too.
The full title is the more specific of the two so it should rarely add
false positives. Another book in the same series still doesn't match,
there is a test for that, and the Dune and Dune Messiah test passes
unchanged. The matching loop moved into a small helper to run it twice,
nothing in it changed.
This only touches the library check. The same short search title also
feeds release searches and auto-download matching, which I left alone
here because loosening or tightening those changes what gets searched
and downloaded for everyone.
Tests fail on current main without the change. The library tests, ruff
and basedpyright are clean.
Logs get pasted into issues, and the formatter from #1488 only masks
`?key=value` query parameters. I went through what else commonly ends up
in a log line and found these unmasked:
- credentials in a URL's userinfo, like a proxy's
`socks5://user:pass@host:1080` or `http://admin:secret@qbittorrent:8080`
- a passkey as a path segment, like `/rss/<key>` or
`/passkey/<key>/get.torrent`, which private trackers use
- the token in a Telegram bot, Discord webhook or Slack webhook URL,
which is what a notification URL looks like when it gets logged
All three are masked now, in `redact_secrets`, so they apply to every
handler. The path rule only fires after a segment that names a secret
(`passkey`, `apikey`, `rsskey`, `authkey`, `token`, `secret`, `rss`,
`torznab`) followed by 16 or more token characters. I left out generic
words like `download` on purpose, because `/api/download/<md5>/cancel`
appears in our own request log and the ids there are useful when
debugging. That is the tradeoff: a passkey in a `/download/<passkey>/`
path still gets through.
Separately, four modules created their logger with plain
`logging.getLogger`, so nothing they logged went through the formatter
at all: `api/websocket.py`, `download/clients/__init__.py`, and the
Jackett, Newznab and Prowlarr package inits. They use `setup_logger` now
like everything else.
Tests: ten cases, seven for the new shapes and three that must stay
readable (an md5 route id, a short `/rss/latest`, an email address),
plus two that check the stray modules now carry the redacting formatter.
Nine of them fail without the change. Full suite passes.
Not covered here: `/api/debug` still zips the existing log files as they
are on disk, so anything logged before this change rides along in a
bundle. That is a separate change in `genDebug.sh`.
If the download coordinator thread dies, every task sits at "Queued" and
nothing says why. The rest of the app keeps answering, `/api/health`
stays green and the container looks fine. #823 and #1166 were both that,
and the loop's `except Exception` cannot catch everything that can end a
thread.
`/api/health` now reports it. The response gets a `coordinator` field,
`running` or `stopped`, and when it is stopped the status is `error` and
the HTTP code is 503. The Docker healthcheck in the Dockerfile and the
one in the readme both use `curl -f`, so the container shows as
unhealthy and anything watching health (Uptime Kuma, autoheal) can act
on it. It needs no login, same as before, and shows no queue contents.
That 503 is the one decision here. Keeping the 200 and only adding a
`degraded` entry would not trip a status-code health check, which is the
whole point, so I went with 503. Easy to flip if you would rather not.
The orchestrator gets a small `coordinator_running()` that asks whether
the thread is alive. `start()` already knows how to restart a dead
thread, so restarting it automatically would be the true fix, but I
would rather the failure be visible first and decide on that separately.
Tests: three for the route (ok, 503, no login needed) and three for
`coordinator_running` (never started, alive, dead). They fail without
the change. One existing test hits `/api/health` through an app that
never starts the coordinator, so it now patches that. The readme's
health section has a sentence about the 503. Full suite passes, ruff and
basedpyright are clean.
`test_cache_hit_flushes_the_index_once_the_window_has_passed` (from
#1544) failed on the first attempt of two of my PRs' Python Tests runs
today (#1576, #1579) and passed on rerun, with nothing in either PR
touching the image cache.
The test sets the clock to exactly `last_saved + ACCESS_FLUSH_SECONDS`
and expects a flush. `(now + 30) - now` isn't always 30.0 in floating
point. For example `105.65207077218265 + 30 - 105.65207077218265` is
`29.999999999999986`, so the `>=` check is false and the flush never
happens. That only shows up when the monotonic value is small, which
fits a freshly booted runner (on this box the uptime is in the millions
of seconds and it never fails). I haven't been able to see the runner's
actual uptime in the CI logs, so that part is an inference, but pinning
`_last_saved` to that value reproduces the exact failure (`assert [] ==
[1]`) locally.
The test now puts the clock one second past the window. The production
check is fine, real elapsed time doesn't have this edge, so only the
test changes.
Six places read the Supported Book Formats and Supported Audiobook
Formats settings on their own: post-processing, the post-process scan,
IRC, slskd, Prowlarr and direct download. For ebooks they already
agreed, since they all fall back to `DEFAULT_EBOOK_FORMATS`. For
audiobooks they did not. The settings registry and slskd fall back to
every audio format plus zip and rar, while post-processing and IRC fell
back to m4b and mp3 only.
They now share `shelfmark/core/formats.py`, with
`get_supported_formats`, `get_supported_audiobook_formats` and a
content-type aware `get_supported_formats_for`, and a shared
`DEFAULT_AUDIOBOOK_FORMATS` in `core/utils.py` next to the ebook one.
`policy.py` re-exports the same functions, so nothing that imports them
from there changes. It is not in `policy.py` itself because importing
the `postprocess` package pulls in the router, which imports the release
sources, so a source could not import from it without a cycle.
One behaviour difference, and only in the fallback: with the audiobook
setting unset, or emptied, post-processing and IRC used to accept m4b
and mp3 and now accept the full default list like slskd and the settings
page already did. A configured list is read exactly as before. Direct
download's own helper used the raw configured list and now reads the
same normalized one, which is identical for any list the settings page
can save.
Also public now: `parse_size` in the Prowlarr source (slskd and Newznab
imported it with the underscore) and
`coerce_completed_path_timeout_seconds` in the client base handler
(slskd imported that too).
Not changed on purpose: slskd's `_coerce_int` looks like a copy of
`coerce_int` but is not. It reads `"12.0"` as 12 and `True` as 0, where
`coerce_int` gives 0 and 1. And the `_parse_size` in the IRC source is a
different function, string to bytes.
Tests: a new set checks that post-processing, IRC and slskd all fall
back to the shared defaults and that the settings default is the shared
one, and the scan-archive and IRC tests now patch the shared config
instead of the per-module copies. Full suite passes.
These are the follow-ups from the review notes on #1516 and #1534 that I
said I would do once the series had merged.
The scheduler waits for the next cycle outside the try that guards the
cycle itself, so an interval setting that raised would end the thread
and nothing would sync again until a restart. It now logs the error and
tries again after six hours. `docs/hardcover-sync.md` says so.
The history check in the sync opened a new SQLite connection for every
book on the shelf. It opens one per sync now and shares it, which adds
up on a slow disk or a small board.
The two test nits from #1534 are in as well: a `parametrize` with a
single case, and an unused argument on a helper.
The thread surviving a failed wait and the shared connection each have a
test that fails without the change. Ruff, basedpyright and the backend
suite pass (5297 passed).
Same idea as #1558, for three more places that hand `str(e)` to whoever
is logged in. These are code scanning alerts 106, 142 and 143.
`/api/status` and `/api/config` returned the raw exception text of an
OSError, RuntimeError and so on with the 500. They now return "Could not
read the download status" and "Could not load the configuration". The
full error is still logged with its trace.
For `/api/releases`, a source failing with an unexpected error put
`source: <exception text>` into the error list that goes back to the
caller. `search_source_releases` takes a new `include_error_detail`
flag. The route passes True for admins (and when auth is off, where
everyone is treated as an admin) and False for everyone else, who get
`source: search failed`. The message of a `SourceUnavailableError` is
written for the user, so it is kept either way, and the background
auto-download caller keeps the default and sees the full text as before.
Left alone on purpose: the `reason` in the release inspect response,
since a test covers that it carries the error text and it looks
intentional.
Tests: two for the routes and two for the helper. All four fail without
the change. Full suite passes (5295), ruff and basedpyright are clean.
Two small leaks from going through the open CodeQL alerts (code scanning
alerts 100 and 123).
The local download route returned `str(e)` on an OSError, and for a
missing or unreadable file that text is the file's path on the server
(`[Errno 2] No such file or directory:
'/audiobooks/Author/Title.epub'`). #1422 cuts a non-admin's download
path down to the file name everywhere else, so this put it back. The
route now answers "Could not read the file" and the detail stays in the
log.
`should_bypass_proxy` logged the whole request URL at debug when it
could not be parsed. URLs can carry credentials, so it logs only the
exception now.
Each change has a test that fails without it. The full non-e2e suite,
ruff and basedpyright pass.
A request whose book turns out to be in the library is closed with the note `[auto] Already in the library.` instead of sitting in Needs Review. A request an admin handled by hand while a pass was running is skipped. A series label in front of a Calibre title no longer hides an owned book, and a Calibre title marked as another edition (full cast, dramatized, GraphicAudio) is not the book.
When no source matches, up to three simpler forms of the title are tried on each source under the same strict match. Optionally (off by default), an unmatched audiobook request is looked up on Audible US and UK through Audiobookshelf's metadata search and the request says what the store answered. It never declines anything.
A Soulseek peer whose recent transfers all failed, and an AudiobookBay post whose comments say it has no seeders, are tried after the other matches and never excluded. A failed or stalled release is kept out for AUTO_DOWNLOAD_RETRY_FAILED_RELEASE_DAYS and then tried again, and a torrent that died for one request is not offered to another. A request stuck on queued or downloading is settled from the download history. Optionally (off by default) a failed download is retried with another release within minutes.
A synced request whose download failed, was interrupted by a restart, or was cancelled by the stall timer goes back to pending after AUTO_DOWNLOAD_RETRY_DAYS (7 by default, 0 turns it off), a few per pass, oldest first. Only what never arrived is retried, the failed release is remembered, and a request is given up on after three failed retries. Downloads cancelled by a person and rejected requests are never retried.
Errors count as errors: a search that failed in one source no longer reads as no match, a throttled Hardcover lookup is an error and not a skip, and a request that blows up doesn't abort the pass. Both summaries carry the error count.
A request for the same Hardcover book and format is skipped when the history already has a finished or running download for it. A request that finds nothing is searched once a day after three no-match passes, then every two days, up to AUTO_DOWNLOAD_BACKOFF_MAX_DAYS (3 by default, 0 turns it off).
A shelf book can be requested as an ebook, an audiobook or both (one request per format), and each request is checked against its own library, source priority list and format guard. Ranking within a source prefers the most downloaded copy, then size, and one m4b is preferred to a set of chapter files unless it is much smaller than the other copies. Soulseek peers are ranked by free slot, queue length and speed.
Automatic downloads refuse multi-book packs, sequels and other works named in the title, study guides, abridged or dramatised versions, two-in-one titles, releases in a language the requester didn't ask for, and an archive for an audiobook request.
Matching folds accents on both sides, drops an alternate title for the search and the match, and reads small numbers the same in any form. AudiobookBay releases with no author in the listing title are judged on the authors credited on the detail page.
Adds an Audiobookshelf provider for the library check, next to the Calibre one. It reads the server over its REST API (title, author, ISBN and ASIN per item), answers for audiobooks, and has its settings in General with a test button that uses the unsaved form values. Read only.
The Hardcover sync and automatic downloads now fail closed when an enabled library cannot be read: the sync skips its cycle and an automatic download holds the request. A library that listed items at the last index and now lists none counts as a failed read. Badges still fail open.
## feat(metadata): add Deutsche Nationalbibliothek (DNB) metadata
provider
This adds the Deutsche Nationalbibliothek (DNB) as a metadata provider.
It covers German editions far better than the existing providers.
No API key, disabled by default, and the same pattern as the other
providers.
It uses DNB's public SRU interface and parses MARC21-XML. Thats one
reason why this needs more code than the Google Books or Open Library
providers. The others are the search (DNB has no relevance ranking) and
the language filter, see below.
### Notes
- **Search box.** Search-box text goes from precise to loose until a
stage has hits: the complete title, then the title as a phrase, then
every word in a title or a person's name, last without German articles.
- **"Only show editions in my book languages"** (optional, on by
default). DNB also catalogs a lot of translations. DNB filters by the
searching user's Default Book Languages, else the system default.
- **No covers.** DNB's cover service is behind Anubis bot protection, so
the provider returns no cover URLs.
- **Proxy support** already included like Open Library since #1526
### Changes outside the provider
- DNB is in the onboarding dropdown.
- Calibre's `dnb-idn` identifier is mapped, so DNB results show library
ownership
- In the env-var docs I added only the DNB section. Regenerating the
whole file on main currently also reorders unrelated sections and drops
`SHELFMARK_API_KEY_READONLY` (and the Databáze knih section sits inside
the Moly.hu block).
### Ideas
- **Deduplication.** DNB lists every edition. Kafka's "Die Verwandlung"
has around 200 German editions of the same text. Grouping them would
touch the API, the frontend and release search.
- **Language-aware metadata search** for all providers. The other
providers ignore Default Book Languages in metadata search. The
`user_id` added here could serve them too.
This replaces #1047, which @InfiniteAvenger opened and calibrain offered
to re-review. It is Caleb's feature commit with his authorship kept,
rebased onto current main, with small fixes, tests and a docs page on
top. Everything is opt-in and off by default, so nothing changes for
anyone who doesn't turn it on.
What it adds is a **Settings, Hardcover Sync** tab with three parts that
can be switched on separately:
- **Shelf sync**: a Hardcover shelf (Want to Read by default) is pulled
into requests on a schedule, or with a Sync now button. Requests carry
the Hardcover metadata and cover.
- **Automatic downloads**: synced requests are approved and a release is
picked from a source priority list. A strict title, author and format
match (plus a seeder minimum for torrents) decides, so anything without
a confident match stays pending for manual review.
- **Skip books you already own**: sync and automatic downloads ask
whichever libraries are enabled under Library Check (Calibre today) and
skip a book that is already there.
On top of Caleb's commit I made the request owner a lookup instead of
assuming user id 1, sent synced cover URLs through the proxy when the
request is created, got ruff and basedpyright clean, and added tests and
`docs/hardcover-sync.md`. His commit also had its own Audiobookshelf
settings and a Test library connection button, written before the
library check was split into providers. The button would have raised a
TypeError against the current `test_connection`, so those settings are
gone from this tab and an Audiobookshelf provider comes as a separate
PR, which is the part of #1047 that is missing here (for now
anyways...).
I've run this daily for about a couple of weeks. Ruff, basedpyright and
the backend suite pass (4426 passed).
This is 1 of 5. As calibrain suggested, the rest are stacked on this
one, each saying which number it is.
Closes
https://github.com/calibrain/shelfmark/pull/1047#issuecomment-6302071588
---------
Co-authored-by: InfiniteAvenger <calebewest02@gmail.com>
Co-authored-by: CaliBrain <calibrain@l4n.xyz>
Two small cuts to work that repeats on every poll or request.
The delivery-state sync behind /api/status and the activity snapshot
listed every fulfilled request (SELECT * plus a JSON parse of book_data
and release_data per row) before it looked at the queue. That history is
never purged, so an idle instance paid for it on every poll. It now
returns straight away when no queue bucket holds a task, since nothing
could map to a request then. Results and emitted updates are unchanged.
The activity snapshot gains less from this, because its status merges
download history rows, so it is usually not empty.
In proxy auth mode the before-request hook looked the user up by name
and by session id on every API call, cover images included, each on a
new SQLite connection. UserDB.get_user_cached answers repeat lookups
from memory for up to five seconds, bounded at 256 entries. create_user,
update_user and delete_user drop the cache as soon as they commit, and a
lookup that overlaps a write is not stored, so a role change or deletion
made in the app applies on the next request. Changes made outside that
UserDB instance (another process, or the separate instances some helpers
open) show within the five seconds. Only the middleware uses the cached
path; every other get_user caller is unchanged.
Each change has a test that failed before it: one counts list_requests
calls for an empty queue, the others count get_user calls across
repeated lookups and middleware runs, check expiry with a patched clock,
and check that writes and caller mutations do not leave a stale row
behind. The full suite passes (4432 passed, 48 skipped, e2e excluded),
as do ruff and basedpyright on the changed files.
I also looked at throttling the full status_update broadcast when only
the progress message changes during torrent and usenet polling, and left
it out. Those handlers poll every 2 seconds and
DOWNLOAD_PROGRESS_UPDATE_INTERVAL defaults to 1 second, so a throttle on
that interval would skip nothing at the default setting. A longer window
would slow the speed and ETA text in the UI, and without a trailing
flush it would hide the last message of a quick burst, such as "Waiting
for bot response" arriving just after "Requesting file from bot", for
the whole wait that follows.
---------
Co-authored-by: CaliBrain <calibrain@l4n.xyz>
#1422 hid `source_id` and the download path from non-admins but left the
status message alone. A failed add stores the client's exception text
there, for example `401 Client Error: Unauthorized for url: <full url>`,
and for a requests error that URL carries the path and query: an indexer
key, a tracker passkey, a peer name. It reached the owner through
`/api/status`, `/api/activity` and the `status_update` socket event.
`redact_download_payload` now replaces every link in `status_message`
with `(link removed)`. That covers the live queue, the socket push and
the history snapshot, since all three go through it. Messages without a
link are unchanged and admins still see the full text.
Tests cover the payload, a whole status snapshot, and messages that must
stay as they are. They fail on main and pass here.
Found in a Claude pass over 1.5.0.
---------
Co-authored-by: CaliBrain <calibrain@l4n.xyz>
I went through what Shelfmark does per search and per cover and measured
the pieces that showed up. Four of them are cheap to fix, so this is one
PR. Shelfmark runs on a single gevent worker, so any of these blocks
every other request while it runs, and I'm assuming a lot of people run
it on small hardware (a Pi, say).
Cover cache. Every cache hit rewrote the whole `cache_index.json` just
to bump the entry's last-access time. On my box that is 3.7 ms of CPU
per hit at 500 entries and about 100 ms at 15,000 (2.2 MB written each
time), and a results page asks for around 40 covers. The access time now
stays in memory and the index is written at most every 30 seconds, or
whenever something else saves it. A crash inside that window loses some
access times, which only changes eviction order.
Settings reload. `config.refresh()` rebuilt the cache by reading and
parsing a tab's JSON file once per field, so about 250 parses per
refresh, 100 ms of CPU, and the metadata search, providers and config
routes each trigger it. It now reads each file once per rebuild (several
tabs share `settings.json`, so the memo is keyed on the file path).
Library match. When the library check is on, each search result is
compared against every library entry, and the title was split into words
again for every entry. The words are now computed once per result. I
measured 33 ms per result at 5,000 library entries and 106 ms at 20,000,
which is a second or more per page of 40 results. The matching itself is
unchanged.
Cover URL probe. `is_covers_cache_enabled()` creates and deletes a file
in the config directory to check it is writable, and it runs for every
cover, queue task and history row. The answer is remembered for 60
seconds. The probe is 0.045 ms on its own, but it is called a few
hundred times per request in the worst case.
Each change has a test that fails without it (call counts on the index
saves, file opens during a refresh, tokenisation calls, and probe
calls). Ruff, basedpyright on the changed files and the non-e2e suite
pass. The timings are from a dev machine with an empty config, so the
real numbers depend on cache and library size.
The start-up reconcile runs at import, before the coordinator starts,
inside a try that named five exception types and left out `TypeError`.
`main.py` already defines `_IMPORT_OPERATIONAL_ERRORS` for this shape
and the other module-level guards use it, so this switches to it. An
unexpected `TypeError` from the reconcile would otherwise fail the
import of `shelfmark.main`, and the worker would fail to boot on every
restart.
I didn't use `except Exception` because BLE001 is on and nothing in
main.py does.
The test reloads main with the reconcile raising `TypeError`, and with
`OSError` as the control (that one already passes on main). Full non-e2e
suite passes, ruff and basedpyright are clean.
Found in a Claude pass over 1.5.0.
The users, download history and activity view-state databases open their
connections with SQLite's default busy timeout of 5 seconds. A start-up
migration, the restart reconcile, or any statement that overlaps a
backup or a second process raised "database is locked" after 5 seconds,
and the reconcile finished only part of its work.
One shared `DB_BUSY_TIMEOUT_SECONDS` (30) in `core/utils.py`, used by
all three. The test reads SQLite's `busy_timeout` pragma back from each
connection, so it doesn't need a real lock fight.
I left `UserDB.initialize()` without a guard on purpose. With 30 seconds
the wait covers a backup, and swallowing a schema failure there would
hide a worker that can't run.
Found in a Claude pass over 1.5.0.
#1429 made the first terminal status win, but the guard only refused
terminal to terminal changes. A worker's poll that reports DOWNLOADING
after a cancel still overwrote CANCELLED (the poll loop calls the status
callback before it waits on the cancel flag), and the COMPLETE or ERROR
that followed was then accepted as a first terminal status, so the
terminal hook ran a second time.
This refuses any status change away from a terminal status. `add()` and
`enqueue_existing()` reset the status themselves, so retries are
unaffected.
The new test cancels a downloading task, sends DOWNLOADING and then
COMPLETE, and expects CANCELLED and one hook call. It fails on main and
passes here. Full non-e2e suite passes, ruff and basedpyright are clean.
Found in a Claude pass over 1.5.0, same as my other small fixes from it.
Fixes#1531.
All Direct Download providers share the one search budget
(`RELEASE_SEARCH_TIMEOUT`, 300s). Anna's Archive runs first, and when
its bypass keeps failing the retries use up the whole budget. OceanofPDF
then sees an expired deadline, skips its queries, and the search raises
Anna's Archive's error even though OceanofPDF never got a turn.
I didn't split the budget evenly, since the deadline module itself says
a cold-start Anna's Archive solve can legitimately take 60 to 120s and a
150s or 37s slice would break setups that work today. Instead
`search_deadline.unit_deadline(waiting=n)` opens a child deadline for
one unit of work. It gets the parent's remaining time minus 30s for each
unit still waiting, never less than 30s and never more than the parent
has left. A single provider still gets everything, a fast provider's
unused time rolls on to the next, and a stuck one stops at its own limit
with the reserve intact.
It's used in the provider loop in `DirectDownloadSource.search`, and in
the source loop in `/api/releases` since it's the same shape and four
lines. With several sources enabled the first one now gets 30s less per
source behind it, which is still well above a cold solve for the usual
two or three.
Tests: a stuck first provider no longer starves the second (fails on
main, passes here), a lone provider gets the full budget, and a child
never outlives its parent. Two existing tests in
`test_releases_api_budget.py` tripped the deadline in force from inside
the source, which is now the child, so they use a short request budget
instead. ruff, basedpyright on the changed files and the non-e2e suite
are clean.
OceanofPDF is inactive on my own instance (no mirror URL set), so I
haven't reproduced this live. It's verified by reading the code and by
the tests. I had Claude do the pass on this one.
The default for Supported Book Formats (`epub, mobi, azw3, fb2, djvu,
cbz, cbr`) was written out in five places: the settings registry,
`postprocess/policy.py`, `irc/parser.py`, Prowlarr's `source.py` (added
in #1490) and slskd's `source.py` (added in #1428). Both of those two
arrived in 1.5.0, and both are mine, so this is cleaning up after myself
as much as anything.
The setting was also read three different ways. Post-processing split on
commas only. slskd split on commas and spaces and stripped a leading
dot. IRC split on commas and kept the dot. Prowlarr split on commas and
kept `""` as a format when the value was an empty string. So `.epub` or
`epub mobi` worked in one source and not another.
This puts `DEFAULT_EBOOK_FORMATS` and `normalize_format_list` in
`core/utils.py` next to `AUDIOBOOK_FORMATS`, and the registry,
post-processing, IRC, Prowlarr and slskd all use them. I took the most
lenient reading of the three (comma or space split, leading dot dropped,
empties ignored, default when nothing is left), so no existing config
gets stricter. I put the reader in `core/utils.py` and not in
`policy.py` because the release sources can't import `policy` without
pulling in the whole post-processing package.
A config with a sensible list reads exactly as before. The only
differences are for malformed values: an emptied setting or an empty
string now falls back to the default in every place, where IRC used to
filter everything out and Prowlarr treated `""` as a format.
The audiobook default is a separate story and I left it alone: the
registry and slskd default to every processable audiobook format plus
the archives, while post-processing and IRC fall back to `m4b, mp3`.
They only differ when the setting is unset or emptied, but they do
differ, and I'd rather ask which one you want than pick one here.
Ruff, basedpyright on the changed files and the backend suite pass (4400
passed, e2e not run). New tests cover the reader and the two fallbacks.
Closes#1130.
Per-user notification route URLs can now use `{Email}` and `{User}`,
filled in from each user's account at send time, the same way `{User}`
already works in destination paths. One default route like
`mailtos://user:pass@smtp.example.com?to={Email}` now reaches every user
at their own address, with no per-user setup.
- Values are URL-encoded (`alice+books@example.com` keeps its `+`);
placeholders are case-insensitive.
- A route that needs a value the user doesn't have (e.g. no email on
file) is skipped for that user; other routes still send.
- The user lookup only runs when a route contains a placeholder.
- The personal "Test notification" button expands the placeholders too,
so testing a `{Email}` route works.
- Added to the routes setting help, `docs/users-and-requests.md`, and
tests.
@calibrain ready for review.
---------
Co-authored-by: CaliBrain <calibrain@l4n.xyz>
A signed-in user picks the release fields of POST /api/releases/download, and a record
id with the "oceanofpdf:" prefix reached the provider without handles() or is_enabled().
The provider then fetched source_url, POSTed to the form action found on that page and
fetched the meta-refresh target, with no check on any of them.
The provider now refuses to run when it is switched off, and the book page, the form
action and every POST redirect hop must be on the configured mirror (same scheme, host
and port). The signed file link may sit on another host but must be http(s).
network.ensure_public_destination refuses a host that resolves to a loopback, private,
link-local or otherwise non-global address (IPv4-mapped IPv6 judged by its IPv4 address).
Hosts the operator configured as mirrors and names that do not resolve pass as before.
html_get_page and download_url call it on their first URL and check each redirect hop
through a response hook. The image cache uses the shared address check too.
Co-authored-by: CaliBrain <calibrain@l4n.xyz>
A user could set any value for their own overridable settings through
PUT /api/users/me. DESTINATION and DESTINATION_AUDIOBOOK accepted any
string, so a download (including one an admin fulfilled for that user)
could be written to any directory the container can reach, and
get_destination created it on demand.
A user's own destination must now resolve inside the global one (with
{User} expanded on both sides). Absolute paths outside it and any ".."
segment are rejected at save time, and get_destination applies the same
check when the path is used, so a stored or hand-written value outside
the global destination is ignored with a warning and the global
destination is used instead. A relative value is taken relative to the
global destination.
Other self-service values are checked as well: select fields must use
one of their options, EMAIL_RECIPIENT must be a plain address (the same
check the orchestrator and the downloads tab use, now shared from
request_helpers), and Grimmory library and path ids must be non-negative
integers. Admin saves on a user's behalf are unchanged.
The self-service 400 response now carries a message line so the reason
reaches the user in the toast.
Co-authored-by: CaliBrain <calibrain@l4n.xyz>
- **Open Library (#1451): keep the work unless the search matched a
translation.** Open Library shows an English edition whenever one
matches the query. So "Mistborn" returned the 2023 "Mistborn" reissue
(2023) instead of "The Final Empire" (2001), and "Children of Dune"
returned "Children of Dune (Dune Chronicles, Book 3)" (1987). The
matched edition's title, ISBNs, cover, language and year now win only
when the edition is in another language and the query doesn't name the
work's own title. Checked against live Open Library: "Children of Dune"
and "Mistborn" give the v1.4.0 results again, and "Der Weg der Könige"
and "Kinder des Nebels" still return the German editions.
- **Google Books (#1434): the ISBN cache decorator is back on
`search_by_isbn`.** It had moved to the new `search_paginated`. Failed
searches were cached as empty pages, and ISBN lookups lost their cache.
- **Notifications: Test notification shows the plugin's failure reason
again.** Apprise 2 delivers on its own worker threads, even for a single
service when that service has a deadline. `_AppriseLogCapture` filtered
records by thread id and dropped all of them, so the UI only said "json:
delivery failed". Apprise runs those workers in a copy of the caller's
context, so the capture now filters on a context variable. That still
keeps concurrent dispatches apart. `AppriseAsset(async_mode=False)`
would not fix this, because the sequential path also uses the worker
pool when a service has a deadline.
- **Activity: `activity_update` no longer sends the raw task id to
non-admins.** The owner's `user_<id>` room got the raw task id. For a
request-linked download that id can hold a private tracker's passkey. It
now goes through `viewer_download_id`, the same mapping `/api/status`
and `download_progress` use. The admins room still gets the raw id.
- **SearchBar (#1476): Escape and an outside click close the homepage's
inline options panel again,** as they did in v1.4.0.
- **genDebug.sh: packages are listed through `importlib.metadata`,**
since #1404 removed pip from the image. The script also now redacts:
- `PROWLARR_MAM_ID`
- `user:pass@` credentials in URLs (proxies), in both the environment
and the config files
- notification route URLs, which carry their service's token (Telegram,
Discord and others)
- `make python-checks`, `make python-test`, `make frontend-checks` and
`make frontend-test` pass on top of current `main`.
- New tests cover:
- Open Library edition selection
- Google Books caching
- log capture against a real Apprise 2 delivery to a local server that
answers 500, plus a concurrent dispatch on another thread
- the `activity_update` payload sent to each room
- genDebug redaction and the package listing
Each one fails on the old code.
- I checked the SearchBar fix in the running app, because the frontend
has no DOM test setup: Escape closes the panel, an outside click closes
it, and a click inside keeps it open.
- I ran the genDebug redaction under GNU sed (Debian image) and BSD sed.
The package listing ran in the pip-less image.
Clear Completed can fail outright with "Only terminal requests can be
dismissed". `dismiss-many` answers 409 and dismisses nothing when any
one request or download in the batch is not terminal. The list a user
clears from can be older than the server, so a request that was reopened
or retried since, or a download that started again, takes the whole
batch down with it. The earlier fix for pending requests (#1438) filters
on the client, which only helps when the list is current.
`dismiss-many` now skips such an item and carries on with the rest. The
response adds `skipped_item_keys` when something was left out, and
`count` is what was actually dismissed. A single `dismiss` of a
non-terminal item still answers 409.
I hit it with a request whose download had completed while the request
itself was still pending, but any reopen between the list and the click
does it.
This touches the same handler as #1507, so whichever lands second needs
a small rebase.
Tests: a batch with a finished and a reopened request dismisses the
finished one and reports the other, and a batch with a finished and a
running download does the same. Ruff, basedpyright and the backend suite
pass (3933 passed, e2e excluded).
Co-authored-by: CaliBrain <calibrain@l4n.xyz>
* fix(torrent): check each redirect hop when fetching a torrent file
The torrent file fetch followed redirects by hand without looking at where a
hop pointed, so a download link could make the server request a loopback,
link-local or LAN address. Each hop (the initial URL and every redirect
Location) must now be http or https and resolve only to global addresses.
The configured Prowlarr/Newznab origin is exempt, as it usually lives on the
LAN. The API key still only goes to that origin.
* fix(socketio): only accept websocket handshakes from the app's own origin
The Socket.IO server allowed every origin. It now uses the same-origin check
python-engineio applies by default (Host, or X-Forwarded-Proto and
X-Forwarded-Host behind a proxy). SOCKETIO_ALLOWED_ORIGINS takes a comma
separated list of extra origins for proxies that change Host without
forwarding it.
* fix(custom-script): do not pass a file name that starts with a dash as the first argument
In relative path mode the script got the organized file's relative path as
its first argument, which many scripts read as an option when the name starts
with a dash. Such a name is now passed with a ./ prefix. Absolute mode is
unchanged.
A release format such as /../../x was appended to the staging file name
unchanged, so the direct-download and libgen handlers could write to, and on
cancel unlink, a path outside TMP_DIR. Normalise the format to a plain
extension at queue time and in build_filename, reject path-like source ids for
the sources that use them as file names, and make both handlers resolve the
staging path and refuse anything that is not a direct child of TMP_DIR.
A user's personal notification routes were sent to any Apprise URL, from the server. Through the test endpoint or a saved route a signed-in user could make the server POST to loopback, link-local and LAN hosts, choosing the path and, with Apprise's + and : parameters, extra headers and body fields.
User routes are now limited to the hosted services that talk to a fixed vendor host (discord, slack, tgram, pover, pbul) and to ntfy and gotify, whose host must resolve only to public addresses (checked on the host urlsplit reads and on the host Apprise reads, including IPv4-mapped and numeric forms). The check runs when a route is tested, when an event is queued and again when it is sent. Admin routes are unchanged.
Two small fixes in `activity_routes.py`.
**Another user's item answers like a missing one.** Dismissing a task or
request id answered 404 when it didn't exist and 403 when it belonged to
someone else, so a user could test whether an arbitrary id exists. A
non-admin now gets the same 404 for both. In dismiss-many a foreign key
is reported with the missing ones and the rest carry on, and the owner
is still written to the log. Admins are unchanged.
**A foreign row in someone's viewer state no longer gives a 500.** If a
non-admin's viewer state referenced another user's download or request
row, building history raised a `RuntimeError`. The row is now skipped
with a warning.
Not changed: history still raises when a row that viewer state points to
has been deleted. That is a different case, and I didn't want to widen
this.
Tests: the existing forbidden test now expects the 404 and the log
context, and four new ones cover a foreign download and a foreign
request answering like a missing one (including dismiss-many), an admin
still dismissing a foreign row, and history skipping foreign rows. Ruff,
basedpyright and the backend suite pass.
The request policy is applied when a user downloads
(`/api/releases/download`) or requests (`/api/requests`), but not when
they list releases. A user whose mode is `request_book` or `blocked` can
still search releases and list a torrent's files through the shared
Prowlarr and qBittorrent credentials. Only the UI hides the list.
This applies the same gate to `/api/releases`, `/api/releases/inspect`
and the release-source records route, with the same codes as the
download route (`policy_blocked`, `policy_requires_request`,
`required_mode`).
- `blocked` and `request_book` are refused. `request_release` is
allowed, because that mode lists releases so the user can pick one. The
UI opens the request dialog directly under `request_book` and never
calls `/api/releases` there.
- `/api/releases` resolves the mode per source for the content type. A
refused source is dropped from the search, so a user blocked on Prowlarr
no longer reaches Prowlarr even when other sources are allowed. It
answers 403 only when no source is left, or when the one source asked
for is refused.
- The records route doesn't know the content type before it fetches, so
it refuses only when every content type the source supports is refused.
- Admins, auth mode `none` and an instance with requests off bypass the
gate, as they do on the download route.
A user whose default is `blocked` now gets a 403 toast in the release
modal, where they used to get a list of "Unavailable" rows. I made no
frontend change.
Tests (14, new file) cover each route for blocked, request_book,
request_release, an admin, and auth mode none, the per-source drop, and
the records route. Ruff, basedpyright and the backend suite pass.
Co-authored-by: CaliBrain <calibrain@l4n.xyz>
`/api/covers/<id>?url=...` cached the fetched image under the id in the
path, and the caller chooses that id. A request with another book's id
and a different URL put its image in that book's cache entry, and every
user then got it.
The key is now derived on the server from the image URL (a hash of it),
and the id in the path stays only so links keep their shape. The SSRF
checks and `fetch_and_cache` are unchanged, and `transform_cover_url`
links already carry the URL, so they keep working. The URL is decoded
first now, so a missing one is a 404 and bad encoding a 400, both before
any fetch.
I made the key the URL alone, not URL plus id. Either stops the
overwrite, and the URL alone lets one image be shared when the same
cover appears under two ids (the viewer redaction rewrites ids and still
hits the cache).
Entries cached under the old keys are not read any more, since there is
no telling which were overwritten. They age out of the LRU and the cache
warms once after the upgrade.
Tests: a request with another id can't replace the legitimate image, in
either order; the normal miss then hit; the same URL under two ids hits;
no URL gives 404; bad encoding gives 400 without a fetch; and a unit
test for the key. Ruff, basedpyright and the backend suite pass.
GET /api/users/me/edit-context served every user-overridable key with the
admin's global value in globalValues and effective, so any user could read
the global destination paths, the admin's EMAIL_RECIPIENT, the Booklore
library and path ids and the global USER_NOTIFICATION_ROUTES table, whose
Apprise URLs embed tokens.
build_user_preferences_payload gains a for_self_service mode that replaces
a global value with None unless the user set the key themselves or the
field is a checkbox, number, select or multi-select switch the form needs
to lay itself out (BOOKLORE_LIBRARY_ID and BOOKLORE_PATH_ID are masked
despite being selects). The self-service route uses it; the admin routes
keep full data.
GET /api/debug returns a bundle with application logs, process listings
and environment names, and GET /api/restart exits the process. Both are
registered only when DEBUG is on, but any logged-in user could call them.
Add an admin_required decorator built on login_required and use it for
both routes. Non-admin sessions get 403 "Admin access required";
authentication mode "none" keeps working as before.
Refs #981.
Hardcover is signed in as one account, the instance owner's, and every
logged-in user shares it. A non-admin can see the owner's lists and
shelves in the List filter, browse one by asking for it, and add or
remove books and change read status from search results, which is what
#981 describes.
Now only an admin may. With authentication set to none the one user
keeps everything.
- `GET` targets and the batch lookup return no options for a non-admin,
so the buttons don't show and the account isn't asked.
- `PUT` targets answers 403.
- The List filter (`hardcover_list`) has no options for a non-admin, and
a request that asks for it has the field dropped before the search.
Providers name these fields in `account_search_fields`, and Hardcover
lists `hardcover_list`.
- Public things are unchanged: author, title and series suggestions, and
a pasted list URL in the search box.
That pasted URL is the one gap I know of. It resolves any list the
account can see, so a user who knows the slug of one of the owner's
private lists could still open it. Closing it means the provider has to
know who is asking, which is a bigger change than this one, so I left
it.
Not included from the issue: a toggle for letting users view lists and
limiting which lists they see. I kept this to the admin-only part so it
stays small, and I can do the toggle as a follow-up if you want it.
Tests cover the routes as an admin, a user and with no authentication
(targets read, batch and write, list options, a crafted list search, and
public fields still passing). Ruff, basedpyright and the backend suite
pass.
Co-authored-by: CaliBrain <calibrain@l4n.xyz>
A request that was retried after a failure has two tasks in the status
built from download history. `sync_delivery_states_from_queue_status`
walks the status buckets in `QueueStatus` order and keeps the last one
it sees for a request. Error comes after complete, so a request that was
delivered by its retry is set back to error and, when no retry is left,
reopened as pending. The next pass can then fetch the book again.
When a request has more than one task, an attempt that is still running
now decides its state, else a completed one, else the failure. Ties keep
the old order, so a request with a single failed task still reopens as
before.
Tests: an earlier failure doesn't undo a later delivery, a running retry
outranks an earlier failure, and a lone failure still reopens. It
happened on my instance to a request whose first try failed and whose
retry succeeded. Ruff and the backend suite pass.
Closes#1445, where you said you were open to it.
A rejected request was final. `update_request` won't move a terminal
status and the admin routes only fulfil or reject, so an admin who
changed their mind had to ask the user to request the book again, and
the user kept seeing the old rejected one.
`POST /api/admin/requests/<id>/reopen` moves a rejected request back to
pending and clears the review fields. What the user asked for (request
level, release, note) is kept. It refuses anything that isn't rejected,
and refuses when that user already has a pending request for the same
book.
Admins get a Reopen action on rejected cards in the activity sidebar and
in History. A rejected request has usually been cleared from the list by
the time anyone wants it back, so reopening also clears the dismissal,
the way the failed-download reopen does. Without that the pending
request stays hidden.
Tests cover the service (state change, what is kept, the refusals, a
non-string admin note) and the route, and the card model for the new
action. Ruff, tsc, oxlint, vitest and the backend suite pass. It has
been running on my instance since Oct 8.
Two things that go wrong when a second request picks a release a first
request already used. A task id comes from the release, so the history
row is reused.
**The row stays with the first request.** Recording a download kept the
first request's id, notes and state. On my instance a card for one book
showed up under the request for a different book, with that request's
note on it. Recording a download now moves the row to the request that
queued it and restarts its queue time. A retry with no request keeps the
request the row had.
**Old rows of a delivered request show as failures forever.** The
startup sweep leaves an old request-linked row alone so its retry stays,
but one whose request already has its book (delivered, or another
download of it finished) is now closed as "Superseded by a completed
download" instead of showing as an interrupted failure for ever.
Also in the first commit: History cards used the saved cover URL as it
was, an external address the page's own-origin image policy blocks,
while running downloads use the cover proxy. They use it too.
The second commit is about restarts. After a restart nothing is in
flight, but a request in downloading, resolving or locating whose row
had moved to another request was stranded, because only queued was
reopened. Those states are reopened too.
Tests cover the row takeover and the retry case, the cover through the
proxy (and a missing cover), the superseded cases (delivered request,
another row finished, an undelivered request is left alone) and the
in-flight states with no row. Ruff and the backend suite pass.
With one global limit, a few tasks that are only waiting hold every
slot. A Soulseek download sitting in a peer's queue is the example I
hit: three of those and a torrent that could start waits behind them.
`MAX_CONCURRENT_DOWNLOADS_PER_SOURCE` (0 keeps the single global limit,
which is the default). With it set, the coordinator starts a download
only when its source is below the limit and passes over tasks of a full
source without losing their place in the queue. `get_next` gets an
optional filter for this, and a cancelled task is still dropped when one
is given. The start-up stagger now applies only between downloads from
the same source, and the pool is sized so every source can reach its
limit.
The second commit: qBittorrent keeps its own queue of torrents waiting
for a free slot. The task polling one of those does no work, so it no
longer counts against its source's limit and the next task of that
source can start. The client does the pacing, as it does for its own
queue.
Tests cover the queue pass-over (order and priority kept, nothing
eligible, cancelled tasks), the coordinator (a full source doesn't block
the others, each source has its own limit, the global limit when unset)
and the client-queue case. Ruff and the backend suite pass.
PUT /api/users/me set a new password from the password field alone, so a live session could replace the password and lock the owner out. A builtin user now has to send current_password with it, checked against the stored hash. A wrong or missing one is a 400, and counts toward the same lockout as a failed login (a locked account gets a 429 without being checked), so the endpoint cannot be used to guess the password.
A password or current_password that is not a string is a 400. A password that was not a string raised a TypeError from len() and answered 500.
The account form asks for the current password when a new one is typed. Profile and settings changes without a password are unchanged.
Download to Browser only ever sent the first file of a multi-file
release
(audiobook chapters, packs): the folder output returned final_paths[0]
and
/api/localdownload served that one path.
The folder output now keeps every final path on the in-memory task
(download_paths), and /api/localdownload streams them as one
uncompressed
ZIP named after the book when there is more than one. The archive is
never
staged on disk or held in memory, and ZIP64 covers large audiobooks.
History still stores only the first path, so a re-download from history
after a dismiss or restart serves a single file, as before.
Also brings every linter back to green on all files:
- pre-commit: bump the ruff hook to v0.17.1 to match uv.lock;
ruff-format
the Python examples in docs/dev and skills
- ruff: fix the remaining issues in skills/shelfmark/download_books.py;
format base_handler.py
- basedpyright: Optional and attrs errors in databazeknih.py
- knip: drop 31 unused exports, reuse ColumnColorHint in colorMaps
- oxfmt 0.72: styles.css
- direct download: fix the "Welib, mirrors" typo and the test that still
expected the pre-OceanofPDF message
Fixes#1467
## Summary
- **Skip files or books in a multi-book pack.** The pack review panel
gets a switch per file and a "Skip this book" switch per book, using the
shared `ToggleSwitch`. A partly skipped book shows e.g. "3 of 5 files"
in the Files column. A fully skipped book is greyed out.
- **Choose formats for a single book.** A new "Choose formats" header
toggle, next to the multi-book one, opens a format picker for a release
that has several book formats (EPUB, MOBI, PDF and so on). It has one
switch per format with file count and size. Releases with one format, or
that can't be inspected, download as before.
- **Skipped files are sent as `ignored_files`.** They are stored on the
task (including restart-safe retry) and removed before grouping and
transfer. This matters for packs because files missing from `book_plan`
would otherwise be filed by the heuristic fallback. Ignored entries
match by release-relative path, then path suffix, then unique basename.
- A pack that ends up with one book after skips keeps the titles you
reviewed instead of falling back to the searched title.
## Screenshots
<img width="1277" height="413" alt="Screenshot 2026-10-19 at 09-39-28
Shelfmark"
src="https://github.com/user-attachments/assets/c005ca4b-3dac-45dd-b2ff-d02fc0f9fcc9"
/>
<img width="1277" height="1210" alt="Screenshot 2026-10-19 at 09-40-23
Shelfmark"
src="https://github.com/user-attachments/assets/2145f3ba-e5fc-4635-9364-1790feb0c8b8"
/>
<img width="1277" height="748" alt="Screenshot 2026-10-19 at 09-39-03
Shelfmark"
src="https://github.com/user-attachments/assets/39023fac-70e3-46d8-af89-1160ebd6e415"
/>
## Test plan
- [x] `pytest` pack tests (new `drop_ignored_files` cases added)
- [x] `vitest`, `tsc`, `oxlint`, `oxfmt`, `ruff`
- [x] Manual check in the UI with a real multi-book pack
🤖 (mostly) Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Closes#1178.
Adds `{AuthorSort}`, the first author surname-first ("Arthur Conan
Doyle" → "Doyle, Arthur Conan"), for rename and organize templates,
alongside `{FirstAuthor}`.
- A value already in "Last, First" form (one word before the comma) is
kept as is; Jr/Sr/II/III/IV stay at the end.
- Known limits, same as calibre's default: the last word is treated as
the surname, so particles ("Le Guin") and surname-first names aren't
detected.
- Added to the settings help, env docs, the frontend variable
picker/preview, and tests.
Searching a translated title on Open Library or Hardcover returned the
original-language work. "Der Weg der Könige" came back as "The Way of
Kings" (2010), and that English title then went into the release search
and the download name. Both providers keep one record per work, titled
after the original, and list translations underneath it.
#483 already searches releases per language with Hardcover's localized
titles, which fixed the release side of #360 when the matching language
is selected. This PR fixes the search result itself, so the book a user
picks is the translation they searched for, and it covers Open Library,
the default provider, which has no titles_by_language.
- Open Library: request the matched edition (editions.*) and prefer its
title, ISBNs, cover, language and publication year when its title
differs from the work's.
- Hardcover: use the alternative title named in the hit's highlights
(alternative_titles, best index first), without the original's subtitle.
A hit whose title also matched keeps the work title.
- /api/releases: when the modal sends a title that differs from the book
record's, it becomes search_title. Before, Hardcover's search_title from
the original won, so picking "Kinder des Nebels" still searched "The
Final Empire" first.
- Release cache: keyed on title and language as well as the provider ID.
All translations share that ID, so opening "The Way of Kings" after "Der
Weg der Könige" showed the German releases until a reload.
Known limitations: neither provider has descriptions per edition, so the
description stays the work's (usually English). Hardcover's search hits
carry no year or language for alternative titles, so those stay the
work's; Open Library's do not. Google Books is unaffected, since it
already returns one volume per edition.
Tests: Open Library edition parsing, Hardcover alternative-title
matching, the /api/releases title override, and the release cache, each
written to fail first. The Python suite passes apart from the 9
test_entrypoint_permissions.py tests, which fail the same way on an
unmodified main on macOS. The frontend typecheck, lint, format check and
unit tests pass. Checked on a live instance with Open Library, Hardcover
and Google Books.
Refs #360, #483
Disclaimer:
This was developed with the help of AI but reviewed and tested as well
as deployed on my server by a real human.
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Kept small on purpose so it's easy to review. I have a few more small
fixes like this from going back over recently merged PRs. Would you
rather get them as separate PRs like this one, or combined into one?
#1361 made `cancel_download` write under the queue lock, which stops a
cancel from overwriting a download that finished first. The other
direction is still open. The worker checks `cancel_flag.is_set()` and
then calls `update_status(COMPLETE)` a few lines later in
`_process_single_download`. If a cancel lands between those two lines
the task goes CANCELLED, then the worker writes COMPLETE over it.
`_apply_status_locked` allows terminal to terminal moves, so the
terminal hook also fires a second time and records two events for one
task.
This makes the first terminal status win: `_apply_status_locked` ignores
a different terminal status once the task already has one. Requeueing
from a terminal status still works because QUEUED isn't terminal.
I replaced the existing race test. It patched `queue.update_status`,
which `cancel_download` no longer calls, so it never exercised the path
it names, and its last assertion only looked at the final hook event.
The new tests call the queue directly: complete then cancel is refused,
cancel then complete stays cancelled with one hook event, error then
complete stays error, and a terminal task can still be requeued. The two
late-write tests fail on current `main`.
Ruff and basedpyright are clean. `tests/core` and `tests/download` pass
apart from 4 `seleniumbase` import failures in my environment that also
fail on `main`.
The one red check is the existing clock-dependent test
`test_a_queued_torrent_asks_for_a_grace_once`, which fails on any runner
that booted recently. It is fixed in #1426 and is not caused by this
change.
splitsec2
ProcyonX1
Amalkrishna A SandCaliBrain
amura11andClaude Sonnet 5.5
HeihoffJandClaude Opus 5.5