Repository navigation
Working on issue #26 - #34
Conversation
|
Warning Review limit reached
More reviews will be available in 41 minutes and 18 seconds. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughImplements Changesbetter-auth Integration
Sequence Diagram(s)sequenceDiagram
participant Client
participant Express as Express (app.ts)
participant userController as user.controller.ts
participant betterAuth as auth.api (better-auth)
participant Prisma as Prisma / PostgreSQL
Client->>Express: POST /api/v1/auth/manual/register
Express->>userController: registerUser(req, res)
userController->>betterAuth: signUpEmail({ email, password, name, headers, asResponse: true })
betterAuth->>Prisma: create user record
Prisma-->>betterAuth: user row
betterAuth-->>userController: Response (Set-Cookie + body)
userController->>Client: copy headers + res.status(N).json(data)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/app.ts`:
- Around line 11-13: Import the `express-async-errors` package at the top of the
app.ts file before any other imports. This package patches Express to
automatically catch errors from async route handlers like registerUser,
loginUser, and logoutUser, ensuring that any unhandled rejections are properly
caught and passed to the errorHandler middleware instead of becoming unhandled
rejections.
In `@apps/api/src/controllers/user.controller.ts`:
- Around line 17-19: The current implementation using response.headers.forEach()
with res.setHeader() incorrectly collapses multiple Set-Cookie headers into a
single comma-separated string, corrupting cookie values. Instead of the generic
forEach loop that handles all headers the same way, use
response.headers.getSetCookie() to extract Set-Cookie headers as an array and
set them individually using res.setHeader() for each cookie. Apply this fix to
all three locations in the file where this pattern appears (the forEach +
setHeader blocks around lines 17-19, 35-37, and 49-51).
- Around line 5-55: The async handlers registerUser, loginUser, and logoutUser
are missing error handling for the auth.api method calls, which can throw
exceptions. Wrap the entire body of each of these three functions in a try/catch
block. In the catch block, invoke next(error) to pass the caught error to
Express's error middleware, ensuring exceptions from auth.api calls are properly
handled by the error middleware instead of causing the request to hang or crash.
In `@apps/api/src/lib/env.ts`:
- Around line 1-8: The config object in env.ts uses insecure dummy fallback
values for critical environment variables like DATABASE_URL, BETTER_AUTH_SECRET,
and BETTER_AUTH_URL. Remove the fallback dummy values and instead throw an error
during initialization if these required environment variables are not set. This
ensures the application fails fast at boot time rather than starting with
insecure or invalid configuration. For each critical variable (DATABASE_URL,
BETTER_AUTH_SECRET, BETTER_AUTH_URL), add validation logic that checks if the
environment variable exists and throws an informative error if it is missing.
In `@apps/api/src/middleware/require-auth.ts`:
- Around line 6-24: The `requireAuth` middleware function is async and calls
`auth.api.getSession()` which can throw errors, but these errors are not caught.
Since Express 4.19.2 does not automatically catch errors in async middleware,
unhandled rejections will bypass the error handler. Wrap the body of the
`requireAuth` function (the call to getSession and subsequent logic) in a
try/catch block, and in the catch block, pass the caught error to the `next()`
function so Express's error handler middleware can process it properly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4b45a564-d379-4c3c-89d9-3ae76f9a93ec
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
.gitignoreapps/api/.env.exampleapps/api/package.jsonapps/api/src/app.tsapps/api/src/controllers/user.controller.tsapps/api/src/index.tsapps/api/src/lib/auth.tsapps/api/src/lib/db.tsapps/api/src/lib/env.tsapps/api/src/middleware/error-handler.tsapps/api/src/middleware/require-auth.tsapps/api/src/routes/admin/auth.routes.tsapps/api/src/routes/index.tsapps/api/src/routes/user/auth.routes.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-06-16T07:39:18.285Z
Learnt from: Basharkhan7776
Repo: Openlabsops/Snap-form PR: 22
File: apps/api/package.json:10-10
Timestamp: 2026-06-16T07:39:18.285Z
Learning: In the Openlabsops/Snap-form monorepo (Bun workspace), internal workspace package dependencies using the `repo/*` scope must use the version specifier `"*"` (not `"workspace:*"`). Apply this consistently when reviewing all `apps/*/package.json` and `packages/*/package.json` files (e.g., `repo/typescript-config`, `repo/eslint-config`, `repo/types`): don’t restrict the check to newly added `repo/*` packages—`"*"` is the repo-wide convention.
Applied to files:
apps/api/package.json
🪛 ast-grep (0.43.0)
apps/api/src/app.ts
[warning] 5-5: Express application should use Helmet
Context: express()
Note: Security best practice.
(missing-helmet-typescript)
🪛 dotenv-linter (4.0.0)
apps/api/.env.example
[warning] 2-2: [UnorderedKey] The BETTER_AUTH_SECRET key should go before the DATABASE_URL key
(UnorderedKey)
[warning] 3-3: [UnorderedKey] The BETTER_AUTH_URL key should go before the DATABASE_URL key
(UnorderedKey)
[warning] 5-5: [UnorderedKey] The NODE_ENV key should go before the PORT key
(UnorderedKey)
🔇 Additional comments (10)
apps/api/src/lib/db.ts (1)
1-3: LGTM!apps/api/src/lib/auth.ts (1)
1-16: LGTM!.gitignore (1)
39-40: LGTM!apps/api/src/middleware/error-handler.ts (1)
1-11: LGTM!apps/api/src/index.ts (1)
1-10: LGTM!apps/api/package.json (1)
12-12: LGTM!apps/api/.env.example (1)
1-5: LGTM!apps/api/src/routes/user/auth.routes.ts (1)
10-14: LGTM!apps/api/src/routes/admin/auth.routes.ts (1)
1-8: LGTM!apps/api/src/routes/index.ts (1)
9-17: LGTM!
Basharkhan7776
left a comment
Basharkhan7776
left a comment
There was a problem hiding this comment.
Great work! merging, we will add alias in future.
2 participants
feat(api): add better-auth with modular architecture
Closes #26
Summary
Installs
better-authand refactorsapps/apiinto a modular, layered architecture with session-based email/password authentication.Changes
Architecture Refactor
src/index.tsintoindex.ts(server bootstrap) andapp.ts(Express factory + middleware wiring)src/routes/index.ts—app.tsno longer imports individual routesNew Files
src/lib/env.tsprocess.envsrc/lib/auth.tsbetterAuth()instance with Prisma adapter + email/passwordsrc/lib/db.tsprismasingleton from@repo/dbsrc/middleware/error-handler.tssrc/middleware/require-auth.tsauth.api.getSession()src/routes/user/auth.routes.tsPOST /register,/login,/logoutsrc/routes/admin/auth.routes.tssrc/controllers/user.controller.tsregisterUser,loginUser,logoutUserhandlers.env.exampleAuth Endpoints
Notes
User,Session,Account,Verificationmodels already in place)Testing
Release Notes
Authentication Framework Installed:
better-authpackage (v1.6.19) added to enable session-based email/password authentication for the API.Three Core Auth Endpoints Implemented:
POST /api/v1/auth/manual/register- User registrationPOST /api/v1/auth/manual/login- Session-based sign-in with cookie supportPOST /api/v1/auth/manual/logout- Session terminationModular Architecture Established: Organized code into
lib/(configuration, database, auth clients),middleware/(error handling, auth guards),routes/(endpoint definitions), andcontrollers/(request handlers) for better maintainability and code reuse.Database Session Storage: Sessions persist to PostgreSQL via Prisma adapter—no external Redis dependency required.
Environment Configuration:
.env.exampledocuments required variables; configuration can be extended to support worker processes using the same auth/database setup.Error Handling: Global error handler middleware in place to catch and respond to server errors uniformly.
Authentication Guard:
requireAuthmiddleware available to protect future routes requiring authenticated sessions.Future Considerations:
lib/modules