Lenny borrow: merge provider loans into the patron's loans page (#13687) - #13691
Draft
mekarpeles wants to merge 12 commits into
Draft
mekarpeles wants to merge 12 commits into
mekarpeles wants to merge 12 commits into
Conversation
Lenny is a self-hosted lending server, and many organisations run their own node. Each node is its own OAuth authorization server; Open Library is a client of each. #13565 harvests a node's OPDS feed into the acquisitions table, so the borrow links already exist as rows -- 94 of them in production as of 2026-09-09. This is the flow behind them. /borrow/lenny/OL51008637M -> node's /authorize -> /borrow/lenny/callback | POST {node}/v1/api/oauth2/borrow It stops one step short of reading the book, on purpose. A Lenny access token authorizes creating a loan; it does not get the patron through Lenny's read gate, which takes a session cookie only and keys the patron on sha256(lowercased email). So a loan created here is invisible to a patron who later signs in to Lenny directly, and the reader still asks for an OTP. Closing that needs a single-use token-to-session exchange on the Lenny side (ArchiveLabs/lenny#211) and a decision about how the two identities reconcile. A "read now" link today would strand the patron *after* they had borrowed, which is worse than not offering one. Three choices that each look like an omission: No tokens are stored, anywhere. A borrow is one-shot, so this requests no refresh token and uses the access token inside the request that obtained it. Lenny rotates refresh tokens and revokes the whole family on reuse, so two concurrent refreshes destroy a patron's grant and log them out with no traceable error. Storing nothing makes the worst case of a bug in here one failed borrow. Anything wanting loans:read on a bookshelf page needs persistence, and that hazard has to be designed for deliberately -- it is a real cost of that feature, not a detail to add casually. No patron identity is sent. The node authenticates the patron itself. Silent authorization -- OL asserting an already-authenticated patron so the node can skip its own login -- would need a pairwise pseudonymous subject (never an email), an OL signing key, and a decision about whether Open Library should act as an identity provider. The plug-in point is marked in the authorize parameters. prompt=none is unsupported by every node today and unknown parameters are ignored, so nothing is sent. Endpoints come from each node's /.well-known/oauth-authorization-server, so a node moving an endpoint does not need an OL deploy. S256 is required by membership rather than by taking the first offered method, so a node advertising something weaker fails closed. The security of this flow is entirely in what it refuses, so the tests are weighted there and each refusal was verified to be load-bearing by breaking it: not enforcing an iss mismatch fails 2, tolerating a missing iss fails 1, leaving state replayable fails 1, dropping the S256 requirement fails 2, and using the verifier as its own challenge fails 1. The iss check earns its own note. One callback path serves every node, so the URL cannot say which node is answering: the node comes from state and iss must match what that node is registered as. Without it a hostile node a patron also uses can replay a code and have Open Library redeem it elsewhere -- the OAuth mix-up attack. Credentials come from a `lenny_nodes` config key, not from feed_registry.data: that blob is treated as printable config throughout the harvest tooling (it is echoed by `bookworm.cli register --show` and named in its drift warning), so a client secret there would reach stdout, the cron log, and cron mail.
Per Mek's decision: use the node's existing OAuth2 PKCE flow as it is, let the patron sign in at the node with its own OTP, and have Open Library keep the resulting token custodially. That sign-in is not an obstacle to route around -- it is what makes the book readable. An access token authorizes OL's *backend* to call a node's API; it gives the patron's browser no session, so no cookie and no reader. Signing in at the node does, which is why the callback can now offer a working "Read it now" link. The token is Fernet-encrypted into the patron's own cookie with the same key and the same pattern as the S3 keys (encrypt_lenny_token / decrypt_lenny_token alongside encrypt_s3_keys), so there is no table and no server-side row to expire. A patron clearing cookies simply signs in again. One hazard comes with that and is inherent rather than a bug: the cookie is the single copy of a rotating refresh token, and a node revokes the whole family when a rotated one is reused. So a failure clears the cookie and sends the patron back through the flow -- a second sign-in rather than a silent dead end. The silent-authorization plug-in point is gone. It would have saved the patron a sign-in but would not have made the book open, so it was solving the wrong half. 30 tests. A node that issues no refresh token is covered, since it is not obliged to.
…gration
Open Library needs a patron's OAuth grant at each trusted book provider so it
can borrow on their behalf and read their loans back. This adds the storage
layer and the refresh handling; it deliberately does not add the table.
`openlibrary/core/provider_tokens.py` keys one grant on (username,
provider_name) and stores the access token, its expiry, the refresh token and
the granted scope. Tokens are Fernet-encrypted with the same secret and the
same pattern as the S3 keys (`encrypt_token`/`decrypt_token`, added next to
`encrypt_s3_keys` at accounts/model.py:109-127). They are encrypted, not
hashed: Open Library presents them to the node, so a digest would be useless
here. Hashing is right on the node, which verifies, and wrong here, which
sends.
`ProviderToken.get_fresh` is the part that matters. A provider that rotates
refresh tokens revokes the entire token family when a spent one is presented
again, so a concurrent double-refresh logs the patron out with nothing in any
log to explain it. Three things guard against that, all in one method:
1. `SELECT ... FOR UPDATE` on the patron's row, held for the whole exchange,
so a second tab blocks rather than presenting the same refresh token
(`openlibrary/data/db.py:143` is the precedent for the clause);
2. the new pair written in the same transaction that consumed the old one;
3. a failed refresh deletes the grant rather than retrying it -- a timeout
and a rejection are indistinguishable from this side, and a retry is what
turns a lost response into a destroyed grant.
`openlibrary/tests/core/test_provider_tokens.py` splits into two levels on
purpose. 25 tests run anywhere, on SQLite, and cover the protocol. Two more
prove the lock itself under real concurrency and need a real Postgres, because
SQLite has no row locks -- substituting a threading lock there would test the
substitute. They skip unless OL_TEST_POSTGRES is set; the class docstring
gives the two commands.
Every assertion here was watched fail. Eight defects were injected one at a
time and each turned at least one test red, the concurrency test included:
with `FOR UPDATE` removed it reports `['rt-0', 'rt-0']` against an expected
`['rt-0']` -- the same refresh token presented twice, which is the patron
being logged out.
Verified against postgres:18.3 and against SQLite; 27 passed with Postgres
present, 25 passed and 2 skipped without. The 4 failures in
openlibrary/tests/core/test_fulltext.py predate this branch and were confirmed
on a clean tree.
No `provider_tokens` table is created here. Whether this is a table at all --
versus a cookie, versus archive.org acting as a vault -- is still open and is
Mek's call, so the DDL lives only in the test file for now. Nothing else in
this commit depends on the answer.
Refs #13685
Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
…ge' into 13600-lenny-oauth-token-store
Replaces the `lenny` cookie this branch added with the per-provider token store from #13689 (`ProviderToken.upsert` after the exchange, `ProviderToken.get_fresh` before presenting a token), and fills in the three things the flow was missing to be usable by the loan lookup that comes next. **Why the cookie had to go, and it is not a preference.** Lenny rotates refresh tokens and revokes the entire family when a spent one is presented again, so the storage has to single-flight the refresh. Two tabs send the same cookie, so both hold the same `R0`; a lock can make the loser wait, but when it wakes the only refresh token it has is the `R0` from its own request headers, because `R1` exists solely in the winner's HTTP response to the other tab. There is nowhere server-side to read it back from, so the loser either presents `R0` -- the reuse that destroys the grant -- or abandons a grant that is alive. `get_fresh` re-reads under the lock, which is the step a cookie cannot perform. The cookie also held one node at a time (`get_custodial_token` returned None for any other node), which is the "does not scale to n providers" objection. `encrypt_lenny_token` / `decrypt_lenny_token` go with it; #13689's `encrypt_token` / `decrypt_token` are the same Fernet pattern without the cookie-shaped three-field plaintext. Also here: - **`scope=loans:read borrow`**, not `borrow` alone, so the merged loan lookup (#13687) does not need a second consent. Both are in the node's `scopes_supported`, checked against `lennyforlibraries.org` discovery. - **`login_hint`** with the patron's email at `/authorize`. See the comment on `authorize_url`: it is inert against every node today and the comment says exactly where it is dropped, so nobody reads the parameter as working. - **The grant is stored before the loan is created**, not after. A loan made with a grant Open Library failed to keep is a live credential at a third-party library that Open Library holds no record of and cannot refresh or revoke. - **`node_refresher` carries `REFRESH_TIMEOUT_SECONDS` on both of its calls**, discovery included, because `get_fresh` calls it with `SELECT ... FOR UPDATE` held on the patron's row. - **`access_token_for`** is the seam #13687 hooks. - The pending state keys on the bare username, not `user.key`, which is what the `provider_tokens` row and `anonymize` both use. The module docstring's claim that the flow "deliberately stops one step short of reading the book" was wrong and is corrected in place. The node's `/authorize` refuses to issue a code without a node session (`lenny/routes/oauth2.py:241`), so the patron's browser already holds the session the reader wants by the time Open Library holds a token. The `read_url` / `render_borrowed` code two hundred lines below the claim already assumed this. 32 new tests, 62 in the file. Every assertion was watched fail: 21 defects were injected one at a time and each turned at least one test red, including `iss` checked after the code is spent (2 red), the grant stored only after the loan succeeds (2), `login_hint` sent for an empty email (1), discovery inside a refresh left unbounded by the refresh timeout (1), and a credential written back to a cookie (1). Two first came back green. One was a defect in the test -- `parse_qsl` drops blank values, so an empty `login_hint` read as absent -- and one was a defect in the mutation, which moved the state into the challenge rather than the verifier and so was not the flaw it claimed to inject. `openlibrary/plugins/upstream/tests/` + `openlibrary/tests/core/` + `openlibrary/tests/accounts/`: 765 passed, 2 skipped. The 2 failures in `test_addbook.py` predate this branch and reproduce on a clean `origin/master`. pre-commit clean on the changed files, mypy and Generate POT included. No `provider_tokens` table exists yet -- the DDL is held back in #13689 pending Mek's ruling -- so this branch is stacked on that one and the callback fails closed until the migration lands. Refs #13685, #13687. Part of #12844. Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
The comment on `authorize_url` said the parameter was inert and that making it work was "three changes on the Lenny side", which read as though nobody had written them. They are written; they are on a branch that has not been pushed. The three places a node running `origin/main` drops it are correct and stay, as evidence for why such a node ignores it rather than as a claim about what exists. Also records the ceiling, because someone will ask for the other thing: even once it ships, `login_hint` pre-fills the email box and does not let the patron skip the email step. `/oauth2/authorize` is an unauthenticated GET, so mailing a one-time code on arrival would let a link or a prefetch send one to any address a caller chose. The privacy cost stays flagged as a cost and is unchanged: the hint discloses the address before consent, including on abandoned flows, and the delta over the completing case is small and real. That is a human's call, not this comment's. Comment only. 6,067 passed, 9 skipped, 1 xfailed; pre-commit clean. Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
A patron's loans page now shows the books they hold at a configured Lenny node alongside their Internet Archive loans (#13687). The merge happens in `account_loans.GET`, not inside `lending.get_loans_of_user`, and that placement is the change rather than an implementation detail. `get_loans_of_user` is not a display lookup: three of its five callers do something Internet Archive-specific with every element they get back. - `User.update_loan_status()` feeds each loan to `lending.sync_loan(loan["ocaid"])`. `account_loans.GET` calls it on the line before the lookup, so a loan with no ocaid there is a KeyError on the page the patron asked for. - `borrow.py` mints an Internet Archive bookreader link from `loan["_key"]` for whichever loan matches the edition. 21 of the 25 borrowable titles in the live feed sit on an edition that also has an `ocaid`, so a Lenny loan reaching that loop would produce a reader link for a loan the Internet Archive has never heard of. - `templates/account/loans.html` keys on `resource_type == 'bookreader'` and falls through to an `else` that reads `loan['loan_link']`. That template `else` is the #13690 shape again, and it is why the actions cell moved into `account/loan_actions.html`: the dispatch is the part that can be wrong, and inline it could only be reached by rendering the whole loans page -- a mock site, covers and a waiting list stand in front of it -- so nothing reached it. Rendered on its own, with the provider branch removed, a provider loan raises `KeyError('loan_link')` when it has no due date and silently advises returning the book through Adobe Digital Editions when it has one. Degradation, which is the requirement this feature turns on: - Nodes are queried concurrently, so four providers cost one timeout and not four. Measured rather than asserted from the shape of the code: against a sequential loop the wall-time test reports 1.61s against a 0.8s bound, while the nine other merge tests pass unchanged. - `provider_loans()` does not raise. A node that is slow, down, or answering nonsense costs its own entry in `unreachable`. - "Could not reach" and "needs reconnecting" are kept apart, for the reason `BORROW_ERRORS` already gives: they ask the patron to do different things. Two things the issue asked for that turned out not to exist as described: - The loans endpoint cannot come from discovery. RFC 8414 registers no field for a resource endpoint and `lennyforlibraries.org` advertises authorization, token and revocation and nothing else. Discovery still decides the origin; the path is built from the issuer, as `borrow()` already does. - The token phase cannot be parallelised alongside the HTTP phase. `access_token_for` is synchronous and takes a row lock, and `web.db.DB` keeps its connection in a `threadeddict` with `has_pooling` false (no `dbutils` in `oldev:latest`), so every worker thread touching `get_db()` opens a Postgres connection that is never released. It stays sequential in the request thread under a deadline. Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
mekarpeles
force-pushed
the
13687/provider-loans-merge
branch
from
September 21, 2026 01:28
2fdb8f7 to
2190a93
Compare
The values in a loans payload are a record of which books a named patron borrowed from a library. The failure they are logged for is a shape mismatch, which the key names answer completely. Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
This was referenced Sep 21, 2026
…timate 'One indexed read' understated it: the read is taken under FOR UPDATE. The variant that avoids the lock was built and measured on #13689 and is slower under contention, because the row an unlocked pre-check reads is expired precisely when a refresh is already in flight -- so it queues on the same lock anyway, having paid for an extra query. Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
This was referenced Sep 21, 2026
Two confirmed defects from independent review of the integration branch, both
this PR's.
**1. A timezone-aware due_at raised, taking the patron's Internet Archive
loans down with it.** The template fed the node's raw due_at to
datetime_from_isoformat -> parse_datetime (openlibrary/api.py:291), which is
re.split + int() on every token. An offset is not a token it can parse:
'2026-10-01T00:00:00+00:00' -> ValueError: invalid literal for int()
'2026-10-01T00:00:00Z' -> ValueError: invalid literal for int()
'2026-10-01T00:00:00-07:00' -> TypeError: tzinfo argument must be None
And that is the shape the node sends, verified against ArchiveLabs/lenny main
through the GitHub API rather than a local checkout: due_date and created_at
are both DateTime(timezone=True) (core/models.py:317,319) and the serialiser
emits a bare .isoformat() (routes/oauth2.py:543-544), which on Postgres carries
the offset.
The third case is why this is normalised in lenny.py rather than guarded at
the template: a negative offset splits into eight tokens and lands in tzinfo,
so it raises TypeError, not ValueError. A downstream guard catching the
obvious exception would still be taken down by a patron borrowing west of UTC.
borrowed_at was guarded from the start by _epoch; due_at was the one field
that bypassed this module and reached the template raw. Both now go through
one _parse_iso, and due_at is returned as naive UTC -- the shape Internet
Archive expiries already have here.
**2. A node 401 was shown as "could not be reached", permanently.** Every
exception was bucketed into unreachable. A grant that is locally unexpired but
rejected at the node stays locally unexpired forever, so the patron saw a
temporary outage that never ended and was never offered the one action that
fixes it. access_token_for cannot see this -- it only knows what the store
knows -- so this is the only place the distinction exists. 401 and 403 now
read as unauthorized; a node's 500 and a timeout still read as outages, which
is what keeps the new branch honest.
Also: return early from _patron_tokens when no node is configured. Every load
of /account/loans reached the token store, Lenny patron or not, and on a
deploy without #13689's migration that is a query against a table that does
not exist.
And two follow-ups from the same review, on judgement rather than instruction:
the notes now name the affected library rather than counting it -- these
arrive as provider names precisely so they can be named, and rendering only
len() made the lists pointless -- and the unauthorized note says how to
reconnect.
The fixtures were part of the defect. Both sides were handed naive
'2026-10-01T00:00:00', which is exactly the value that parses, so the unit
test and the template test each passed while nothing connected them.
TestTheExpiryTheNodeActuallySends now renders what loan_from_node returns from
what the node actually sends. Against the unfixed code 10 tests go red,
including that end-to-end pair.
Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
'It does not raise' was true while the loans page was broken: provider_loans never raised on a timezone-aware due_at, it passed the node's string through to a renderer that could not parse it, and the patron lost every loan they had. Not raising is not the property worth promising -- every value handed out of here being one the loans page can render is. Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #13687. Part of the Lenny ↔ Open Library borrow flow (epic ArchiveLabs/lenny#219). Domain notes:
ol-kb/wiki/lenny-oauth.md.Draft, and stacked. Branched from #13600 (
12844/lenny-borrow-oauth), which is itself stacked on #13689 (13685/provider-oauth-token-storage).Review the top two commits only — the feature, and one follow-up on what its failure logs record. Everything below them belongs to #13600 and #13689 and arrives here only because neither has merged. Nothing here merges before they do.
Base is
masterrather than #13600's branch, matching what #13600 does with #13689. That is not cosmetic:python_tests.ymltriggers onpull_request: branches: [master], so a PR based on the parent branch gets pre-commit and no test job at all. Retargeted after seeing exactly that on the first push.What
/account/loansnow shows the books a patron holds at a configured Lenny node alongside their Internet Archive loans. The node's loans come fromGET /v1/api/oauth2/loanswith the patron's access token, one call per node the patron holds a grant at, all of them at once.Tokens come from
lenny.access_token_for(username, provider_name)(#13600) and nothing here reaches past it — no second refresh path, no second storage accessor. Enumeration usesProviderToken.get_providers(username), which #13689 already provides.The placement is the change
The issue asked to extend the existing loan lookup. The merge is in
account_loans.GETinstead, becauselending.get_loans_of_useris not a display lookup — three of its callers do something Internet Archive-specific with every element:User.update_loan_status()lending.sync_loan(loan["ocaid"])— andaccount_loans.GETcalls it on the line before the lookup, so a loan without an ocaid is aKeyErroron the page the patron asked forborrow.py:292loan["_key"]for whichever loan matches the editiontemplates/account/loans.htmlresource_type == 'bookreader', else readsloan['loan_link']The middle one is the quiet one. Per the wiki's survey of the live feed, 21 of the 25 borrowable titles sit on an edition that also has an
ocaid— so a Lenny loan reaching that loop produces an Internet Archive reader link for a loan the Internet Archive has never heard of.It is also memcache-memoized for five minutes and called on hot borrow paths, so putting one to four HTTP calls inside it would be wrong even without the shape problem.
The template, which is #13690 again
The actions cell moved out of
account/loans.htmlintoaccount/loan_actions.html. Not tidying: the dispatch is the part that can be wrong, and inline it could only be reached by rendering the whole loans page — a mock site, covers and a waiting list stand in front of it — so in practice nothing reached it. #13690 is what that costs.Rendered on its own with the provider branch removed, a provider loan:
KeyError('loan_link')when it has no due date, andBoth are pinned by
test_loan_actions.py, and the IA and ACS branches are pinned unchanged alongside them.Degrading, which is the requirement this turns on
provider_loans()does not raise. A node that is slow, down, or returning a list of strings costs its own entry inunreachable. Proven by mutation: droppingreturn_exceptions=Trueturns two tests red.BORROW_ERRORSalready gives in this module: they ask the patron to do different things. Each names the affected library.Two defects found by independent review, and what my own tests got wrong
Both were real, both are fixed in
11d441e4, and in both cases my tests passed while the code was broken — worth stating plainly, because the reason is the same twice.1. A timezone-aware
due_attook the whole loans page down, the patron's Internet Archive loans included. The template fed the node's rawdue_attodatetime_from_isoformat→parse_datetime(openlibrary/api.py:291), which isre.split+int()per token:And that is the shape the node sends —
due_dateandcreated_atare bothDateTime(timezone=True)(lenny/core/models.py:317,319) serialised with a bare.isoformat()(routes/oauth2.py:543-544), which on Postgres carries the offset. Verified againstArchiveLabs/lennymainthrough the GitHub API, not a local checkout.The third case is why this is normalised in
lenny.pyrather than guarded at the template: a negative offset splits into eight tokens and lands intzinfo, so it raisesTypeError, notValueError. A downstream guard catching the obvious exception would still be taken down by a patron borrowing from a node west of UTC.The asymmetry that caused it is in my own code:
_epochguardedborrowed_atfrom the start, anddue_atwas the one field that bypassed this module entirely. Both now go through one_parse_iso.2. A node 401 was shown as "could not be reached", permanently. Every exception was bucketed into
unreachable. A grant that is locally unexpired but rejected at the node stays locally unexpired forever, so the patron saw a temporary outage that never ended and was never offered the action that fixes it.access_token_forcannot see this — it only knows what the store knows — so this is the only place the distinction exists. 401/403 now read asunauthorized; a node's 500 and a timeout still read as outages, and those counterpart tests are what keep the new branch honest.Why my tests agreed with me.
test_lenny.pyprovedloan_from_nodenormalised the offset;test_loan_actions.pyproved the template rendered an already-normalised expiry. Both sides were handed naive"2026-10-01T00:00:00"— precisely the value that parses — so each passed while nothing connected them. Same shape as thetest_a_naive_timestamp_is_read_as_utcproblem I caught earlier in this PR: a fixture chosen to be convenient is a fixture that agrees with you.TestTheExpiryTheNodeActuallySendsnow renders whatloan_from_nodeactually returns from what the node actually sends, andtest_every_normalised_due_at_survives_ols_own_expiry_parsercalls the real OL parser rather than asserting a string, so the test cannot drift from the thing it protects. 10 tests go red against the unfixed code.Also fixed:
_patron_tokensnow returns early when no node is configured. Every load of/account/loansreached the token store, Lenny patron or not — and without #13689's migration that is a query against a table that does not exist.Two things in the issue that turned out not to be true
The loans endpoint cannot come from discovery. RFC 8414 registers no field for a resource endpoint, and the live document has no loans entry:
Discovery still decides the origin, which is the part a node can move. The path is built from the issuer, exactly as
borrow()already does. (scopes_supporteddoes confirm #13600'sSCOPESclaim.)The token phase cannot join the concurrent phase.
access_token_foris synchronous and takesSELECT ... FOR UPDATE. Moving it ontoasyncio.to_threadwould leak a Postgres connection per worker thread —web.db.DBkeeps its connection in athreadeddict,dbutilsis absent sohas_poolingis false, and_unload_contextonly runs when pooling is on:So it stays sequential in the request thread, under
LOANS_DEADLINE_SECONDS, which stops it after the grants it has resolved rather than letting N expired grants at N hanging nodes add up. In the ordinary case it does no network at all.Verified vs. assumed
Verified: every test below run in
oldev:latest; every failure mode above watched failing against mutated source; mypy run withvendor/infogamiinitialised and with a deliberate type error to confirm it was analysing rather than passing silently; discovery fetched live.Not verified: no end-to-end run against a node — the authorize leg needs an OTP to a human inbox, which nobody can do yet (wiki, "Nobody can test this end to end"). The node's loans response shape is taken from the documented contract, not observed.
Deliberately not in scope
mybooks.py) also renders loans and is untouched. It needs onlyloan["book"]andloan["loaned_at"], both of which provider loans carry, but its buttons are IA-shaped — worth its own change./account/loans.jsonis unchanged. Adding provider loans there changes an API contract and should be a decision, not a side effect.The
get_freshlock: raised here, measured on #13689, and it staysWorth recording because the conclusion is the opposite of the one this PR originally reached.
get_freshtakesSELECT ... FOR UPDATEon every call, not only when it refreshes. That was cheap whileaccess_token_forwas called once per borrow; this PR is the first caller to invoke it at page-render frequency, once per provider. So I flagged it — concurrent loads of one patron's loans page appearing to serialise on a lock built for the rare path — and did not touch it, since the seam is not mine to reach past.Measured on #13689 against real Postgres rather than argued, and the read-then-lock variant loses:
It recovers 0.071 ms where things are already fast and is worse where they are slow. The reason generalises: a caller arriving mid-refresh reads the pre-refresh row without the lock, and that row is expired precisely because that is why the first caller is refreshing — so it concludes a refresh is needed and queues on the same lock anyway, having paid for an extra query first.
At this PR's frequency it is a non-issue: 8 threads on one row sustained ~3,400 calls/s at p95 2.6 ms, two concurrent loads of one patron's page cost ~0.2 ms more than one, and total lock overhead is 0.023 ms per call over a plain
SELECT. No change here or in #13689 — it is now a written decision with numbers in theget_freshdocstring.Test plan:
test_addbook.pyhas two failures on this branch; they reproduce in isolation on the base branch and are unrelated.https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas