Setup and very basic version #1
Conversation
📝 WalkthroughWalkthroughRebrands project to Libroo, migrates persisted storage from PostgreSQL to SQLite with new migrations, introduces Effect-TS layered services (Db, Storage, Auth, Book, OpenLibrary), adds many server API routes, pages, components, auth composables/middleware, and updates build/CI/package configuration. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser
participant Middleware as Auth Middleware
participant AuthClient as better-auth
participant Server as Nitro/API
participant DB as SQLite
User->>Browser: Navigate to protected page (/library)
Browser->>Middleware: route guard (meta.auth)
Middleware->>AuthClient: authClient.useSession(useFetch)
AuthClient->>Server: server-side session check
Server->>DB: query session data
alt session exists
DB-->>Server: session record
Server-->>AuthClient: session data
Middleware-->>Browser: allow navigation
else no session
Middleware-->>Browser: redirect to /auth/login?redirect=...
end
sequenceDiagram
actor User
participant Browser
participant API as /api/books/lookup
participant EffectHandler
participant BookService
participant Repo as BookRepository
participant OpenLibrary
participant Storage
User->>Browser: Submit ISBN
Browser->>API: POST /api/books/lookup { isbn }
API->>EffectHandler: validate & auth
EffectHandler->>BookService: lookupBook(isbn)
BookService->>Repo: findByIsbn(isbn)
alt found locally
Repo-->>BookService: local data
else not found
BookService->>OpenLibrary: lookupByISBN
OpenLibrary-->>BookService: metadata + coverUrl
BookService->>Storage: download & putBlob
Storage-->>BookService: blob pathname
BookService->>Repo: insert book + user_books
end
BookService-->>API: return lookup result
API-->>Browser: 200 + payload
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📚 Learning: 2026-01-13T21:08:13.933ZApplied to files:
📚 Learning: 2026-01-13T21:24:30.028ZApplied to files:
📚 Learning: 2026-01-15T15:10:03.761ZApplied to files:
🔇 Additional comments (7)
✏️ Tip: You can disable this entire section by setting Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/TemplateMenu.vue (1)
1-48: Remove the unused TemplateMenu component.This component is not imported or used anywhere in the codebase and contains only external Nuxt UI Pro template links. The AppHeader component already provides all necessary Libroo-branded navigation.
🤖 Fix all issues with AI agents
In @app/middleware/auth.ts:
- Around line 1-8: The auth middleware's logic is inverted: when no session
exists (from authClient.useSession in defineNuxtRouteMiddleware) it currently
redirects only if to.meta.auth === false; change this to redirect when the route
requires auth (i.e., to.meta.auth is not strictly false). Update the conditional
around navigateTo('/auth/login') so it triggers when !session.value &&
to.meta.auth !== false (or equivalently to.meta.auth !== false/Boolean check for
truthy requirement), ensuring protected routes (to.meta.auth !== false) send
unauthenticated users to the login page.
In @app/pages/auth/login.vue:
- Around line 52-84: The catch block in onSubmit uses the banned catch (e: any);
change it to catch (e: unknown) and safely extract a message: check if e is an
instance of Error and use e.message, otherwise coerce to string (e.g., String(e)
or JSON.stringify fallback) before assigning to error.value and passing to
toast; update the catch handling in onSubmit to perform this type-safe
extraction and keep the rest of the error toast logic unchanged.
- Around line 16-25: The redirect query is not sanitized, enabling open-redirect
risks; update the computed redirectPath (where you read route.query.redirect) to
validate the value before use: only accept strings that start with a single '/'
and not with '//' (e.g., check redirectStr.startsWith('/') &&
!redirectStr.startsWith('//') or use a regex like /^\/(?!\/)/), otherwise fall
back to '/library'; keep the existing watch(session...) and
navigateTo(redirectPath.value) but ensure redirectPath.value always returns a
safe in-app path.
- Around line 44-48: The Zod schema uses legacy positional message args
(z.email('...'), z.string('...')) which is invalid in Zod v4; update the schema
definition used in schema to pass error messages via the unified error option
object: call z.email({ error: 'Please enter a valid email address' }) and
z.string({ error: 'Password is required' }).min(1, { error: 'Password is
required' }) so the email and password validators use the { error: ... } form
instead of positional string arguments.
In @app/pages/auth/register.vue:
- Around line 68-104: The catch block in onSubmit uses catch (e: any) which
fails CI; change it to catch (e: unknown) and safely narrow the error before
using .message (e.g., check if e is an instance of Error and extract message,
otherwise stringify or fallback to 'An unexpected error occurred'), then assign
error.value and use that safe message in the toast; update only the catch
section inside onSubmit to perform this type-safe error handling.
- Around line 55-64: The Zod schema uses outdated positional message args;
update the validation to use the v4 object-form error option by replacing
occurrences like z.string('...') and z.email('...') with z.string({ error: '...'
}) and z.email({ error: '...' }) respectively inside the const schema =
z.object({...}) definition (leave .min(...) and the .refine(...) intact),
ensuring each field (name, email, password, confirmPassword) uses the { error:
'...' } form so Zod v4.3.5 validates and reports messages correctly.
In @app/pages/library/[id].vue:
- Around line 37-67: The formatDate function incorrectly parses year-only
strings like "2015" as full Dates so the year-only branch never runs; update
formatDate to test the /^\d{4}$/ year-only pattern before attempting new
Date(...) and return the raw year when matched, then proceed with parsing other
formats; also remove any trailing whitespace on the affected lines in formatDate
to satisfy CI.
- Around line 74-94: The removeBook function currently uses an unsafe catch
signature (catch (error: any)) and fragile message extraction; change to catch
(error: unknown) and narrow the type before reading properties: check if it's a
string, an Error instance (use (error instanceof Error).message), or an object
with a data?.message or message field, and fall back to a generic message; then
pass that computed message into toast.add. Update references in removeBook
(userBookId, $fetch, toast) accordingly so CI no longer complains about any
typing and error messages are robust.
In @app/pages/library/index.vue:
- Around line 33-83: The problem is that books is a computed view over
data.value.items so when loadMore increments page and useFetch replaces data,
the UI shows only the new page; fix by maintaining an accumulated reactive list
and appending new page items instead of replacing them: introduce a ref like
accumulatedBooks and update books computed to return accumulatedBooks.value (and
update allSelected/hasBooks/selectedCount to use it), then change loadMore (or
the fetch response handler) to fetch the next page and push/concat new items
into accumulatedBooks.value while updating pagination.value from
data.value.pagination; ensure selectedBooks logic still references the book ids
from accumulatedBooks.
In @server/api/books/lookup.post.ts:
- Around line 11-36: localBookEffect is swallowing DB errors (returning null),
subjects JSON is parsed without try/catch, and OpenLibrary lookup uses raw
body.isbn instead of the normalized value; fix by letting database errors
propagate (or map them to a clear Effect failure) instead of catching to null in
localBookEffect so real DB failures surface, wrap JSON.parse of
localBook.subjects in a safe try/catch (or JSON.parse wrapper) and fall back to
null on parse errors, and ensure the OpenLibrary request uses normalizedISBN
(not body.isbn) so both local and remote lookups use the same normalized key.
In @server/db/schema/auth.ts:
- Around line 27-41: The account table schema defined by sqliteTable(account) is
missing a uniqueness constraint on the (providerId, accountId) pair; update the
sqliteTable call for account to include a constraints callback (e.g., add the
third argument or the (t) => [...] callback) and return
unique().on(t.providerId, t.accountId) so Drizzle enforces uniqueness for
providerId + accountId, keeping all existing columns (id, accountId, providerId,
userId, etc.) unchanged.
In @server/repositories/book.repository.ts:
- Around line 16-28: The BookNotFoundError defined in this file conflicts with
OpenLibrary's error tag; rename the repository-specific error tag (change class
BookNotFoundError to use a distinct tag like 'BookRepositoryNotFoundError' or
similarly unique identifier) and update all places that reference that class
(throws and imports) to the new name, then update effectHandler.ts to map/handle
the new tag and branch property access so it reads bookId for the repository
error and isbn for the OpenLibrary error.
In @server/utils/auth.ts:
- Around line 7-8: The current config uses a hardcoded fallback for the auth
secret (secret: process.env.BETTER_AUTH_SECRET ||
'libroo-dev-secret-change-in-production'), which is a security risk; change the
logic so that when NODE_ENV === 'production' (or isProduction flag) and
process.env.BETTER_AUTH_SECRET is missing/empty you throw an error during
startup, otherwise allow a dev-only fallback; update the code paths referencing
the secret key (the secret config property and any initialization that reads
BETTER_AUTH_SECRET) to validate presence and fail fast in production with a
clear error message.
In @server/utils/drizzle.ts:
- Around line 1-14: This file is dead/commented-out code; either delete it or
replace the commented blocks with a clear placeholder explaining intent. Remove
the file entirely if the Drizzle integration was intentionally removed (git
history preserves it); otherwise, if you want to keep a placeholder for future
Drizzle work, remove all commented code and add a single TODO comment at the top
referencing an issue/track ID and explaining that useDrizzle, DrizzleDB, and
schema were intentionally removed and will be reintroduced when the migration is
complete. Ensure no commented implementations remain and update any docs or
issue links referenced in the TODO.
In @server/utils/effect.ts:
- Around line 14-33: MainLive is redundantly re-providing BaseServicesLive
because RepositoriesLive already includes BookRepositoryLive wired with
BaseServicesLive; fix by making MainLive just the already-composed
RepositoriesLive (i.e., export MainLive as RepositoriesLive / directly re-export
the composed layer) instead of calling Layer.provideMerge(RepositoriesLive,
BaseServicesLive). Alternatively, for clearer separation move
OpenLibraryRepositoryLive into the RepositoriesLive composition alongside
BookRepositoryLive and then export MainLive as the single composed
RepositoriesLive so base services aren’t provided twice.
🟡 Minor comments (6)
README.md-17-17 (1)
17-17: Documentation inconsistency: Stack mentions PostgreSQL but implementation uses SQLite.The README states "Drizzle ORM + PostgreSQL", but
drizzle.config.tsshows the project now uses SQLite with a local file at.data/db/sqlite.db. Update the documentation to reflect the actual stack.📝 Suggested fix
-- [Drizzle ORM](https://orm.drizzle.team/) + PostgreSQL +- [Drizzle ORM](https://orm.drizzle.team/) + SQLiteapp/pages/library/add.vue-33-35 (1)
33-35: Incorrect Zod 4 API usage forz.string().In Zod 4,
z.string()no longer accepts an error message as the first argument. The'ISBN is required'message will be ignored. Use the unifiederrorparameter or rely on.min()for the validation message.Suggested fix
const isbnSchema = z.object({ - isbn: z.string('ISBN is required').min(10, 'ISBN must be at least 10 characters').max(17, 'ISBN is too long') + isbn: z.string().min(10, 'ISBN must be at least 10 characters').max(17, 'ISBN is too long') })Based on library documentation for Zod 4.
app/composables/auth.ts-3-4 (1)
3-4: Add explicitbaseURLto match server configuration.The client-side
createAuthClientshould be configured with the samebaseURLpattern as the server-side auth setup. Currently, the server explicitly setsbaseURL: process.env.BETTER_AUTH_URL || 'http://localhost:3000', but the client has no baseURL configuration and will attempt to infer it from requests, which the better-auth library explicitly marks as not recommended. Update the client configuration to:export const authClient = createAuthClient({ baseURL: process.env.BETTER_AUTH_URL || 'http://localhost:3000' })This ensures consistent behavior across development and production environments.
app/components/AppHeader.vue-42-58 (1)
42-58: Fix linting errors flagged by CI.The pipeline is failing due to formatting issues:
- Line 51: More than 1 blank line not allowed
- Lines 45-46, 53: Attributes should be on separate lines per
vue/max-attributes-per-line- Line 58: Missing newline at end of file
🔧 Proposed fix
<template> <UHeader :links="links"> <template #left> - <NuxtLink to="/" class="flex items-center gap-2"> - <UIcon name="i-lucide-book-open-check" class="text-2xl text-primary" /> + <NuxtLink + to="/" + class="flex items-center gap-2" + > + <UIcon + name="i-lucide-book-open-check" + class="text-2xl text-primary" + /> <span class="font-bold text-xl">Libroo</span> </NuxtLink> </template> - <template #right> - <UNavigationMenu :items="links" class="hidden lg:flex" variant="link" /> + <UNavigationMenu + :items="links" + class="hidden lg:flex" + variant="link" + /> <UColorModeButton /> </template> </UHeader> </template> +server/api/books/lookup.post.ts-17-28 (1)
17-28: Inconsistent ISBN format in responses.The endpoint returns different ISBN formats depending on the outcome: normalized from OpenLibrary (line 61) or local DB (line 44), but raw
body.isbnin the not-found error (line 75). AlthoughlookupByISBN()normalizes internally (openLibrary.repository.ts line 119), the inconsistency creates confusing API responses.Additionally, the only validation is a basic string check (line 18)—a clearly-invalid value like "abc" will still hit the database and API.
Use
normalizedISBNconsistently across all response paths, and add basic format validation:Suggested fix
@@ + const normalizedISBN = body.isbn.replace(/[-\s]/g, '') + if (!/^\d{10}(\d{3})?$/.test(normalizedISBN)) { + return yield* Effect.fail( + createError({ statusCode: 400, message: 'ISBN must be 10 or 13 digits' }) + ) + } @@ const lookupEffect = lookupByISBN(body.isbn).pipe( Effect.catchTag('BookNotFoundError', () => Effect.succeed({ found: false, - isbn: body.isbn, + isbn: normalizedISBN, message: 'Book not found on OpenLibrary' }) ) )server/repositories/book.repository.ts-288-302 (1)
288-302: Misleading error property inremoveFromLibrary.When the deletion fails to find a record,
BookNotFoundErroris created withbookId: userBookId. This is semantically incorrect—it's auserBookId, not abookId.🐛 Suggested fix
if (result.length === 0) { - return yield* Effect.fail(new BookNotFoundError({ bookId: userBookId })) + return yield* Effect.fail(new BookNotFoundError({ bookId: userBookId })) // Note: This is actually userBookId }Or better, add a
userBookIdfield to the error type if you rename it as suggested earlier.
🧹 Nitpick comments (31)
Pitch.md (1)
1-52: Excellent pitch document with clear vision and structure!The document effectively articulates Libroo's purpose, target audience, and differentiators. The problem-solution structure is compelling, and the core pillars are well-defined.
Minor: Consider adjusting markdown heading hierarchy
The document starts with h3 (
###) headings. Standard markdown practice is to start with h2 (##) after the main title (h1#), incrementing by one level at a time. This is flagged by markdownlint but is purely a formatting convention.If you'd like to follow standard markdown conventions:
-### **Vision** +## **Vision**Apply the same change to other top-level sections (The Problem, The Solution, Target Audience, Philosophy & Values).
drizzle.config.ts (1)
7-9: Consider making the database path configurable.The hardcoded SQLite path works for local development, but consider using an environment variable for flexibility across different environments (development, testing, production).
💡 Optional improvement
+import process from 'node:process' + export default defineConfig({ dialect: 'sqlite', schema: './server/db/schema/index.ts', out: './server/db/migrations', dbCredentials: { - url: '.data/db/sqlite.db' + url: process.env.DATABASE_URL || '.data/db/sqlite.db' } })app/components/AppLogo.vue (1)
9-37: Inconsistent fill color usage across paths.The first two paths (lines 10-11, 14-15) use
currentColorwhile the remaining paths usevar(--ui-primary). If the parent element'scolorproperty differs from--ui-primary, parts of the logo will render in different colors.Consider using
var(--ui-primary)consistently across all paths, orcurrentColorthroughout if the logo should inherit color from its context.♻️ Suggested fix for consistency
<path d="M377 200C379.16 200 381 198.209 381 196V103C381 103 386 112 395 127L434 194C435.785 197.74 439.744 200 443 200H470V50H443C441.202 50 439 51.4941 439 54V148L421 116L385 55C383.248 51.8912 379.479 50 376 50H350V200H377Z" - fill="currentColor" + fill="var(--ui-primary)" /> <path d="M726 92H739C742.314 92 745 89.3137 745 86V60H773V92H800V116H773V159C773 169.5 778.057 174 787 174H800V200H783C759.948 200 745 185.071 745 160V116H726V92Z" - fill="currentColor" + fill="var(--ui-primary)" />server/db/migrations/sqlite/meta/0002_snapshot.json (1)
277-295: Consider adding indexes on foreign key columns for query performance.The
loanstable has a foreign key onuser_book_idbut no index. Similarly,user_bookslacks indexes onuser_idandbook_id. As the dataset grows, queries joining these tables or filtering by these columns will benefit from indexes.This is a generated snapshot file, so the change should be made in the schema definition (
server/db/schema/domain.ts) and regenerated.server/db/schema/domain.ts (1)
27-34: Consider adding a composite unique constraint on(userId, bookId).The
userBooksjunction table allows inserting the same user-book pair multiple times. If a user should only own each book once, add a unique constraint to prevent duplicate entries.♻️ Suggested fix using Drizzle's unique constraint
+import { sqliteTable, text, integer, unique } from 'drizzle-orm/sqlite-core' -import { sqliteTable, text, integer } from 'drizzle-orm/sqlite-core' import { user } from './auth' // ... books table ... // User's book ownership (junction table) // Links users to books they own -export const userBooks = sqliteTable('user_books', { +export const userBooks = sqliteTable('user_books', { id: text('id').primaryKey(), userId: text('user_id').notNull().references(() => user.id, { onDelete: 'cascade' }), bookId: text('book_id').notNull().references(() => books.id, { onDelete: 'cascade' }), addedAt: integer('added_at', { mode: 'timestamp' }).notNull() -}) +}, (table) => [ + unique().on(table.userId, table.bookId) +])app/components/BookCard.vue (3)
2-12: UnusedbookIdprop.The
bookIdprop is defined but never used in the component. Either remove it or clarify its intended purpose.Suggested fix
interface Props { id: string - bookId: string title: string author: string isbn?: string | null coverPath?: string | null addedAt?: string | Date selected?: boolean selectable?: boolean }
84-88: Add keyboard accessibility to selectable mode.The selectable
<div>is only click-accessible. For keyboard users, addrole="button",tabindex="0", and handle@keydown.enter/@keydown.space.Suggested fix
<div v-else class="block cursor-pointer" + role="button" + tabindex="0" @click="handleClick" + @keydown.enter="handleClick" + @keydown.space.prevent="handleClick" >
63-68: Formatting:classattribute should be on a new line.Static analysis flagged this line. Consider reformatting for consistency.
Suggested fix
- <div v-else class="w-full h-full flex items-center justify-center bg-muted aspect-[1/1.5]"> + <div + v-else + class="w-full h-full flex items-center justify-center bg-muted aspect-[1/1.5]" + >package.json (2)
21-21: Consider moving@nuxt/test-utilsto devDependencies.Test utilities are typically development-only and shouldn't be bundled with production code.
Suggested fix
"dependencies": { ... - "@nuxt/test-utils": "3.22.0", ... }, "devDependencies": { "@nuxt/eslint": "^1.12.1", + "@nuxt/test-utils": "3.22.0", "drizzle-kit": "^0.31.8", ... },
14-30: Inconsistent version pinning strategy.Some packages use exact versions (e.g.,
"@nuxt/image": "2.0.0") while others use caret ranges (e.g.,"nuxt": "^4.2.2"). Consider standardizing for predictable builds—either pin all versions or use ranges consistently.server/db/schema/auth.ts (1)
43-50: Inconsistent nullable timestamps inverificationtable.
createdAtandupdatedAtare nullable here but required (.notNull()) in other tables. Verify this is intentional per Better Auth requirements.app/components/AppHeader.vue (1)
7-10: Consider adding error handling to sign-out flow.If
signOut()fails, the user is still redirected to/, potentially leaving them in an inconsistent auth state.🔧 Optional improvement
async function handleSignOut() { - await signOut() - router.push('/') + try { + await signOut() + } finally { + router.push('/') + } }app/pages/library/[id].vue (1)
237-269: Addrel="noopener noreferrer"to external links opened withtarget="_blank".
Prevents tabnabbing for the Open Library links.
-->app/pages/auth/login.vue (1)
90-92: Fix template formatting to satisfy CI (indentation/linebreak rules).
This is currently failing lint on Line 91.-->
server/api/books/[id]/index.get.ts (1)
22-31: Preserve original DB error details safely.
new Error(\Database error: ${error}`)often becomes[object Object]; preferString(error)(or includecause`) so logs are actionable.
-->server/api/books/lookup.post.ts (1)
8-10: Public endpoint: consider rate-limit + caching to protect OpenLibrary and your server.
Since this is unauthenticated, it’s an easy target for request bursts.Also applies to: 57-82
server/services/db.service.ts (1)
1-18: LGTM: clean Effect Tag/Layer facade for db access.
Optional: endpoints likeserver/api/books/lookup.post.tscould usegetDb/DbServiceinstead of importingdbdirectly for consistency with the DI setup.server/utils/effect.ts (1)
42-49:runEffectunused_eventparam: either use it or drop it for now.
If the goal is request-scoped dependencies, consider a follow-up pattern to inject per-request context; otherwise remove_eventto keep the API tight.app/pages/library/index.vue (1)
84-117: Bulk delete: preferallSettledso one failure doesn’t cancel the batch.
WithPromise.all(Line 97), a single failing delete prevents reporting partial success and skips cleanup logic you might want.Proposed fix (partial success reporting)
@@ - await Promise.all(deletePromises) + const results = await Promise.allSettled(deletePromises) + const successCount = results.filter(r => r.status === 'fulfilled').length + const failureCount = results.length - successCount @@ toast.add({ title: 'Books removed', - description: `${selectedBooks.value.size} book(s) removed from your library`, + description: failureCount === 0 + ? `${successCount} book(s) removed from your library` + : `${successCount} removed, ${failureCount} failed`, color: 'success' })server/utils/effectHandler.ts (3)
38-58: Consider logging unexpected errors before returning a generic 500.The
errorToH3Errorfunction silently swallows unrecognized errors by returning a generic message. In production, logging the original error (without exposing it to the client) would help with debugging.♻️ Suggested improvement
// Default to internal server error - const message = error instanceof Error ? error.message : 'Internal server error' + // Log unexpected errors for debugging (consider using a proper logger) + console.error('Unexpected error in effect handler:', error) + const message = 'Internal server error' return createError({ statusCode: 500, message })
91-98: Passing a dummy user whenrequireAuth: falseis error-prone.When
needsAuthisfalse, a dummy user object{ id: '', name: '', email: '' }is passed to the handler. This is misleading—handlers might inadvertently use this empty user assuming it's valid. Consider usingeffectHandlerPublicfor non-auth endpoints instead, or make the user parameter optional/nullable.♻️ Suggested alternative: Remove the `requireAuth` option
Since
effectHandlerPublicalready exists for public endpoints, consider removing therequireAuthoption fromeffectHandlerto avoid confusion:-export function effectHandler<A>( - handler: ( - event: H3Event, - user: { id: string, name: string, email: string } - ) => Effect.Effect<A, any, any>, - options: EffectHandlerOptions = {} -) { - const { requireAuth: needsAuth = true } = options +export function effectHandler<A>( + handler: ( + event: H3Event, + user: { id: string, name: string, email: string } + ) => Effect.Effect<A, any, any> +) { return defineEventHandler(async (event) => { const effect = Effect.gen(function* () { - let user: { id: string, name: string, email: string } - - if (needsAuth) { - user = yield* requireAuth(event) - } else { - user = { id: '', name: '', email: '' } - } - + const user = yield* requireAuth(event) return yield* handler(event, user) })
104-105: Consider refining the handler type signature to reduce reliance on type assertion.The assertion
as Effect.Effect<A, any, never>works correctly becauseMainLivedoes provide all services that handlers require:DbService,StorageService,AuthService, andBookRepository. However, the handler acceptsEffect.Effect<A, any, any>, allowing theoretically unbounded service requirements. A more type-safe approach would be to constrain the handler signature toEffect.Effect<A, any, MainServices>or similar, eliminating the need for the assertion entirely.server/services/auth.service.ts (2)
63-85: SimplifyrequireAuthby removing redundantEffect.flatMap.The
Effect.flatMap(..., user => Effect.succeed(user))is a no-op that adds unnecessary complexity.♻️ Simplified implementation
requireAuth: event => - Effect.flatMap( - Effect.tryPromise({ - try: async () => { - const session = await auth.api.getSession({ - headers: event.headers - }) - - if (!session) { - throw new UnauthorizedError({ message: 'Authentication required' }) - } - - return session.user as AuthUser - }, - catch: (error) => { - if (error instanceof UnauthorizedError) { - return error - } - return new UnauthorizedError({ message: 'Authentication failed' }) - } - }), - user => Effect.succeed(user) - ) + Effect.tryPromise({ + try: async () => { + const session = await auth.api.getSession({ + headers: event.headers + }) + + if (!session) { + throw new UnauthorizedError({ message: 'Authentication required' }) + } + + return session.user as AuthUser + }, + catch: (error) => { + if (error instanceof UnauthorizedError) { + return error + } + return new UnauthorizedError({ message: 'Authentication failed' }) + } + })
41-61: Consider reusingrequireAuthingetCurrentUserto reduce duplication.Both methods have nearly identical session-fetching logic.
getCurrentUsercould delegate torequireAuthand wrap the result.server/services/storage.service.ts (2)
27-79: Dynamic import is repeated in every method.Consider caching the imported
blobmodule or importing it once at the Layer level to avoid repeated dynamic imports on each operation.♻️ Alternative: Import once at Layer creation
-export const StorageServiceLive = Layer.succeed(StorageService, { - put: (pathname, data, options) => - Effect.tryPromise({ - try: async () => { - const { blob } = await import('hub:blob') +export const StorageServiceLive = Layer.effect(StorageService, + Effect.gen(function* () { + // Import blob module once + const { blob } = yield* Effect.promise(() => import('hub:blob')) + + return { + put: (pathname, data, options) => + Effect.tryPromise({ + try: async () => { + const result = await blob.put(pathname, data, {
44-44: Error message may produce[object Object]for non-Error exceptions.When
erroris not an Error instance, string interpolation will produce unhelpful messages.♻️ Suggested fix
- catch: error => new Error(`Failed to put blob: ${error}`) + catch: error => new Error(`Failed to put blob: ${error instanceof Error ? error.message : String(error)}`)Apply similarly to lines 53, 62, and 77.
server/repositories/openLibrary.repository.ts (4)
80-86: Remove dead code:extractWorkKeyFromUrlalways returnsnull.This function is declared but its implementation just returns
null. It's never used in the codebase.♻️ Remove unused function
-// Extract work key from edition key -function extractWorkKeyFromUrl(url?: string): string | null { - if (!url) return null - // URL like: https://openlibrary.org/books/OL24303521M/... - // We need to find the works link from the book data - return null -}
88-112:checkCoverExistslacks timeout protection.HEAD requests to external services can hang indefinitely. Consider adding an
AbortControllerwith a timeout.♻️ Add timeout to HEAD request
async function checkCoverExists(coverUrl: string): Promise<boolean> { try { const urlWithParam = coverUrl.includes('?') ? `${coverUrl}&default=false` : `${coverUrl}?default=false` - const response = await fetch(urlWithParam, { method: 'HEAD' }) + const controller = new AbortController() + const timeoutId = setTimeout(() => controller.abort(), 5000) + + const response = await fetch(urlWithParam, { + method: 'HEAD', + signal: controller.signal + }) + + clearTimeout(timeoutId)
114-240:lookupByISBNmakes up to 3 sequential API calls without timeout protection.The function fetches from
/api/books, then optionally/editions/{key}.json, then/works/{key}.json. Each call can hang, and failures in the optional calls are silently ignored. Consider:
- Adding timeouts to fetch calls
- Logging when optional enrichment fails (even if not propagating the error)
♻️ Add fetch helper with timeout
async function fetchWithTimeout(url: string, timeoutMs = 10000): Promise<Response> { const controller = new AbortController() const timeoutId = setTimeout(() => controller.abort(), timeoutMs) try { const response = await fetch(url, { signal: controller.signal }) return response } finally { clearTimeout(timeoutId) } }
242-287: Silently returningnullon storage failure may hide operational issues.Line 284 catches all blob storage errors and returns
null. While this prevents a missing cover from blocking book creation, it could mask persistent storage problems.Consider logging the error before returning
null:- Effect.catchAll(() => Effect.succeed(null)) + Effect.catchAll((error) => { + console.error('Failed to store cover in blob storage:', error) + return Effect.succeed(null) + })server/repositories/book.repository.ts (1)
104-200: Non-null assertions onbook!after conditional reassignment.After line 168 (
book = newBook), the code usesbook!multiple times. TypeScript can't track thatbookis definitely assigned after theif (!book)block. Consider refactoring to avoid the assertions.♻️ Refactor to avoid non-null assertions
- // If book doesn't exist, fetch from OpenLibrary and create it - if (!book) { + // If book doesn't exist, fetch from OpenLibrary and create it + const finalBook = book ?? (yield* Effect.gen(function* () { const openLibraryData = yield* lookupByISBN(isbn) - - // Download cover to local storage const coverPath = yield* downloadCover(isbn, 'L') - const newBookId = generateId() const now = new Date() - const newBook = { id: newBookId, // ... rest of newBook } - yield* Effect.tryPromise({ try: () => dbService.db.insert(books).values(newBook), catch: error => new BookCreateError({ message: `Failed to insert book: ${error}` }) }) - - book = newBook - } + return newBook + })) // Create userBooks entry // ... use finalBook instead of book!
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (64)
.env.example.github/workflows/ci.ymlAgents.mdPitch.mdREADME.mdapp/app.config.tsapp/app.vueapp/assets/css/main.cssapp/components/AppHeader.vueapp/components/AppLogo.vueapp/components/BookCard.vueapp/components/TemplateMenu.vueapp/composables/auth.tsapp/composables/use-auth-client.tsapp/layouts/default.vueapp/middleware/auth.tsapp/pages/auth/login.vueapp/pages/auth/register.vueapp/pages/index.vueapp/pages/library/[id].vueapp/pages/library/add.vueapp/pages/library/index.vuedrizzle.config.tsnuxt.config.tspackage.jsonpnpm-workspace.yamlrenovate.jsonserver/api/blob/[...pathname].get.tsserver/api/books/[id].delete.tsserver/api/books/[id]/index.get.tsserver/api/books/index.get.tsserver/api/books/index.post.tsserver/api/books/lookup.post.tsserver/db/auth-schema.tsserver/db/index.tsserver/db/migrations/0000_regular_malice.sqlserver/db/migrations/0001_jazzy_living_lightning.sqlserver/db/migrations/meta/0000_snapshot.jsonserver/db/migrations/meta/0001_snapshot.jsonserver/db/migrations/meta/_journal.jsonserver/db/migrations/sqlite/0000_equal_blade.sqlserver/db/migrations/sqlite/0001_glossy_shooting_star.sqlserver/db/migrations/sqlite/0002_jazzy_gauntlet.sqlserver/db/migrations/sqlite/meta/0000_snapshot.jsonserver/db/migrations/sqlite/meta/0001_snapshot.jsonserver/db/migrations/sqlite/meta/0002_snapshot.jsonserver/db/migrations/sqlite/meta/_journal.jsonserver/db/relations.tsserver/db/schema.tsserver/db/schema/auth.tsserver/db/schema/domain.tsserver/repositories/book.repository.tsserver/repositories/openLibrary.repository.tsserver/services/auth.service.tsserver/services/db.service.tsserver/services/storage.service.tsserver/tasks/db/migrate.tsserver/tsconfig.jsonserver/utils/auth.tsserver/utils/drizzle.tsserver/utils/effect.tsserver/utils/effectHandler.tsserver/utils/nitro-hooks.tstsconfig.json
💤 Files with no reviewable changes (15)
- server/db/migrations/meta/_journal.json
- server/db/index.ts
- server/utils/nitro-hooks.ts
- server/db/migrations/0001_jazzy_living_lightning.sql
- server/db/migrations/meta/0000_snapshot.json
- renovate.json
- server/tasks/db/migrate.ts
- app/composables/use-auth-client.ts
- server/db/migrations/0000_regular_malice.sql
- .env.example
- server/db/migrations/meta/0001_snapshot.json
- server/db/relations.ts
- server/tsconfig.json
- server/db/auth-schema.ts
- server/db/schema.ts
🧰 Additional context used
🧬 Code graph analysis (12)
server/api/books/index.get.ts (3)
server/utils/effectHandler.ts (1)
effectHandler(79-113)server/db/schema/auth.ts (1)
user(6-14)server/repositories/book.repository.ts (2)
getLibrary(311-312)UserBook(41-46)
server/db/schema/domain.ts (1)
server/db/schema/auth.ts (1)
user(6-14)
app/middleware/auth.ts (2)
server/db/schema/auth.ts (1)
session(16-25)app/composables/auth.ts (1)
authClient(3-4)
server/services/db.service.ts (1)
server/utils/effect.ts (3)
Context(52-52)Layer(52-52)Effect(52-52)
server/api/blob/[...pathname].get.ts (2)
server/utils/effectHandler.ts (1)
effectHandlerPublic(118-133)server/services/storage.service.ts (1)
getBlob(85-86)
server/api/books/[id].delete.ts (2)
server/utils/effectHandler.ts (1)
effectHandler(79-113)server/repositories/book.repository.ts (1)
removeFromLibrary(317-318)
server/api/books/lookup.post.ts (3)
server/utils/effectHandler.ts (1)
effectHandlerPublic(118-133)server/db/schema/domain.ts (1)
books(8-25)server/repositories/openLibrary.repository.ts (1)
lookupByISBN(291-292)
server/api/books/[id]/index.get.ts (3)
server/utils/effectHandler.ts (1)
effectHandler(79-113)server/services/db.service.ts (1)
DbService(10-10)server/db/schema/domain.ts (2)
userBooks(29-34)books(8-25)
server/services/auth.service.ts (3)
server/utils/effect.ts (4)
Data(52-52)Effect(52-52)Context(52-52)Layer(52-52)server/db/schema/auth.ts (2)
session(16-25)user(6-14)server/utils/auth.ts (1)
auth(6-27)
server/utils/effect.ts (5)
server/services/db.service.ts (2)
DbServiceLive(13-15)DbService(10-10)server/services/storage.service.ts (2)
StorageServiceLive(28-79)StorageService(25-25)server/services/auth.service.ts (2)
AuthServiceLive(41-86)AuthService(38-38)server/repositories/openLibrary.repository.ts (2)
OpenLibraryRepositoryLive(115-288)OpenLibraryRepository(70-73)server/repositories/book.repository.ts (2)
BookRepositoryLive(98-305)BookRepository(90-90)
server/repositories/book.repository.ts (4)
server/repositories/openLibrary.repository.ts (5)
BookNotFoundError(7-10)OpenLibraryApiError(12-14)OpenLibraryRepository(70-73)lookupByISBN(291-292)downloadCover(294-295)server/services/db.service.ts (1)
DbService(10-10)server/services/storage.service.ts (1)
StorageService(25-25)server/db/schema/domain.ts (2)
userBooks(29-34)books(8-25)
server/utils/effectHandler.ts (2)
server/utils/effect.ts (2)
Effect(52-52)MainLive(29-32)server/services/auth.service.ts (1)
requireAuth(92-93)
🪛 GitHub Actions: ci
app/components/AppHeader.vue
[warning] 45-45: ESLint: vue/max-attributes-per-line violation. 'class' should be on a new line. Command: pnpm run lint.
[warning] 46-46: ESLint: vue/max-attributes-per-line violation. 'class' should be on a new line. Command: pnpm run lint.
[error] 51-51: ESLint: @stylistic/no-multiple-empty-lines violation. More than 1 blank line not allowed. Command: pnpm run lint.
🪛 GitHub Check: ci (ubuntu-latest, 22)
app/pages/auth/login.vue
[failure] 91-91:
Expected indentation of 17 spaces but found 8 spaces
[warning] 90-90:
'icon' should be on a new line
[warning] 90-90:
'title' should be on a new line
[warning] 90-90:
':loading' should be on a new line
[warning] 90-90:
':fields' should be on a new line
[warning] 90-90:
Expected a linebreak before this attribute
[failure] 74-74:
Unexpected any. Specify a different type
app/components/BookCard.vue
[warning] 63-63:
'class' should be on a new line
app/pages/library/[id].vue
[failure] 87-87:
Unexpected any. Specify a different type
[failure] 64-64:
Trailing spaces not allowed
[failure] 55-55:
Trailing spaces not allowed
[failure] 50-50:
Trailing spaces not allowed
[failure] 40-40:
Trailing spaces not allowed
app/components/AppHeader.vue
[failure] 58-58:
Newline required at end of file but not found
[warning] 53-53:
'variant' should be on a new line
[warning] 53-53:
'class' should be on a new line
[failure] 51-51:
More than 1 blank line not allowed
[warning] 46-46:
'class' should be on a new line
[warning] 45-45:
'class' should be on a new line
app/pages/auth/register.vue
[failure] 94-94:
Unexpected any. Specify a different type
🪛 LanguageTool
Pitch.md
[style] ~51-~51: Replacing this phrase with a shorter alternative might make your text sound more refined.
Context: ...ion is your business. Your library data belongs to you. * Aesthetic Utility: A clean inte...
(BELONG_TO_PRP)
🪛 markdownlint-cli2 (0.18.1)
Pitch.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🔇 Additional comments (43)
.github/workflows/ci.yml (1)
14-25: LGTM! Action version upgrades are valid.The
actions/checkout@v6is the latest version (v6.0.1, last published December 2, 2025), andactions/setup-node@v6is also current as shown in the official documentation.The workflow setup is well-structured with appropriate caching via pnpm.
pnpm-workspace.yaml (1)
1-6: LGTM!The
ignoredBuiltDependenciesconfiguration is appropriate for avoiding unnecessary rebuilds of native/compiled dependencies during installation.tsconfig.json (1)
1-10: LGTM!This follows Nuxt 4's recommended TypeScript configuration using project references to the auto-generated tsconfig files.
server/db/migrations/sqlite/meta/_journal.json (1)
1-27: LGTM!Auto-generated migration journal from drizzle-kit. The structure correctly tracks the three SQLite migrations.
server/db/migrations/sqlite/meta/0002_snapshot.json (1)
453-516: Missing composite unique constraint onuser_bookstable.The
user_booksjunction table allows a user to add the same book multiple times since there's no unique constraint on(user_id, book_id). If a user should only have one entry per book, consider adding a composite unique constraint.Agents.md (1)
1-112: Good project documentation for onboarding.The documentation provides clear guidance on project architecture, conventions, and development workflow. This will help maintain consistency as the project grows.
nuxt.config.ts (2)
3-11: LGTM on module configuration.The module additions align well with the project requirements: NuxtHub for database/blob storage, UI components, image handling, and test utilities.
25-32: Verify image provider configuration.Setting
provider: 'none'disables Nuxt Image's optimization features. Ensure this is intentional—if cover images are served from blob storage, you may still benefit from image optimization. Alternatively, consider using theipxprovider for local optimization.server/db/schema/domain.ts (1)
8-25: Schema looks well-structured for the domain.The
bookstable design with unique ISBN for deduplication and optional metadata fields is appropriate. The timestamp handling withmode: 'timestamp'enables proper Date object mapping.One minor consideration: storing
subjectsas a JSON text string works for SQLite, but you'll need to handle serialization/deserialization in application code.server/db/migrations/sqlite/0000_equal_blade.sql (2)
18-28: Initial migration establishes base schema correctly.The
bookstable in this initial migration contains the core fields. The enhanced metadata columns (work_key,description,subjects, etc.) visible indomain.tsare presumably added in subsequent migrations (0001, 0002), which aligns with incremental schema evolution.
63-70: Verify foreign key execution order.The
user_bookstable references bothuserandbooks. Since SQLite's foreign key enforcement requires referenced tables to exist, ensure this migration runs afteruser(line 52) andbooks(line 18) table creation. The current ordering in this file handles this correctly.app/pages/library/add.vue (2)
40-67: LGTM!The lookup function properly handles loading state, clears previous results, and provides appropriate user feedback via toast notifications for both success and error scenarios.
69-99: LGTM!The add-to-library flow properly guards against invalid state, manages loading indicators, and provides clear user feedback with navigation on success.
app/composables/auth.ts (1)
6-34: LGTM!The
useAuthcomposable provides a clean abstraction over the auth client with properly typed methods for session management and authentication flows.server/db/schema/auth.ts (1)
6-25: LGTM!The
userandsessiontables are well-structured with appropriate constraints, unique indexes, and cascade delete behavior for the foreign key relationship.app/app.config.ts (1)
1-9: LGTM!The color configuration aligns with the new Libroo branding. Ensure the
royal-blueandvivid-ambercolor scales are properly defined in the CSS.app/layouts/default.vue (1)
1-13: LGTM!Clean and simple default layout structure. The flex column with
min-h-screenensures proper full-height rendering, and the slot pattern correctly allows page content to be injected.server/db/migrations/sqlite/0001_glossy_shooting_star.sql (1)
1-5: LGTM!Migration correctly adds nullable columns for book metadata. The use of
texttype forsubjectsandpublisherssuggests JSON-serialized storage, which is a reasonable approach for SQLite.server/db/migrations/sqlite/0002_jazzy_gauntlet.sql (1)
1-1: LGTM!Simple migration adding the
work_keycolumn for OpenLibrary work identification.server/api/books/index.get.ts (1)
9-34: LGTM!Clean implementation with proper pagination bounds. The response DTO appropriately includes only essential fields for list views, keeping the payload lightweight.
server/api/books/[id].delete.ts (1)
5-23: LGTM!Correct implementation with proper parameter validation and Effect-based error handling. The repository function signature matches the call order (
userBookId,userId).server/api/books/index.post.ts (1)
5-35: LGTM! Clean effect-based POST handler implementation.The handler correctly:
- Validates the request body with proper error handling
- Checks ISBN presence and type
- Delegates to the repository layer
- Returns a well-structured response
app/app.vue (1)
1-11: LGTM! Clean app root using Nuxt layout system.The simplified structure properly delegates UI to the layout system while keeping the
UAppwrapper for @nuxt/ui styling.server/api/blob/[...pathname].get.ts (2)
31-33: Good caching strategy for immutable blobs.The 1-year
max-agewithimmutableis appropriate for content-addressed storage. Make sure the blob pathnames include content hashes or are otherwise guaranteed to be immutable.
19-36: No changes needed - the return value is correct.The
getBlobservice returnsEffect.Effect<Blob | null, Error>, soblobDatais a standard WebBlobobject. Returning it directly is correct; the framework will handle the Blob response body properly. AccessingblobData.typeto set the Content-Type header is also correct, as it's a standard property on Blob objects. The code is working as intended.server/db/migrations/sqlite/meta/0000_snapshot.json (1)
367-374: SQLite boolean default representation.
email_verifiedisintegerbut default isfalse; confirm migration tooling maps this to0correctly in the emitted SQL.
-->app/assets/css/main.css (1)
1-61: Theme variable setup looks coherent for Nuxt UI.
Only thing to double-check: setting--ui-radius: 0remglobally is a strong design choice—confirm it’s intentional for all components.
-->server/api/books/[id]/index.get.ts (1)
46-61: Frontend type mismatch risk foraddedAt(string vs Date/number).
The page typesaddedAt: string, but this handler returnsrow.user_books.addedAt(likelyDatedepending on drizzle column mode). Consider normalizing to ISO string explicitly to lock the API contract.-->
app/pages/index.vue (1)
1-71: LGTM (copy + public homepage meta + auth links).
Just ensure your auth middleware/layout respectsdefinePageMeta({ auth: false })on this route.server/utils/effectHandler.ts (3)
1-10: LGTM!Clean imports and well-defined
HttpErrorMappinginterface for error translation.
12-36: LGTM!Error mappings are well-structured with appropriate HTTP status codes for each domain error type.
118-133: LGTM!
effectHandlerPublicprovides a clean way to handle public endpoints without authentication overhead.server/services/auth.service.ts (2)
1-8: LGTM!Clean imports and well-defined tagged error type for authentication failures.
10-29: LGTM!The
AuthUserandSessioninterfaces align with the database schema (server/db/schema/auth.ts) and Better Auth's session structure.server/services/storage.service.ts (2)
1-25: LGTM!Well-defined types and interface for blob storage operations.
81-92: LGTM!Helper effects follow the established pattern for service access.
server/repositories/openLibrary.repository.ts (3)
1-14: LGTM!Clean error types using Effect's
Data.TaggedErrorpattern.
16-62: LGTM!Comprehensive type definitions for OpenLibrary API responses.
290-295: LGTM!Helper effects follow the established pattern.
server/repositories/book.repository.ts (4)
1-14: LGTM!Clean imports with proper type-only imports for service dependencies.
30-63: LGTM!Well-structured output types with proper pagination support.
65-95: LGTM!Repository interface clearly defines the contract with proper Effect error types.
307-318: LGTM!Helper effects follow the established pattern for service access.
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Fix all issues with AI agents
In @app/middleware/auth.global.ts:
- Around line 1-18: The middleware references authClient but never imports it,
causing a runtime error; add the correct import for authClient at the top of the
file (matching how it’s exported from its module) and ensure you import any
required helpers used with it (e.g., useSession/useFetch usage stays unchanged);
verify whether authClient is a default or named export and update the import
statement accordingly so defineNuxtRouteMiddleware can call
authClient.useSession successfully.
In @app/pages/library/add.vue:
- Around line 33-35: The zod schema uses z.string('ISBN is required') which
won’t flag empty/missing fields in Zod 4; update the isbnSchema (and account for
formState.isbn initializing as an empty string) to assert presence explicitly by
replacing the error-parameter usage with a presence check such as
z.string().min(1, 'ISBN is required') and then keep the length constraint (e.g.,
.min(10, 'ISBN must be at least 10 characters').max(17, 'ISBN is too long')) so
the schema enforces both required and length validation for isbnSchema.
In @app/pages/library/index.vue:
- Around line 77-82: The loadMore flow currently just increments page (in
loadMore) which causes data to be refetched and replaces data.value/items;
change to maintain an accumulated list (e.g., allBooks ref) and update the UI to
use allBooks instead of data.items/books: create allBooks (ref<LibraryBook[]>),
watch data (or the fetch result) and if page.value === 1 replace allBooks.value,
otherwise append new items to allBooks.value; update loadMore to only increment
page (same) but ensure new fetch appends via the watcher; also ensure any
search/filter reset logic resets page.value to 1 and clears allBooks.value so
new queries start fresh; use identifiers loadMore, page, data, allBooks, books,
pagination in your changes.
- Around line 86-120: The deleteSelected function currently uses Promise.all
which aborts on the first failure and reports a generic error; change it to use
Promise.allSettled on the array built from
Array.from(selectedBooks.value).map(id => $fetch(`/api/books/${id}`, { method:
'DELETE' })), then compute how many settled as "fulfilled" vs "rejected",
collect failed ids (from the original selectedBooks list and the corresponding
settled results), and update the toast message to report successes and failures
(e.g., "X removed, Y failed" and include failed ids or error messages). Ensure
you still clear selectedBooks.value and toggle isSelectMode.value only for
successfully removed items (or leave selection for failed ids), keep
isDeleting.value toggled in finally, and call refresh() after handling partial
results.
In @server/api/blob/[...pathname].get.ts:
- Around line 5-37: The handler currently enforces authentication via
effectHandler/requireAuth but leaves the user param unused and sets a public
Cache-Control header; decide intended access model and adjust accordingly: if
blobs are public, switch to a non-authenticated handler (remove requireAuth
usage in effectHandler call or use a public variant) and keep the public
Cache-Control header; if blobs are private, remove the unused _user parameter
from the effectHandler callback, ensure requireAuth remains in effectHandler,
and change the setHeader call that sets "Cache-Control" to a private/no-cache
value appropriate for authenticated content. Reference
effectHandler/requireAuth, the unused _user parameter in the default export,
getBlob, and the setHeader lines when making the change.
In @server/api/books/[id]/index.get.ts:
- Line 5: The import in index.get.ts references symbols books and userBooks from
'hub:db:schema' which do not exist; open the schema module to discover the
actual exported table names (the ripgrep command suggested in the comment can
help), then either update this file's import to use the real exported
identifiers (e.g., whatever the schema actually exports) or add matching
re-exports in 'hub:db:schema' so that books and userBooks are exported; ensure
all subsequent code in index.get.ts uses the corrected identifiers consistently.
- Line 56: The code does an unsafe JSON.parse on bookData.subjects which can
throw; add a safe parser (e.g., safeJsonParse<T>(json: string | null): T | null)
or inline try-catch around JSON.parse and return null on failure, then replace
the current assignment subjects: bookData.subjects ?
JSON.parse(bookData.subjects) : null with subjects:
safeJsonParse(bookData.subjects) (or the inline try-catch result); ensure the
helper is exported/available in the same module or imported and that the
subjects property type allows null on parse failure.
In @server/api/books/lookup.post.ts:
- Line 58: The code uses a normalizedISBN for the local DB lookup but still
passes the raw body.isbn into lookupByISBN when building lookupEffect; update
the call so lookupByISBN receives normalizedISBN (not body.isbn) to ensure
consistent, normalized ISBN usage throughout (e.g., change the argument in
lookupEffect where lookupByISBN is invoked to normalizedISBN).
- Line 49: The inline JSON.parse of localBook.subjects in the response mapping
can throw on malformed JSON; update the mapping logic (where subjects:
localBook.subjects ? JSON.parse(localBook.subjects) : null) to safely parse:
wrap JSON.parse in a try/catch (or call a safeParse helper) and return null (or
an empty array) on parse errors, and optionally log the error via the existing
logger; make sure this change is applied where the response object for localBook
is constructed so the request cannot crash due to bad subjects JSON.
- Around line 30-36: The Effect.tryPromise wrapper around the DB lookup
(localBookEffect) swallows all errors by using catch: () => null, making it
impossible to tell "not found" from DB failures; change the catch to surface the
error (e.g., catch: (err) => { processLogger.error('DB lookup failed for ISBN',
err); throw err }) or simply rethrow the error so callers can distinguish
failures from a missing book, leaving only a null/undefined return for the
actual not-found case returned by db.select().from(books).where(eq(books.isbn,
normalizedISBN)).limit(1).
In @server/repositories/openLibrary.repository.ts:
- Line 167: bookData.notes can be an object, so change the description
assignment in openLibrary.repository.ts to normalize notes like description: if
bookData.notes is a string use it, otherwise use bookData.notes.value (or
notes?.value) and fall back to bookData.excerpts?.[0]?.text; also similarly
guard bookData.description if used elsewhere. Update the BookData interface (the
one that defines bookData / notes types) to allow notes?: string | { type?:
string; value?: string } so the compiler knows notes can be a string or an
object.
🧹 Nitpick comments (9)
app/pages/auth/login.vue (2)
26-30: Consider immediate check vs. watch for redirect.The watch with
{ immediate: true }will trigger on initial load. However, ifsessionis initiallyundefinedor loading, this may not redirect immediately. Ensure the session is properly initialized before this check runs to avoid a flash of the login page for authenticated users.
114-122: Placeholder link for "Forgot password" functionality.The "Forgot password" link points to
#, indicating incomplete functionality. Consider either implementing this feature or removing the link to avoid user confusion.Would you like me to open an issue to track implementing the password reset functionality?
server/utils/effectHandler.ts (2)
17-24: Consider adding error tags for completeness.The error mappings handle several domain errors. As the application grows, consider extracting these mappings to a separate configuration file for easier maintenance, or using a discriminated union type to ensure all error types are handled.
108-112: Use H3'sisError()utility for robust error detection instead of duck-typing.The property check
'statusCode' in result && 'message' in resultis fragile and may incorrectly identify unrelated objects as H3 errors. H3 provides a dedicated utility for this purpose.♻️ Proposed fix
+import { isError } from 'h3' + // ... - if (result && typeof result === 'object' && 'statusCode' in result && 'message' in result) { + if (isError(result)) { throw result }app/pages/library/add.vue (2)
58-66: Consider extracting the duplicate error message extraction.The same error message extraction pattern appears in both
lookupISBNandaddBookToLibrary. A shared helper would reduce duplication.Example helper
function extractErrorMessage(err: unknown, fallback: string): string { if (err instanceof Error) return err.message return (err as { data?: { message?: string } })?.data?.message || fallback }Also applies to: 92-100
15-28:existsLocallyflag is defined but not utilized in the UI.The
LookupResultinterface includesexistsLocally(line 27), which could be used to warn users that a book already exists in their library before they attempt to add it again. Consider displaying a notification or disabling the "Add to Library" button whenexistsLocallyis true.server/repositories/book.repository.ts (3)
205-215: Consider validating pagination parameters.If
pageis 0 or negative,offsetbecomes negative. IfpageSizeis 0,Math.ceil(totalItems / pageSize)causes division by zero returningInfinity.Consider adding validation or defensive defaults:
🛠️ Suggested validation
try: async () => { - const { page, pageSize } = pagination + const page = Math.max(1, pagination.page) + const pageSize = Math.max(1, pagination.pageSize) const offset = (page - 1) * pageSizeAlternatively, validate at the API layer before calling this method.
280-280: Type assertion suggests a type mismatch with the schema.The
book.createdAt as unknown as stringcast indicates the database may return a string rather than aDate. This is common with SQLite which stores dates as text.Consider handling this consistently by either:
- Configuring Drizzle to handle date serialization
- Creating a helper function for date normalization across all repositories
🛠️ Helper function approach
function toDate(value: Date | string | number): Date { return value instanceof Date ? value : new Date(value) }Then use:
createdAt: toDate(book.createdAt)
294-297: Misleading error property:bookIdcontains auserBookId.In
removeFromLibrary, theBookNotFoundErroris constructed withbookId: userBookId, which is semantically incorrect and could confuse debugging.🛠️ Suggested fix
Consider creating a dedicated
UserBookNotFoundErrortype, or extendBookNotFoundErrorto acceptuserBookId:export class BookNotFoundError extends Data.TaggedError('BookNotFoundError')<{ bookId?: string isbn?: string + userBookId?: string }> { }Then:
-return yield* Effect.fail(new BookNotFoundError({ bookId: userBookId })) +return yield* Effect.fail(new BookNotFoundError({ userBookId }))
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
Agents.mdapp/components/AppHeader.vueapp/components/BookCard.vueapp/middleware/auth.global.tsapp/pages/auth/login.vueapp/pages/auth/register.vueapp/pages/library/[id].vueapp/pages/library/add.vueapp/pages/library/index.vuenuxt.config.tsserver/api/blob/[...pathname].get.tsserver/api/books/[id]/index.get.tsserver/api/books/lookup.post.tsserver/repositories/book.repository.tsserver/repositories/openLibrary.repository.tsserver/utils/effectHandler.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- app/pages/library/[id].vue
- app/components/AppHeader.vue
- nuxt.config.ts
- app/pages/auth/register.vue
- Agents.md
🧰 Additional context used
🧬 Code graph analysis (3)
server/repositories/openLibrary.repository.ts (3)
server/repositories/book.repository.ts (1)
BookNotFoundError(17-20)server/utils/effect.ts (4)
Data(52-52)Effect(52-52)Context(52-52)Layer(52-52)server/services/storage.service.ts (2)
StorageService(25-25)putBlob(82-83)
app/middleware/auth.global.ts (2)
server/db/schema/auth.ts (1)
session(16-25)app/composables/auth.ts (1)
authClient(3-4)
server/api/blob/[...pathname].get.ts (2)
server/utils/effectHandler.ts (1)
effectHandler(86-116)server/services/storage.service.ts (1)
getBlob(85-86)
🪛 GitHub Actions: ci
server/api/books/[id]/index.get.ts
[error] 5-5: TypeScript error TS2305: Module '"hub:db:schema"' has no exported member 'books'.
🪛 GitHub Check: ci (ubuntu-latest, 22)
server/api/books/lookup.post.ts
[failure] 4-4:
Module '"hub:db:schema"' has no exported member 'books'.
[failure] 4-4:
Module '"hub:db:schema"' has no exported member 'books'.
server/api/books/[id]/index.get.ts
[failure] 5-5:
Module '"hub:db:schema"' has no exported member 'userBooks'.
[failure] 5-5:
Module '"hub:db:schema"' has no exported member 'books'.
[failure] 5-5:
Module '"hub:db:schema"' has no exported member 'userBooks'.
[failure] 5-5:
Module '"hub:db:schema"' has no exported member 'books'.
server/repositories/book.repository.ts
[failure] 14-14:
Module '"hub:db:schema"' has no exported member 'userBooks'.
[failure] 14-14:
Module '"hub:db:schema"' has no exported member 'books'.
[failure] 14-14:
Module '"hub:db:schema"' has no exported member 'userBooks'.
[failure] 14-14:
Module '"hub:db:schema"' has no exported member 'books'.
🔇 Additional comments (21)
app/pages/auth/login.vue (3)
17-23: Good security practice with redirect path validation.The regex
/^\/(?!\/)/.test(redirect)correctly prevents open redirects by ensuring the path starts with a single/and not//(which could be interpreted as a protocol-relative URL).
58-90: LGTM!The submit handler properly handles both API errors (via
result.error) and exceptions, shows appropriate toast notifications, and manages loading state correctly with thefinallyblock.
51-54: Verify the project's actual Zod version compatibility.The code uses
z.email()syntax, which is a Zod 4 feature (top-level format). In Zod 3, this would bez.string().email()instead. Confirm the project'spackage.jsonspecifies Zod 4 as a dependency; if using Zod 3, the code will fail at runtime.server/utils/effectHandler.ts (1)
42-67: LGTM on centralized error handling.The
handleErrorfunction provides a clean pattern for converting domain errors to HTTP errors. The use of Effect.logError for structured logging is appropriate. The tag-based error mapping is extensible.server/api/blob/[...pathname].get.ts (1)
31-33: Long cache duration is appropriate for immutable content.The 1-year cache with
immutabledirective is a good practice for content-addressable storage where the pathname includes a hash. Ensure pathnames are indeed content-addressed; otherwise, stale content could be served.server/api/books/[id]/index.get.ts (1)
23-34: Good ownership verification pattern.The query correctly enforces ownership by joining on
userIdand checking against the authenticated user. This prevents unauthorized access to other users' books.app/components/BookCard.vue (2)
1-37: LGTM - Clean component setup with proper TypeScript typing.The props interface, defaults, and event emission are well-structured. The
coverUrlcomputed property correctly handles the blob path prefix.Minor observation: the
bookIdprop (line 4) appears unused within this component - verify if it's needed for parent component logic or if it can be removed.
40-144: Dual rendering modes are well-implemented.The template correctly separates navigable and selectable modes with appropriate event handling. The selection indicator with visual feedback (ring/shadow) provides good UX.
app/pages/library/add.vue (1)
113-346: Well-structured two-step UI flow.The template cleanly separates the lookup and confirmation steps with appropriate loading states, error feedback via toasts, and clear visual hierarchy. Good use of Nuxt UI components.
app/pages/library/index.vue (1)
1-75: Good implementation of selection mode and UI states.The selection logic with Set-based tracking, toggle functions, and visual feedback is well-implemented. The loading/empty states and responsive grid provide good UX.
Also applies to: 121-262
server/repositories/openLibrary.repository.ts (4)
7-14: Error types follow the codebase pattern correctly.The
BookNotFoundErrorandOpenLibraryApiErrorextendData.TaggedErrorconsistently with the existing pattern inbook.repository.ts.
80-104: Cover existence check is thorough.The
checkCoverExistsfunction properly handles thedefault=falseparameter and content-length check to detect placeholder images. Good defensive implementation.
234-279: Cover download with WebP conversion is well-implemented.The graceful degradation (returning
nullon failure rather than propagating errors) is appropriate for non-critical cover images. The sharp conversion to WebP with quality setting is a good optimization.
106-232: Complex but well-structured lookup implementation.The multi-stage fetch (books API → edition → works) with proper error handling and data enrichment is comprehensive. The Effect pattern with
tryPromiseand typed error propagation is correctly implemented.One consideration: the sequential cover URL validation (lines 146-151) adds latency. For a future optimization, these could be parallelized, but the current approach is acceptable for correctness.
server/api/books/lookup.post.ts (1)
4-4: The importimport { books } from 'hub:db:schema'is valid. Thehub:import alias is automatically configured by the@nuxthub/coremodule (loaded innuxt.config.ts) based on thehubconfiguration. Thebooksexport exists inserver/db/schema/domain.tsand is successfully used across multiple files without build or runtime failures.Likely an incorrect or invalid review comment.
server/repositories/book.repository.ts (6)
16-28: LGTM!Error types are well-structured using Effect's
Data.TaggedErrorpattern, enabling type-safe error handling and pattern matching.
30-63: LGTM!Well-defined interfaces with a reusable generic
PaginatedResult<T>type for pagination.
65-87: LGTM!The interface properly declares Effect signatures with comprehensive error types and service dependencies in the requirements channel.
89-95: LGTM!Standard Effect context tag pattern, and
crypto.randomUUID()is appropriate for generating unique IDs.
303-314: LGTM!Idiomatic Effect-TS helper functions that provide convenient access to repository methods.
14-14: No issue found.The
booksanduserBookstables are properly exported fromserver/db/schema/domain.ts. Thehub:db:schemapath alias is provided by NuxtHub and is correctly resolving throughout the codebase. The imports are valid and will not cause a CI failure.Likely an incorrect or invalid review comment.
app/pages/library/add.vue
Outdated
| const isbnSchema = z.object({ | ||
| isbn: z.string('ISBN is required').min(10, 'ISBN must be at least 10 characters').max(17, 'ISBN is too long') | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and content
fd -e vue app/pages/library/add.vueRepository: niklhut/libroo
Length of output: 40
🌐 Web query:
Zod 4 z.string() error message parameter required field validation
💡 Result:
Use the new unified error param and check whether the input is undefined. Example:
const schema = z.string({
error: (issue) => issue.input === undefined ? 'This field is required' : 'Must be a string'
});
- The Zod v4 API unifies previous required_error/invalid_type_error into the single error option (the error function receives the issue and can return a string). [1][2][3]
Sources:
[1] Zod API docs (Defining schemas).
[2] Zod v4 migration / changelog (shows required_error removed and example using error).
[3] Zod custom-error docs (example using issue.input === undefined).
🏁 Script executed:
fd add.vueRepository: niklhut/libroo
Length of output: 83
🏁 Script executed:
# Also check the repository structure
git ls-files | grep -E "add\.(vue|ts|js)$"Repository: niklhut/libroo
Length of output: 83
🏁 Script executed:
cat -n app/pages/library/add.vue | head -50Repository: niklhut/libroo
Length of output: 1534
Zod 4 requires unified error parameter for required-field validation.
In Zod 4, z.string('ISBN is required') passes the string to the error parameter for type coercion errors only—when non-strings are coerced to strings. This message won't appear for missing/undefined fields. Since formState.isbn initializes as an empty string, use .min(1, 'ISBN is required') to catch empty input, or use the unified error parameter with a function checking issue.input === undefined for complete control over both required and type-error messages.
🤖 Prompt for AI Agents
In @app/pages/library/add.vue around lines 33 - 35, The zod schema uses
z.string('ISBN is required') which won’t flag empty/missing fields in Zod 4;
update the isbnSchema (and account for formState.isbn initializing as an empty
string) to assert presence explicitly by replacing the error-parameter usage
with a presence check such as z.string().min(1, 'ISBN is required') and then
keep the length constraint (e.g., .min(10, 'ISBN must be at least 10
characters').max(17, 'ISBN is too long')) so the schema enforces both required
and length validation for isbnSchema.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/ci.yml:
- Around line 25-32: The CI workflow runs the "Lint" step before "Prepare",
which can fail if postinstall generates artifacts the linter needs; reorder the
steps so the "Prepare" step (run: pnpm run postinstall) executes before "Lint"
(run: pnpm run lint), keeping "Typecheck" after both, to ensure generated type
definitions/stubs exist before linting.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
11-19: No action required — all specified versions exist and are current.
actions/checkout@v6(latest: v6.0.2),actions/setup-node@v6(latest: v6.1.0), and Node.js 24 (LTS "Krypton") are all released and available as of January 2026.
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 14-32: The dependency versions in package.json are inconsistently
pinned (some use exact versions like "@nuxt/image": "2.0.0" and "@nuxt/scripts":
"0.13.2", while others use caret ranges like "@nuxt/ui": "^4.3.0"), causing
unpredictable installs; pick a consistent strategy (either switch all entries to
exact versions for reproducible installs or use caret ranges for flexibility),
update the dependency entries such as "@nuxt/image", "@nuxt/scripts",
"@nuxt/ui", "@nuxt/hints", "@nuxt/test-utils", etc. to follow that chosen
strategy, and ensure you commit the lockfile (package-lock.json or
pnpm-lock.yaml/yarn.lock) to guarantee deterministic installs.
- Line 23: Move the "@nuxt/test-utils" entry out of dependencies into
devDependencies in package.json: remove the "@nuxt/test-utils": "3.22.0" line
from the top-level "dependencies" object and add the same entry under
"devDependencies" so test utilities are not bundled in production; after
editing, run your package manager (npm/yarn/pnpm) to update lockfile and
node_modules.
- Around line 41-42: Update the packageManager field value from "pnpm@10.26.1"
to "pnpm@10.27.0" (or a later 10.27+ version) in package.json to address
CVE-2025-69262; after changing the packageManager entry, run pnpm install to
regenerate the lockfile (pnpm-lock.yaml) so the lockfile reflects the upgraded
pnpm version.
In `@server/repositories/book.repository.ts`:
- Line 2: The import of HttpClient in server/repositories/book.repository.ts is
type-only and should use a type-only import to satisfy the compiler/CI; change
the statement importing HttpClient to a type import (e.g., use "import type {
HttpClient } from '...'" for the symbol HttpClient) and ensure any usages remain
only in type positions (function signatures or type annotations) so no runtime
import is emitted.
In `@server/services/auth.service.ts`:
- Around line 37-69: Extract the duplicate session-fetching logic inside
AuthServiceLive into a shared helper (e.g., fetchSession or getSessionFromEvent)
that calls auth.api.getSession({ headers: event.headers }) via Effect.tryPromise
and throws the same UnauthorizedError on failure or missing session; then update
AuthServiceLive.getCurrentUser to call that helper and return the Session, and
update AuthServiceLive.requireAuth to call the same helper and return
session.user as AuthUser. Ensure the helper reuses the same error types/messages
used currently (UnauthorizedError) and reference auth.api.getSession,
getCurrentUser, and requireAuth when making the replacements.
In `@server/services/book.service.ts`:
- Line 2: The import of HttpClient in book.service.ts is only used for type
annotations; replace the value import with a type-only import by changing the
import statement to use "import type" for HttpClient (e.g., update the import
that currently references HttpClient from '@effect/platform' to an import type
for HttpClient) so the compiler/tree-shaker treats it as a type-only import.
In `@server/utils/effect.ts`:
- Around line 88-93: The handleError function uses duck-typing to detect H3
errors; replace that check with H3's isError utility for consistency with
runEffect: import and call isError(error) in handleError and, when true, return
error as H3Error; keep the rest of the Effect.gen flow unchanged and ensure the
same H3Error typecast is used.
♻️ Duplicate comments (8)
app/pages/library/[id].vue (1)
49-73: Consider adding confirmation before removing a book.The
removeBookfunction performs an irreversible destructive action without user confirmation. This was flagged in a previous review as a nitpick but remains unaddressed.♻️ Example using a confirmation pattern
async function removeBook() { + if (!confirm('Are you sure you want to remove this book from your library?')) { + return + } + try { await $fetch(`/api/books/${userBookId}`, { method: 'DELETE' })server/repositories/openLibrary.repository.ts (2)
45-46: Thenotesfield may be an object, not just a string.OpenLibrary API can return
notesas either a string or{ type, value }object (similar todescriptionon lines 62-63). Line 160 only handles the string case, which could result in[object Object]being displayed.🐛 Proposed fix
Update the interface:
- notes?: string + notes?: string | { type: string; value: string }And update line 160:
- let description = bookData.notes || bookData.excerpts?.[0]?.text + let description = ( + typeof bookData.notes === 'string' + ? bookData.notes + : bookData.notes?.value + ) || bookData.excerpts?.[0]?.text
1-5: Missing import:putBlobis not imported but used on line 274.The
putBlobhelper fromstorage.service.tsis called indownloadCoverbut not imported. Additionally,StorageServiceis referenced in the interface type on line 72 but not imported.🐛 Proposed fix
import { Context, Effect, Layer, Data, Duration } from 'effect' import { HttpClient } from '@effect/platform' import * as HCError from '@effect/platform/HttpClientError' import type { HttpClient as HttpClientType } from '@effect/platform' import sharp from 'sharp' +import { putBlob, StorageService } from '../services/storage.service'Agents.md (1)
1-72: LGTM! Comprehensive onboarding documentation.The agent instructions document is well-structured, covering the 3-tier architecture, Effect-TS patterns, and development workflow. The guardrails (no fat handlers, local-first lookups, etc.) align with the PR's implementation.
Note: The missing trailing newline at line 72 was already flagged in a previous review.
server/repositories/book.repository.ts (2)
363-363: WrapJSON.parseto handle malformed data.If
bookData.subjectscontains invalid JSON, this will throw an unhandled exception. This was flagged in a previous review.Proposed safe parsing helper
function safeJsonParse<T>(json: string | null): T | null { if (!json) return null try { return JSON.parse(json) } catch { return null } } // Usage: subjects: safeJsonParse<string[]>(bookData.subjects),
184-197: ReturnedBookobject is missing optional fields.The
Bookinterface (lines 27-41) includes optional fields (description,subjects,publishDate,publishers,numberOfPages,workKey), but the returned object only includes base fields. This inconsistency could cause issues for consumers expecting complete data.Include all Book fields
return { id: userBookId, bookId: book.id, book: { id: book.id, isbn: book.isbn, title: book.title, author: book.author, coverPath: book.coverPath, openLibraryKey: book.openLibraryKey, - createdAt: book.createdAt + createdAt: book.createdAt, + description: book.description ?? null, + subjects: book.subjects ?? null, + publishDate: book.publishDate ?? null, + publishers: book.publishers ?? null, + numberOfPages: book.numberOfPages ?? null, + workKey: book.workKey ?? null }, addedAt }The same issue applies to
getLibrary(lines 241-250) andgetBookById(lines 285-293).server/services/book.service.ts (1)
140-140: WrapJSON.parseto handle malformed data.Same issue as in the repository - if
localBook.subjectscontains invalid JSON, this will throw an unhandled exception.Use a shared safe parsing utility:
subjects: safeJsonParse<string[]>(localBook.subjects),server/utils/effect.ts (1)
36-40: Redundant layer provisioning inMainLive.
ServicesLivealready containsRepositoriesLivewhich containsBaseServicesLive. ProvidingBaseServicesLiveagain inMainLivecreates unnecessary redundancy.Simplify MainLive
// Combined live layer for all services -export const MainLive = Layer.provideMerge( - ServicesLive, - BaseServicesLive -) +export const MainLive = ServicesLiveAlternatively, if
BaseServicesLiveneeds to be explicitly available at the top level for some reason, the current approach works but is redundant.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
Agents.mdapp/pages/library/[id].vueapp/pages/library/index.vuepackage.jsonserver/repositories/book.repository.tsserver/repositories/openLibrary.repository.tsserver/services/auth.service.tsserver/services/book.service.tsserver/services/storage.service.tsserver/utils/effect.tsserver/utils/effectHandler.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-13T21:08:13.933Z
Learnt from: niklhut
Repo: niklhut/libroo PR: 1
File: server/utils/effect.ts:30-38
Timestamp: 2026-01-13T21:08:13.933Z
Learning: In Effect-TS code, when combining service tags (Context.Tag) for multiple services, use a union type (Context1.Tag | Context2.Tag) rather than an intersection (&). Service tags are nominal types and cannot be meaningfully intersected; a union correctly represents combined service requirements in the Effect<R> parameter, while runtime Layer composition handles provisioning. Apply this guidance to TypeScript files that define or compose Effect-TS service contexts (e.g., server/utils/effect.ts and similar files).
Applied to files:
server/services/auth.service.tsserver/utils/effectHandler.tsserver/repositories/openLibrary.repository.tsserver/utils/effect.tsserver/services/book.service.tsserver/services/storage.service.tsserver/repositories/book.repository.ts
📚 Learning: 2026-01-13T21:24:30.028Z
Learnt from: niklhut
Repo: niklhut/libroo PR: 1
File: shared/utils/schemas.ts:11-13
Timestamp: 2026-01-13T21:24:30.028Z
Learning: In TypeScript files using Zod v4.3.5 or newer, use the error option for both schema-level configuration and validators (min, max, etc.). The message parameter is deprecated. Example: z.string({ error: 'Required' }).min(10, { error: 'Too short' }). Apply this pattern to all Zod schema definitions in TS files to avoid deprecated API usage.
Applied to files:
server/services/auth.service.tsserver/utils/effectHandler.tsserver/repositories/openLibrary.repository.tsserver/utils/effect.tsserver/services/book.service.tsserver/services/storage.service.tsserver/repositories/book.repository.ts
🧬 Code graph analysis (6)
server/services/auth.service.ts (3)
server/utils/effect.ts (4)
Data(134-134)Effect(134-134)Context(134-134)Layer(134-134)server/db/schema/auth.ts (1)
session(16-25)server/utils/auth.ts (1)
auth(6-27)
server/utils/effectHandler.ts (3)
server/db/schema/auth.ts (1)
user(6-14)server/utils/effect.ts (3)
Effect(134-134)MainServices(43-50)runEffect(116-131)server/services/auth.service.ts (1)
requireAuth(75-76)
server/repositories/openLibrary.repository.ts (2)
server/utils/effect.ts (4)
Data(134-134)Effect(134-134)Context(134-134)Layer(134-134)server/services/storage.service.ts (2)
StorageService(31-31)putBlob(100-101)
server/utils/effect.ts (6)
server/services/db.service.ts (2)
DbServiceLive(13-15)DbService(10-10)server/services/storage.service.ts (2)
StorageServiceLive(34-97)StorageService(31-31)server/services/auth.service.ts (2)
AuthServiceLive(41-69)AuthService(38-38)server/repositories/openLibrary.repository.ts (2)
OpenLibraryRepositoryLive(118-290)OpenLibraryRepository(76-79)server/repositories/book.repository.ts (2)
BookRepositoryLive(92-374)BookRepository(84-84)server/services/book.service.ts (2)
BookServiceLive(51-176)BookService(47-47)
server/services/storage.service.ts (1)
server/utils/effect.ts (4)
Data(134-134)Effect(134-134)Context(134-134)Layer(134-134)
server/repositories/book.repository.ts (5)
server/repositories/openLibrary.repository.ts (3)
OpenLibraryBookNotFoundError(8-11)OpenLibraryApiError(13-15)OpenLibraryRepository(76-79)server/services/db.service.ts (1)
DbService(10-10)shared/types/pagination.ts (2)
PaginationParams(1-4)PaginatedResult(6-15)server/db/schema/domain.ts (2)
books(8-25)userBooks(29-34)shared/types/book.ts (1)
BookDetails(31-46)
🪛 GitHub Actions: Typecheck and Lint
server/repositories/book.repository.ts
[error] 2-2: ESLint: All imports in the declaration are only used as types. Use 'import type'.
🪛 GitHub Check: typecheck
server/repositories/book.repository.ts
[failure] 324-324:
Unexpected parentheses around single function argument having a body with no curly braces
[failure] 304-304:
Unexpected parentheses around single function argument having a body with no curly braces
[failure] 274-274:
Unexpected parentheses around single function argument having a body with no curly braces
[failure] 232-232:
Unexpected parentheses around single function argument having a body with no curly braces
[failure] 212-212:
Unexpected parentheses around single function argument having a body with no curly braces
[failure] 181-181:
Unexpected parentheses around single function argument having a body with no curly braces
[failure] 163-163:
Unexpected parentheses around single function argument having a body with no curly braces
[failure] 127-127:
Unexpected parentheses around single function argument having a body with no curly braces
[failure] 109-109:
Unexpected parentheses around single function argument having a body with no curly braces
[failure] 2-2:
All imports in the declaration are only used as types. Use import type
🔇 Additional comments (24)
app/pages/library/[id].vue (3)
1-19: LGTM! Well-structured data fetching and cover URL computation.The
useFetchwith cookie forwarding for authenticated requests and thecoverUrlcomputed property are implemented correctly.
20-43: LGTM! TheformatDatelogic is now correct.The function properly handles year-only strings (e.g., "2015") by checking with the regex first, before attempting Date parsing. This addresses the earlier concern where year-only strings would be parsed as January 1st of that year.
76-252: LGTM! Well-structured template with proper loading, error, and content states.The template correctly handles all UI states (loading, not-found, book details) and renders the book information with proper responsiveness. The external links to OpenLibrary are implemented correctly with
target="_blank".app/pages/library/index.vue (6)
1-33: LGTM! Well-structured state management and data fetching.The type imports, pagination state, and
useFetchwith computed query params are properly set up. UsingshallowReffor theSetis appropriate.
35-53: LGTM! Accumulated books pattern is correctly implemented.The watcher properly handles pagination by replacing items on page 1 and appending on subsequent pages, which addresses the previous "Load More replaces items" issue.
55-81: LGTM! Selection functions now use proper reactivity patterns.All selection functions (
toggleSelectMode,toggleBookSelection,toggleSelectAll) correctly reassign the Set to trigger reactivity, addressing the previousshallowRef.clear()issues.
83-94: LGTM! Load more with proper loading state.The
loadMorefunction correctly manages theisLoadingMorestate and guards against concurrent requests.
96-161: LGTM! Batch delete with proper partial failure handling.The implementation correctly uses a batch delete endpoint and handles partial success/failure scenarios with appropriate user feedback. The previous concerns about
Promise.allpartial failure are addressed by using a dedicated batch endpoint that returnsremovedIdsandfailedIds.
164-305: LGTM! Well-structured template with comprehensive UI states.The template properly handles selection mode, loading states, empty states, book grid with selectable cards, and pagination controls. The "Load More" button correctly shows loading state via
:loading="isLoadingMore".server/services/auth.service.ts (2)
1-35: LGTM! Well-designed error types and interfaces.The
UnauthorizedErrorusingData.TaggedErrorfollows Effect-TS patterns correctly. TheAuthUserandSessioninterfaces are clearly defined and align with Better Auth's session structure.
71-76: LGTM! Helper effects follow Effect-TS idioms correctly.The
flatMappattern for accessing the service and delegating to its methods is the idiomatic way to create helper effects in Effect-TS.server/repositories/openLibrary.repository.ts (2)
86-115: LGTM! Helper functions with proper timeouts and error handling.The
fetchJsonandcheckCoverExistshelpers are well-implemented with 5-second timeouts usingDuration.seconds(5), which addresses the previous timeout concerns.
117-217: LGTM!lookupByISBNis well-structured with proper enrichment fallbacks.The implementation correctly:
- Normalizes ISBN and queries OpenLibrary
- Handles description as both string and object types (lines 180-185)
- Merges subjects from works data when needed
- Uses
Effect.catchAllwithEffect.logDebugfor optional enrichment failuresserver/services/storage.service.ts (3)
1-31: LGTM! Well-designed storage service interface and error types.The
StorageErrorwith operation context,BlobMetadatainterface, andStorageServiceInterfaceare cleanly defined following Effect-TS patterns.
33-97: LGTM! Robust live implementation with lazy imports.The
StorageServiceLiveimplementation correctly:
- Uses dynamic import for
hub:blobto avoid build-time issues- Maps errors to
StorageErrorwith operation context- Normalizes timestamps in
listresults (line 94)- Uses
Effect.tryPromisefor proper error handling
99-110: LGTM! Helper effects follow Effect-TS conventions.The
putBlob,getBlob,deleteBlob, andlistBlobshelpers use the idiomaticEffect.flatMappattern for service access.server/utils/effectHandler.ts (1)
23-41: LGTM! Clean Effect-based handler wrapper.The implementation correctly:
- Enforces authentication via
requireAuthbefore invoking the handler- Composes the Effect pipeline with proper dependency injection
- Delegates error handling and execution to
runEffectThe design aligns well with the 3-tier architecture documented in Agents.md.
server/utils/effect.ts (1)
115-131: LGTM! Well-structured effect runner with proper error handling.The
runEffectfunction correctly:
- Provides the
MainLivelayer- Catches all errors and converts them to H3 errors
- Throws H3 errors for proper HTTP response handling
This addresses concerns from previous reviews about unhandled errors.
server/repositories/book.repository.ts (1)
91-198: LGTM! Well-structuredaddBookByISBNimplementation.The method correctly:
- Checks for existing ownership before adding
- Reuses shared book records when available
- Fetches from OpenLibrary and downloads covers for new books
- Uses proper error types for different failure scenarios
The Effect-based error handling with
DatabaseError,BookAlreadyOwnedError, andBookCreateErrorprovides good granularity.server/services/book.service.ts (2)
93-120: LGTM! Well-implemented batch deletion with bounded concurrency.The implementation correctly:
- Uses bounded concurrency (
concurrency: 10) to avoid overwhelming the database- Wraps each operation in
Eitherto capture individual success/failure- Properly yields
Effect.logErrorto execute the logging effect- Returns a structured result with
removedIdsandfailedIdsThis addresses concerns from previous reviews.
125-173: LGTM! Clean local-first lookup implementation.The
lookupBookmethod follows the documented "local-first" pattern from Agents.md:
- First checks
BookRepositoryfor existing local data- Falls back to
OpenLibraryRepositoryif not found locally- Gracefully handles
OpenLibraryBookNotFoundErrorwith a proper fallback responseThe ISBN normalization at line 127 is correctly applied before both the local lookup and OpenLibrary call.
package.json (3)
2-2: LGTM on project rename and lint:fix script.The project rename to "librooapp" aligns with the Libroo rebranding, and the
lint:fixscript is a useful addition for developer workflow.Also applies to: 11-11
20-20: Alpha dependency in production.
@nuxt/hintsat version1.0.0-alpha.5is an alpha release. Alpha versions may have breaking changes or instability. Ensure this is intentional and consider pinning to a stable release when available.
32-32: No action required — codebase is already compatible with Zod v4.The Zod schemas in
shared/utils/schemas.tscorrectly use the v4 unified error API with theerrorparameter (e.g.,z.string({ error: '...' }),.min(10, { error: '...' })). No deprecated v3 patterns detected. The version specified in package.json ("zod": "^4.3.5") is the current latest stable release.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@server/repositories/book.repository.ts`:
- Around line 376-387: The module exports helper Effect wrappers for several
BookRepository methods but omits helpers for findByIsbn and
getUserBookWithDetails; add exported helper functions named findByIsbn and
getUserBookWithDetails that call Effect.flatMap(BookRepository, repo =>
repo.findByIsbn(isbn)) and Effect.flatMap(BookRepository, repo =>
repo.getUserBookWithDetails(userBookId, userId)) respectively so all interface
methods have consistent helpers and can be imported elsewhere.
In `@server/services/book.service.ts`:
- Around line 62-70: The mapping from repository userBook to LibraryBook is
duplicated in getUserLibrary and addBookToLibrary; extract a single helper
function (e.g., toLibraryBook(userBook): LibraryBook) that constructs { id,
bookId, title: userBook.book.title, author: userBook.book.author, isbn:
userBook.book.isbn, coverPath: userBook.book.coverPath, addedAt:
userBook.addedAt } and replace the inline mappings in both getUserLibrary and
addBookToLibrary to call toLibraryBook(userBook); declare the helper near the
top of the module with the correct repository input type (or export it if needed
for tests) to ensure type safety and reuse.
- Around line 163-169: The not-found handler inside
Effect.catchTag('OpenLibraryBookNotFoundError') returns the original isbn
instead of the normalizedISBN, causing inconsistency with successful lookups;
update the returned object in that handler to use normalizedISBN (the same
normalized value used on success) so the BookLookupResult's isbn field is
consistent with other responses.
In `@server/services/storage.service.ts`:
- Around line 36-54: The dynamic import of 'hub:blob' inside Effect.gen can
throw before Effect.tryPromise runs, so wrap the entire async operation (both
import('hub:blob') and the blob.put call) in a single Effect.tryPromise (or
perform a try/catch inside Effect.gen and convert any thrown error to a
StorageError) so import failures are caught and mapped to StorageError;
specifically, ensure the block that loads the module and calls blob.put (the
import('hub:blob') line and the blob.put(pathname, data, { contentType:
options?.contentType, prefix: options?.prefix })) returns via Effect.tryPromise
or rethrows as new StorageError so callers of Effect.gen receive a consistent
StorageError on failure.
♻️ Duplicate comments (13)
server/utils/effect.ts (3)
1-5: Missing explicit imports for service layers and utilities.The file uses
DbServiceLive,StorageServiceLive,AuthServiceLive,OpenLibraryRepositoryLive,BookRepositoryLive,BookServiceLive,DbService,StorageService,AuthService,BookRepository,OpenLibraryRepository,BookService,createError, andisErrorwithout explicit imports. While Nuxt/Nitro may auto-import these, explicit imports improve clarity and catch errors at compile time.
36-40: SimplifyMainLiveto avoid redundant layer provisioning.
BaseServicesLiveis already provided viaRepositoriesLive(line 25-28), which is included inServicesLive. Providing it again here is redundant.Proposed simplification
// Combined live layer for all services -export const MainLive = Layer.provideMerge( - ServicesLive, - BaseServicesLive -) +export const MainLive = ServicesLive
88-93: Consider usingisErrorfrom H3 for consistent error detection.The
handleErrorfunction uses duck-typing ('statusCode' in error) whilerunEffect(line 126) uses theisErrorutility. For consistency and robustness, consider usingisErrorin both places.server/repositories/openLibrary.repository.ts (3)
1-5: Missing import:putBlobis called but not imported.The
downloadCoverimplementation usesputBlobon line 274 but it's not imported. This will cause a runtime error.Proposed fix
import { Context, Effect, Layer, Data, Duration } from 'effect' import { HttpClient } from '@effect/platform' import * as HCError from '@effect/platform/HttpClientError' import type { HttpClient as HttpClientType } from '@effect/platform' import sharp from 'sharp' +import { putBlob, type StorageService } from '../services/storage.service'
45-46:notesfield may be an object, not just a string.OpenLibrary API can return
notesas either a string or{ type, value }object (similar todescriptionhandling on lines 181-185). This could cause the description to display as[object Object].Proposed fix
Update the interface at line 45:
- notes?: string + notes?: string | { type: string, value: string }And update line 160:
- let description = bookData.notes || bookData.excerpts?.[0]?.text + let description = ( + typeof bookData.notes === 'string' + ? bookData.notes + : (bookData.notes as { value?: string })?.value + ) || bookData.excerpts?.[0]?.textAlso applies to: 159-160
219-260: Remove debugconsole.logstatements before merging.The
downloadCoverfunction contains multipleconsole.logstatements (lines 221, 231, 239, 244, 253, 260) that appear to be debugging artifacts. Replace them withEffect.log/Effect.logDebugfor consistent observability, or remove them entirely.Proposed cleanup
downloadCover: (isbn, size = 'L') => Effect.gen(function* () { - console.log('downloadCover', isbn, size) const normalizedISBN = normalizeISBN(isbn) const coverUrl = `https://covers.openlibrary.org/b/isbn/${normalizedISBN}-${size}.jpg?default=false` const imageBuffer = yield* HttpClient.get(coverUrl).pipe( Effect.timeout(Duration.seconds(10)), Effect.flatMap((response) => { if (response.status < 200 || response.status >= 300) { - console.log('No cover available for ISBN', normalizedISBN) return Effect.succeed(null as ArrayBuffer | null) } const contentLength = response.headers['content-length'] if (contentLength && parseInt(contentLength) < 1000) { - console.log('Too small, probably the placeholder image', normalizedISBN) return Effect.succeed(null as ArrayBuffer | null) } - console.log('Read the body within the same scope as the request', normalizedISBN) return response.arrayBuffer }), // ... ) - console.log('image Buffer', imageBuffer?.byteLength) if (!imageBuffer) { yield* Effect.log(`[OpenLibrary] No cover found for ISBN ${normalizedISBN}`) return null } - console.log('image Buffer', imageBuffer.byteLength)server/repositories/book.repository.ts (5)
184-197: ReturnedBookobject is missing optional fields.The return value omits
description,subjects,publishDate,publishers,numberOfPages, andworkKeyfields that are defined in theBookinterface and available in thebookvariable.Proposed fix
return { id: userBookId, bookId: book.id, book: { id: book.id, isbn: book.isbn, title: book.title, author: book.author, coverPath: book.coverPath, openLibraryKey: book.openLibraryKey, - createdAt: book.createdAt + createdAt: book.createdAt, + description: book.description ?? null, + subjects: book.subjects ?? null, + publishDate: book.publishDate ?? null, + publishers: book.publishers ?? null, + numberOfPages: book.numberOfPages ?? null, + workKey: book.workKey ?? null }, addedAt }
238-251: IncompleteBookmapping ingetLibraryresults.Similar to
addBookByISBN, theBookobjects are missing optional fields (description,subjects,publishDate,publishers,numberOfPages,workKey).
285-293: IncompleteBookmapping: missing optional fields.The
getBookByIdmethod returns aBooktype but omits optional fields likedescription,subjects,publishDate,publishers,numberOfPages, andworkKey.Proposed fix
return { id: book.id, isbn: book.isbn, title: book.title, author: book.author, coverPath: book.coverPath, openLibraryKey: book.openLibraryKey, - createdAt: book.createdAt instanceof Date ? book.createdAt : new Date(book.createdAt as unknown as string) + createdAt: book.createdAt instanceof Date ? book.createdAt : new Date(book.createdAt as unknown as string), + description: book.description ?? null, + subjects: book.subjects ?? null, + publishDate: book.publishDate ?? null, + publishers: book.publishers ?? null, + numberOfPages: book.numberOfPages ?? null, + workKey: book.workKey ?? null }
363-363: UnsafeJSON.parsemay throw on malformed data.If
bookData.subjectscontains malformed JSON,JSON.parsewill throw an unhandled exception, crashing the request.Proposed fix: Add safe JSON parsing
+// Helper for safe JSON parsing +function safeJsonParse<T>(json: string | null): T | null { + if (!json) return null + try { + return JSON.parse(json) + } catch { + return null + } +} // In getUserBookWithDetails: - subjects: bookData.subjects ? JSON.parse(bookData.subjects) : null, + subjects: safeJsonParse<string[]>(bookData.subjects),
1-5: Fix missing imports preventing compilation.The import from
'hub:db:schema'does not support destructuring in this context. Additionally, the following types and functions are used but not imported:DbService,StorageService,OpenLibraryRepository,PaginationParams,PaginatedResult,BookDetails,OpenLibraryBookNotFoundError,OpenLibraryApiError,lookupByISBN, anddownloadCover.Required imports
import { Context, Effect, Layer, Data } from 'effect' import type { HttpClient } from '@effect/platform' import { eq, and, count, desc } from 'drizzle-orm' import type { InferSelectModel } from 'drizzle-orm' -import { books, userBooks } from 'hub:db:schema' +import { books, userBooks } from '~/server/db/schema/domain' +import { DbService } from '~/server/services/db.service' +import { StorageService } from '~/server/services/storage.service' +import { + OpenLibraryRepository, + OpenLibraryBookNotFoundError, + OpenLibraryApiError, + lookupByISBN, + downloadCover +} from '~/server/repositories/openLibrary.repository' +import type { PaginationParams, PaginatedResult } from '~/shared/types/pagination' +import type { BookDetails } from '~/shared/types/book'server/services/book.service.ts (2)
1-2: Missing imports for types and services used throughout the file.The file references numerous types, error classes, services, and repository items that are not imported. This will cause TypeScript compilation errors.
Missing imports include:
- Types:
PaginationParams,PaginatedResult,LibraryBook,BookLookupResult,BatchDeleteResult,BookDetails- Errors:
DatabaseError,BookCreateError,BookAlreadyOwnedError,OpenLibraryBookNotFoundError,OpenLibraryApiError,BookNotFoundError- Services:
DbService,StorageService- Repository:
BookRepository,OpenLibraryRepository,lookupByISBN,OpenLibraryBookDataProposed imports
import { Context, Effect, Layer, Either } from 'effect' import type { HttpClient } from '@effect/platform' +import { DbService, type DatabaseError } from '~/server/services/db.service' +import { StorageService } from '~/server/services/storage.service' +import { + OpenLibraryRepository, + lookupByISBN, + type OpenLibraryBookData, + OpenLibraryBookNotFoundError, + OpenLibraryApiError +} from '~/server/repositories/openLibrary.repository' +import { + BookRepository, + BookNotFoundError, + BookCreateError, + BookAlreadyOwnedError +} from '~/server/repositories/book.repository' +import type { PaginationParams, PaginatedResult } from '~/shared/types/pagination' +import type { LibraryBook, BookLookupResult, BatchDeleteResult, BookDetails } from '~/shared/types/book'
140-140:JSON.parsecan throw on malformed data.If
localBook.subjectscontains invalid JSON, this will throw an unhandled exception. Consider using a safe parsing approach.Proposed fix using try-catch
- subjects: localBook.subjects ? JSON.parse(localBook.subjects) : null, + subjects: localBook.subjects ? (() => { + try { + return JSON.parse(localBook.subjects) + } catch { + return null + } + })() : null,Or better, extract a reusable safe parse utility:
const safeParseJson = (str: string | null): unknown | null => { if (!str) return null try { return JSON.parse(str) } catch { return null } }
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
server/repositories/book.repository.tsserver/repositories/openLibrary.repository.tsserver/services/book.service.tsserver/services/storage.service.tsserver/utils/effect.tsserver/utils/effectHandler.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-13T21:08:13.933Z
Learnt from: niklhut
Repo: niklhut/libroo PR: 1
File: server/utils/effect.ts:30-38
Timestamp: 2026-01-13T21:08:13.933Z
Learning: In Effect-TS code, when combining service tags (Context.Tag) for multiple services, use a union type (Context1.Tag | Context2.Tag) rather than an intersection (&). Service tags are nominal types and cannot be meaningfully intersected; a union correctly represents combined service requirements in the Effect<R> parameter, while runtime Layer composition handles provisioning. Apply this guidance to TypeScript files that define or compose Effect-TS service contexts (e.g., server/utils/effect.ts and similar files).
Applied to files:
server/utils/effectHandler.tsserver/utils/effect.tsserver/services/storage.service.tsserver/repositories/openLibrary.repository.tsserver/services/book.service.tsserver/repositories/book.repository.ts
📚 Learning: 2026-01-13T21:24:30.028Z
Learnt from: niklhut
Repo: niklhut/libroo PR: 1
File: shared/utils/schemas.ts:11-13
Timestamp: 2026-01-13T21:24:30.028Z
Learning: In TypeScript files using Zod v4.3.5 or newer, use the error option for both schema-level configuration and validators (min, max, etc.). The message parameter is deprecated. Example: z.string({ error: 'Required' }).min(10, { error: 'Too short' }). Apply this pattern to all Zod schema definitions in TS files to avoid deprecated API usage.
Applied to files:
server/utils/effectHandler.tsserver/utils/effect.tsserver/services/storage.service.tsserver/repositories/openLibrary.repository.tsserver/services/book.service.tsserver/repositories/book.repository.ts
🧬 Code graph analysis (4)
server/utils/effectHandler.ts (2)
server/utils/effect.ts (3)
Effect(134-134)MainServices(43-50)runEffect(116-131)server/services/auth.service.ts (1)
requireAuth(75-76)
server/utils/effect.ts (6)
server/services/db.service.ts (2)
DbServiceLive(13-15)DbService(10-10)server/services/storage.service.ts (2)
StorageServiceLive(34-97)StorageService(31-31)server/services/auth.service.ts (2)
AuthServiceLive(41-69)AuthService(38-38)server/repositories/openLibrary.repository.ts (2)
OpenLibraryRepositoryLive(118-290)OpenLibraryRepository(76-79)server/repositories/book.repository.ts (2)
BookRepositoryLive(92-374)BookRepository(84-84)server/services/book.service.ts (2)
BookServiceLive(51-176)BookService(47-47)
server/repositories/openLibrary.repository.ts (2)
server/utils/effect.ts (4)
Data(134-134)Effect(134-134)Context(134-134)Layer(134-134)server/services/storage.service.ts (2)
StorageService(31-31)putBlob(100-101)
server/repositories/book.repository.ts (7)
server/utils/effect.ts (4)
Data(134-134)Effect(134-134)Context(134-134)Layer(134-134)server/repositories/openLibrary.repository.ts (5)
OpenLibraryBookNotFoundError(8-11)OpenLibraryApiError(13-15)OpenLibraryRepository(76-79)lookupByISBN(293-294)downloadCover(296-297)server/services/db.service.ts (1)
DbService(10-10)server/services/storage.service.ts (1)
StorageService(31-31)shared/types/pagination.ts (2)
PaginationParams(1-4)PaginatedResult(6-15)server/db/schema/domain.ts (2)
books(8-25)userBooks(29-34)shared/types/book.ts (1)
BookDetails(31-46)
🪛 GitHub Actions: Typecheck and Lint
server/repositories/book.repository.ts
[error] 5-5: TypeScript TS2305: Module '"hub:db:schema"' has no exported member 'books'.
🪛 GitHub Check: typecheck
server/repositories/book.repository.ts
[failure] 5-5:
Module '"hub:db:schema"' has no exported member 'userBooks'.
[failure] 5-5:
Module '"hub:db:schema"' has no exported member 'books'.
[failure] 5-5:
Module '"hub:db:schema"' has no exported member 'userBooks'.
[failure] 5-5:
Module '"hub:db:schema"' has no exported member 'books'.
🔇 Additional comments (10)
server/utils/effect.ts (2)
42-51: LGTM!The union type for
MainServicesis correct for Effect-TS service tags, as service tags are nominal types that cannot be intersected. Based on learnings from previous review.
115-131: LGTM!The
runEffectfunction properly handles errors by converting them to H3 errors viahandleError, then throwing if the result is an error. This integrates well with H3/Nitro's error handling.server/utils/effectHandler.ts (1)
23-41: LGTM!The
effectHandlercleanly wraps handlers with authentication and delegates torunEffectfor error handling. The implementation correctly enforces authentication and provides proper error handling through the centralized utilities.server/services/storage.service.ts (2)
3-28: LGTM!The error type and interfaces are well-structured with clear semantics. The
StorageErrorproperly captures the operation context for debugging.
99-110: LGTM!The helper effects follow the standard Effect-TS pattern for service delegation.
server/repositories/openLibrary.repository.ts (1)
292-297: LGTM!The helper effects follow the standard Effect-TS delegation pattern.
server/repositories/book.repository.ts (2)
7-24: LGTM!The error types are well-defined with distinct tags.
BookNotFoundErrorandOpenLibraryBookNotFoundErrorhave different_tagvalues, so there's no collision.
26-48: LGTM!The
BookandUserBookinterfaces are well-structured. Thesubjectsfield is correctly typed asstring | nullto represent the JSON-stringified array stored in the database.server/services/book.service.ts (2)
6-43: Well-structured Effect-TS service interface.The interface properly declares explicit error channels and dependency requirements for each method. The use of union types for errors and dependencies follows Effect-TS conventions correctly.
178-196: LGTM!The helper effects follow the standard Effect-TS accessor pattern, providing convenient service access via
Effect.flatMap. This is idiomatic and improves ergonomics for consumers.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…to RepositoriesLive.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.github/workflows/ci.yml:
- Line 6: The workflow job currently named "typecheck" is misleading because it
only runs the lint script; rename the job key from typecheck to lint in the CI
workflow (update the "typecheck:" job header to "lint:") and adjust any
references or dependencies that reference the old job name (e.g., needs:, job
outputs or badges) so they point to "lint" instead to keep the job name
consistent with its behavior.
In `@server/services/storage.service.ts`:
- Around line 55-92: The get, delete, and list methods reference an undefined
blob variable; fix them the same way as the put method by performing the dynamic
import/initialization of the storage client inside each Effect.gen before
calling blob (e.g., obtain the blob client with the same async helper or import
used in put), then use that local blob to call get/delete/list and preserve the
current StorageError wrapping if the client is unavailable or the call fails;
ensure uploadedAt normalization in list remains and throw/return a StorageError
if the dynamic import or client resolution fails.
In `@server/utils/auth.ts`:
- Around line 37-52: getAuthUrl currently silently falls back to
'http://localhost:3000'; update it to emit a clear production warning when no
explicit URL is configured: detect production via process.env.NODE_ENV ===
'production' (and/or config.isProd if available), and if in production and
neither process.env.BETTER_AUTH_URL nor config.betterAuthUrl were found, call
console.warn (or a module logger if available) with a message referencing
getAuthUrl and advising to set BETTER_AUTH_URL; keep the existing fallback for
non-production but do not suppress the warning in production.
In `@server/utils/effect.ts`:
- Around line 111-127: runEffect uses Effect.catchAll(handleError) which
converts errors into a successful value (an H3Error) so the function can inspect
it and re-throw; add a short clarifying comment above runEffect (and/or just
above the Effect.catchAll line) explaining that handleError returns
Effect.succeed(h3Error) intentionally to surface H3Error as a return value that
is then detected via isError and thrown, referencing the runEffect function, the
Effect.catchAll(handleError) usage, and the isError check to help future
maintainers understand this control flow.
♻️ Duplicate comments (13)
nuxt.config.ts (1)
3-11:@nuxt/test-utilsshould be conditionally loaded for non-production environments.This module is intended for development/testing only and should not be included in production builds.
package.json (1)
23-23: Move@nuxt/test-utilstodevDependencies.Test utilities should not be in production dependencies. This was flagged in a previous review and appears to still be unaddressed.
♻️ Suggested fix
"dependencies": { ... - "@nuxt/test-utils": "3.22.0", ... }, "devDependencies": { "@nuxt/eslint": "^1.12.1", + "@nuxt/test-utils": "3.22.0", "drizzle-kit": "^0.31.8", ... },server/repositories/openLibrary.repository.ts (3)
37-56:notesfield type is inconsistent with actual API response.The
notesfield on line 45 is typed asstring, but OpenLibrary API can returnnotesas either a string or{ type, value }object (similar todescriptioninOpenLibraryWorksApiResponseon line 62). This type mismatch could cause runtime issues when accessingbookData.noteson line 160.🐛 Proposed fix
interface OpenLibraryBooksApiResponse { [key: string]: { title: string authors?: Array<{ name: string, url?: string }> publishers?: Array<{ name: string }> publish_date?: string number_of_pages?: number - notes?: string + notes?: string | { type: string, value: string } excerpts?: Array<{ text: string }>Then update line 160:
- let description = bookData.notes || bookData.excerpts?.[0]?.text + let description = ( + typeof bookData.notes === 'string' + ? bookData.notes + : bookData.notes?.value + ) || bookData.excerpts?.[0]?.text
263-273: Critical:putBlobis called but not imported.The
putBlobfunction is used on line 266 but is not imported fromstorage.service.ts. This will cause a runtime error when attempting to download and store covers.🐛 Proposed fix
Add to imports at the top of the file:
import { Context, Effect, Layer, Data, Duration } from 'effect' import { HttpClient } from '@effect/platform' import * as HCError from '@effect/platform/HttpClientError' import type { HttpClient as HttpClientType } from '@effect/platform' import sharp from 'sharp' +import { putBlob } from '../services/storage.service'
1-5: Missing import forStorageServicetype.The
StorageServicetype is referenced on line 72 in the interface definition but is not imported. This will cause a TypeScript compilation error.🐛 Proposed fix
import { Context, Effect, Layer, Data, Duration } from 'effect' import { HttpClient } from '@effect/platform' import * as HCError from '@effect/platform/HttpClientError' import type { HttpClient as HttpClientType } from '@effect/platform' import sharp from 'sharp' +import { putBlob, type StorageService } from '../services/storage.service'server/repositories/book.repository.ts (5)
1-5: Missing imports for types and functions used throughout the file.The file references
DbService,StorageService,OpenLibraryRepository,PaginationParams,PaginatedResult,BookDetails,OpenLibraryBookNotFoundError,OpenLibraryApiError,lookupByISBN, anddownloadCoverwithout importing them. This will cause TypeScript compilation errors.🐛 Proposed fix
import { Context, Effect, Layer, Data } from 'effect' import type { HttpClient } from '@effect/platform' import { eq, and, count, desc } from 'drizzle-orm' import type { InferSelectModel } from 'drizzle-orm' import { books, userBooks } from 'hub:db:schema' +import { DbService } from '../services/db.service' +import { StorageService } from '../services/storage.service' +import { + OpenLibraryRepository, + OpenLibraryBookNotFoundError, + OpenLibraryApiError, + lookupByISBN, + downloadCover +} from './openLibrary.repository' +import type { PaginationParams, PaginatedResult } from '~/shared/types/pagination' +import type { BookDetails } from '~/shared/types/book'
184-198: ReturnedBookobject is missing optional fields.The returned
bookobject omitsdescription,subjects,publishDate,publishers,numberOfPages, andworkKeyfields that are part of theBookinterface. This could cause issues if consumers expect these fields.🐛 Proposed fix
return { id: userBookId, bookId: book.id, book: { id: book.id, isbn: book.isbn, title: book.title, author: book.author, coverPath: book.coverPath, openLibraryKey: book.openLibraryKey, - createdAt: book.createdAt + createdAt: book.createdAt, + description: book.description ?? null, + subjects: book.subjects ?? null, + publishDate: book.publishDate ?? null, + publishers: book.publishers ?? null, + numberOfPages: book.numberOfPages ?? null, + workKey: book.workKey ?? null }, addedAt }
238-251: IncompleteBookmapping ingetLibraryresults.Similar to
addBookByISBN, theBookobjects ingetLibraryresults are missing optional fields (description,subjects,publishDate,publishers,numberOfPages,workKey).🐛 Proposed fix
const items = result.map(row => ({ id: row.user_books.id, bookId: row.books.id, book: { id: row.books.id, isbn: row.books.isbn, title: row.books.title, author: row.books.author, coverPath: row.books.coverPath, openLibraryKey: row.books.openLibraryKey, - createdAt: row.books.createdAt + createdAt: row.books.createdAt, + description: row.books.description ?? null, + subjects: row.books.subjects ?? null, + publishDate: row.books.publishDate ?? null, + publishers: row.books.publishers ?? null, + numberOfPages: row.books.numberOfPages ?? null, + workKey: row.books.workKey ?? null }, addedAt: row.user_books.addedAt }))
285-294: IncompleteBookmapping ingetBookById.The returned object is missing optional
Bookfields.🐛 Proposed fix
return { id: book.id, isbn: book.isbn, title: book.title, author: book.author, coverPath: book.coverPath, openLibraryKey: book.openLibraryKey, - createdAt: book.createdAt instanceof Date ? book.createdAt : new Date(book.createdAt as unknown as string) + createdAt: book.createdAt instanceof Date ? book.createdAt : new Date(book.createdAt as unknown as string), + description: book.description ?? null, + subjects: book.subjects ?? null, + publishDate: book.publishDate ?? null, + publishers: book.publishers ?? null, + numberOfPages: book.numberOfPages ?? null, + workKey: book.workKey ?? null }
355-371: UnsafeJSON.parsemay throw on malformed data.If
bookData.subjectscontains malformed JSON,JSON.parseon line 363 will throw an unhandled exception, crashing the request.🐛 Proposed fix: Add safe JSON parsing
+// Helper for safe JSON parsing +function safeJsonParse<T>(json: string | null): T | null { + if (!json) return null + try { + return JSON.parse(json) + } catch { + return null + } +} // Then in getUserBookWithDetails: return { id: row.user_books.id, bookId: bookData.id, title: bookData.title, author: bookData.author, isbn: bookData.isbn, coverPath: bookData.coverPath, description: bookData.description ?? null, - subjects: bookData.subjects ? JSON.parse(bookData.subjects) : null, + subjects: safeJsonParse<string[]>(bookData.subjects), publishDate: bookData.publishDate ?? null,server/services/book.service.ts (2)
1-13: Missing imports for types and dependencies.The file uses
LibraryBook,PaginationParams,PaginatedResult,DatabaseError,DbService,StorageService,OpenLibraryRepository,BookCreateError,BookAlreadyOwnedError,OpenLibraryBookNotFoundError,OpenLibraryApiError,BookNotFoundError,BatchDeleteResult,BookDetails,BookLookupResult,BookRepository,lookupByISBN, andOpenLibraryBookDatawithout importing them.🐛 Proposed imports
import { Context, Effect, Layer, Either } from 'effect' import type { HttpClient } from '@effect/platform' import type { UserBook } from '../repositories/book.repository' +import { + BookRepository, + BookNotFoundError, + BookCreateError, + BookAlreadyOwnedError, + DatabaseError +} from '../repositories/book.repository' +import { + OpenLibraryRepository, + OpenLibraryBookNotFoundError, + OpenLibraryApiError, + lookupByISBN, + type OpenLibraryBookData +} from '../repositories/openLibrary.repository' +import { DbService } from './db.service' +import { StorageService } from './storage.service' +import type { PaginationParams, PaginatedResult } from '~/shared/types/pagination' +import type { + LibraryBook, + BookLookupResult, + BatchDeleteResult, + BookDetails +} from '~/shared/types/book'
127-141: UnsafeJSON.parseonlocalBook.subjects.The
JSON.parsecall on line 135 could throw iflocalBook.subjectscontains malformed JSON. Apply the same safe parsing approach as suggested for the repository.🐛 Proposed fix
+// Helper for safe JSON parsing (or import from shared utils) +function safeJsonParse<T>(json: string | null): T | null { + if (!json) return null + try { + return JSON.parse(json) + } catch { + return null + } +} if (localBook) { return { found: true, isbn: localBook.isbn || normalizedISBN, title: localBook.title, author: localBook.author, coverUrl: localBook.coverPath ? `/api/blob/${localBook.coverPath}` : null, description: localBook.description ?? undefined, - subjects: localBook.subjects ? JSON.parse(localBook.subjects) : null, + subjects: safeJsonParse<string[]>(localBook.subjects), publishDate: localBook.publishDate ?? undefined,server/services/storage.service.ts (1)
33-53: Critical:blobis used but not defined or imported.The
StorageServiceLiveimplementation referencesblob(lines 38, 58, 69, 80) without importing or defining it. The comment on line 33 mentions "uses dynamic import to avoid build issues," but no dynamic import is present. This will cause a runtime error.🐛 Proposed fix: Add dynamic import
// Live implementation - uses dynamic import to avoid build issues export const StorageServiceLive = Layer.succeed(StorageService, { put: (pathname, data, options) => - Effect.gen(function* () { - const result = yield* Effect.tryPromise({ - try: () => blob.put(pathname, data, { + Effect.tryPromise({ + try: async () => { + const { blob } = await import('hub:blob') + const result = await blob.put(pathname, data, { contentType: options?.contentType, prefix: options?.prefix - }), - catch: error => new StorageError({ - message: `Failed to put blob: ${error}`, - operation: 'put' }) - }) - return { - pathname: result.pathname, - contentType: result.contentType, - size: result.size, - uploadedAt: new Date(result.uploadedAt) - } + return { + pathname: result.pathname, + contentType: result.contentType, + size: result.size, + uploadedAt: new Date(result.uploadedAt) + } + }, + catch: error => new StorageError({ + message: `Failed to put blob: ${error}`, + operation: 'put' + }) }),Apply similar pattern to
get,delete, andlistmethods.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.github/workflows/ci.ymlnuxt.config.tspackage.jsonserver/repositories/book.repository.tsserver/repositories/openLibrary.repository.tsserver/services/auth.service.tsserver/services/book.service.tsserver/services/storage.service.tsserver/utils/auth.tsserver/utils/effect.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-13T21:08:13.933Z
Learnt from: niklhut
Repo: niklhut/libroo PR: 1
File: server/utils/effect.ts:30-38
Timestamp: 2026-01-13T21:08:13.933Z
Learning: In Effect-TS code, when combining service tags (Context.Tag) for multiple services, use a union type (Context1.Tag | Context2.Tag) rather than an intersection (&). Service tags are nominal types and cannot be meaningfully intersected; a union correctly represents combined service requirements in the Effect<R> parameter, while runtime Layer composition handles provisioning. Apply this guidance to TypeScript files that define or compose Effect-TS service contexts (e.g., server/utils/effect.ts and similar files).
Applied to files:
server/utils/auth.tsserver/repositories/openLibrary.repository.tsnuxt.config.tsserver/services/auth.service.tsserver/repositories/book.repository.tsserver/utils/effect.tsserver/services/storage.service.tsserver/services/book.service.ts
📚 Learning: 2026-01-13T21:24:30.028Z
Learnt from: niklhut
Repo: niklhut/libroo PR: 1
File: shared/utils/schemas.ts:11-13
Timestamp: 2026-01-13T21:24:30.028Z
Learning: In TypeScript files using Zod v4.3.5 or newer, use the error option for both schema-level configuration and validators (min, max, etc.). The message parameter is deprecated. Example: z.string({ error: 'Required' }).min(10, { error: 'Too short' }). Apply this pattern to all Zod schema definitions in TS files to avoid deprecated API usage.
Applied to files:
server/utils/auth.tsserver/repositories/openLibrary.repository.tsnuxt.config.tsserver/services/auth.service.tsserver/repositories/book.repository.tsserver/utils/effect.tsserver/services/storage.service.tsserver/services/book.service.ts
🧬 Code graph analysis (4)
server/services/auth.service.ts (2)
server/utils/auth.ts (1)
auth(54-75)server/db/schema/auth.ts (1)
session(16-25)
server/repositories/book.repository.ts (7)
server/utils/effect.ts (4)
Data(130-130)Effect(130-130)Context(130-130)Layer(130-130)server/repositories/openLibrary.repository.ts (5)
OpenLibraryBookNotFoundError(8-11)OpenLibraryApiError(13-15)OpenLibraryRepository(76-79)lookupByISBN(285-286)downloadCover(288-289)server/services/db.service.ts (1)
DbService(10-10)server/services/storage.service.ts (1)
StorageService(31-31)shared/types/pagination.ts (2)
PaginationParams(1-4)PaginatedResult(6-15)server/db/schema/domain.ts (2)
books(8-25)userBooks(29-34)shared/types/book.ts (1)
BookDetails(31-46)
server/utils/effect.ts (6)
server/services/db.service.ts (2)
DbServiceLive(13-15)DbService(10-10)server/services/storage.service.ts (2)
StorageServiceLive(34-93)StorageService(31-31)server/services/auth.service.ts (2)
AuthServiceLive(40-49)AuthService(22-22)server/repositories/book.repository.ts (2)
BookRepositoryLive(92-374)BookRepository(84-84)server/repositories/openLibrary.repository.ts (1)
OpenLibraryRepository(76-79)server/services/book.service.ts (2)
BookServiceLive(62-171)BookService(58-58)
server/services/storage.service.ts (1)
server/utils/effect.ts (4)
Data(130-130)Effect(130-130)Context(130-130)Layer(130-130)
🔇 Additional comments (30)
nuxt.config.ts (5)
30-37: LGTM!The import directories align with the layered architecture (services/repositories) described in the PR.
39-42: LGTM!NuxtHub configuration with SQLite and blob storage is properly set up for the deployment target.
44-53: LGTM!ESLint stylistic rules are consistent and follow common conventions for TypeScript/JavaScript projects.
55-57: Verify intent: Image optimization is disabled.Setting
provider: 'none'disables all image optimization. This may be intentional for initial setup, but consider enabling a provider (e.g.,ipxfor self-hosted or a CDN provider) before production to optimize image delivery.
19-22: No action needed – configuration is correct.
betterAuthUrlcorrectly remains in server-only config. The client-side auth composable (createAuthClient()inapp/composables/auth.ts) is initialized without requiring a URL, and the actualbetterAuthUrlvalue is only accessed server-side inserver/utils/auth.ts. Exposing it to the client would be unnecessary and create a potential security concern.server/utils/auth.ts (2)
6-35: Security improvement: Production guard for auth secret is well implemented.The
getAuthSecrethelper properly validates the secret is present in production and fails fast with a clear error message. The fallback to a dev secret is only used in non-production environments.
54-77: LGTM!The auth configuration is properly set up with the helper functions, SQLite adapter, and email/password authentication enabled. The placeholder for social providers is appropriate for the initial setup.
package.json (1)
41-42: LGTM: Package manager updated to safe version.The
pnpm@10.28.0version addresses the previously flagged CVE-2025-69262 vulnerability (fixed in 10.27.0+).server/services/auth.service.ts (2)
24-37: Well-structured session fetching helper.The
fetchSessionhelper correctly extracts the shared session-fetching logic, addressing the previous review comment about code duplication. The implementation properly uses Effect.tryPromise with appropriate error handling.
39-56: LGTM!The
AuthServiceLiveimplementation is clean and leverages thefetchSessionhelper effectively. The helper effects at the bottom provide a convenient API for consuming the service.server/utils/effect.ts (3)
7-13: Good implementation of redirect-following HTTP client.The
HttpClientLivelayer correctly configures the HTTP client to follow up to 10 redirects, which is necessary for OpenLibrary cover URLs. The layer composition withNodeHttpClient.layeris properly structured.
15-36: Clean layer composition hierarchy.The layer structure follows a clear dependency hierarchy (Base → Repositories → Services → Main) without redundant provisioning. This addresses the previous review feedback.
1-4: Reliance on Nuxt auto-imports is acceptable.The file uses
DbServiceLive,StorageServiceLive,AuthServiceLive,isError,createError, and other symbols without explicit imports. This is valid in Nuxt/Nitro projects where server utilities are auto-imported. While explicit imports would improve IDE support in some setups, this is a standard Nuxt pattern..github/workflows/ci.yml (1)
11-19: No issues found. Bothactions/checkout@v6andactions/setup-node@v6are released and stable versions. The checkout action is the current stable major release, and setup-node v6 has been released and maintained (v6.1.0 as of Dec 2025). Using these versions is appropriate.server/repositories/openLibrary.repository.ts (5)
86-95: LGTM!The
fetchJsonhelper properly implements timeout handling withDuration.seconds(5)and maps HTTP errors to domain-specificOpenLibraryApiError.
97-115: LGTM!The
checkCoverExistshelper correctly handles timeouts and gracefully catches all errors to returnfalse, preventing cover availability checks from failing the overall operation.
117-217: LGTM!The
lookupByISBNimplementation has a well-structured flow: ISBN normalization, primary API fetch, cover validation, and optional enrichment from works data with proper error handling viaEffect.catchAlland debug logging.
274-281: LGTM!Good defensive error handling — the
catchTagonOpenLibraryCoverErrorensures cover download failures don't fail the entire operation, logging a warning and returningnullinstead.
284-289: LGTM!Helper effect exports follow the standard Effect-TS pattern for ergonomic service method access.
server/repositories/book.repository.ts (5)
7-24: LGTM!Error types are properly defined using Effect's
Data.TaggedErrorpattern with distinct tags (BookNotFoundError,BookCreateError,BookAlreadyOwnedError,DatabaseError). TheBookNotFoundErrortag is distinct fromOpenLibraryBookNotFoundErrorin the OpenLibrary repository.
26-48: LGTM!The
BookandUserBookinterfaces properly define the domain types with appropriate optional fields matching the database schema.
50-84: LGTM!The interface properly defines Effect-based method signatures with explicit error and dependency types. Based on learnings, the union types for service requirements are correctly used.
296-330: LGTM!The
removeFromLibraryandfindByIsbnimplementations are correctly structured with proper error handling.
376-393: LGTM!All repository methods now have corresponding helper effect exports following the standard Effect-TS pattern.
server/services/storage.service.ts (1)
95-106: LGTM!Helper effect exports follow the standard Effect-TS pattern for ergonomic service access.
server/services/book.service.ts (5)
5-13: LGTM!Good refactor extracting
toLibraryBookhelper to eliminate duplicate mapping logic. This addresses the previous review feedback.
15-58: LGTM!The
BookServiceInterfaceproperly defines Effect-based method signatures with explicit error and dependency types following Effect-TS patterns.
88-115: LGTM!The
batchRemoveFromLibraryimplementation properly uses bounded concurrency ({ concurrency: 10 }) and correctly logs errors by yieldingEffect.logError. This addresses previous review feedback.
120-168: LGTM!The
lookupBookimplementation correctly usesnormalizedISBNfor both the local lookup and OpenLibrary fallback, and the not-found response consistently uses the normalized ISBN.
173-191: LGTM!Helper effect exports provide ergonomic access to all service methods.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@server/utils/auth.ts`:
- Around line 37-59: In getAuthUrl, enhance the production warning to treat
empty-string values as unset by checking url.trim() as well as null/undefined;
update the condition that currently tests for !url to instead consider
url?.trim() === '' (or compute an isEmpty = !url || url.trim() === '' and use
that) so the warning and fallback to 'http://localhost:3000' behave consistently
with getAuthSecret.
In `@server/utils/effect.ts`:
- Around line 149-150: The module re-exports Effect, Layer, Context, and Data
but omits pipe; if consumers need pipe for composing effects, add pipe to the
export list so it’s publicly available (i.e., update the export statement that
currently exports Effect, Layer, Context, Data to also export pipe from
'effect'). Ensure the symbol name is exactly pipe to match imports elsewhere.
- Around line 37-46: The union type declaration for MainServices is
mis-indented; adjust continuation lines so binary-op continuation (the '=' and
subsequent '|' lines) use 4-space indentation to satisfy
`@stylistic/indent-binary-ops`. Specifically, align the "=" line and each union
member line (DbService, StorageService, AuthService, BookRepository,
OpenLibraryRepository, BookService, HttpClient.HttpClient) with a 4-space
continuation indent relative to the "export type MainServices" line so the '|'
tokens are indented correctly.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/ci.ymlserver/utils/auth.tsserver/utils/effect.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-13T21:08:13.933Z
Learnt from: niklhut
Repo: niklhut/libroo PR: 1
File: server/utils/effect.ts:30-38
Timestamp: 2026-01-13T21:08:13.933Z
Learning: In Effect-TS code, when combining service tags (Context.Tag) for multiple services, use a union type (Context1.Tag | Context2.Tag) rather than an intersection (&). Service tags are nominal types and cannot be meaningfully intersected; a union correctly represents combined service requirements in the Effect<R> parameter, while runtime Layer composition handles provisioning. Apply this guidance to TypeScript files that define or compose Effect-TS service contexts (e.g., server/utils/effect.ts and similar files).
Applied to files:
server/utils/auth.tsserver/utils/effect.ts
📚 Learning: 2026-01-13T21:24:30.028Z
Learnt from: niklhut
Repo: niklhut/libroo PR: 1
File: shared/utils/schemas.ts:11-13
Timestamp: 2026-01-13T21:24:30.028Z
Learning: In TypeScript files using Zod v4.3.5 or newer, use the error option for both schema-level configuration and validators (min, max, etc.). The message parameter is deprecated. Example: z.string({ error: 'Required' }).min(10, { error: 'Too short' }). Apply this pattern to all Zod schema definitions in TS files to avoid deprecated API usage.
Applied to files:
server/utils/auth.tsserver/utils/effect.ts
📚 Learning: 2026-01-15T15:10:03.761Z
Learnt from: niklhut
Repo: niklhut/libroo PR: 1
File: server/services/storage.service.ts:55-92
Timestamp: 2026-01-15T15:10:03.761Z
Learning: In NuxtHub server-side code, the blob API from Cloudflare R2 is provided via the virtual module hub:blob and is auto-imported. In files under server/ (e.g., server/services/storage.service.ts), you can call blob.put(), blob.get(), blob.delete(), blob.list(), etc., without an explicit import. Do not add import statements for blob in server code. Ensure the files are under the server directory and that TypeScript paths resolve to the virtual module in NuxtHub.
Applied to files:
server/utils/auth.tsserver/utils/effect.ts
🧬 Code graph analysis (1)
server/utils/effect.ts (6)
server/services/db.service.ts (2)
DbServiceLive(13-15)DbService(10-10)server/services/storage.service.ts (2)
StorageServiceLive(34-93)StorageService(31-31)server/services/auth.service.ts (2)
AuthServiceLive(40-49)AuthService(22-22)server/repositories/book.repository.ts (2)
BookRepositoryLive(92-374)BookRepository(84-84)server/repositories/openLibrary.repository.ts (2)
OpenLibraryRepositoryLive(118-282)OpenLibraryRepository(76-79)server/services/book.service.ts (2)
BookServiceLive(62-171)BookService(58-58)
🪛 GitHub Actions: Lint
server/utils/effect.ts
[error] 40-40: Expected indentation of 4 spaces @stylistic/indent-binary-ops
🪛 GitHub Check: lint
server/utils/effect.ts
[failure] 45-45:
Expected indentation of 4 spaces
[failure] 44-44:
Expected indentation of 4 spaces
[failure] 43-43:
Expected indentation of 4 spaces
[failure] 42-42:
Expected indentation of 4 spaces
[failure] 41-41:
Expected indentation of 4 spaces
[failure] 40-40:
Expected indentation of 4 spaces
🔇 Additional comments (12)
server/utils/auth.ts (4)
1-4: LGTM!The imports correctly use NuxtHub virtual modules (
hub:dbandhub:db:schema) along with the better-auth library and its drizzle adapter.
6-35: Well implemented production security guard.The function properly validates the auth secret and fails fast in production with a clear, actionable error message. The defensive try/catch around
useRuntimeConfighandles CLI usage gracefully, and the empty string check on line 24 is thorough.
61-82: LGTM!The auth configuration correctly uses the helper functions and is properly configured for the SQLite migration. The commented social providers placeholder is reasonable for the initial setup.
83-84: LGTM!The type export enables proper type inference for consumers of the auth instance.
.github/workflows/ci.yml (2)
1-7: LGTM! Workflow and job naming are now consistent.The workflow correctly uses
actions/checkout@v6and the job namelintaccurately reflects its purpose. The previous review comment about the misleadingtypecheckjob name has been addressed.
11-26: Action and Node.js versions are current.The
actions/checkoutlatest version is v6.0.1, released December 2, 2025. Theactions/setup-node@v6withnode-version: 24is the documented configuration, and Node.js 24.x is now in Active LTS (Krypton).The workflow is appropriately streamlined for linting. If type checking is still needed in the project, consider whether a separate workflow or step should be added.
server/utils/effect.ts (6)
1-12: LGTM!Clean import structure and well-documented HttpClientLive layer. The redirect-following behavior with a 10-redirect limit is appropriate for handling OpenLibrary cover URL redirects.
14-35: LGTM!The layer composition correctly follows the dependency hierarchy: base services (DB, Storage, Auth, HttpClient) → repositories (Book, OpenLibrary) → services (Book). The simplified
MainLive = ServicesLiveavoids the redundant double-provisioning that was previously flagged.
47-53: LGTM!The
getProphelper provides safe property extraction from unknown objects with proper type guards. This is appropriate for extracting properties from tagged error objects.
55-77: LGTM!Comprehensive error-to-status-code mapping with appropriate HTTP codes. The custom message formatters for
BookNotFoundErrorandBookAlreadyOwnedErrorprovide user-friendly context by extracting the ISBN.
79-111: LGTM!The
handleErrorfunction cleanly transforms errors into H3Errors usingEffect.die(). The documentation clearly explains the pattern, and the use ofisError()is now consistent throughout the file.
113-147: LGTM!The
runEffectfunction provides a clean execution wrapper with comprehensive documentation explaining the error-handling flow. The use ofEffect.runPromiseExitwithExit.matchand defect extraction fromCause.defectsis idiomatic Effect-TS.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary by CodeRabbit
New Features
Improvements
Infrastructure
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.