Refine auth, database, and CI scaffolding#2
Conversation
WalkthroughAdds a Bun-based monorepo with four packages (api, auth, db, types), TypeScript configs, CI, Hono API app factory and fetch handler, routes (health/categories/tasks) with auth, layered queries/services/repositories, DB schema and RLS support, Supabase/header auth providers, tests, and developer docs/tooling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as Hono App
participant Auth as AuthenticationProvider
participant Queries as CategoryQueries
participant Service as CategoryService
participant Repo as CategoryRepository
Note over API: GET /users/:userId/categories
Client->>API: GET /users/:userId/categories?limit&cursor (Authorization)
API->>Auth: authenticate({ request })
alt auth success
Auth-->>API: { user }
API->>Queries: listByUserId({ userId, limit, cursor })
Queries->>Service: listByUserId(...)
Service->>Repo: listByUserId(...)
Repo-->>Service: { items, nextCursor, hasMore }
Service-->>Queries: result
Queries-->>API: result
API-->>Client: 200 { data, meta }
else auth failure
Auth-->>API: AuthenticationError / null
API-->>Client: 401
end
sequenceDiagram
autonumber
participant Client
participant API as Hono App
participant DBHealth as DatabaseHealthChecker
Note over API: GET /healthz/database
Client->>API: GET /healthz/database
API->>DBHealth: check()
alt ok true
DBHealth-->>API: { ok: true }
API-->>Client: 200 { status: "ok" }
else ok false
DBHealth-->>API: { ok: false, error }
API-->>Client: 503 { status: "error", error }
else throws
DBHealth--x API: throws
API-->>Client: 500 { status: "error", error }
end
sequenceDiagram
autonumber
participant Route
participant SupabaseAuth as SupabaseAuthProvider
participant JOSE as jose (jwtVerify / remote JWKs)
Note over Route: Supabase JWT verification flow
Route->>SupabaseAuth: authenticate({ request })
SupabaseAuth->>SupabaseAuth: extractAuthorizationToken(header, scheme)
SupabaseAuth->>JOSE: jwtVerify(token, remoteJWKSet, opts)
JOSE-->>SupabaseAuth: payload (sub, role, ...)
alt requiredRole satisfied or not set
SupabaseAuth-->>Route: { user }
Route-->>Client: proceed
else role missing/invalid
SupabaseAuth-->>Route: AuthenticationError
Route-->>Client: 401
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (38)
packages/api/src/queries/task-queries.ts (2)
11-16: Thin wrapper can directly pass through paramsNo transformation occurs. Forwarding the object reduces duplication and future drift.
- async function listByCategory(params: ListTasksParams) { - return dependencies.service.listByCategory({ - categoryId: params.categoryId, - userId: params.userId, - }); - } + async function listByCategory(params: ListTasksParams) { + return dependencies.service.listByCategory(params); + }
18-23: Same here: direct pass‑throughKeeps the query layer minimal and consistent with category queries.
- async function findById(params: FindTaskParams) { - return dependencies.service.findById({ - taskId: params.taskId, - userId: params.userId, - }); - } + async function findById(params: FindTaskParams) { + return dependencies.service.findById(params); + }packages/auth/src/authentication/header.ts (2)
10-15: Authorization scheme comparison likely case‑sensitive in shared extractorMost clients treat the auth scheme as case‑insensitive (RFC 7235). Consider normalizing in extractAuthorizationToken().
Proposed change in packages/auth/src/authentication/shared.ts (for reference):
const headerValue = context.request.headers.get(headerName); if (headerValue === null) throw new AuthenticationError("Missing authorization header"); const expectedPrefix = `${scheme} `; if (!headerValue.toLowerCase().startsWith(expectedPrefix.toLowerCase())) { throw new AuthenticationError("Invalid authorization scheme"); }
10-15: Safer defaults to avoid accidental prod misuseDefaulting to "authorization"/"Bearer" suggests real tokens. For a dev/test header provider, consider
headerName: "x-user-id"and a custom scheme or no scheme.packages/api/src/services/category-service.ts (1)
13-17: Clamp page size to protect the repository/DBGuard against excessive limits and invalid values at the service boundary.
async function listByUserId( params: ListCategoriesRepositoryParams, ): Promise<PaginatedResult<Category>> { - return dependencies.repository.listByUserId(params); + const MAX_LIMIT = 100; + const limit = Math.min(Math.max(params.limit, 1), MAX_LIMIT); + return dependencies.repository.listByUserId({ ...params, limit }); }If the repository already clamps, feel free to skip—just confirm where the guard lives. Based on learnings
packages/api/src/services/task-service.ts (1)
12-22: Drop unnecessary async wrappers and destructure dependenciesSlightly reduces overhead and noise; behavior unchanged.
Apply:
export function createTaskService( dependencies: TaskServiceDependencies, ): TaskService { - async function listByCategory( - params: ListTasksRepositoryParams, - ): Promise<readonly Task[]> { - return dependencies.repository.listByCategory(params); - } + const { repository } = dependencies; + + function listByCategory(params: ListTasksRepositoryParams) { + return repository.listByCategory(params); + } - async function findById( - params: FindTaskRepositoryParams, - ): Promise<Task | null> { - return dependencies.repository.findById(params); - } + function findById(params: FindTaskRepositoryParams) { + return repository.findById(params); + }AGENTS.md (1)
6-11: Docs consistency: Vitest vs Bun test runnerLine 4 still references vitest.config.ts while Line 16 standardizes on Bun’s test runner. Clarify whether Vitest is legacy or remove it to avoid confusion. I can open a follow-up PR to adjust wording.
Also applies to: 16-16
packages/api/src/routes/health.ts (1)
9-12: Hardening: add no-store and a HEAD endpointHealth responses should be non-cacheable; a HEAD /healthz is useful for lightweight probes.
Apply:
app.get("/healthz", (context) => { + context.header("Cache-Control", "no-store"); return context.json({ status: "ok" }); }); + app.head("/healthz", (context) => { + context.header("Cache-Control", "no-store"); + return context.body(null, 200); + }); + app.get("/healthz/database", async (context) => { + context.header("Cache-Control", "no-store"); const checker = options.databaseHealth; if (!checker) { return context.json({ status: "unknown" }, 200); }Also applies to: 13-41
packages/api/src/app.test.ts (3)
61-124: Add an unauthorized test for category endpointsEnsure 401 on missing/invalid auth (per guidelines to cover representative error paths).
Apply:
test("lists categories for a user", async () => { @@ }); + test("returns 401 when Authorization header is missing", async () => { + const authentication = createHeaderAuthentication(); + const app = createApp({ categoryQueries, authentication }); + const res = await app.fetch(createRequest("/users/user-1/categories")); + expect(res.status).toBe(401); + });As per coding guidelines.
126-173: Add an unauthorized test for task endpointsMirror the 401 check for tasks.
Apply:
test("finds task by id", async () => { @@ }); + test("returns 401 when Authorization header is missing (tasks)", async () => { + const authentication = createHeaderAuthentication(); + const app = createApp({ taskQueries, authentication }); + const res = await app.fetch(createRequest(`/tasks/${tasks[0].id}`)); + expect(res.status).toBe(401); + });As per coding guidelines.
175-277: Deduplicate test fixturescreateCategory/createTask and in-memory query factories are useful; consider moving to a shared test util to reuse across suites.
packages/api/src/queries/category-queries.ts (1)
8-32: Keep default limit, but drop unnecessary async and centralize the defaultSlight cleanup and a single source for the default page size.
Apply:
export function createCategoryQueries( dependencies: CategoryQueriesDependencies, ): CategoryQueries { - async function listByUserId(params: ListCategoriesParams) { - const limit = params.limit ?? 20; + const DEFAULT_PAGE_SIZE = 20 as const; + + function listByUserId(params: ListCategoriesParams) { + const limit = params.limit ?? DEFAULT_PAGE_SIZE; return dependencies.service.listByUserId({ userId: params.userId, limit, cursor: params.cursor, }); } - async function findById(params: FindCategoryParams) { + function findById(params: FindCategoryParams) { return dependencies.service.findById({ categoryId: params.categoryId, userId: params.userId, }); }README.md (1)
4-4: Consider clarifying the incremental adoption timeline.While the documentation mentions "incremental" adoption for auth and other packages, it would be helpful to indicate whether the auth package (including the Supabase provider) is already available in this PR or still pending.
packages/api/src/app.ts (1)
7-21: Consider validating required dependencies for production deployments.While using default empty objects allows flexible wiring during development, production deployments might benefit from validation that ensures critical dependencies (like authentication) are properly configured.
Consider adding a validation helper or warning when critical dependencies are missing:
export function createApp(dependencies: AppDependencies = {}): Hono { const app = new Hono(); // Consider logging a warning in production if authentication is missing if (process.env.NODE_ENV === 'production' && !dependencies.authentication) { console.warn('Authentication provider not configured'); } registerHealthRoutes(app, { databaseHealth: dependencies.databaseHealth }); // ... rest of the codepackages/auth/src/authentication/supabase.test.ts (1)
141-158: Consider adding coverage for audience/issuer and missing claimsYou already cover role-mismatch and missing header. Add small tests for invalid audience, invalid issuer, and missing role/subject to harden behavior.
As per coding guidelines (error paths in tests).
packages/api/src/repositories/category-repository.ts (2)
36-42: Clamp limit to a sane range to protect the DBUnbounded/zero/negative limits can degrade performance or error. Clamp, e.g., 1–100.
Apply:
- const limit = params.limit; + const limit = Math.max(1, Math.min(params.limit, 100));
25-43: Add composite index to back the pagination patternFor performance at scale, add an index like (created_by, created_at DESC, id DESC).
Example migration (Postgres):
CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_categories_owner_createdat_id ON categories (created_by, created_at DESC, id DESC);packages/api/src/routes/tasks.ts (2)
56-61: Return WWW-Authenticate header with 401 responsesHelps clients understand the auth scheme and aligns with RFC 6750.
Apply:
- if (authResult === null) { - return context.json({ error: "Unauthorized" }, 401); - } + if (authResult === null) { + context.header("WWW-Authenticate", 'Bearer realm="listee"'); + return context.json({ error: "Unauthorized" }, 401); + } @@ - if (authResult === null) { - return context.json({ error: "Unauthorized" }, 401); - } + if (authResult === null) { + context.header("WWW-Authenticate", 'Bearer realm="listee"'); + return context.json({ error: "Unauthorized" }, 401); + }Also applies to: 71-75
45-55: Reduce duplication by authenticating via middlewareUse Hono middleware to authenticate once and stash user in context; route handlers can then assume presence.
Sketch:
app.use("/categories/*", async (c, next) => { const auth = await tryAuthenticate(authentication, c.req.raw); if (!auth) { c.header("WWW-Authenticate", 'Bearer realm="listee"'); return c.json({ error: "Unauthorized" }, 401); } c.set("user", auth.user); await next(); });Then read c.get("user") in handlers.
Also applies to: 56-89
packages/types/src/db.ts (1)
22-26: Minor: Consider ReadonlyArray for API surfaceUsing ReadonlyArray instead of readonly T[] can be slightly clearer for consumers and aligns with many TS API surfaces. Optional, stylistic.
packages/auth/src/authentication/shared.ts (1)
34-41: Make scheme check case-insensitive and trim firstAuthorization schemes are case-insensitive (e.g., “bearer”). Trim then compare lowercased to avoid false negatives.
- const expectedPrefix = `${scheme} `; - if (!headerValue.startsWith(expectedPrefix)) { + const normalizedHeader = headerValue.trim(); + const expectedPrefix = `${scheme} `; + if (!normalizedHeader.toLowerCase().startsWith(expectedPrefix.toLowerCase())) { throw new AuthenticationError("Invalid authorization scheme"); } - const tokenValue = headerValue.slice(expectedPrefix.length).trim(); + const tokenValue = normalizedHeader.slice(expectedPrefix.length).trim();packages/auth/src/authentication/supabase.ts (1)
70-83: Small hardening: set typ and cap token sizeTighten verification a bit by asserting typ and bounding token size to reduce parser abuse.
- const verifyOptions: JWTVerifyOptions = {}; + const verifyOptions: JWTVerifyOptions = { typ: "JWT", maxTokenSize: 8 * 1024 }; if (issuer.length > 0) { verifyOptions.issuer = issuer; }packages/db/src/schema/index.ts (2)
13-20: updatedAt won’t auto-change without triggersdefaultNow() sets only the initial value. Consider a trigger (e.g., BEFORE UPDATE) to keep updatedAt current.
26-33: Optional: FK for defaultCategoryIdIf you want referential integrity, consider referencing categories.id with ON DELETE SET NULL. Beware the cycle; acceptable if your migration order handles it.
packages/api/src/routes/categories.ts (2)
67-69: Don’t silently no-op when dependencies are missing. Log or fail fast.Returning early hides misconfiguration and makes routes disappear with no signal. At least warn in non-prod.
- if (queries === undefined || authentication === undefined) { - return; - } + if (queries === undefined || authentication === undefined) { + if (process.env.NODE_ENV !== "production") { + console.warn("[registerCategoryRoutes] Skipping route registration: missing queries or authentication"); + } + return; + }
82-88: Enforce a sane max for “limit”.Prevents accidental abuse and unbounded queries.
- const limitParam = context.req.query("limit"); + const limitParam = context.req.query("limit"); const cursor = context.req.query("cursor") ?? null; const limit = parsePositiveInteger(limitParam); if (limitParam !== undefined && limit === undefined) { return context.json({ error: "Invalid limit parameter" }, 400); } + const MAX_LIMIT = 100; + if (limit !== undefined && limit > MAX_LIMIT) { + return context.json({ error: `limit must be <= ${MAX_LIMIT}` }, 400); + }packages/db/src/index.ts (3)
72-74: Enable connection reuse in production too.Right now, reuse only caches in dev; production creates a new connection per call. Cache globally in all envs.
- if (process.env.NODE_ENV !== "production") { - globalThis.__pgConn = connection; - } + globalThis.__pgConn = connection;
87-93: Harden role handling: normalize + whitelist.Lowercase to match PG identifier behavior and restrict to allowed roles to avoid privilege escalation via tokens.
-function sanitizeRole(role: unknown): string { - if (typeof role === "string" && /^[A-Za-z0-9_]+$/.test(role)) { - return role; - } +function sanitizeRole(role: unknown): string { + if (typeof role === "string") { + const normalized = role.toLowerCase(); + if (/^[a-z0-9_]+$/.test(normalized) && (normalized === "anon" || normalized === "authenticated")) { + return normalized; + } + } return "anon"; }
152-157: Naming:createDrizzlereturns an RLS client, not a Drizzle DB.Avoid confusion with
drizzle()from the ORM; consider renaming tocreateRlsClient(keep old as alias).packages/types/src/api.ts (9)
32-35: Paginate task listings for parity and scalabilityCategories are paginated, tasks are not. Returning unbounded arrays will become a perf and API‑stability issue later.
Apply:
export interface TaskQueries { - listByCategory(params: ListTasksParams): Promise<readonly Task[]>; + listByCategory(params: ListTasksParams): Promise<ListTasksResult>; findById(params: FindTaskParams): Promise<Task | null>; }And add the result alias (see near Line 31 insertion in a separate comment).
83-86: Repository should also return a paginated result for tasksAlign repository shape with queries to avoid later breaking changes.
export interface TaskRepository { - listByCategory(params: ListTasksRepositoryParams): Promise<readonly Task[]>; + listByCategory(params: ListTasksRepositoryParams): Promise<PaginatedResult<Task>>; findById(params: FindTaskRepositoryParams): Promise<Task | null>; }
88-91: Service should mirror paginated repository outputKeep service and repository contracts consistent.
export interface TaskService { - listByCategory(params: ListTasksRepositoryParams): Promise<readonly Task[]>; + listByCategory(params: ListTasksRepositoryParams): Promise<PaginatedResult<Task>>; findById(params: FindTaskRepositoryParams): Promise<Task | null>; }
22-25: Add pagination inputs to task listing paramsExpose limit/cursor at the public API like categories.
export interface ListTasksParams { readonly categoryId: string; readonly userId?: string; + readonly limit?: number; + readonly cursor?: string; }
31-31: Introduce ListTasksResult aliasKeeps symmetry with categories and eases refactors.
+export type ListTasksResult = PaginatedResult<Task>;
73-76: Add pagination inputs to task repository paramsRequired limit matches the category repository shape and centralizes defaulting in higher layers.
export interface ListTasksRepositoryParams { readonly categoryId: string; readonly userId?: string; + readonly limit: number; + readonly cursor?: string; }
37-40: Type the error field as unknown to avoid leaking internalsStringly‑typed error encourages passing raw messages. unknown is safer and matches “prefer unknown for external inputs”.
export interface DatabaseHealthStatus { readonly ok: boolean; - readonly error?: string; + readonly error?: unknown; }Optionally add a redacted
errorMessage?: stringat the HTTP boundary. As per coding guidelines.
62-67: Decouple service contracts from repository paramsHave services use domain/query params; keep repository specifics (like required limit) in the repo layer. This preserves layering and makes defaults/caps a service concern.
export interface CategoryService { - listByUserId( - params: ListCategoriesRepositoryParams, - ): Promise<PaginatedResult<Category>>; - findById(params: FindCategoryRepositoryParams): Promise<Category | null>; + listByUserId(params: ListCategoriesParams): Promise<ListCategoriesResult>; + findById(params: FindCategoryParams): Promise<Category | null>; }
4-8: Reduce nullable states for cursorsInputs don’t need
null;undefinedalready represents “no cursor”. Outputs can still usenullfor “no next page”.readonly limit?: number; - readonly cursor?: string | null; + readonly cursor?: string;readonly limit: number; - readonly cursor?: string | null; + readonly cursor?: string;Also applies to: 44-48
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (41)
AGENTS.md(1 hunks)README.md(2 hunks)biome.json(1 hunks)package.json(1 hunks)packages/api/package.json(1 hunks)packages/api/src/app.test.ts(1 hunks)packages/api/src/app.ts(1 hunks)packages/api/src/index.ts(1 hunks)packages/api/src/infrastructure/database-health.ts(1 hunks)packages/api/src/queries/category-queries.ts(1 hunks)packages/api/src/queries/task-queries.ts(1 hunks)packages/api/src/repositories/category-repository.ts(1 hunks)packages/api/src/repositories/task-repository.ts(1 hunks)packages/api/src/routes/categories.ts(1 hunks)packages/api/src/routes/health.ts(1 hunks)packages/api/src/routes/tasks.ts(1 hunks)packages/api/src/services/category-service.ts(1 hunks)packages/api/src/services/task-service.ts(1 hunks)packages/api/src/utils/error.ts(1 hunks)packages/api/tsconfig.json(1 hunks)packages/auth/package.json(1 hunks)packages/auth/src/authentication/errors.ts(1 hunks)packages/auth/src/authentication/header.ts(1 hunks)packages/auth/src/authentication/index.ts(1 hunks)packages/auth/src/authentication/shared.ts(1 hunks)packages/auth/src/authentication/supabase.test.ts(1 hunks)packages/auth/src/authentication/supabase.ts(1 hunks)packages/auth/src/index.ts(1 hunks)packages/auth/tsconfig.json(1 hunks)packages/db/package.json(1 hunks)packages/db/src/index.test.ts(1 hunks)packages/db/src/index.ts(1 hunks)packages/db/src/schema/index.ts(1 hunks)packages/db/tsconfig.json(1 hunks)packages/types/package.json(1 hunks)packages/types/src/api.ts(1 hunks)packages/types/src/authentication.ts(1 hunks)packages/types/src/db.ts(1 hunks)packages/types/src/index.ts(1 hunks)packages/types/tsconfig.json(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
packages/*/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all package source files under packages//src/
Files:
packages/auth/src/authentication/index.tspackages/auth/src/index.tspackages/api/src/routes/health.tspackages/auth/src/authentication/shared.tspackages/auth/src/authentication/supabase.tspackages/api/src/queries/task-queries.tspackages/types/src/index.tspackages/api/src/queries/category-queries.tspackages/auth/src/authentication/header.tspackages/types/src/api.tspackages/db/src/schema/index.tspackages/api/src/repositories/category-repository.tspackages/auth/src/authentication/errors.tspackages/types/src/authentication.tspackages/api/src/services/task-service.tspackages/api/src/app.tspackages/api/src/utils/error.tspackages/api/src/index.tspackages/auth/src/authentication/supabase.test.tspackages/api/src/services/category-service.tspackages/api/src/routes/categories.tspackages/api/src/infrastructure/database-health.tspackages/api/src/repositories/task-repository.tspackages/db/src/index.test.tspackages/api/src/routes/tasks.tspackages/types/src/db.tspackages/db/src/index.tspackages/api/src/app.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid implicit any in TypeScript
Prefer unknown for external inputs in TypeScript
Replace as casts with dedicated type guards or the satisfies operator where appropriate
Use PascalCase for types and enums
Use camelCase for variables and functions
Files:
packages/auth/src/authentication/index.tspackages/auth/src/index.tspackages/api/src/routes/health.tspackages/auth/src/authentication/shared.tspackages/auth/src/authentication/supabase.tspackages/api/src/queries/task-queries.tspackages/types/src/index.tspackages/api/src/queries/category-queries.tspackages/auth/src/authentication/header.tspackages/types/src/api.tspackages/db/src/schema/index.tspackages/api/src/repositories/category-repository.tspackages/auth/src/authentication/errors.tspackages/types/src/authentication.tspackages/api/src/services/task-service.tspackages/api/src/app.tspackages/api/src/utils/error.tspackages/api/src/index.tspackages/auth/src/authentication/supabase.test.tspackages/api/src/services/category-service.tspackages/api/src/routes/categories.tspackages/api/src/infrastructure/database-health.tspackages/api/src/repositories/task-repository.tspackages/db/src/index.test.tspackages/api/src/routes/tasks.tspackages/types/src/db.tspackages/db/src/index.tspackages/api/src/app.test.ts
{tsconfig.json,biome.json,vitest.config.ts,bun.lock}
📄 CodeRabbit inference engine (AGENTS.md)
Review cross-package impact before modifying shared root tooling files (tsconfig.json, biome.json, vitest.config.ts, bun.lock)
Files:
tsconfig.jsonbiome.json
tsconfig.json
📄 CodeRabbit inference engine (AGENTS.md)
Ensure TypeScript strict mode is enabled
Files:
tsconfig.json
**/{__tests__/**/*.ts,*.test.ts}
📄 CodeRabbit inference engine (AGENTS.md)
**/{__tests__/**/*.ts,*.test.ts}: Co-locate tests as *.test.ts files or inside tests/ directories
Name test suites with behavior-focused sentences
For new features, cover both success paths and representative error paths in tests
Files:
packages/auth/src/authentication/supabase.test.tspackages/db/src/index.test.tspackages/api/src/app.test.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: listee-dev/listee-libs#0
File: AGENTS.md:0-0
Timestamp: 2025-09-16T13:20:43.061Z
Learning: Applies to {tsconfig.json,biome.json,vitest.config.ts,bun.lock} : Review cross-package impact before modifying shared root tooling files (tsconfig.json, biome.json, vitest.config.ts, bun.lock)
📚 Learning: 2025-09-16T13:20:43.061Z
Learnt from: CR
PR: listee-dev/listee-libs#0
File: AGENTS.md:0-0
Timestamp: 2025-09-16T13:20:43.061Z
Learning: Applies to {tsconfig.json,biome.json,vitest.config.ts,bun.lock} : Review cross-package impact before modifying shared root tooling files (tsconfig.json, biome.json, vitest.config.ts, bun.lock)
Applied to files:
packages/api/tsconfig.jsonpackages/types/tsconfig.jsonpackages/db/tsconfig.jsontsconfig.jsonpackage.jsonAGENTS.mdbiome.jsonpackages/auth/tsconfig.json
📚 Learning: 2025-09-16T13:20:43.061Z
Learnt from: CR
PR: listee-dev/listee-libs#0
File: AGENTS.md:0-0
Timestamp: 2025-09-16T13:20:43.061Z
Learning: Applies to tsconfig.json : Ensure TypeScript strict mode is enabled
Applied to files:
packages/types/tsconfig.jsonpackages/db/tsconfig.jsonpackages/auth/tsconfig.json
📚 Learning: 2025-09-16T13:20:43.061Z
Learnt from: CR
PR: listee-dev/listee-libs#0
File: AGENTS.md:0-0
Timestamp: 2025-09-16T13:20:43.061Z
Learning: Applies to **/{__tests__/**/*.ts,*.test.ts} : Co-locate tests as *.test.ts files or inside __tests__/ directories
Applied to files:
packages/types/tsconfig.jsonpackages/db/tsconfig.jsonpackages/auth/tsconfig.json
📚 Learning: 2025-09-16T13:20:43.061Z
Learnt from: CR
PR: listee-dev/listee-libs#0
File: AGENTS.md:0-0
Timestamp: 2025-09-16T13:20:43.061Z
Learning: Applies to **/{__tests__/**/*.ts,*.test.ts} : For new features, cover both success paths and representative error paths in tests
Applied to files:
packages/api/src/app.test.ts
🧬 Code graph analysis (20)
packages/api/src/routes/health.ts (3)
packages/api/src/index.ts (1)
registerHealthRoutes(20-20)packages/types/src/api.ts (1)
RegisterHealthRoutesOptions(115-117)packages/api/src/utils/error.ts (1)
toErrorMessage(1-15)
packages/auth/src/authentication/shared.ts (2)
packages/auth/src/authentication/errors.ts (1)
AuthenticationError(1-6)packages/types/src/authentication.ts (1)
AuthenticationContext(8-10)
packages/auth/src/authentication/supabase.ts (3)
packages/types/src/authentication.ts (4)
SupabaseAuthenticationOptions(25-33)AuthenticationProvider(16-18)AuthenticationContext(8-10)AuthenticationResult(12-14)packages/auth/src/authentication/shared.ts (2)
extractAuthorizationToken(24-45)assertNonEmptyString(16-22)packages/auth/src/authentication/errors.ts (1)
AuthenticationError(1-6)
packages/api/src/queries/task-queries.ts (1)
packages/types/src/api.ts (4)
TaskQueriesDependencies(101-103)TaskQueries(32-35)ListTasksParams(22-25)FindTaskParams(27-30)
packages/api/src/queries/category-queries.ts (1)
packages/types/src/api.ts (4)
CategoryQueriesDependencies(97-99)CategoryQueries(17-20)ListCategoriesParams(4-8)FindCategoryParams(12-15)
packages/auth/src/authentication/header.ts (2)
packages/types/src/authentication.ts (4)
HeaderAuthenticationOptions(20-23)AuthenticationProvider(16-18)AuthenticationContext(8-10)AuthenticationResult(12-14)packages/auth/src/authentication/shared.ts (1)
extractAuthorizationToken(24-45)
packages/types/src/api.ts (2)
packages/types/src/db.ts (1)
PaginatedResult(22-26)packages/types/src/authentication.ts (1)
AuthenticationProvider(16-18)
packages/api/src/repositories/category-repository.ts (4)
packages/db/src/index.ts (2)
db(85-85)Database(82-82)packages/types/src/api.ts (3)
CategoryRepository(55-60)ListCategoriesRepositoryParams(44-48)FindCategoryRepositoryParams(50-53)packages/types/src/db.ts (2)
PaginatedResult(22-26)Category(11-11)packages/db/src/schema/index.ts (2)
Category(100-100)categories(57-98)
packages/types/src/authentication.ts (1)
packages/types/src/db.ts (1)
SupabaseToken(11-11)
packages/api/src/services/task-service.ts (3)
packages/api/src/index.ts (1)
createTaskService(23-23)packages/types/src/api.ts (4)
TaskServiceDependencies(93-95)TaskService(88-91)ListTasksRepositoryParams(73-76)FindTaskRepositoryParams(78-81)packages/db/src/schema/index.ts (1)
Task(157-157)
packages/api/src/app.ts (4)
packages/types/src/api.ts (1)
AppDependencies(119-124)packages/api/src/routes/health.ts (1)
registerHealthRoutes(5-42)packages/api/src/routes/categories.ts (1)
registerCategoryRoutes(60-124)packages/api/src/routes/tasks.ts (1)
registerTaskRoutes(45-89)
packages/auth/src/authentication/supabase.test.ts (3)
packages/auth/src/authentication/errors.ts (1)
AuthenticationError(1-6)packages/types/src/authentication.ts (1)
SupabaseAuthenticationOptions(25-33)packages/auth/src/authentication/supabase.ts (1)
createSupabaseAuthentication(52-109)
packages/api/src/services/category-service.ts (3)
packages/types/src/api.ts (4)
CategoryServiceDependencies(69-71)CategoryService(62-67)ListCategoriesRepositoryParams(44-48)FindCategoryRepositoryParams(50-53)packages/types/src/db.ts (2)
PaginatedResult(22-26)Category(11-11)packages/db/src/schema/index.ts (1)
Category(100-100)
packages/api/src/routes/categories.ts (3)
packages/types/src/api.ts (1)
RegisterCategoryRoutesOptions(105-108)packages/auth/src/index.ts (3)
AuthenticationProvider(4-4)AuthenticationResult(5-5)AuthenticationError(8-8)packages/types/src/authentication.ts (2)
AuthenticationProvider(16-18)AuthenticationResult(12-14)
packages/api/src/infrastructure/database-health.ts (3)
packages/db/src/index.ts (3)
db(85-85)Database(82-82)sql(159-159)packages/types/src/api.ts (1)
DatabaseHealthChecker(42-42)packages/api/src/utils/error.ts (1)
toErrorMessage(1-15)
packages/api/src/repositories/task-repository.ts (3)
packages/db/src/index.ts (2)
db(85-85)Database(82-82)packages/types/src/api.ts (3)
TaskRepository(83-86)ListTasksRepositoryParams(73-76)FindTaskRepositoryParams(78-81)packages/db/src/schema/index.ts (2)
Task(157-157)tasks(102-155)
packages/db/src/index.test.ts (1)
packages/db/src/index.ts (2)
createPostgresConnection(55-80)createRlsClient(118-150)
packages/api/src/routes/tasks.ts (3)
packages/types/src/api.ts (1)
RegisterTaskRoutesOptions(110-113)packages/db/src/schema/index.ts (1)
tasks(102-155)packages/types/src/authentication.ts (2)
AuthenticationProvider(16-18)AuthenticationResult(12-14)
packages/types/src/db.ts (1)
packages/db/src/schema/index.ts (3)
profiles(22-53)categories(57-98)tasks(102-155)
packages/api/src/app.test.ts (5)
packages/api/src/app.ts (1)
createApp(7-21)packages/api/src/queries/category-queries.ts (1)
createCategoryQueries(8-32)packages/auth/src/authentication/index.ts (1)
createHeaderAuthentication(2-2)packages/api/src/queries/task-queries.ts (1)
createTaskQueries(8-29)packages/types/src/api.ts (3)
CategoryQueries(17-20)ListCategoriesResult(10-10)TaskQueries(32-35)
🔇 Additional comments (34)
biome.json (1)
1-28: Biome configuration looks good for the monorepo.The configuration uses Biome 2.2.4 which is relatively current. The formatter settings are appropriate with space indentation and double quotes for JavaScript, organizeImports is properly enabled, and the linter uses recommended rules. The file includes pattern correctly targets the monorepo structure.
Based on learnings: Confirms compliance with the requirement to review cross-package impact for shared tooling files.
packages/db/package.json (1)
1-20: Package structure follows modern TypeScript library patterns.The package configuration is well-structured with proper module type declarations, build scripts, and export paths. The
sideEffects: falseflag is appropriate for a database utility library.packages/types/package.json (2)
16-18: Workspace dependency on @listee/db is correctly defined.The dependency uses
workspace:^range which is appropriate for monorepo internal dependencies and allows for automatic version resolution within the workspace.
1-19: Package manifest follows consistent patterns with the db package.The structure mirrors the db package appropriately with matching build scripts, export configuration, and TypeScript setup. This consistency will help with maintainability across the monorepo.
packages/types/src/authentication.ts (4)
1-1: Import statement follows TypeScript best practices.The type-only import is correctly used for the SupabaseToken type from the db module.
3-6: AuthenticatedUser interface is well-designed.The interface properly uses readonly modifiers for immutability and has a clear, focused responsibility with just the essential user identification and token data.
25-33: SupabaseAuthenticationOptions interface provides comprehensive configuration.The interface extends HeaderAuthenticationOptions appropriately and includes all necessary Supabase-specific configuration options. The optional fields with sensible defaults (like
jwksPathandclockToleranceSeconds) provide flexibility while maintaining ease of use.
16-18: AuthenticationProvider interface follows good async patterns.The provider interface properly returns a Promise for the async authentication operation, which is appropriate for JWT verification and external service calls.
packages/types/tsconfig.json (2)
3-8: TypeScript configuration is properly set up for library compilation.The configuration correctly extends the root tsconfig, enables declaration generation for type definitions, and uses
verbatimModuleSyntaxfor proper ESM output. TheoutDiris appropriately set to match the package.json exports.
9-11: Include and exclude patterns are correctly configured.The patterns properly target the source files while excluding the build output directory to prevent circular references during compilation.
packages/api/src/services/category-service.ts (1)
25-28: LGTM: simple pass‑throughs keep the service focusedThe delegation to the repository is clean and aligns with the types.
packages/api/src/queries/task-queries.ts (1)
25-29: Export wiring verified createTaskQueries is re-exported in packages/api/src/index.ts (line 16) and consumed in task routes tests in packages/api/src/app.test.ts [line 127]packages/api/src/services/task-service.ts (1)
9-28: Thin service wrapper looks goodStraightforward delegation to the repository; types align with @listee/types.
AGENTS.md (1)
33-41: Architecture playbook reads wellClear dependency direction and auth flow; matches the code structure in this PR.
packages/api/src/app.test.ts (2)
16-59: Health route coverage looks solidCovers ok/unknown/error paths and status codes.
1-3: Ensure Supabase authentication tests are executed in CI
I didn’t find any CI configuration invokingsupabase.test.ts; please confirm your CI workflow includes and runs the Supabase auth tests.packages/auth/src/index.ts (1)
1-11: LGTM! Clean module exports with proper separation of concerns.The barrel export pattern correctly separates type exports from implementation exports, maintaining a clear distinction between the type system (
@listee/types) and runtime code (./authentication/index.js).README.md (2)
15-15: Repository root installation command is correct.Good practice to specify catalog-aware installation at the repository root for Bun workspaces.
36-51: Excellent architecture guidelines! Clear separation of concerns.The guidelines effectively establish:
- Clear responsibility boundaries between layers
- Unidirectional dependency flow (routes → queries → services → repositories)
- Proper separation of authentication and authorization concerns
This architecture supports both testability and runtime flexibility.
packages/api/src/app.ts (2)
23-23: Good type extraction pattern.The
AppFetchtype alias cleanly captures the Hono app's fetch signature for reuse.
25-35: Clean fetch handler wrapper implementation.The
createFetchHandlerfunction correctly delegates to the Hono app's fetch method while preserving the full signature including optional environment and execution context parameters.packages/db/src/index.test.ts (5)
1-8: Excellent lazy loading pattern for test setup.The approach of importing only types and deferring module evaluation until after mocks are configured ensures proper test isolation and prevents binding issues with the mocked dependencies.
110-115: Good test environment setup.Setting the initial
POSTGRES_URLinbeforeAlland then usingbeforeEachfor test-specific setup ensures proper isolation between test cases.
125-156: Comprehensive connection caching tests.The test suite thoroughly validates:
- Connection reuse behavior
- Explicit opt-out via
reuseConnection: false- Error handling for missing connection strings
- Override behavior with explicit connection strings
158-201: Thorough RLS client testing with good attention to detail.The tests effectively validate:
- Transaction wrapping with setup/teardown queries
- Token serialization and claim injection
- Role sanitization (checking
rawValues.at(-1)for the sanitized role)- Proper cleanup with NULL values and role reset
The use of
renderSqlhelper to inspect generated SQL is particularly useful for validation.
187-187: Resolve: Default role fallback is correctly set to “anon” and the test assertion is valid.packages/auth/src/authentication/supabase.test.ts (1)
95-107: Nice JWKS mocking and fetch restoreThe JWKS mock with preserved fetch.preconnect and restore in finally is clean and Bun-friendly. Good isolation for tests.
Also applies to: 111-133, 160-165
packages/api/src/routes/tasks.ts (1)
21-43: Response mapper looks goodConsistent ISO conversion and explicit shape. Keep it colocated here unless shared elsewhere.
packages/api/src/repositories/task-repository.ts (1)
20-25: The script above will locate and display the start of your TaskRepository file so we can confirm the filtering logic. Please review the output and we’ll proceed to verify RLS configuration next.packages/types/src/db.ts (1)
1-11: SupabaseToken import and re-export is correct
@listee/db exportsSupabaseToken(packages/db/src/index.ts:95), so the import and re-export in packages/types/src/db.ts is valid.Likely an incorrect or invalid review comment.
packages/api/src/index.ts (1)
1-23: TS module settings validated; no changes needed. Roottsconfig.jsonuses"module": "ESNext"and"moduleResolution": "Bundler", and all package-level configs extend it withverbatimModuleSyntax.packages/types/src/api.ts (3)
105-113: Route options look solidOptional injection of queries and authentication is flexible for public vs. protected routes.
119-124: AppDependencies shape LGTMClear DI surface for wiring health, queries, and auth.
12-15: Optional userId usage verified safe
All service, query, and route layers consistently supplyuserId, so there’s no path that omits it and exposes unscoped data.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
AGENTS.md(1 hunks)README.md(2 hunks)packages/api/src/repositories/category-repository.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- AGENTS.md
🧰 Additional context used
📓 Path-based instructions (2)
packages/*/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all package source files under packages//src/
Files:
packages/api/src/repositories/category-repository.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid implicit any in TypeScript
Prefer unknown for external inputs in TypeScript
Replace as casts with dedicated type guards or the satisfies operator where appropriate
Use PascalCase for types and enums
Use camelCase for variables and functions
Files:
packages/api/src/repositories/category-repository.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: listee-dev/listee-libs#0
File: AGENTS.md:0-0
Timestamp: 2025-09-16T13:20:43.061Z
Learning: Applies to {tsconfig.json,biome.json,vitest.config.ts,bun.lock} : Review cross-package impact before modifying shared root tooling files (tsconfig.json, biome.json, vitest.config.ts, bun.lock)
🧬 Code graph analysis (1)
packages/api/src/repositories/category-repository.ts (4)
packages/db/src/index.ts (2)
db(85-85)Database(82-82)packages/types/src/api.ts (3)
CategoryRepository(55-60)ListCategoriesRepositoryParams(44-48)FindCategoryRepositoryParams(50-53)packages/types/src/db.ts (2)
PaginatedResult(22-26)Category(11-11)packages/db/src/schema/index.ts (2)
Category(100-100)categories(57-98)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/db/src/schema/index.ts (2)
13-20: Add automatic update behavior forupdatedAt.The
updatedAtfield currently usesdefaultNow(), which only sets the timestamp on insert. It will not automatically update when rows are modified. Use$onUpdateto ensureupdatedAtreflects the last modification time.Apply this diff to add automatic update behavior:
const timestamps = { createdAt: timestamp("created_at", { withTimezone: true }) .notNull() .defaultNow(), updatedAt: timestamp("updated_at", { withTimezone: true }) .notNull() - .defaultNow(), + .defaultNow() + .$onUpdate(() => new Date()), };Based on learnings
22-53: Add foreign key constraint fordefaultCategoryId.The
defaultCategoryIdfield references a category but lacks a foreign key constraint, which could lead to referential integrity issues (e.g., orphaned references when categories are deleted).Apply this diff to add the constraint:
name: text("name"), - defaultCategoryId: uuid("default_category_id"), + defaultCategoryId: uuid("default_category_id") + .references((): AnyPgColumn => categories.id, { onDelete: "set null" }), ...timestamps,Note: Use
onDelete: "set null"to automatically clear the default when the referenced category is deleted, oronDelete: "restrict"to prevent deletion of categories that are set as defaults.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/db/src/schema/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/*/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all package source files under packages//src/
Files:
packages/db/src/schema/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid implicit any in TypeScript
Prefer unknown for external inputs in TypeScript
Replace as casts with dedicated type guards or the satisfies operator where appropriate
Use PascalCase for types and enums
Use camelCase for variables and functions
Files:
packages/db/src/schema/index.ts
🔇 Additional comments (6)
packages/db/src/schema/index.ts (6)
1-11: LGTM!The imports are appropriate for defining PostgreSQL tables with RLS policies using drizzle-orm and Supabase integration.
55-55: LGTM!The
Profiletype export correctly uses drizzle-orm's$inferSelectfor type-safe schema inference.
57-98: LGTM!The categories table definition is well-structured. The RLS policies correctly enforce ownership checks, and the update policy now properly mirrors the
usingcondition inwithCheck(addressing the previous privilege drift concern).
100-100: LGTM!The
Categorytype export correctly uses$inferSelectfor type inference.
102-155: LGTM!The tasks table definition is well-designed with proper access control. The
hasAccesspolicy correctly grants access to both task creators and category owners via the EXISTS subquery, and the update policy now properly useswithCheck: hasAccessto prevent privilege drift.
157-157: LGTM!The
Tasktype export correctly uses$inferSelect.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/api/src/routes/auth-utils.ts(1 hunks)packages/api/src/routes/categories.ts(1 hunks)packages/api/src/routes/tasks.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/api/src/routes/tasks.ts
🧰 Additional context used
📓 Path-based instructions (2)
packages/*/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all package source files under packages//src/
Files:
packages/api/src/routes/auth-utils.tspackages/api/src/routes/categories.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid implicit any in TypeScript
Prefer unknown for external inputs in TypeScript
Replace as casts with dedicated type guards or the satisfies operator where appropriate
Use PascalCase for types and enums
Use camelCase for variables and functions
Files:
packages/api/src/routes/auth-utils.tspackages/api/src/routes/categories.ts
🧬 Code graph analysis (1)
packages/api/src/routes/categories.ts (2)
packages/types/src/api.ts (1)
RegisterCategoryRoutesOptions(105-108)packages/api/src/routes/auth-utils.ts (1)
tryAuthenticate(7-20)
🔇 Additional comments (1)
packages/api/src/routes/auth-utils.ts (1)
7-19: Helper cleanly normalizes auth failures
Good separation between expectedAuthenticationErrorcases (returningnull) and unexpected errors that should bubble up. 👍
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/db/tsconfig.build.json (1)
4-4: Consider including declaration files in the build.Right now the program only includes
.tsfiles. Any hand-written.d.tsinsrcwould be ignored, which can drop ambient declarations during the build. Add the pattern so TSC picks them up if they appear.- "include": ["src/**/*.ts"] + "include": ["src/**/*.ts", "src/**/*.d.ts"]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/workflows/ci.yml(1 hunks)packages/api/package.json(1 hunks)packages/api/tsconfig.build.json(1 hunks)packages/api/tsconfig.json(1 hunks)packages/auth/package.json(1 hunks)packages/auth/src/authentication/supabase.test.ts(1 hunks)packages/auth/tsconfig.build.json(1 hunks)packages/auth/tsconfig.json(1 hunks)packages/db/package.json(1 hunks)packages/db/tsconfig.build.json(1 hunks)packages/db/tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/api/tsconfig.build.json
- .github/workflows/ci.yml
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/db/tsconfig.json
- packages/auth/package.json
- packages/api/package.json
- packages/db/package.json
🧰 Additional context used
📓 Path-based instructions (3)
packages/*/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all package source files under packages//src/
Files:
packages/auth/src/authentication/supabase.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid implicit any in TypeScript
Prefer unknown for external inputs in TypeScript
Replace as casts with dedicated type guards or the satisfies operator where appropriate
Use PascalCase for types and enums
Use camelCase for variables and functions
Files:
packages/auth/src/authentication/supabase.test.ts
**/{__tests__/**/*.ts,*.test.ts}
📄 CodeRabbit inference engine (AGENTS.md)
**/{__tests__/**/*.ts,*.test.ts}: Co-locate tests as *.test.ts files or inside tests/ directories
Name test suites with behavior-focused sentences
For new features, cover both success paths and representative error paths in tests
Files:
packages/auth/src/authentication/supabase.test.ts
🧠 Learnings (1)
📚 Learning: 2025-09-16T13:20:43.061Z
Learnt from: CR
PR: listee-dev/listee-libs#0
File: AGENTS.md:0-0
Timestamp: 2025-09-16T13:20:43.061Z
Learning: Applies to {tsconfig.json,biome.json,vitest.config.ts,bun.lock} : Review cross-package impact before modifying shared root tooling files (tsconfig.json, biome.json, vitest.config.ts, bun.lock)
Applied to files:
packages/api/tsconfig.json
🧬 Code graph analysis (1)
packages/auth/src/authentication/supabase.test.ts (3)
packages/auth/src/authentication/errors.ts (1)
AuthenticationError(1-6)packages/types/src/authentication.ts (2)
SupabaseAuthenticationOptions(33-41)AuthenticatedToken(9-9)packages/auth/src/authentication/supabase.ts (1)
createSupabaseAuthentication(52-109)
🔇 Additional comments (8)
packages/api/tsconfig.json (1)
3-12: API tsconfig aligns with package build requirementsDeclaration emit, Bun types, and the shared rootDirs setup match the package’s needs without introducing new risks. Nice addition.
packages/auth/tsconfig.json (1)
1-11: LGTM! Clean package-level TypeScript configuration.The config correctly extends the root tsconfig, enables declaration/map generation for consumers, outputs to
dist, and usesverbatimModuleSyntaxfor ESM correctness. Source inclusion and exclusion patterns are appropriate.packages/auth/src/authentication/supabase.test.ts (6)
1-8: LGTM! Clean and well-typed imports.All imports are explicit and type-safe. The use of the
typekeyword for @listee/types imports is good practice, and the jose imports align with current best practices for JWT handling.
10-81: Excellent test coverage and structure.The test suite follows best practices:
- Behavior-focused test names clearly describe the scenario.
- Success path and representative error paths are covered (valid token, role mismatch, missing header).
- Proper cleanup with try/finally ensures global fetch is restored.
- Past issues with missing
awaiton rejection assertions have been addressed.
83-98: LGTM! Well-typed and consistent test helper interfaces.The interfaces use indexed access types to stay aligned with
SupabaseAuthenticationOptions, enforce immutability withreadonly, and follow PascalCase naming conventions. Type safety is maintained withReturnType<typeof ...>for the provider.
100-170: LGTM! Robust and well-structured test helper.The implementation is excellent:
- Correctly uses jose's
generateKeyPair,exportJWK, andSignJWTfollowing current best practices.- Mock fetch properly intercepts JWKS requests and preserves the
preconnectstatic property.- Uses
satisfies typeof fetchto ensure type correctness of the mock.- The
signTokenhelper builds valid RS256 JWTs with appropriate claims and headers.- Cleanup via
restoreensures no side effects between tests.Based on learnings about jose 6.1.0.
172-185: LGTM! Clean type guard implementation.The assertion function correctly narrows
AuthenticatedTokentoSupabaseTokenwith appropriate runtime checks and a clear error message. This follows guidelines by using a type guard instead of type assertions.
187-197: LGTM! Correct URL resolution utility.The function correctly handles all variants of
RequestInfo | URL(string, URL instance, Request object) and extracts the URL string appropriately. Logic is sound and type-safe.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/api/src/routes/categories.ts (1)
43-58: Validation improved, but redundant check remains.The regex
/^[1-9]\d*$/correctly addresses the past review concern by rejecting mixed alphanumeric inputs like "10abc". However, the subsequent checkparsed <= 0at line 53 is now redundant since the regex already ensures the number starts with 1-9 and will always be positive.Consider simplifying the function:
function parsePositiveInteger(value: string | undefined): number | undefined { if (value === undefined) { return undefined; } if (!/^[1-9]\d*$/.test(value)) { return undefined; } - const parsed = Number.parseInt(value, 10); - if (parsed <= 0) { - return undefined; - } - - return parsed; + return Number.parseInt(value, 10); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/api/src/routes/categories.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/*/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all package source files under packages//src/
Files:
packages/api/src/routes/categories.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid implicit any in TypeScript
Prefer unknown for external inputs in TypeScript
Replace as casts with dedicated type guards or the satisfies operator where appropriate
Use PascalCase for types and enums
Use camelCase for variables and functions
Files:
packages/api/src/routes/categories.ts
🧬 Code graph analysis (1)
packages/api/src/routes/categories.ts (3)
packages/api/src/index.ts (1)
registerCategoryRoutes(19-19)packages/types/src/api.ts (1)
RegisterCategoryRoutesOptions(105-108)packages/api/src/routes/auth-utils.ts (1)
tryAuthenticate(7-20)
🔇 Additional comments (5)
packages/api/src/routes/categories.ts (5)
5-21: LGTM! Well-structured type definitions.The interfaces follow TypeScript best practices with readonly modifiers and proper PascalCase naming conventions.
23-41: LGTM! Clean data transformation.The function correctly maps internal category objects to API response format with proper ISO timestamp serialization.
71-105: LGTM! Solid authentication and authorization flow.The endpoint correctly:
- Authenticates the request and returns 401 on failure
- Ensures authenticated user matches path userId (403 on mismatch)
- Validates limit parameter (400 on invalid input)
- Handles pagination with cursor support
- Maps results to proper API response format
107-123: LGTM! Clean and secure single-resource endpoint.The endpoint correctly:
- Authenticates the request (401 on failure)
- Enforces user-scoped access control by passing userId to findById
- Returns 404 when category not found or user lacks access
- Maps result to proper API response format
60-69: LGTM! Safe dependency handling.The function signature is well-typed and the early exit pattern safely handles missing dependencies without registering incomplete routes.
gentamura
changed the title
1 participant
Summary
Testing
Summary by CodeRabbit
New Features
Documentation
Tests
Chores