feat: personal library over XRPC with atproto service auth - #205
Merged
Conversation
Makes the whole personal library reachable over /xrpc/*, authenticated with atproto inter-service auth so a script or e-reader can use it rather than only a browser session. Asked for in https://bsky.md/profile/pds.dad/post/3msrmxae2tk2g. Binary bodies: uploadPersonalBook declares an explicit ebook MIME allowlist instead of */* (which made @atcute/lex-cli emit no validator at all) and takes `filename` as a required param, replacing an x-file-name header that defaulted to "unknown". application/octet-stream is in the list deliberately — the iOS app, mobile pickers and curl all send it; detectFormat checking magic bytes against the extension is the real gate. OPDS parity: new getPersonalBookFile and getPersonalBookCover (blob outputs owning their own ETag/304), new listPersonalShelves (the root call — /opds root was 431 of 551 OPDS hits in 48h of production traffic), and q/sort/storage on getPersonalLibrary. Service auth via ServiceJwtVerifier. Methods declare auth: "identity" | "pdsWrite" and the registration wrapper derives the lxm from the schema's NSID, so a method's route and its token binding cannot drift. pdsWrite refuses service auth outright — it proves key control, not that we hold an OAuth grant. isKnownAccount gates it so an arbitrary DID can't open a quota's worth of disk. One upload implementation. processBookUpload and /library/upload had drifted in four ways; both are now thin adapters over src/utils/uploadPersonalBook.ts. That closed real memory problems: bodyLimit() buffered chunked bodies twice, and the epub/cbz parsers inflated every image in an archive to keep one cover. Worst-case upload RSS goes from unbounded to a bounded ~630 MB cluster-wide. Storage quota (2 GB), enforced as a SUM evaluated inside the INSERT so two concurrent uploads can't both slip past. Verified safe against production: the heaviest user holds 39 MB. Also fixes the KOSync progress bridge (an upload before the first sync left sync_document.hiveId null forever), persists admin backfill progress across restarts, and gives OPDS covers an ETag — production served 43 cover fetches in 48h and could never answer a single one with a 304. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds shared personal-book uploads, filename-based sync matching, personal-library XRPC methods, binary retrieval, service-auth JWT verification, replay protection, KV-backed backfill progress, storage quotas, and related tests and documentation. ChangesPersonal library platform
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Client
participant XRPCRouter
participant ServiceJwtVerifier
participant UploadPersonalBook
participant Database
participant LibraryStorage
Client->>XRPCRouter: Upload personal book with Bearer token
XRPCRouter->>ServiceJwtVerifier: Verify audience and method binding
ServiceJwtVerifier-->>XRPCRouter: Return service identity
XRPCRouter->>UploadPersonalBook: Stream file for authenticated DID
UploadPersonalBook->>Database: Validate duplicate, quota, and matching state
UploadPersonalBook->>LibraryStorage: Move validated book and cover
UploadPersonalBook-->>XRPCRouter: Return book view and storage usage
XRPCRouter-->>Client: Return upload result
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 27
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 553-557: Correct the `writeCapped` description in AGENTS.md to
match the implementation’s 1 MB stream highWaterMark and stated memory bound.
Replace the inaccurate ~64 KB RSS figure while preserving the explanation of
streaming to `.part` files and capping the body.
- Around line 886-901: Add an explicit language identifier to the fenced code
block containing the PDS and BookHive HTTP examples, using http or text to
satisfy markdownlint MD040 while leaving the example content unchanged.
- Line 471: Update the personal_book schema documentation to name the sizeBytes
column instead of fileSize, and correct the primary-key description to match the
composite key defined in src/db.ts so identical content uploaded by different
users is represented by separate rows.
In `@lexicons/getPersonalLibrary.json`:
- Around line 134-138: Update the description schema in getPersonalLibrary so
its declared limit matches stored catalog data: either remove maxLength or
enforce a UTF-8-aware truncation before returning descriptions, ensuring values
longer than 5000 bytes cannot violate the contract.
In `@lexicons/uploadPersonalBook.json`:
- Around line 32-44: Update the required-property declaration for the
uploadPersonalBook response schema to include both storageUsedBytes and
storageQuotaBytes alongside book, ensuring generated client types treat these
always-returned fields as mandatory.
In `@src/client/components/LibraryManager.tsx`:
- Around line 121-135: Update handleDelete so a successful deletion refreshes
the active list and storage state by invoking fetchBooks, or accurately
subtracts the deleted book’s size from storage. Ensure the quota meter reflects
the freed space immediately and no longer blocks uploads based on stale usage.
In `@src/context.ts`:
- Around line 362-372: Update the replay sweep setup around isPrimaryWorker to
require env.XRPC_SERVICE_AUTH in addition to env.XRPC_SERVICE_AUTH_REPLAY before
creating the interval, preventing sweeps when replay storage was not initialized
by the service-auth branch.
In `@src/db.ts`:
- Around line 940-967: The migration 022 backfill must use normalization logic
that cannot change with shared application helpers. Replace the calls to
filenameKey and koreaderFilenameHash in the personal_book and sync_document
backfill loops with frozen local implementations copied into the migration, or
add a subsequent migration that recomputes both columns when those helpers
change; ensure SAME_BOOK_FILE continues comparing consistently normalized
values.
In `@src/routes/library.tsx`:
- Around line 26-31: Extract the duplicated formatBytes helper from the route
and LibraryManager into a shared module, export it there, and import and reuse
that shared symbol in both consumers so quota formatting remains consistent.
- Around line 76-102: Apply Hono’s zValidator form middleware to the /upload
route and define a Zod schema for the multipart file field, replacing the manual
formData/file validation while preserving the existing error responses. Also
validate the error query parameter before it reaches LibraryPage using
zValidator query and its schema, reusing the validated value in the page flow.
In `@src/routes/sync/kosync.test.ts`:
- Around line 143-147: Update the afterEach cleanup around personalBookDir so it
resolves the configured LIBRARY_DIR and verifies the root is the expected
test-only temporary directory before invoking recursive rm; if the guard fails,
do not delete anything. Preserve cleanup of the test user directory when the
assertion passes.
In `@src/utils/bookMetadata/cbz.ts`:
- Around line 43-44: Update the cover metadata construction around extOf and
cover to call extOf(first.name) once, store its result, then normalize "jpeg" to
"jpg" before passing the extension to mimeForExt and assigning ext.
In `@src/utils/bookMetadata/epub.ts`:
- Around line 126-138: Enforce MAX_COVER_BYTES against the actual inflated data,
not only the ZIP metadata: update inflateCover in src/utils/bookMetadata/epub.ts
(lines 126-138) to reject data when data.length exceeds the cap, and apply the
same post-inflate or streaming limit to the page-1 inflation in
src/utils/bookMetadata/cbz.ts (lines 33-41).
In `@src/utils/bookMetadata/testFixtures.ts`:
- Around line 105-121: Update the files declaration in the fixture-building
function to support per-file ZIP options, then define the mimetype entry using
the proposed tuple with compression level 0 so it is stored rather than
deflated. Leave the other entries unchanged and preserve the existing cover,
padding, and zipSync behavior.
In `@src/utils/catalogBookService.ts`:
- Around line 273-301: Update getBackfillProgress so backfillProgress is
returned locally only while its status is "running"; after completion or
failure, read the KV state as authoritative. Change persistProgress to write an
immutable snapshot rather than the mutable global object, serialize progress
writes to prevent stale delayed writes from overwriting newer records, and await
the final terminal write while handling errors locally. Add a regression test
covering a worker observing newer progress after another worker starts a
subsequent backfill.
In `@src/utils/syncMatching.ts`:
- Around line 206-234: Update the query construction before the FTS loop so
author signals cannot consume all MAX_FTS_QUERIES slots: reserve at least one
slot for a candidate title query when title queries exist, while retaining the
existing author and title deduplication behavior. Ensure the slice passed to the
FTS execution loop includes that reserved title query and still respects
MAX_FTS_QUERIES.
- Around line 336-347: Guard the personal_book update in the hiveId/file
writeback block with a database-side null check, adding a where condition that
requires hiveId to still be null before setting it. Keep the existing user_book
ownership update unchanged, and anchor the change to the personal_book update in
the surrounding sync flow.
In `@src/utils/uploadPersonalBook.test.ts`:
- Around line 456-534: Add coverage in the rejection tests for the untested busy
result and byte-source cap path: saturate parseSemaphore while invoking
uploadPersonalBook and assert it returns { ok: false, reason: "busy" }, then
invoke uploadPersonalBook with source.kind "bytes" containing data over
MAX_PERSONAL_BOOK_BYTES and assert a too-large rejection with no temporary
entries or persisted record.
In `@src/utils/uploadPersonalBook.ts`:
- Around line 70-85: The deployed process count is three, so update all related
documentation to use WEB_CONCURRENCY=3 and recalculate the cluster-wide upload
memory ceiling as approximately 630 MB. In src/utils/uploadPersonalBook.ts,
correct the parseSemaphore comment and the “four independent processes”
reference near the upload flow; in AGENTS.md, make the production process-count
statement consistent with the existing WEB_CONCURRENCY=3 and ~630 MB guidance.
- Around line 429-432: Move the cover write in the upload flow before the
insertIfUnderQuota commit, or handle Bun.write failure by clearing the committed
row’s coverPath before propagating the error. Ensure the success payload’s
coverUrl reflects the final persisted cover state, including a null/absent value
when cover storage fails.
- Around line 210-224: Update insertIfUnderQuota to add ON CONFLICT (userDid,
contentHash) DO NOTHING to the INSERT, then distinguish a duplicate conflict
from a quota rejection when no row is affected. Return the established
discriminated duplicate result for the conflict, while preserving the
quota-exceeded result for rows excluded by the quota condition.
In `@src/xrpc/auth.ts`:
- Around line 44-48: Make isKnownAccount required in XrpcAuthContext and update
the service-authentication path around the serviceJwtVerifier handling to call
it unconditionally before returning authenticated access, denying authentication
when the DID is not known; remove the optional/missing-callback bypass while
preserving existing JWT verification behavior.
In `@src/xrpc/personalLibrary.test.ts`:
- Around line 419-426: Update the test around the “defaults to newest first”
case to include a non-empty q query parameter and verify that setting q does not
alter the default ordering. Make the assertion deterministic by avoiding
dependence on equal createdAt timestamps, or by asserting the documented stable
ordering with an explicit tiebreaker consistent with the router behavior.
In `@src/xrpc/replay-store.ts`:
- Around line 42-48: Update the production Dockerfile’s oven/bun image reference
from the floating 1-alpine tag to Bun 1.3.14 or newer, ensuring the replay-store
SQL flow in the shown INSERT conflict path runs against a supported SQLite
version.
In `@src/xrpc/router.ts`:
- Around line 1495-1502: Update the three sort branches in the query ordering
expression to append personal_book.id as the final
ascending/descending-consistent tiebreaker after title, authors/title, and
createdAt respectively. Preserve the existing primary sort directions and ensure
every limit/offset ordering is deterministic.
- Around line 191-194: Make isKnownAccount a required member of XrpcContext,
while leaving serviceJwtVerifier optional. In resolveXrpcAuth, remove the
truthiness guard and always await ctx.isKnownAccount(issuer) before granting
service-auth access, preserving rejection for unknown accounts.
- Around line 1734-1743: Update the catalog redirect branch guarded by
book.hiveId to return a mutable Response constructed with status 302 and the
computed Location header instead of using Response.redirect. Preserve the
existing absolute image-proxy URL, and add a regression test covering a book
with hiveId but no coverPath.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8ad6e1b4-b999-4428-8478-aa704d54969d
⛔ Files ignored due to path filters (7)
bun.lockis excluded by!**/*.locksrc/bsky/lexicon/generated/index.tsis excluded by!**/generated/**src/bsky/lexicon/generated/types/buzz/bookhive/getPersonalBookCover.tsis excluded by!**/generated/**src/bsky/lexicon/generated/types/buzz/bookhive/getPersonalBookFile.tsis excluded by!**/generated/**src/bsky/lexicon/generated/types/buzz/bookhive/getPersonalLibrary.tsis excluded by!**/generated/**src/bsky/lexicon/generated/types/buzz/bookhive/listPersonalShelves.tsis excluded by!**/generated/**src/bsky/lexicon/generated/types/buzz/bookhive/uploadPersonalBook.tsis excluded by!**/generated/**
📒 Files selected for processing (52)
AGENTS.mdbunfig.tomllexicons/auth.jsonlexicons/getPersonalBookCover.jsonlexicons/getPersonalBookFile.jsonlexicons/getPersonalLibrary.jsonlexicons/listPersonalShelves.jsonlexicons/uploadPersonalBook.jsonpackage.jsonsrc/app.tssrc/auth/client.tssrc/auth/router.tsxsrc/auth/session.test.tssrc/auth/token-refresh.test.tssrc/bsky/id-resolver.tssrc/client/components/LibraryManager.tsxsrc/context.tssrc/db.tssrc/env.tssrc/pages/library.test.tsxsrc/pages/library.tsxsrc/routes/admin.tssrc/routes/library.test.tssrc/routes/library.tsxsrc/routes/opds.test.tssrc/routes/opds.tssrc/routes/sync/kosync.test.tssrc/routes/sync/kosync.tssrc/test/env-setup.tssrc/types.tssrc/utils/account.tssrc/utils/bookMatching.tssrc/utils/bookMetadata/cbz.tssrc/utils/bookMetadata/cover.tssrc/utils/bookMetadata/epub.tssrc/utils/bookMetadata/hash.tssrc/utils/bookMetadata/index.tssrc/utils/bookMetadata/testFixtures.tssrc/utils/catalogBookService.test.tssrc/utils/catalogBookService.tssrc/utils/filenameMatching.test.tssrc/utils/filenameMatching.tssrc/utils/personalLibrary.tssrc/utils/syncMatching.test.tssrc/utils/syncMatching.tssrc/utils/uploadPersonalBook.test.tssrc/utils/uploadPersonalBook.tssrc/xrpc/auth.test.tssrc/xrpc/auth.tssrc/xrpc/personalLibrary.test.tssrc/xrpc/replay-store.tssrc/xrpc/router.ts
Comment on lines
+42
to
+48
| const result = await sql<{ inserted: number }>` | ||
| INSERT INTO ${sql.table(REPLAY_TABLE)} (id, value, created_at, updated_at) | ||
| VALUES (${`${iss}|${jti}`}, ${expiresAt}, ${now}, ${now}) | ||
| ON CONFLICT(id) DO NOTHING | ||
| RETURNING 1 as inserted | ||
| `.execute(kvDb); | ||
| return result.rows.length > 0; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the SQLite version available to the runtime and look for replay-store tests.
bun --version
bun -e 'import { Database } from "bun:sqlite"; const d = new Database(":memory:"); console.log(d.query("select sqlite_version() as v").get());'
fd -t f 'replay' -x echo {}
rg -n 'createKvReplayStore|sweepReplayStore|ensureReplayTable' --type=tsRepository: nperez0111/bookhive
Length of output: 258
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- replay-store.ts ---'
cat -n src/xrpc/replay-store.ts
printf '%s\n' '--- repository manifests and runtime references ---'
git ls-files | rg '(^|/)(package\.json|bun\.lockb?|bunfig\.toml|Dockerfile.*|compose.*|.*lock|README.*)$' | head -80
printf '%s\n' '--- package/runtime references ---'
rg -n 'bun:sqlite|sqlite|better-sqlite|libsql|XRPC_SERVICE_AUTH_REPLAY|replay-store|replay' \
--glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' \
package.json bunfig.toml Dockerfile* docker-compose* src test tests .github 2>/dev/null || true
printf '%s\n' '--- relevant manifest files ---'
for f in package.json bunfig.toml; do
if [ -f "$f" ]; then cat -n "$f"; fi
doneRepository: nperez0111/bookhive
Length of output: 38431
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- runtime image and Bun pin ---'
for f in Dockerfile .cursor/Dockerfile compose.yaml; do
if [ -f "$f" ]; then
printf '%s\n' "--- $f ---"
cat -n "$f"
fi
done
printf '%s\n' '--- Bun references and CI setup ---'
rg -n 'oven/bun|BUN_VERSION|bun-version|setup-bun|1\.3\.14|bun:sqlite' \
--glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' \
.github Dockerfile .cursor/README* .cursor/Dockerfile package.json README.md 2>/dev/null || true
printf '%s\n' '--- replay-related test and auth setup locations ---'
rg -n -C 8 'ensureReplayTable|createKvReplayStore|XRPC_SERVICE_AUTH_REPLAY|replayStore' \
src/xrpc/auth.test.ts src/context.ts src/env.ts src/xrpc 2>/dev/null || trueRepository: nperez0111/bookhive
Length of output: 18269
🌐 Web query:
Bun 1.3.14 bundled SQLite version bun:sqlite
💡 Result:
In Bun 1.3.14, the bundled version of SQLite is officially 3.53.0 [1][2]. However, the actual SQLite version experienced by users may differ depending on the platform [3]: * On Linux and Windows, Bun typically uses the bundled SQLite 3.53.0 amalgamation [3][4]. * On macOS, Bun historically linked against the system-provided libsqlite3.dylib, which meant that bun:sqlite would often report an older version (such as 3.43.2) regardless of the version bundled in the Bun release notes [3][5][6]. Due to these discrepancies, users on macOS who require the features or fixes present in newer SQLite versions (such as 3.53.0) may need to point bun:sqlite to a custom SQLite build using Database.setCustomSQLite(path) [6]. Note that Bun development has actively worked on addressing this by moving toward statically linking the bundled SQLite on all platforms to ensure consistency [3].
Citations:
- 1: https://bun.com/blog/bun-v1.3.14
- 2: https://github.com/oven-sh/bun/releases/tag/bun-v1.3.14
- 3: bun:sqlite on macOS arm64 still ships SQLite 3.43.2 in Bun 1.3.14 / 1.4.0-canary, despite blog claiming 3.53.0 oven-sh/bun#31247
- 4: https://github.com/oven-sh/bun/blob/6618e7f7/scripts/build/deps/sqlite.ts
- 5: SQLite version is incorrect oven-sh/bun#16717
- 6: https://bun.com/docs/runtime/sqlite
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Bun setup workflows ---'
for f in .github/workflows/*.yml .github/workflows/*.yaml; do
[ -f "$f" ] || continue
if rg -q 'setup-bun|bun test|oven/bun' "$f"; then
printf '%s\n' "--- $f ---"
cat -n "$f"
fi
done
printf '%s\n' '--- replay test files ---'
git ls-files '*replay*' '*auth.test.ts' | sortRepository: nperez0111/bookhive
Length of output: 1724
Pin the production Bun image to a supported version. Bun 1.3.14 bundles SQLite 3.53.0, but Dockerfile uses the floating oven/bun:1-alpine tag. Pin it to Bun 1.3.14 or newer, or add replay-store coverage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/xrpc/replay-store.ts` around lines 42 - 48, Update the production
Dockerfile’s oven/bun image reference from the floating 1-alpine tag to Bun
1.3.14 or newer, ensuring the replay-store SQL flow in the shown INSERT conflict
path runs against a supported SQLite version.
Review follow-ups on the personal-library XRPC work, plus two things found while testing it against a real book. Review fixes: - isKnownAccount is required on XrpcAuthContext/XrpcContext and called unconditionally. It was optional and guarded by truthiness, so a context that merely forgot to wire it opened the personal library to any DID on the network. - getPersonalBookCover builds its 302 by hand rather than with Response.redirect, whose headers are immutable — the downstream Cache-Control middleware mutating them throws a TypeError and turns the redirect into a 500. - Every getPersonalLibrary sort ends on personal_book.id. None of the leading keys are unique, so LIMIT/OFFSET pages could repeat one book and drop another. - insertIfUnderQuota gained ON CONFLICT DO NOTHING and tells a duplicate apart from a quota rejection, instead of raising a UNIQUE violation when the same file races past the duplicate check. - A failed cover write clears coverPath instead of propagating, so a committed row can never point at a file that isn't there. - The replay sweep is gated on XRPC_SERVICE_AUTH as well as ..._REPLAY; the svc_jti table only exists when both are on. - syncMatching: the hiveId-null guard is enforced in the UPDATE, and author FTS queries can no longer starve the title query out of all four slots. - epub/cbz re-check inflated cover bytes against MAX_COVER_BYTES; the ZIP's declared originalSize is attacker-controlled and covers bypass the quota. - LibraryManager refetches after a delete so the storage meter shows freed space; GET /library validates ?error= against a closed set. SVG covers (image-meta + @resvg/resvg-js): Standard Ebooks ships every cover as an SVG holding the artwork in an <image> element plus the title as ~40 vector paths. Bun.Image rejects SVG as an "unrecognised format", so that whole corpus uploaded with no cover at all. prepareCover now rasterizes via resvg and encodes to JPEG, inside the parse semaphore so it shares the upload's native-memory bound. Both shortcuts were tried and each loses half the cover: unwrapping the embedded raster drops the lettering, rendering with @takumi-rs drops the artwork. @resvg/resvg-js* is in traceDeps because a missing native binding does not crash — it silently restores the bug. Also fixes PNG_32, which was never a valid PNG: it built IDAT with Bun.deflateSync (raw deflate) where the format requires zlib. Bun's decoder is lenient, resvg's is not, so the fixture only worked by accident. AGENTS.md, audited against the codebase: seed:db documented a script whose entrypoint was deleted long ago, 44 lexicons not ~27, 5 worker bundles not 4, personal_book/personal_shelf_item key columns, the @ alias (configured in Vite but absent from tsconfig, so unusable in src/), and the unreferenced screenshot count. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/utils/bookMetadata/epub.ts (1)
126-142: 🩺 Stability & Availability | 🟡 Minor | ⚖️ Poor tradeoffBoth cover extractors allocate the full inflated entry before they reject it. Each file now re-checks the real byte length against
MAX_COVER_BYTES, which closes the storage half of the earlier finding. The memory half remains:unzipSyncinflates the selected entry completely before the check runs, so a crafted archive that understatesoriginalSizestill forces the full decompressed allocation.parseSemaphorebounds the concurrency, not the per-entry size.
src/utils/bookMetadata/epub.ts#L126-L142: replace theunzipSynccall ininflateCoverwith a streaming inflate that aborts once the output passesMAX_COVER_BYTES.src/utils/bookMetadata/cbz.ts#L40-L44: apply the same streaming inflate with an abort cap to the page-1 extraction.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/bookMetadata/epub.ts` around lines 126 - 142, Both cover extractors must enforce MAX_COVER_BYTES during decompression rather than after full allocation. In src/utils/bookMetadata/epub.ts lines 126-142, update inflateCover to use a streaming inflate that aborts once output exceeds MAX_COVER_BYTES; in src/utils/bookMetadata/cbz.ts lines 40-44, apply the same capped streaming extraction to page 1. Preserve the existing rejection and cover-validation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/bookMetadata/cover.ts`:
- Around line 63-85: Update rasterizeSvgCover’s Resvg configuration to disable
per-instance system-font discovery by setting font.loadSystemFonts to false.
Preserve the existing rendering options, and supply any required fonts through
Resvg’s fontFiles or fontDirs configuration if the SVG rendering flow depends on
them.
In `@src/utils/syncMatching.ts`:
- Around line 352-361: The sync flow around the guarded `personal_book` update
must use the persisted link when the update affects zero rows. Inspect the
result of the update in the surrounding function, read the existing
`personal_book.hiveId` when another request won the race, and use that resolved
ID for the subsequent `user_book` ownership update and return value; retain the
requested `hiveId` only when this update establishes the link.
In `@src/utils/uploadPersonalBook.ts`:
- Around line 450-472: Wrap the post-commit rename in the upload flow around
ensureDir, rename, and the committed-row state so a rename failure compensates
by deleting the corresponding personal_book row before rethrowing the error. Use
the existing userDid and contentHash predicates, and preserve successful rename
and subsequent cover handling unchanged.
In `@src/xrpc/personalLibrary.test.ts`:
- Around line 367-379: Remove the duplicated response-parsing fragments in
src/xrpc/personalLibrary.test.ts at lines 367-379 and 450-470: delete the
repeated completed json() expression after the upload result assertion, and
remove the duplicate books type member plus the repeated searched json()
expression. Preserve the surrounding test logic and the original response
parsing.
---
Duplicate comments:
In `@src/utils/bookMetadata/epub.ts`:
- Around line 126-142: Both cover extractors must enforce MAX_COVER_BYTES during
decompression rather than after full allocation. In
src/utils/bookMetadata/epub.ts lines 126-142, update inflateCover to use a
streaming inflate that aborts once output exceeds MAX_COVER_BYTES; in
src/utils/bookMetadata/cbz.ts lines 40-44, apply the same capped streaming
extraction to page 1. Preserve the existing rejection and cover-validation
behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 309eb1ac-0f63-4c2b-ac9f-7bd35b3384f0
⛔ Files ignored due to path filters (3)
bun.lockis excluded by!**/*.locksrc/bsky/lexicon/generated/types/buzz/bookhive/getPersonalLibrary.tsis excluded by!**/generated/**src/bsky/lexicon/generated/types/buzz/bookhive/uploadPersonalBook.tsis excluded by!**/generated/**
📒 Files selected for processing (24)
AGENTS.mdlexicons/getPersonalLibrary.jsonlexicons/uploadPersonalBook.jsonpackage.jsonsrc/client/components/LibraryManager.tsxsrc/context.tssrc/db.tssrc/routes/library.tsxsrc/utils/bookMetadata/bookMetadata.test.tssrc/utils/bookMetadata/cbz.tssrc/utils/bookMetadata/cover.tssrc/utils/bookMetadata/epub.tssrc/utils/bookMetadata/index.tssrc/utils/bookMetadata/testFixtures.tssrc/utils/catalogBookService.tssrc/utils/filenameMatching.tssrc/utils/imageProxy.tssrc/utils/syncMatching.tssrc/utils/uploadPersonalBook.test.tssrc/utils/uploadPersonalBook.tssrc/xrpc/auth.tssrc/xrpc/personalLibrary.test.tssrc/xrpc/router.tsvite.config.ts
Comment on lines
+367
to
+379
|
|
||
| it("redirects to the catalog image for a linked book with no stored cover", async () => { | ||
| // The branch that used to be built with `Response.redirect`, whose headers | ||
| // are immutable — the Cache-Control middleware setting a header on it threw | ||
| // a TypeError and turned the 302 into a 500. | ||
| const app = createApp(); | ||
| const hash = ( | ||
| (await ( | ||
| await uploadRequest(app, makeFb2(), "x.fb2", { | ||
| contentType: "application/x-fictionbook+xml", | ||
| }) | ||
| ).json()) as { book: { contentHash: string } } | ||
| ).book.contentHash; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the duplicated response-parsing fragments.
The duplicated statements make src/xrpc/personalLibrary.test.ts invalid TypeScript. Line 378 repeats a completed json() expression. Line 457 repeats the books type member. Line 465 repeats another completed json() expression.
src/xrpc/personalLibrary.test.ts#L367-L379: remove the second response-parsing fragment after the upload result type assertion.src/xrpc/personalLibrary.test.ts#L450-L470: remove the duplicatebooksmember and the second response-parsing fragment forsearched.
📍 Affects 1 file
src/xrpc/personalLibrary.test.ts#L367-L379(this comment)src/xrpc/personalLibrary.test.ts#L450-L470
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/xrpc/personalLibrary.test.ts` around lines 367 - 379, Remove the
duplicated response-parsing fragments in src/xrpc/personalLibrary.test.ts at
lines 367-379 and 450-470: delete the repeated completed json() expression after
the upload result assertion, and remove the duplicate books type member plus the
repeated searched json() expression. Preserve the surrounding test logic and the
original response parsing.
Service auth on /xrpc/* no longer requires that a DID have used BookHive before — any valid inter-service token is accepted. Signup is open, so the gate bought little; the per-user storage quota is the real backstop on what a caller can consume, and pdsWrite methods still refuse service auth. Removes markAccount/isKnownAccount, the account: KV mount, and the isKnownAccount field from XrpcAuthContext/XrpcContext/AppContext. Deletes src/utils/account.ts. Updates the auth tests (a stranger's valid token now 200s) and AGENTS.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- cover.ts: disable resvg per-instance system-font scanning (loadSystemFonts: false); these SVG covers carry outlined text, so no fonts are needed and the scan was pure per-upload cost. - syncMatching.ts: when the guarded personal_book link update loses a race (0 rows), adopt the persisted hiveId for the ownership update and return value instead of writing our own guess — a wrong link is worse than no link. - uploadPersonalBook.ts: roll back the committed personal_book row if the post-commit rename fails, so a failed move can't leave a dead download and phantom quota usage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/utils/uploadPersonalBook.ts (1)
450-465: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winInclude
ensureDirin the rollback scope.If
ensureDirfails after Line 418 commits the row, execution bypasses the catch block. The database then retains apersonal_bookrow with no file, and a retry returnsduplicate.Move
ensureDirinside thetryblock so every post-commit filesystem failure deletes the committed row before rethrowing.Proposed fix
- await ensureDir(personalBookDir(userDid, contentHash)); try { + await ensureDir(personalBookDir(userDid, contentHash)); await rename(tmp, filePath); } catch (err) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/uploadPersonalBook.ts` around lines 450 - 465, Move the ensureDir call into the try block that currently wraps rename in the post-commit flow. Keep the existing personal_book deletion and rethrow in the catch so failures from either ensureDir or rename roll back the committed row before cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/syncMatching.ts`:
- Around line 367-374: Update the fallback logic after the `personal_book`
update in `syncMatching` so `hiveId` is always synchronized with the persisted
value: assign the stored `current.hiveId` when usable, otherwise clear it to
`null` when absent or equal to `NO_HIVE_MATCH`, preventing later ownership logic
from using the stale local guess.
---
Duplicate comments:
In `@src/utils/uploadPersonalBook.ts`:
- Around line 450-465: Move the ensureDir call into the try block that currently
wraps rename in the post-commit flow. Keep the existing personal_book deletion
and rethrow in the catch so failures from either ensureDir or rename roll back
the committed row before cleanup.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7ae52cb2-ac99-434a-adc5-6909296c2aca
📒 Files selected for processing (3)
src/utils/bookMetadata/cover.tssrc/utils/syncMatching.tssrc/utils/uploadPersonalBook.ts
When the guarded personal_book link update loses the race, clear hiveId to null if the persisted value is absent or NO_HIVE_MATCH instead of keeping the stale local guess. This stops the ownership update and return value from establishing a wrong link when the winner dismissed the match or the row was deleted concurrently. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Service auth is now always on (no XRPC_SERVICE_AUTH kill switch), the max-age window is a constant (SERVICE_AUTH_MAX_AGE_SECONDS = 3600) rather than an env var, and replay protection is removed entirely — a token is already scoped to one lxm and one audience, so within its short window "authed is authed". Deletes src/xrpc/replay-store.ts and its svc_jti table plumbing. XRPC_SERVICE_AUTH_AUDIENCES stays configurable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
parseBook (fflate unzipSync) and prepareCover (synchronous resvg raster) are the only whole-file, CPU-bound steps of an upload; running them inline stalled the request process's event loop. They now run in a throwaway Bun Worker spawned per upload (parse-client.ts -> parse-worker.ts), reading the file from the temp path so nothing large crosses the thread boundary. The parse semaphore still bounds concurrency — now the number of live workers — and sheds excess as busy (503). The worker is terminated on every path (reply, error, or 60s deadline). Registered as the 6th standalone worker bundle; resvg's native binding is emitted alongside it by bun build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
iOS client side of the storage-quota feature: a StorageMeter component, storage usage read off getPersonalLibrary results, and quota-aware upload handling in the library/sync screens. Authored in parallel; committed here as part of the branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
nperez0111
added a commit
that referenced
this pull request
Aug 12, 2026
…users Three fixes for the session/auth failure paths surfaced after the re-signin deploy (#205): - Tolerate a corrupt/tampered/rotated-secret `sid` cookie. iron-session throws `Wrong mac prefix` when the HMAC doesn't verify; that throw was uncaught in both `getSessionAgent` and the `getProfile` fast path, so a modified cookie hard-500'd every route for that browser (and a COOKIE_SECRET rotation would do it to every existing user). `readIronSession` now catches, treats it as no session, and expires the bad cookie so the browser recovers on its own. - Give the 401 "Invalid Session" error page a way back in. `ErrorPage` only offered "Go back home" and "Contact support" — it told users to log in with no login link. It now shows a primary "Sign in" button (→ /login) whenever the status is 401, covering every "Invalid Session" call site without touching each route. - Add a regression test for the tampered-cookie path. Note: the scope change in #205 does not invalidate existing OAuth grants — sessions are only destroyed on genuinely terminal errors — so existing credentials keep working; these fixes only harden the invalid/failed paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
E-reader sync now works for the majority of KOReader users:
filenameHash/filenameKeycolumns (migration 022), a three-waySAME_BOOK_FILEpredicate, andmatchSyncDocumentForUser, which resolves a synced document against the user's uploaded files rather than only the title/author a default-configured KOReader never sends. Every OPDS capability now has an XRPC equivalent —getPersonalBookFile/getPersonalBookCover(blob outputs),listPersonalShelves, andq/sort/storageongetPersonalLibrary— authenticated with atproto inter-service auth viaServiceJwtVerifier, so a script or e-reader can use the library instead of only a browser session, withuploadPersonalBooktaking a real MIME allowlist and a requiredfilenameparam in place of*/*and anx-file-nameheader. The two drifted upload implementations collapse into one shared core that streams to disk instead of buffering, which removes a double-buffering bug inbodyLimit()and stops the epub/cbz parsers inflating every image in an archive to keep one cover, taking worst-case upload RSS from unbounded to a bounded ~630 MB cluster-wide. Adds a 2 GB per-user storage quota evaluated inside the INSERT so concurrent uploads cannot both slip past — checked against production first, where the heaviest user holds 39 MB. Also supersedes #204 (carrying its backfill-progress persistence forward with a fix: the stored value wasJSON.stringifyd, which unstorage reads back as an object, so every persisted run was silently discarded) and gives OPDS covers an ETag, which production served 43 of in 48h without ever answering a single 304.Verified: 559 tests pass (up from 475 on main, stable across repeated runs), typecheck and lint clean, production build succeeds, and the real app was booted to confirm the new routes authenticate rather than 404.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes