Repository navigation
Lenny borrow: a popup, so the patron never leaves the book page (#13688) - #13693
Closed
mekarpeles wants to merge 12 commits into
Closed
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
…13686) One entry in `PROVIDER_ORDER`, which every surface already reads -- book page, search results, carousels -- plus the two things that entry turns out to need. The access kind is read from the harvested row, not synthesized from the identifier. Every other provider here builds its URL out of `identifiers.*` and calls the result open-access, which for Lenny is wrong half the time: 25 of the 50 publications in the live feed are `open-access` and 25 are `borrow`, and `identifiers.lenny` is identical for both. The endpoints do not forgive a guess -- `/v1/api/items/46539165/read` on a borrowable title answers 401 with an OPDS Authentication Document for `Accept: text/html` as well as `*/*`, so a "Read" link there is a dead end in a browser. The access lives only in the Trusted Book Provider `acquisitions` table, which is the store the harvester writes; `Edition.providers` is a different store with the same class name and nothing in the import pipeline assigns it. `book_providers/read_button.html` had branches for `open-access` and `sample` only, so a `borrow` acquisition fell off the end of the if/elif chain and rendered the empty string. Reverting just that file turns `test_a_borrow_acquisition_renders_a_button` red with `assert 'Borrow' in '\n\n\n\n\n\n\n'` -- a provider registered and the patron shown nothing. `handle_borrow_async` forwarded only `open-access` acquisitions to their provider. A `borrow` one fell through to the Internet Archive branch, where `edition.ocaid` on a non-IA edition is infogami's `Nothing` and `__radd__` returns the left operand, so the patron reached a bare `archive.org/stream/`. Lenny is placed below `InternetArchiveProvider` deliberately, and that is the reviewable decision. Of the 50 publications in the feed, checked against production on 2026-09-20: all 25 open-access ones are Standard Ebooks editions that already render a Read button; 21 of the 25 borrowable ones sit on an edition with an `ocaid`; 1 has a hand-added `providers` URL; 3 have nothing else offering them. So this placement changes no button that exists today and newly surfaces Lenny on 3. Above IA, 21 would swap from an IA button to a Lenny one. `get_access` is overridden because the base class answers PUBLIC unconditionally and that value becomes Solr's `ebook_access`, which drives `public_scan_b` and `has_fulltext`. A borrowable title would have been indexed as publicly readable. A per-request memo on `web.ctx` was written and removed. Only web.py clears `web.ctx` per request, so the entry outlived its request under the test runner and editions with no row began answering with another edition's -- eleven tests in `TestNothingToOffer` and `TestEbookAccess` went red on a full-suite run and green in isolation. Batching belongs at the call site, where `WorkSearchScheme.add_non_solr_fields` already holds every row for the page. Verified against live services rather than the wiki, which was dated: 94 works on production carry `id_lenny`; `/books/OL37044817M.json` carries `identifiers.lenny` and no `providers`; `/v1/api/items/46539165/borrow` answers 303 to the node's own "Sign In - Lenny" page for a browser Accept header, so the borrow CTA completes with nothing built on this side. The OAuth handshake that keeps the patron on Open Library is #13688. Ref #13270, #13688 Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
`test_the_borrow_button_does_not_link_straight_out_to_the_node` was written without it and stayed green against the unfixed template, because the empty string that branch rendered also contains no node URL. Someone reading that test in a year is in an editor, not in the PR thread, so the reasoning goes on the test. Also name DirectProvider on the template branch: `access` in the edit-book form's `providers` field may be `borrow`, so this branch is not Lenny-only and was not a Lenny-only gap. 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
…13688/lenny-borrow-popup
Clicking Borrow on a Lenny title opened another library's website in a new tab. The loan worked; it did not feel like Open Library. This puts the same navigation in a popup, so the page the patron started on is still there when the loan is made, and refreshes it. The popup's first page is the interstitial, carrying the one sentence that explains the relationship -- "borrowable for free from <library>, a trusted Open Library book provider" -- then it navigates to the node's authorize endpoint, the patron signs in and consents, and Open Library's callback exchanges the code and creates the loan. The callback's page then closes the popup and refreshes the opener. The interstitial is inside the popup rather than before it, which is a deviation from the issue's ordering and not a preference: `window.open` needs user activation, so a popup opened from a five-second countdown is blocked. Opening it on the click is the only version that works, and it costs the patron nothing -- they see the same two screens in the same order. A popup, not an iframe, and the reason is at the `window.open` call site because that is where someone will be standing when they propose simplifying it. The node's consent screen sets `X-Frame-Options: DENY` and `frame-ancestors 'none'` and has a test pinning them, so a frame renders nothing in any browser; and its session cookie is `SameSite=Lax`, so no browser would send it back from a cross-site frame. That cookie is the only thing the node's reader accepts. The issue gives this as a Safari/ITP tradeoff; it is not a tradeoff. Three things found on the way, each with its reason in the code: - The popup's endings are `delegate.RawText`, which looks like a styling choice. It is the only way the confirmation reaches the page behind the popup: `get_flash_messages()` drains `web.ctx.flash` as a side effect of reading it, the site layout calls it on every render, and `flash_processor` then writes no cookie because the drained value matches the empty one that arrived. - A signed-out patron was sent to `/account/login?redirect=<the book>`, which loses the click -- they arrive signed in, looking at the same button. Now they come back to the borrow they asked for. - The library is named from the node's own config rather than from the feed's provider name, because a node is a library and "Lenny" is the software. An operator who sets no `name` still gets a whole sentence. What this does not do: the refreshed book page still says "Borrow", not "Read". That button renders from the harvested acquisitions row, which a loan does not touch, and `get_lending_state` has no "patron holds a provider loan" state at all. Closing it needs per-patron loan state on a hot render path and must not go through `get_acquisitions`, which also feeds Solr. Until then the popup's success page keeps the node's read link for the paths with no opener, and the loan is confirmed by a flash on the refreshed page. Every test here was watched failing first: against a plain string instead of RawText, against the flash removed, against the interstitial's borrow wording dropped, against the mediated branch reverted, against the provider attribute deleted, against the message type renamed on one side only, and against the popup's origin check and its preventDefault ordering. Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
…13694 Three comment changes, no behaviour change. The popup's reliance on `Cross-Origin-Opener-Policy` being absent was written up as a finding -- true, checkable, and framed as a curiosity. It is a dependency, and one on a server Open Library does not own. The failure it produces is silent and will be attributed elsewhere: the loan is created, the popup closes, the page behind it never refreshes, nothing errors. A node operator could cause it tomorrow by hardening their own server for reasons having nothing to do with Open Library, and nobody would connect the two. So the call site now says what to check first and what the fix would be -- a BroadcastChannel between same-origin pages survives a context-group swap. The two places that explained why the success page keeps the node's read link pointed at "the PR for #13688". A PR thread is the wrong artifact for that: someone reading this code in a year is in an editor. They now point at openlibrary#13694, which tracks the button flip, and say to delete the link when it lands -- because that is the moment `read` stops being the only route to the book. Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
mekarpeles
added a commit
that referenced
this pull request
Sep 30, 2026
Resolving the test_lenny.py conflict by concatenating both sides glued two of #13693's methods onto #13691's last class, because #13693's own `class TestCallback:` line sat above the conflict start. Inside the wrong class their `flow` fixture is out of scope, so they errored rather than failed — 2 errors, which is how a fixture-scope mistake presents. Re-homed onto TestCallback, which owns them on origin/13688. Worth recording: 'keep both sides' is the right call for an additive conflict and is not sufficient on its own. Concatenation preserves text and not structure, and Python's block structure is exactly what a conflict marker cuts through. The tell was cheap — two ERRORs naming methods attributed to a class that does not define their fixture.
1 of 14 tasks
2 tasks
7 tasks
Member
Author
|
Superseded by #13865, which composes this branch together with the rest of the Lenny OAuth work onto current |
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
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.
State: ready-for-review
Updated: 2026-10-05T03:33Z
Owner: lenny (Division Lead)
Base drift: 76 commits behind
master; newestpython_testsis from 2026-09-21. A green check here is a claim about a September base, not about today's master — rebase before reading it as evidence.Composition note — green alone, red in the union. This PR adds
from openlibrary.i18n import gettext as _tolenny.py; #13691 addsfor (provider_name, _), result in zip(...)toprovider_loansin the same module. Neither branch contains both, so both pass ruff alone and only a branch carrying both tripsF402(shadowed import). Latent rather than live — nothing calls_()after that loop today — but the rebinding survives to the end of the function. Fixed onlenny/integration-v2by renaming the loop variable to_grant. Whichever of the two merges second will need that rename.Verified by RUNNING
provider_popup_result.html.jinjawithdata-message-type="ol-provider-borrow"— matching the JS constant —data-ok="1", and apostMessageto the opener.javascript_testspasses).Unverified / READ only
window.open, a real cross-windowpostMessage, a real opener. The tests synthesize both sides; nothing has ever run in a browser.Worth knowing
npx jest <file>reportsTests: 0 totaland a failed suite onwindowbeing undefined — which reads exactly like "these tests have never executed". They execute. Checkgh pr checksbefore concluding a test never runs.Next steps
Blocked on
Closes #13688 steps 1–5 and the popup-closes-and-refreshes half of step 6. The other half of step 6 — the button reading Read — is #13694, and this PR is not held on it.
Clicking Borrow on a Lenny title opened another library's website in a new tab. The loan worked; it did not feel like Open Library. This puts the same navigation in a popup, so the page the patron started on is still there when the loan is made, and refreshes it.
The flow, as the patron experiences it
/-/borrow?action=borrowURL the button already pointed at. Its first page is the interstitial, carrying the one sentence that explains the relationship: "This book is borrowable for free from Archive Labs Lenny, a trusted Open Library book provider." No destination URL is printed — it is an openlibrary.org path, and showing it would raise the question the screen exists to answer./v1/api/oauth2/authorize. The patron enters their one-time code and consents.The interstitial is inside the popup, not before it
This is a deviation from the issue's ordering, and it is not a preference.
window.openneeds user activation, so a popup opened later — from the interstitial's five-second countdown — is blocked. Opening it on the click is the only version that works. The patron sees the same two screens in the same order; only the window boundary moved.Popup, not iframe — the reason is at the
window.opencall siteprovider_borrow_popup.jscarries the argument in full, because that is where someone will be standing when they propose simplifying it. The issue frames this as Safari/ITP dropping a third-party cookie. That is true and understates it, in a way that matters: as a Safari problem it reads like a tradeoff someone could accept. It is not one.X-Frame-Options: DENYandContent-Security-Policy: frame-ancestors 'none'(lenny/routes/oauth2.py:289-290on ArchiveLabs/lennyorigin/main=75b0906), citing RFC 6749 §10.13 and RFC 9700 §4.16, with a node-side test pinning it (tests/test_oauth2_endpoints.py:798). A frame renders nothing, in every browser. Worth knowing the trap: a livecurlof/v1/api/oauth2/authorizeshows neither header, because an invalidclient_idreturns JSON from an earlier path. Checking from outside is what makes an iframe look viable.SameSite=Lax(lenny/routes/oauth.py:232,343,371,448,655;api.py:465), so no browser sends it from a cross-site frame — and that cookie is the only thing the node's reader accepts. An iframe that somehow rendered would still produce a loan the patron cannot open.Dependency: this needs
Cross-Origin-Opener-Policyto stay absent on the nodeCalling this out as a dependency of the design, not as a finding, because it is one and it is not on a server we control.
Neither openlibrary.org nor the lending node sends
Cross-Origin-Opener-Policy(checked live 2026-09-20; nothing indocker/*.conf, nothing in the node's repo).same-originon either side would swap the browsing context group when the popup navigates to the node, sowindow.openerwould be null on the way back — andwindow.openeris the entire channel the popup uses to tell the book page a loan exists.The failure is silent and will be attributed to something else. The loan is created, the popup closes, the page behind it never refreshes, and nothing errors. A node operator could cause it tomorrow by hardening their own server for reasons having nothing to do with Open Library, and nobody would connect the two events. So: if this flow ever stops refreshing the page behind it, check for that header before anything else. If it appears, the fix is a channel that does not depend on the opener — a
BroadcastChannelbetween same-origin Open Library pages survives a context-group swap.This is stated at the
window.opencall site inprovider_borrow_popup.jsand inol-kb/wiki/lenny-oauth.md, not only here, because a PR thread is not where someone debugging it in a year will be standing.All of the above is in
ol-kb/wiki/lenny-oauth.mdwith the commands that check it.Three things found on the way
The popup's endings are
delegate.RawText, and that looks like a styling choice. It is the only way the confirmation reaches the page behind the popup.get_flash_messages()drainsweb.ctx.flashas a side effect of reading it (vendor/infogami/infogami/utils/flash.py:18-21), the site layout calls it on every render (openlibrary/core/layout.py:101-112), andflash_processorthen writes no cookie because the drained value matches the empty one that arrived (flash.py:42-50). A handler whose output the patron will not read — a popup that closes itself, a callback — must skiprender_sitefor its flash to reach anywhere. This generalises past Lenny; it is written up in the wiki.A signed-out patron was losing the click.
lenny_borrowsent them to/account/login?redirect=<the book page>, so they arrived signed in, looking at the same Borrow button. Now they come back to the borrow they asked for. It mattered already in a tab; inside the popup it also meant finishing on a book page 520px wide.The library is named from the node's own config, not from the feed's provider name. A node is a library — "Archive Labs Lenny" names an operator, "Lenny" names the software — and a second node will want its own name in the same sentence.
node_display_namereadslenny_nodes.<name>.nameand falls back to the provider name, so an operator who sets none gets "Lenny" rather than a sentence with a hole in it.What this does not do
The refreshed book page still says "Borrow", not "Read" — now tracked as #13694. Issue step 6 asks for the flip, and it is not reachable from the presentation layer:
read_button.htmlpicks Borrow or Read fromacquisition.access, which for Lenny comes from the harvested TBPacquisitionsrow (_harvested_lenny_entries, Register Lenny as a book provider so the existing button offers it (#13686) #13690). A loan writes a Loan row on the node and touches nothing in that table.lending.get_lending_state(openlibrary/core/lending.py:908) returns"partner"at step 2 for any non-IA provider and never reaches its loan checks, which areocaid-keyed anyway (lending.py:938-947). There is no "this patron holds a provider loan" state on the book page at all.templates/account/loans.htmlandloan_actions.html— the loans page, not the book page.Two constraints on whatever does close it, both carried into #13694 as constraints rather than notes:
get_acquisitions, because that same call feeds Solr'sebook_access(Register Lenny as a book provider so the existing button offers it (#13686) #13690'sget_accesssays so). Per-patron loan state reaching it would put one patron's loans into the search index — the same class of mistake as crawling availability in this domain.get_lending_stateruns on every one), so a slow node must degrade to showing Borrow rather than hanging. Borrow is wrong-but-harmless there, because the node's borrow is idempotent for a loan that already exists.Not smuggled in here: this PR is not held on #13694.
Until then: the popup's success page keeps the node's read link — the only route to the book on the paths with no opener — and the loan is confirmed by a plain-text flash on the refreshed page. The flash cannot carry a link, because the layout renders it escaped (
site.html.jinja:23).Also not done, and belongs upstairs: the callback's error copy (
render_errorcall sites andBORROW_ERRORS) is not wrapped in_(), so it shows English in every locale. Those strings were authored in #13600 and are still being reviewed there, which is where wrapping them belongs.Stacking
Base is
master, and the branch carries its parents' commits: #13600 (e2e449b01, the OAuth client) merged with #13690 (6f6903188, Lenny as a book provider). Two commits are mine:28fbea1b5(the change) and477ebb3f3(comments only — the COOP dependency above, and pointing the "still says Borrow" comments at #13694 instead of at this thread). Review with:gh pr diff 13688 --repo internetarchive/openlibrary # or: git show 28fbea1b5 477ebb3f3What I verified, and what nobody can
Verified, with the mutation each test was watched failing against:
_popup_pagereturning a plainstr— 11 tests redadd_flash_messageremoved — 3 redmediated_borrowreturning the node's URL — 2 redaccess == "borrow"guard widened — 1 red{% if borrowing %}branch dropped — 3 reddata-ol-providerdeleted — 2 redpreventDefaultmoved beforewindow.open— 1 redDirectProvider's borrow is left alone"direct"added to the allow-list — 1 redRan:
pytest openlibrary/plugins/upstream/tests/ openlibrary/tests/(2268 passed), the full JS suite (913 passed, afternode scripts/build_icon_sprite.mjs),ruff check openlibrary/,ruff format --check,mypyon the five changed Python files withvendor/infogamiinitialised,eslint, andscripts/i18n-messages extract.Two pre-existing failures in this environment,
test_addbook.py::TestMakeWork::{test_make_author_adds_the_correct_key,test_make_work_does_indeed_make_a_work}, plustest_jinja.py::test_site_layout_template_uses_jinja_templatewhen that file runs alone. All three reproduce on a cleanorigin/mastertree in the same container, so they are not from this branch.Nobody can test this end to end. The authorize leg needs a one-time code delivered to a human inbox, so nothing here has exercised a real sign-in, a real consent, or a real popup closing in a real browser. What is untested by machine: that the popup actually opens as a window rather than a tab under each browser's popup settings; that
window.openersurvives the round trip to the node in practice; and that the countdown-then-navigate inside the popup feels right rather than abrupt. Someone with an inbox on the node should walk it once before this leaves draft.https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas