fix(hardcover): pause bulk work until daily quota resets by magrhino · Pull Request #2699 · vavallee/bindery

magrhino · 2026-09-19T00:14:37Z

Summary

When Hardcover reports daily quota exhaustion, continuing a bulk import cannot succeed until the reset. This replacement for #2607 implements the smaller scope requested in its maintainer review: persist a server-reported daily hold, stop bulk work with a distinct error and reset time, and show the pause in Settings. Refs #2593 and #2615; the broader accounting and hydration requirements in #2593 remain deferred.

Parse the daily RateLimit bucket and validate its RateLimit-Policy window when supplied. Persist a hold only for an explicit zero remaining count with a valid reset; leave short throttling and Retry-After handling unchanged.
Share the hold across configured Hardcover clients, scoped by a hash of the token. Preserve holds across restart, retry failed persistence before admitting requests, and keep network requests outside the store lock.
Stop ABS imports without advancing the interrupted item's checkpoint; stop CSV/Readarr metadata resolution and apply the existing Goodreads breaker immediately. A rerun requeues committed authors whose catalogues have never populated, while preserving deliberately emptied catalogues.
Stop background refresh/discovery/catalogue work on exhaustion and show the configured token's reset time under Settings → API Keys.
No request accounting, plan allowance estimates, interactive reserve, account-discovery query, deferred-hydration queue, schema migration, or new dependency. Distinct tokens retain separate holds even if they belong to the same account. Retry interrupted manual imports after reset; scheduled work runs on its next pass.

The implementation is roughly 350 production lines plus regression tests. The caching work in #2615 remains separate and should retain this hold when constructing token-bound clients.

Checklist

Commits signed off with git commit -s
Tests added or updated
docs/DEPLOYMENT.md updated
Added one changelog fragment under changelog.d/; CHANGELOG.md untouched
Wiki pages updated if user-facing behaviour changed — deployment guidance is in this repository; external wiki not edited

Test plan

codecov · 2026-09-19T00:19:39Z

Codecov Report

❌ Patch coverage is 82.92683% with 35 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/metadata/hardcover/daily_quota.go	76.19%	19 Missing and 16 partials ⚠️

📢 Thoughts on this report? Let us know!

vavallee · 2026-09-27T00:29:19Z

Sorry for the silence on the PR itself. I scoped this on #2593 this morning and should have said so here too. Short version: this is the in scope half and I want it in.

One change before merge: the only red check is CodeQL on `daily_quota.go:58`, the `sha256` of the normalised token. The finding is wrong on substance (it's a password hashing rule, and this is a scope key from a high entropy bearer token), but an HMAC with an install local salt is about five lines and makes the argument go away, which beats a dismissal the next reviewer has to re trust.

One thing to look at, not a blocker: `Check` calls `q.persist(ctx)` on every admission while holding `q.mu`. It's guarded by `dirty`, but the hot path is taking a lock that can cover a settings write.

It's a week behind main, so a rebase plus a full test run against merged main would be good. Full notes on #2593.

magrhino · 2026-09-28T22:52:11Z

Thanks. All three points are addressed, and the branch is rebased onto current main (84a0ab24). The two original commits are unchanged apart from the rebase, and each fix is its own commit on top:

CodeQL (daily_quota.go) (7975398): token fingerprints now use HMAC-SHA256 with a random 32-byte key generated once per install and stored in auth.hardcover_daily_hold_secret. It's kept separate from the session secret, so rotating sessions doesn't orphan live holds. The settings API hides it through the existing _secret pattern, and it's added to TestSettings_SecretLeakRegression. New tests cover the HMAC keying, reuse of the key across a restart, and a corrupt key raising an error.
Check persisting under q.mu (same commit): a version counter replaces dirty. persist copies the holds under q.mu and writes them outside it, with a separate write lock that keeps writes in order. A Check with nothing pending never waits on a settings write, and a token under a live hold still gets DailyQuotaError even if the retried write fails.
Rebase: the only conflict was in hardcoverlistsyncer/syncer.go, and both sides are kept. It exposed one interaction (348f87c): A book made wanted by a Hardcover list sync, or monitored after the fact, is never searched immediately #2722 now queues searches until the end of the pass, so the per-book hold check returning early dropped searches for books that pass had already made wanted. The loop now breaks, dispatches those searches, then returns the hold. There's a regression test for it.

docs/DEPLOYMENT.md is updated for both behaviour changes. go vet ./... and make test pass on the rebased branch, as does go test -race on the hardcover and list-syncer packages. On macOS, make test needs TMPDIR=/private/tmp because of #2868, which is unrelated.

vavallee

Thanks, the core holds up: the admission checks are real (removing them fails the two transport tests), the reset is relative and capped so timezones can't move it, nothing does network I/O under the lock, and the token is pinned per operation. A few things to change though.

A Hardcover hold breaks ISBN lookups on the default setup. GetBookByISBNWithOutcome (aggregator.go:921) returns on DailyQuotaError from any provider, so with OpenLibrary primary and Hardcover as an enricher, a found OpenLibrary answer is thrown away, and DNB is never asked when OpenLibrary misses. Short circuiting only when the held provider is the primary (idx == 0) and otherwise falling through to the existing failed provider handling fixes it.
An enricher's hold stops all bulk work. Aggregator.CheckQuota loops over enrichers too, so on an OpenLibrary primary install a Hardcover hold stops ABS imports, CSV and Readarr imports, metadata refresh and every catalogue sync for up to a day, including a user adding an author. None of that needs Hardcover, and held enricher calls already fail fast without touching the network. I'd check the primary only. I know my #2607 review asked for imports to stop, but that was about hammering an exhausted key, which the client level check now prevents.
runCatalogueSync returns early and strands books it already created (authors.go:2297, :2708). Those books skip matching files on disk, auto search, deferred series linking and the summary, and the next sync treats them as existing so it never happens. The ctx.Err() check next to it uses break for this reason; quotaErr = err; break and returning it at the end would match.
The per book hold check in the list syncer (syncer.go:562) guards a loop that makes no Hardcover calls, so a hold raised mid pass just drops local writes of data already fetched. I'd remove it and let the calls before the loop fail.
A corrupt holds row blocks every token for good. load (daily_quota.go:65) fails closed on bad JSON on every call, and the row isn't writable through the API. Unlikely, but total. Logging and starting from an empty map (and regenerating a corrupt secret) would be safer.

Small one: in migrate/authors.go:150-166 a failed CataloguePopulatedAt read counts as both skipped and an error.

Also, this and #2615 both add a section at the same spot in docs/DEPLOYMENT.md, so whichever merges second needs a trivial rebase.

magrhino · 2026-10-04T01:19:36Z

Thanks for the detailed review. Each point is done the way you suggested, in new commits on top, and the branch is rebased onto current main (c3b16571).

ISBN lookups (75f8bbf): only a held primary (idx == 0) short-circuits. A held enricher falls through to the existing failed-provider handling, so OpenLibrary's answer is kept and DNB is still asked.
Bulk work (75f8bbf): Aggregator.CheckQuota checks the primary only. Doing that exposed a related bug (21162d4): the import breaker in migrate/outage.go tripped on any daily error in FirstErr, and with an OpenLibrary primary that error can be the enricher's. So CSV, Readarr and Goodreads imports would still fail every row after the first. It now also requires PrimaryFailed.
runCatalogueSync (fa1f297): at :2297, quotaErr = err; break. At :2708, the hold is recorded instead of returned. Created books get hydration, file matching, search, series linking and the summary, and the hold is returned at the end. Author discovery also counted those books as zero, because it broke on Backoff before adding them, so I fixed that too.
List syncer (12920c3): removed the per-book check. A hold now stops the sync at GetUserLists / GetListBooks.
Corrupt state (37df99a): a bad holds row logs a warning and starts empty. A bad or empty key is regenerated, and holds under the old key just expire.

Small one (21162d4): a failed CataloguePopulatedAt read now counts once, as an error.

docs/DEPLOYMENT.md is updated in ae9e110. Noted on #2615; whichever lands second gets the rebase.

Each fix has a regression test that fails without it. go vet ./... and make test pass, as does go test -race on the metadata, hardcover, list-syncer, migrate and scheduler packages.

Signed-off-by: magrhino <12738722+magrhino@users.noreply.github.com>

…e lock Token fingerprints were a plain sha256 of the bearer token, which CodeQL flags as weak hashing of sensitive data. Key them with HMAC-SHA256 under a random 32-byte install-local secret stored in the private setting auth.hardcover_daily_hold_secret. It is separate from the session secret so rotating sessions does not orphan live holds. Check no longer retries a pending hold write while holding the store lock. A version counter replaces the dirty flag, and persist snapshots under the lock and writes outside it behind its own write lock. Admissions with nothing pending never wait on a settings write, and a token under a live hold still gets DailyQuotaError even when the retried write fails. Signed-off-by: magrhino <12738722+magrhino@users.noreply.github.com>

The per-book daily-hold check returned from the list-sync loop, dropping the searches queued for books that pass had already made wanted (vavallee#2722). The next pass sees those books as tracked, so they waited for the scheduled sweep instead. Break out of the loop instead, dispatch the queued searches, then return the hold. Signed-off-by: magrhino <12738722+magrhino@users.noreply.github.com>

A held enricher aborted ISBN lookups, discarding the primary's answer and never asking later fallbacks, and Aggregator.CheckQuota stopped every bulk job on an enricher hold. Only a held primary short-circuits a lookup now; a held enricher is an ordinary failed provider. CheckQuota checks the primary only, since held enricher calls already fail fast without a request. Signed-off-by: magrhino <12738722+magrhino@users.noreply.github.com>

The import breaker tripped on any daily-hold error in FirstErr, which is the enricher's error when the primary answered and only an enricher was held, so CSV, Readarr and Goodreads imports failed every row after the first. Require PrimaryFailed as well. Also count a failed catalogue marker read once, as an error, instead of as both skipped and failed. Signed-off-by: magrhino <12738722+magrhino@users.noreply.github.com>

runCatalogueSync returned on a daily hold, so books it had already created skipped file matching, auto search, deferred series linking and the summary, and the next sync treated them as existing. Record the hold and break, matching the ctx.Err() check beside it, finish the created books, then return the hold. Author discovery now also counts the books a held run created instead of dropping them at the backoff break. Signed-off-by: magrhino <12738722+magrhino@users.noreply.github.com>

The pass loop makes no Hardcover calls, so a hold raised mid-pass only dropped local writes of data already fetched. Holds still stop the sync at the list fetches, which go through the quota-aware client. Signed-off-by: magrhino <12738722+magrhino@users.noreply.github.com>

A corrupt holds row or fingerprint key failed every load, blocking every token permanently, and neither setting is writable through the API. Log a warning and start with no holds, and replace a corrupt key; holds keyed by the old one expire on their own. Signed-off-by: magrhino <12738722+magrhino@users.noreply.github.com>

…ecovery Signed-off-by: magrhino <12738722+magrhino@users.noreply.github.com>

Waiting out a primary's hold made a refusal that never ends unbounded: each row waited 3 times, then failed as rate limited, which the outage breaker ignores, so a 1000 row import against a refusing provider ran for hours and sent thousands of requests. The Readarr and Goodreads imports run detached and could not be stopped. A lookup still refused after its waits now counts toward the outage streak, so three such lookups in a row trip the breaker and the rest of the rows fail at once with "primary metadata provider X is rate limiting requests, try the import again later". A run also has a 10 minute budget for waiting out holds; once spent, a refused lookup is not waited out and counts straight away. isRateLimited moves into outage.go, which keeps the errors and hardcover imports in use, so the file still builds when #2699, which adds an errors.As case to the same switch, lands alongside this. Signed-off-by: vavallee <vavallee@protonmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9

…cover GraphQL rate limits (#2369, #2075, #2791) (#3100) * fix(metadata): share one retry loop and refusal hold across HTTP providers (#2369, #2075) Only OpenLibrary retried a 429. Google Books, DNB, NB, Audible and audnex made one bare request, so a refusal was a hard "metadata lookup failed" and, for all but NB, carried no providererr mark for discovery to back off on (#2369). OpenLibrary's loop moves into internal/metadata/providerhttp and all six HTTP providers use it: retry 429/502/503/504 and transport errors up to 3 times, honour Retry-After (capped at 30s) or jittered backoff, drain error bodies, and mark what is left with ErrRateLimited or ErrUnavailable. Google Books keeps flattening transport errors so its key never reaches a log (#1144); OpenLibrary keeps its error chain. Each client also gets a Gate. A refusal used to back off only the request that was refused, so the other catalogue fetches and edition samples a bulk import had in flight kept hitting a provider that had asked for quiet, which is how #2075's 429s escalated into timeouts and refused connections. Now a refusal holds every request through that client, and a request whose hold would outlast its deadline fails at once as rate limited instead of sleeping into a timeout. Hardcover keeps its own adaptive pacer. Signed-off-by: vavallee <vavallee@protonmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 * fix(hardcover): treat a GraphQL rate limit in a 200 as a refusal (#2791) query called pacer().succeed() before looking at the body, so a 200 carrying a GraphQL errors array counted as a healthy request. A rate limit delivered that way never penalised the throttle and advanced its decay counter, unwinding pacing earlier refusals had set up. The envelope is now classified first. An errors array whose message or extensions code reads as a rate limit or quota refusal penalises the pacer with Hardcover's own hint and is retried like a 429, marked ErrRateLimited. Any other GraphQL error is returned without touching the pacer. Only a clean answer counts as a success. Signed-off-by: vavallee <vavallee@protonmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 * fix(metadata): report holds by cause, and do not retry a Google Books daily quota (#2369) Review follow ups on the shared provider request loop: - A hold now remembers what set it. A request turned away by a hold a 502, 503 or 504 set reports ErrUnavailable; only a 429 hold reports ErrRateLimited. - Request.Final lets a provider stop the loop on an answer waiting will not fix. Google Books uses it for a 403 or 429 whose reason is dailyLimitExceeded or quotaExceeded, returned at once and marked as a rate limit; a per minute rateLimitExceeded is still retried. - A transport failure whose backoff would outlast the caller's deadline returns the error at once instead of sleeping into the timeout. - An unparseably large Retry-After is clamped to the cap, not ignored. - SetBaseDelay lets provider test suites shorten the backoff; nb, dnb, googlebooks, audible and audnex use it (nb went from 26s to 0.1s). - The DNB num= fallback test pins the retry count and the fallback query. Signed-off-by: vavallee <vavallee@protonmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 * fix(migrate): wait out a primary's hold instead of tripping the import breaker (#2075) The shared provider loop fails a held request at once rather than sleeping into the lookup's deadline. The import outage breaker only exempted hardcover.ErrRateLimited, so after a single OpenLibrary 429 every following row was turned away within a millisecond, three of them tripped the breaker, and CSV, Readarr and Goodreads imports failed every remaining row. Before the shared loop that took about 24s of refusals. The breaker now leaves any rate limit out of its streak, whichever provider refused, and the importers wait out a primary's hold before asking again (the hold's own end time, or 2s for a refusal that names none, at most 3 times per lookup) instead of failing the row. A hold an outage set is still waited out once its end is known, then counts toward the breaker as before. Also gives the Add Book ISBN and ASIN lookups one 20 second deadline: they ask the providers in turn and each now retries, so two dead providers could outlast the server's 120s write timeout. Signed-off-by: vavallee <vavallee@protonmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 * fix(migrate): stop an import whose primary keeps refusing (#2075) Waiting out a primary's hold made a refusal that never ends unbounded: each row waited 3 times, then failed as rate limited, which the outage breaker ignores, so a 1000 row import against a refusing provider ran for hours and sent thousands of requests. The Readarr and Goodreads imports run detached and could not be stopped. A lookup still refused after its waits now counts toward the outage streak, so three such lookups in a row trip the breaker and the rest of the rows fail at once with "primary metadata provider X is rate limiting requests, try the import again later". A run also has a 10 minute budget for waiting out holds; once spent, a refused lookup is not waited out and counts straight away. isRateLimited moves into outage.go, which keeps the errors and hardcover imports in use, so the file still builds when #2699, which adds an errors.As case to the same switch, lands alongside this. Signed-off-by: vavallee <vavallee@protonmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 --------- Signed-off-by: vavallee <vavallee@protonmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

github-actions Bot added the bindery-notified Discord notification already sent for this PR label Sep 19, 2026

github-advanced-security Bot found potential problems Sep 19, 2026

View reviewed changes

Comment thread internal/metadata/hardcover/daily_quota.go Fixed

magrhino mentioned this pull request Sep 19, 2026

feat(metadata): protect hardcover daily quotas (#2593) #2607

Closed

13 of 17 tasks

magrhino marked this pull request as ready for review September 19, 2026 01:23

This was referenced Sep 26, 2026

Reduce Hardcover API usage and support daily quotas #2592

Closed

Daily quota protection #2593

Open

This was referenced Sep 27, 2026

perf(metadata): cache provider searches and deduplicate requests #2615

Merged

GetBook, GetAuthor and GetEditions hand every caller the cached object, and two callers mutate it #2809

Closed

magrhino force-pushed the codex/hardcover-daily-hold branch from 55365e7 to 348f87c Compare September 28, 2026 22:51

vavallee requested changes Oct 2, 2026

View reviewed changes

magrhino force-pushed the codex/hardcover-daily-hold branch from 43936cb to ae9e110 Compare October 4, 2026 01:27

magrhino added 10 commits October 7, 2026 18:14

fix(hardcover): pause bulk work until daily quota resets

2905d0a

Signed-off-by: magrhino <12738722+magrhino@users.noreply.github.com>

fix(hardcover): bound quota holds and cover pause paths

537a6e0

Signed-off-by: magrhino <12738722+magrhino@users.noreply.github.com>

docs(metadata): describe primary-only daily holds and corrupt-state r…

beffd2a

…ecovery Signed-off-by: magrhino <12738722+magrhino@users.noreply.github.com>

magrhino force-pushed the codex/hardcover-daily-hold branch from ae9e110 to beffd2a Compare October 7, 2026 23:19

This was referenced Oct 8, 2026

fix(metadata): shared retry loop and provider wide refusal hold, Hardcover GraphQL rate limits (#2369, #2075, #2791) #3100

Merged

A rejected Hardcover token is retried on every call during a bulk import #3101

Open

Navigation Menu

Repository navigation

fix(hardcover): pause bulk work until daily quota resets - #2699

fix(hardcover): pause bulk work until daily quota resets#2699
magrhino wants to merge 10 commits into
vavallee:mainfrom
magrhino:codex/hardcover-daily-hold

magrhino commented Sep 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

codecov Bot commented Sep 19, 2026 •

edited

Loading

Uh oh!

vavallee commented Sep 27, 2026

Uh oh!

magrhino commented Sep 28, 2026

Uh oh!

vavallee left a comment

Uh oh!

magrhino commented Oct 4, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

magrhino commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Test plan

Uh oh!

Uh oh!

codecov Bot commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vavallee commented Sep 27, 2026

Uh oh!

magrhino commented Sep 28, 2026

Uh oh!

vavallee left a comment

Choose a reason for hiding this comment

Uh oh!

magrhino commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

magrhino commented Sep 19, 2026 •

edited

Loading

codecov Bot commented Sep 19, 2026 •

edited

Loading

magrhino commented Oct 4, 2026 •

edited

Loading