Skip to content

Conversation

@senutpal
Copy link
Owner

@senutpal senutpal commented Jan 1, 2026

  • Add Fastify 5 HTTP server with CORS, Helmet, JWT, rate limiting
  • Integrate Prisma 7 with PostgreSQL adapter and ESM support
  • Add Redis service with separate pub/sub clients for presence
  • Implement WebSocket handler with JWT auth and heartbeat protocol
  • Create GitHub OAuth authentication flow
  • Add REST routes: /auth, /users, /friends with full CRUD
  • Set up structured Pino logging with sensitive data redaction
  • Add hierarchical error system distinguishing operational errors
  • Configure graceful shutdown with connection cleanup

Summary by CodeRabbit

  • New Features

    • GitHub OAuth login with token refresh/logout
    • Follow/unfollow, followers/following lists, mutual friends
    • User profiles with privacy mode, editable display name, and search
    • Real-time presence, friend notifications, poke/heartbeat over WebSocket
  • Chores

    • New backend server wiring (DB, Redis, WebSocket), structured logging, health checks, robust error handling, Prisma schema and config
    • Lint/config updates and package metadata adjustments
  • CI

    • Added Prisma client generation step to CI build

✏️ Tip: You can customize this high-level summary in your review settings.

- Add Fastify 5 HTTP server with CORS, Helmet, JWT, rate limiting
- Integrate Prisma 7 with PostgreSQL adapter and ESM support
- Add Redis service with separate pub/sub clients for presence
- Implement WebSocket handler with JWT auth and heartbeat protocol
- Create GitHub OAuth authentication flow
- Add REST routes: /auth, /users, /friends with full CRUD
- Set up structured Pino logging with sensitive data redaction
- Add hierarchical error system distinguishing operational errors
- Configure graceful shutdown with connection cleanup
@coderabbitai
Copy link

coderabbitai bot commented Jan 1, 2026

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Adds a new Fastify server app (apps/server) with Prisma schema, Redis-backed presence, GitHub OAuth + JWT auth, REST routes for users/friends, WebSocket real-time presence/poke handling, structured logging/errors, and related configs, scripts, and CI changes.

Changes

Cohort / File(s) Summary
Project & Tooling
\.gitignore`, apps/server/tsconfig.json, apps/server/eslint.config.js, apps/server/package.json`
New server scaffold: .gitignore entry for generated Prisma client, TS config, ESLint config (extends base + ignores prisma generated), and package.json with scripts, ESM module, and dependencies.
Prisma & Schema
apps/server/prisma.config.ts, apps/server/prisma/schema.prisma
Prisma 7 CLI config and PostgreSQL schema added (models: User, Follow, Team, TeamMember; enums Tier, Role) with migrations and datasource config.
Config & Core libs
apps/server/src/config.ts, apps/server/src/lib/errors.ts, apps/server/src/lib/logger.ts
Zod-validated environment config and Env type; hierarchical AppError classes and helpers; Pino-based structured logger with redaction and child logger helper.
Services
apps/server/src/services/db.ts, apps/server/src/services/redis.ts, apps/server/src/services/github.ts
Prisma client singleton + lifecycle and health checks; Redis command/sub/pub clients with presence APIs and token blacklist; GitHub OAuth helpers (auth URL, token exchange, upsert user, getters).
API Routes
apps/server/src/routes/auth.ts, apps/server/src/routes/users.ts, apps/server/src/routes/friends.ts
Auth routes (GitHub redirect, callback, refresh, logout); user profile endpoints (me, get, patch, search); friends/follow endpoints (list, followers, follow, unfollow, mutuals) with validation and error handling.
Server & WebSocket
apps/server/src/server.ts, apps/server/src/types/fastify.d.ts, apps/server/src/ws/types.ts, apps/server/src/ws/handler.ts
Fastify bootstrap (plugins, hooks, auth decorator, error handling, health), Fastify type augmentations, WebSocket types and handler with JWT auth, heartbeats, presence subscription, poke routing, connection tracking, and broadcast API.
Shared packages & validators
packages/eslint-config/package.json, packages/shared/package.json, packages/shared/src/validators.ts
package.json export/dep updates; downgraded zod in shared package; validators import updated to zod; WebSocket message schema correlationId changed to z.string().uuid().optional().
CI & Orchestration
.github/workflows/ci.yml, turbo.json
CI step added to generate Prisma client for server; turbo task dependencies for check-types/lint/lint:fix now depend on ^build instead of topo.

Sequence Diagram(s)

sequenceDiagram
  participant Browser as User/Browser
  participant Server as Fastify Server
  participant GitHub as GitHub API
  participant DB as PostgreSQL

  Browser->>Server: GET /auth/github
  Server->>Server: generate state, set oauth_state cookie
  Server-->>Browser: 302 redirect -> GitHub auth URL
  Browser->>GitHub: authorize, redirect back with code & state
  Browser->>Server: GET /auth/callback?code&state
  Server->>Server: validate state & params
  Server->>GitHub: POST /login/oauth/access_token (code)
  GitHub-->>Server: access_token
  Server->>GitHub: GET /user (access_token)
  GitHub-->>Server: user profile
  Server->>DB: upsert user record
  DB-->>Server: user
  Server->>Server: sign JWT (userId, username, tier)
  Server-->>Browser: { token, user }
Loading
sequenceDiagram
  participant Client as WS Client
  participant Server as Fastify WS Handler
  participant DB as PostgreSQL
  participant Redis as Redis Pub/Sub

  Client->>Server: WS connect /ws?token=JWT
  Server->>Server: validate JWT -> userId
  Server->>DB: fetch friendIds
  DB-->>Server: friendIds[]
  Server->>Redis: SUBSCRIBE presence:friendId* (per friend)
  Redis-->>Server: subscription ack
  Server-->>Client: CONNECTED + initial FRIEND_STATUS list
  Client->>Server: HEARTBEAT {status, activity}
  Server->>Redis: set presence:userId, PUBLISH presence:userId
  Redis-->>Server: deliver presence updates to subscribers
  Server-->>Client: PONG and broadcast FRIEND_STATUS to subscribers
  Client->>Server: POKE { toUserId, message }
  Server->>Server: validate friend relation
  alt target online
    Server->>Server: deliver POKE to target's connections
  else
    Server->>Server: ignore or persist (noop)
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • feat: Setup Monorepo foundation #1: Overlaps changes to packages/shared/src/validators.ts (zod import and WebSocketMessageSchema correlationId) used by the new WebSocket handler.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat(server): implement Phase 1.1 backend foundation' clearly summarizes the main change—establishing the foundational backend infrastructure with Fastify, Prisma, Redis, and related services.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/backend-implementation-phase-1

📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between add8b14 and d555a07.

📒 Files selected for processing (1)
  • turbo.json
🔇 Additional comments (1)
turbo.json (1)

15-23: LGTM! Task dependencies correctly updated.

The change from ["topo"] to ["^build"] ensures that workspace dependencies (e.g., @devradar/shared) are fully compiled before type-checking and linting run. This prevents errors when dependent packages import from shared code that hasn't been built yet.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link

greptile-apps bot commented Jan 1, 2026

Greptile Summary

Implements Phase 1.1 backend foundation with Fastify 5 HTTP server, Prisma 7 database, Redis pub/sub for real-time presence, and WebSocket support with JWT authentication. The architecture is well-structured with clean separation of concerns, comprehensive error handling, and structured logging.

Critical Issues Found:

  • CSRF Vulnerability: OAuth state parameter generated but never validated in callback, allowing potential CSRF attacks
  • Memory Leak: Redis pub/sub message listeners added on every WebSocket connection but never removed, causing memory to grow unbounded
  • Broken Graceful Shutdown: Shutdown handlers don't properly exit the process and uncaught exception handlers incorrectly attempt graceful shutdown

Architecture Highlights:

  • Clean hexagonal architecture with service adapters for database and Redis
  • Hierarchical error system distinguishing operational vs programming errors
  • Proper use of Prisma 7 adapter pattern with PostgreSQL
  • Type-safe configuration with Zod validation
  • Comprehensive request validation using shared Zod schemas
  • Structured logging with sensitive data redaction

Other Concerns:

  • Unnecessary JWT verification attempt in WebSocket handler (will always fail and fall through to query token)
  • Potential performance issue with Redis publish on every heartbeat (every 30s per user)
  • Duplicate auth route registration may cause confusion

The code quality and architecture are excellent, but the critical security and stability issues must be resolved before merging.

Confidence Score: 2/5

  • This PR has critical security and stability issues that must be fixed before production deployment
  • Score reflects critical CSRF vulnerability in OAuth flow, memory leak in WebSocket handler from unremoved Redis listeners, and broken graceful shutdown that won't exit the process. These are production-blocking issues that could lead to security breaches and server instability.
  • Pay close attention to apps/server/src/routes/auth.ts (CSRF vulnerability), apps/server/src/ws/handler.ts (memory leak), and apps/server/src/server.ts (shutdown issues)

Important Files Changed

Filename Overview
apps/server/src/routes/auth.ts OAuth flow implementation with critical CSRF vulnerability - state parameter not validated
apps/server/src/ws/handler.ts WebSocket handler with memory leak - Redis pub/sub listeners never removed on disconnect
apps/server/src/server.ts Server setup with graceful shutdown issues - handlers don't properly exit process
apps/server/src/services/redis.ts Redis service with separate pub/sub clients, potential performance concern with frequent publishes

Sequence Diagram

sequenceDiagram
    participant Client
    participant Server as Fastify Server
    participant GitHub
    participant DB as PostgreSQL
    participant Redis
    participant WS as WebSocket

    Note over Client,WS: OAuth Authentication
    Client->>Server: GET /auth/github
    Server->>Client: Redirect to GitHub
    Client->>GitHub: Authorize app
    GitHub->>Server: Callback with code
    Server->>GitHub: Exchange code for access
    GitHub->>Server: User profile
    Server->>DB: Upsert user
    Server->>Client: Return signed JWT

    Note over Client,WS: WebSocket Connection
    Client->>WS: Connect with JWT
    WS->>DB: Fetch friend IDs
    WS->>Redis: Subscribe to presence
    Redis->>WS: Current presences
    WS->>Client: CONNECTED + friend statuses

    Note over Client,WS: Real-time Updates
    Client->>WS: HEARTBEAT with status
    WS->>Redis: Update presence (SETEX + PUBLISH)
    Redis-->>WS: Notify subscribers
    WS->>Client: Friend status updates

    Note over Client,WS: REST Operations
    Client->>Server: POST /friends/:id
    Server->>DB: Create follow
    Server->>Client: Success response
    
    Client->>Server: GET /friends
    Server->>DB: Query follows
    Server->>Redis: Get presences
    Server->>Client: Friends with status
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

22 files reviewed, 9 comments

Edit Code Review Agent Settings | Greptile

Comment on lines 36 to 40
// Generate state for CSRF protection
const state = crypto.randomUUID();

// Store state in session/cookie for validation (simplified for now)
// In production, use secure cookies or session storage
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: CSRF state validation is not implemented. The state parameter is generated but never stored or validated in the callback, creating a CSRF vulnerability.

Suggested change
// Generate state for CSRF protection
const state = crypto.randomUUID();
// Store state in session/cookie for validation (simplified for now)
// In production, use secure cookies or session storage
// Generate state for CSRF protection
const state = crypto.randomUUID();
// Store state in secure cookie for validation
reply.setCookie('oauth_state', state, {
httpOnly: true,
secure: isProduction,
sameSite: 'lax',
maxAge: 600, // 10 minutes
});
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/routes/auth.ts
Line: 36:40

Comment:
**logic:** CSRF state validation is not implemented. The state parameter is generated but never stored or validated in the callback, creating a CSRF vulnerability.

```suggestion
    // Generate state for CSRF protection
    const state = crypto.randomUUID();

    // Store state in secure cookie for validation
    reply.setCookie('oauth_state', state, {
      httpOnly: true,
      secure: isProduction,
      sameSite: 'lax',
      maxAge: 600, // 10 minutes
    });
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 54 to 74
app.get('/auth/callback', async (request: FastifyRequest, reply: FastifyReply) => {
const result = CallbackQuerySchema.safeParse(request.query);

if (!result.success) {
throw new ValidationError('Invalid callback parameters', {
details: { errors: result.error.issues },
});
}

const { code, error, error_description } = result.data;

// Handle OAuth errors from GitHub
if (error) {
logger.warn({ error, error_description }, 'GitHub OAuth error');
return reply.status(400).send({
error: {
code: 'OAUTH_ERROR',
message: error_description ?? error,
},
});
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Missing state validation in OAuth callback creates CSRF vulnerability.

Suggested change
app.get('/auth/callback', async (request: FastifyRequest, reply: FastifyReply) => {
const result = CallbackQuerySchema.safeParse(request.query);
if (!result.success) {
throw new ValidationError('Invalid callback parameters', {
details: { errors: result.error.issues },
});
}
const { code, error, error_description } = result.data;
// Handle OAuth errors from GitHub
if (error) {
logger.warn({ error, error_description }, 'GitHub OAuth error');
return reply.status(400).send({
error: {
code: 'OAUTH_ERROR',
message: error_description ?? error,
},
});
}
app.get('/auth/callback', async (request: FastifyRequest, reply: FastifyReply) => {
const result = CallbackQuerySchema.safeParse(request.query);
if (!result.success) {
throw new ValidationError('Invalid callback parameters', {
details: { errors: result.error.issues },
});
}
const { code, state, error, error_description } = result.data;
// Validate CSRF state
const storedState = request.cookies.oauth_state;
if (!state || !storedState || state !== storedState) {
logger.warn({ providedState: state }, 'OAuth state mismatch');
return reply.status(400).send({
error: {
code: 'INVALID_STATE',
message: 'Invalid OAuth state parameter',
},
});
}
// Clear the state cookie
reply.clearCookie('oauth_state');
// Handle OAuth errors from GitHub
if (error) {
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/routes/auth.ts
Line: 54:74

Comment:
**logic:** Missing state validation in OAuth callback creates CSRF vulnerability.

```suggestion
  app.get('/auth/callback', async (request: FastifyRequest, reply: FastifyReply) => {
    const result = CallbackQuerySchema.safeParse(request.query);

    if (!result.success) {
      throw new ValidationError('Invalid callback parameters', {
        details: { errors: result.error.issues },
      });
    }

    const { code, state, error, error_description } = result.data;

    // Validate CSRF state
    const storedState = request.cookies.oauth_state;
    if (!state || !storedState || state !== storedState) {
      logger.warn({ providedState: state }, 'OAuth state mismatch');
      return reply.status(400).send({
        error: {
          code: 'INVALID_STATE',
          message: 'Invalid OAuth state parameter',
        },
      });
    }

    // Clear the state cookie
    reply.clearCookie('oauth_state');

    // Handle OAuth errors from GitHub
    if (error) {
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 118 to 128

// Handle incoming presence updates
subscriber.on('message', (channel: string, messageData: string) => {
try {
const data = JSON.parse(messageData) as FriendStatusPayload;
send(ws, 'FRIEND_STATUS', data);
} catch (error) {
logger.error({ error, channel }, 'Failed to parse presence message');
}
});
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Redis pub/sub listener is registered for every WebSocket connection without cleanup, causing memory leak. Each new connection adds a new 'message' listener that's never removed.

Suggested change
// Handle incoming presence updates
subscriber.on('message', (channel: string, messageData: string) => {
try {
const data = JSON.parse(messageData) as FriendStatusPayload;
send(ws, 'FRIEND_STATUS', data);
} catch (error) {
logger.error({ error, channel }, 'Failed to parse presence message');
}
});
}
// Store the message handler so it can be removed later
const messageHandler = (channel: string, messageData: string) => {
try {
const data = JSON.parse(messageData) as FriendStatusPayload;
send(ws, 'FRIEND_STATUS', data);
} catch (error) {
logger.error({ error, channel }, 'Failed to parse presence message');
}
};
// Attach handler to ws for cleanup on close
ws.presenceMessageHandler = messageHandler;
// Subscribe to friend channels
for (const friendId of ws.friendIds) {
const channel = REDIS_KEYS.presenceChannel(friendId);
await subscriber.subscribe(channel);
logger.debug({ userId: ws.userId, friendId, channel }, 'Subscribed to friend presence');
}
subscriber.on('message', messageHandler);
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/ws/handler.ts
Line: 118:128

Comment:
**logic:** Redis pub/sub listener is registered for every WebSocket connection without cleanup, causing memory leak. Each new connection adds a new `'message'` listener that's never removed.

```suggestion
  // Store the message handler so it can be removed later
  const messageHandler = (channel: string, messageData: string) => {
    try {
      const data = JSON.parse(messageData) as FriendStatusPayload;
      send(ws, 'FRIEND_STATUS', data);
    } catch (error) {
      logger.error({ error, channel }, 'Failed to parse presence message');
    }
  };

  // Attach handler to ws for cleanup on close
  ws.presenceMessageHandler = messageHandler;

  // Subscribe to friend channels
  for (const friendId of ws.friendIds) {
    const channel = REDIS_KEYS.presenceChannel(friendId);
    await subscriber.subscribe(channel);
    logger.debug({ userId: ws.userId, friendId, channel }, 'Subscribed to friend presence');
  }

  subscriber.on('message', messageHandler);
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 260 to 267

// Unsubscribe from friend channels
const subscriber = getRedisSubscriber();
for (const friendId of ws.friendIds) {
const channel = REDIS_KEYS.presenceChannel(friendId);
await subscriber.unsubscribe(channel);
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Redis pub/sub message handler is never removed when unsubscribing, only channels are unsubscribed.

Suggested change
// Unsubscribe from friend channels
const subscriber = getRedisSubscriber();
for (const friendId of ws.friendIds) {
const channel = REDIS_KEYS.presenceChannel(friendId);
await subscriber.unsubscribe(channel);
}
}
// Unsubscribe from friend channels
const subscriber = getRedisSubscriber();
// Remove the message handler to prevent memory leak
if (ws.presenceMessageHandler) {
subscriber.off('message', ws.presenceMessageHandler);
}
for (const friendId of ws.friendIds) {
const channel = REDIS_KEYS.presenceChannel(friendId);
await subscriber.unsubscribe(channel);
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/ws/handler.ts
Line: 260:267

Comment:
**logic:** Redis pub/sub message handler is never removed when unsubscribing, only channels are unsubscribed.

```suggestion
  // Unsubscribe from friend channels
  const subscriber = getRedisSubscriber();
  
  // Remove the message handler to prevent memory leak
  if (ws.presenceMessageHandler) {
    subscriber.off('message', ws.presenceMessageHandler);
  }
  
  for (const friendId of ws.friendIds) {
    const channel = REDIS_KEYS.presenceChannel(friendId);
    await subscriber.unsubscribe(channel);
  }
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 270 to 295
const shutdown = (signal: string): void => {
logger.info({ signal }, 'Shutdown signal received');

// Set a timeout for graceful shutdown
const shutdownTimeout = setTimeout(() => {
logger.error('Graceful shutdown timed out, forcing exit');
throw new Error('Shutdown timeout');
}, 30_000);

Promise.all([
app?.close().then(() => {
logger.info('HTTP server closed');
}),
disconnectDb(),
disconnectRedis(),
])
.then(() => {
clearTimeout(shutdownTimeout);
logger.info('Graceful shutdown complete');
})
.catch((error: unknown) => {
logger.error({ error }, 'Error during shutdown');
clearTimeout(shutdownTimeout);
throw error;
});
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Shutdown handler doesn't properly exit the process after cleanup, and errors during shutdown incorrectly throw (which will crash without cleanup).

Suggested change
const shutdown = (signal: string): void => {
logger.info({ signal }, 'Shutdown signal received');
// Set a timeout for graceful shutdown
const shutdownTimeout = setTimeout(() => {
logger.error('Graceful shutdown timed out, forcing exit');
throw new Error('Shutdown timeout');
}, 30_000);
Promise.all([
app?.close().then(() => {
logger.info('HTTP server closed');
}),
disconnectDb(),
disconnectRedis(),
])
.then(() => {
clearTimeout(shutdownTimeout);
logger.info('Graceful shutdown complete');
})
.catch((error: unknown) => {
logger.error({ error }, 'Error during shutdown');
clearTimeout(shutdownTimeout);
throw error;
});
};
const shutdown = async (signal: string): Promise<void> => {
logger.info({ signal }, 'Shutdown signal received');
// Set a timeout for graceful shutdown
const shutdownTimeout = setTimeout(() => {
logger.error('Graceful shutdown timed out, forcing exit');
process.exit(1);
}, 30_000);
try {
await Promise.all([
app?.close().then(() => {
logger.info('HTTP server closed');
}),
disconnectDb(),
disconnectRedis(),
]);
clearTimeout(shutdownTimeout);
logger.info('Graceful shutdown complete');
process.exit(0);
} catch (error: unknown) {
logger.error({ error }, 'Error during shutdown');
clearTimeout(shutdownTimeout);
process.exit(1);
}
};
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/server.ts
Line: 270:295

Comment:
**logic:** Shutdown handler doesn't properly exit the process after cleanup, and errors during shutdown incorrectly throw (which will crash without cleanup).

```suggestion
    const shutdown = async (signal: string): Promise<void> => {
      logger.info({ signal }, 'Shutdown signal received');

      // Set a timeout for graceful shutdown
      const shutdownTimeout = setTimeout(() => {
        logger.error('Graceful shutdown timed out, forcing exit');
        process.exit(1);
      }, 30_000);

      try {
        await Promise.all([
          app?.close().then(() => {
            logger.info('HTTP server closed');
          }),
          disconnectDb(),
          disconnectRedis(),
        ]);
        
        clearTimeout(shutdownTimeout);
        logger.info('Graceful shutdown complete');
        process.exit(0);
      } catch (error: unknown) {
        logger.error({ error }, 'Error during shutdown');
        clearTimeout(shutdownTimeout);
        process.exit(1);
      }
    };
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 297 to 313
process.on('SIGTERM', () => {
shutdown('SIGTERM');
});
process.on('SIGINT', () => {
shutdown('SIGINT');
});

// Handle uncaught errors
process.on('uncaughtException', (error) => {
logger.fatal({ error }, 'Uncaught exception');
shutdown('uncaughtException');
});

process.on('unhandledRejection', (reason) => {
logger.fatal({ reason }, 'Unhandled rejection');
shutdown('unhandledRejection');
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Signal handlers call shutdown() but don't await it, causing process to continue running. Also, uncaughtException and unhandledRejection shouldn't call shutdown - they should exit immediately.

Suggested change
process.on('SIGTERM', () => {
shutdown('SIGTERM');
});
process.on('SIGINT', () => {
shutdown('SIGINT');
});
// Handle uncaught errors
process.on('uncaughtException', (error) => {
logger.fatal({ error }, 'Uncaught exception');
shutdown('uncaughtException');
});
process.on('unhandledRejection', (reason) => {
logger.fatal({ reason }, 'Unhandled rejection');
shutdown('unhandledRejection');
});
process.on('SIGTERM', () => {
void shutdown('SIGTERM');
});
process.on('SIGINT', () => {
void shutdown('SIGINT');
});
// Handle uncaught errors - exit immediately, don't attempt graceful shutdown
process.on('uncaughtException', (error) => {
logger.fatal({ error }, 'Uncaught exception');
process.exit(1);
});
process.on('unhandledRejection', (reason) => {
logger.fatal({ reason }, 'Unhandled rejection');
process.exit(1);
});
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/server.ts
Line: 297:313

Comment:
**logic:** Signal handlers call `shutdown()` but don't await it, causing process to continue running. Also, `uncaughtException` and `unhandledRejection` shouldn't call shutdown - they should exit immediately.

```suggestion
    process.on('SIGTERM', () => {
      void shutdown('SIGTERM');
    });
    process.on('SIGINT', () => {
      void shutdown('SIGINT');
    });

    // Handle uncaught errors - exit immediately, don't attempt graceful shutdown
    process.on('uncaughtException', (error) => {
      logger.fatal({ error }, 'Uncaught exception');
      process.exit(1);
    });

    process.on('unhandledRejection', (reason) => {
      logger.fatal({ reason }, 'Unhandled rejection');
      process.exit(1);
    });
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 286 to 297
let decoded: { userId: string };
try {
decoded = await request.jwtVerify<{ userId: string }>({ onlyCookie: false });
} catch {
// Try from query token as header might not be available
try {
decoded = app.jwt.verify<{ userId: string }>(token);
} catch {
ws.close(WsCloseCodes.INVALID_TOKEN, 'Invalid token');
return;
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: JWT verification attempts request.jwtVerify() which requires the JWT to be in the Authorization header, but WebSocket connections can't set custom headers from browsers. The fallback to query token will always execute.

Suggested change
let decoded: { userId: string };
try {
decoded = await request.jwtVerify<{ userId: string }>({ onlyCookie: false });
} catch {
// Try from query token as header might not be available
try {
decoded = app.jwt.verify<{ userId: string }>(token);
} catch {
ws.close(WsCloseCodes.INVALID_TOKEN, 'Invalid token');
return;
}
}
// Verify JWT from query parameter
// (WebSocket connections from browsers can't set Authorization headers)
let decoded: { userId: string };
try {
decoded = app.jwt.verify<{ userId: string }>(token);
} catch {
ws.close(WsCloseCodes.INVALID_TOKEN, 'Invalid token');
return;
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/ws/handler.ts
Line: 286:297

Comment:
**syntax:** JWT verification attempts `request.jwtVerify()` which requires the JWT to be in the Authorization header, but WebSocket connections can't set custom headers from browsers. The fallback to query token will always execute.

```suggestion
      // Verify JWT from query parameter
      // (WebSocket connections from browsers can't set Authorization headers)
      let decoded: { userId: string };
      try {
        decoded = app.jwt.verify<{ userId: string }>(token);
      } catch {
        ws.close(WsCloseCodes.INVALID_TOKEN, 'Invalid token');
        return;
      }
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 224 to 226
// Also register auth at root for OAuth redirects
app.register(authRoutes, { prefix: '/auth' });

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Registering auth routes twice (at /api/v1/auth and /auth) may cause confusion. The comment says "for OAuth redirects" but the callback URL should be configured to match one specific path. Is the dual registration intentional? Which path should GITHUB_CALLBACK_URL use?

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/server.ts
Line: 224:226

Comment:
**style:** Registering auth routes twice (at `/api/v1/auth` and `/auth`) may cause confusion. The comment says "for OAuth redirects" but the callback URL should be configured to match one specific path. Is the dual registration intentional? Which path should `GITHUB_CALLBACK_URL` use?

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 159 to 168
export async function setPresence(userId: string, data: PresenceData): Promise<void> {
const redis = getRedis();
const key = REDIS_KEYS.presence(userId);

await redis.setex(key, PRESENCE_TTL_SECONDS, JSON.stringify(data));

// Publish presence update for real-time subscribers
const pub = getRedisPublisher();
await pub.publish(REDIS_KEYS.presenceChannel(userId), JSON.stringify(data));
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Publishing to Redis after every setPresence call could create significant Redis load with frequent heartbeats (every 30s per connected user). Consider batching or throttling presence updates.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/services/redis.ts
Line: 159:168

Comment:
**style:** Publishing to Redis after every `setPresence` call could create significant Redis load with frequent heartbeats (every 30s per connected user). Consider batching or throttling presence updates.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 26

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b06f22 and a720654.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (22)
  • .gitignore
  • apps/server/eslint.config.js
  • apps/server/package.json
  • apps/server/prisma.config.ts
  • apps/server/prisma/schema.prisma
  • apps/server/src/config.ts
  • apps/server/src/lib/errors.ts
  • apps/server/src/lib/logger.ts
  • apps/server/src/routes/auth.ts
  • apps/server/src/routes/friends.ts
  • apps/server/src/routes/users.ts
  • apps/server/src/server.ts
  • apps/server/src/services/db.ts
  • apps/server/src/services/github.ts
  • apps/server/src/services/redis.ts
  • apps/server/src/types/fastify.d.ts
  • apps/server/src/ws/handler.ts
  • apps/server/src/ws/types.ts
  • apps/server/tsconfig.json
  • packages/eslint-config/package.json
  • packages/shared/package.json
  • packages/shared/src/validators.ts
🧰 Additional context used
🧬 Code graph analysis (9)
apps/server/src/lib/logger.ts (1)
apps/server/src/config.ts (2)
  • env (75-75)
  • isDevelopment (85-85)
apps/server/src/routes/users.ts (5)
packages/shared/src/types.ts (1)
  • UserDTO (89-99)
apps/server/src/services/db.ts (2)
  • db (85-85)
  • getDb (25-39)
apps/server/src/lib/errors.ts (2)
  • NotFoundError (117-125)
  • ValidationError (63-77)
apps/server/src/services/redis.ts (1)
  • getPresence (176-188)
packages/shared/src/validators.ts (1)
  • UserUpdateSchema (82-85)
apps/server/src/services/redis.ts (3)
apps/server/src/lib/logger.ts (1)
  • logger (76-76)
apps/server/src/config.ts (1)
  • env (75-75)
packages/shared/src/constants.ts (2)
  • REDIS_KEYS (30-36)
  • PRESENCE_TTL_SECONDS (5-5)
apps/server/src/server.ts (7)
apps/server/src/config.ts (3)
  • isProduction (80-80)
  • isDevelopment (85-85)
  • env (75-75)
apps/server/src/lib/errors.ts (2)
  • AuthenticationError (91-99)
  • toAppError (207-217)
apps/server/src/lib/logger.ts (1)
  • logger (76-76)
apps/server/src/services/db.ts (3)
  • isDbHealthy (74-82)
  • connectDb (45-55)
  • disconnectDb (61-67)
apps/server/src/services/redis.ts (3)
  • isRedisHealthy (132-140)
  • connectRedis (98-111)
  • disconnectRedis (117-127)
apps/server/src/ws/handler.ts (2)
  • getConnectionCount (384-386)
  • registerWebSocketHandler (272-367)
apps/server/src/routes/auth.ts (1)
  • authRoutes (30-144)
apps/server/src/routes/auth.ts (3)
apps/server/src/services/github.ts (2)
  • getGitHubAuthUrl (64-77)
  • authenticateWithGitHub (147-185)
apps/server/src/lib/logger.ts (1)
  • logger (76-76)
apps/server/src/lib/errors.ts (1)
  • ValidationError (63-77)
apps/server/src/services/db.ts (2)
apps/server/src/config.ts (2)
  • env (75-75)
  • isDevelopment (85-85)
apps/server/src/lib/logger.ts (1)
  • logger (76-76)
apps/server/src/services/github.ts (4)
apps/server/src/config.ts (1)
  • env (75-75)
apps/server/src/lib/logger.ts (1)
  • logger (76-76)
apps/server/src/lib/errors.ts (2)
  • AuthenticationError (91-99)
  • InternalError (162-170)
apps/server/src/services/db.ts (2)
  • db (85-85)
  • getDb (25-39)
apps/server/prisma.config.ts (1)
apps/server/src/config.ts (1)
  • env (75-75)
apps/server/src/ws/types.ts (1)
packages/shared/src/types.ts (1)
  • UserStatusType (4-4)
🪛 GitHub Actions: CI
apps/server/src/lib/logger.ts

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/tsconfig.json

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/src/types/fastify.d.ts

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/src/routes/users.ts

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/src/routes/friends.ts

[error] 61-61: eslint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment).

apps/server/src/services/redis.ts

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/eslint.config.js

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/src/ws/handler.ts

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/src/server.ts

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/prisma/schema.prisma

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/package.json

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/src/routes/auth.ts

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/src/services/db.ts

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/src/config.ts

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/src/services/github.ts

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/prisma.config.ts

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/src/lib/errors.ts

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

apps/server/src/ws/types.ts

[error] 1-1: ESLint lint step failed for server. 1 error reported in server lint run.

🪛 GitHub Check: Build and Test
apps/server/src/routes/friends.ts

[failure] 82-82:
Unsafe member access .follow on an error typed value


[failure] 82-82:
Unsafe call of a(n) error type typed value


[failure] 81-81:
Unsafe assignment of an error typed value


[failure] 75-75:
Unsafe member access .data on an error typed value


[failure] 74-74:
Unsafe member access .success on an error typed value


[failure] 74-74:
Unsafe assignment of an error typed value


[failure] 73-73:
Unsafe member access .safeParse on an error typed value


[failure] 73-73:
Unsafe call of a(n) error type typed value


[failure] 73-73:
Unsafe assignment of an error typed value


[failure] 61-61:
Unsafe assignment of an error typed value

🪛 GitHub Check: CodeQL
apps/server/src/ws/handler.ts

[failure] 273-364: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.

apps/server/src/routes/auth.ts

[failure] 54-105: Missing rate limiting
This route handler performs authorization, but is not rate-limited.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Greptile Review
🔇 Additional comments (43)
packages/eslint-config/package.json (1)

8-8: LGTM! Good addition of default export entry.

Adding the "." export provides a clean default import path for consumers of this ESLint config package.

packages/shared/package.json (1)

44-46: No action needed. The dependency versions are correct and up-to-date.

TypeScript 5.9.3 (released October 1, 2025) is newer than 5.7.2 (released November 22, 2024), making the change an upgrade, not a downgrade. ESLint 9.39.2 is a recent stable patch release (December 12, 2025), and rimraf 6.1.2 is the latest v6 release (November 19, 2025). All versions are valid and reflect current stable releases as of January 2026.

Likely an incorrect or invalid review comment.

apps/server/tsconfig.json (1)

1-16: LGTM!

The TypeScript configuration is correctly set up for ESM with bundler module resolution and path aliases. The configuration appropriately extends the monorepo base config and establishes the project structure.

apps/server/src/types/fastify.d.ts (1)

1-28: LGTM!

The Fastify type augmentations are correctly implemented using TypeScript declaration merging. The authenticate decorator and JWT payload types are properly structured and align with the authentication flow described in the PR objectives.

apps/server/package.json (2)

8-20: LGTM!

The npm scripts are well-structured and include all necessary commands for development, building, type-checking, linting, and Prisma database operations.


36-36: No action needed. The codebase uses only Zod v3 APIs and contains no Zod v4-specific patterns (standalone format schemas, z.strictObject/z.looseObject, unified error parameters, etc.). The specified version "^3.24.0" is compatible with the actual implementation.

Likely an incorrect or invalid review comment.

.gitignore (1)

48-48: Verify Prisma output path consistency across ignore configurations.

The .gitignore ignores apps/server/src/generated/ but the ESLint config (apps/server/eslint.config.js) ignores prisma/generated/**. If Prisma generates output to src/generated/, the ESLint ignore pattern should match this path to prevent linting generated code.

apps/server/eslint.config.js (1)

1-8: Remove the incorrect Prisma ignore path concern; verify actual ESLint pipeline failure.

The configuration structure is valid for ESLint flat config format. However, the review comment incorrectly references .gitignore containing apps/server/src/generated/ — the root .gitignore has no Prisma-related entries, and the Prisma config specifies prisma/migrations as the migration path, not a generated directory. The ignore path prisma/generated/** should be verified against actual Prisma outputs if it is necessary.

The pipeline failure claim requires diagnosis with actual ESLint error logs to identify the root cause, as the configuration structure itself is syntactically correct.

packages/shared/src/validators.ts (2)

1-1: LGTM!

The import path 'zod' is correct for Zod 3.24.0.


60-60: LGTM!

The change from z.uuid() to z.string().uuid() is correct for Zod 3.x, where UUID validation is a string refinement rather than a standalone schema type.

apps/server/prisma.config.ts (2)

10-11: LGTM!

The imports are correctly structured for Prisma 7 CLI configuration. The env function from prisma/config is distinct from the application's env export in src/config.ts - Prisma's env() is specifically for CLI commands to read environment variables.


13-26: LGTM!

The Prisma 7 configuration is well-structured with explicit schema path, migrations directory, and datasource configuration. The dotenv/config import ensures environment variables are loaded before the CLI accesses them.

apps/server/src/config.ts (1)

60-69: LGTM!

The parseEnv() function provides clear error formatting when validation fails, following fail-fast principles. The use of safeParse with explicit error handling is correct.

apps/server/src/routes/auth.ts (1)

111-130: LGTM!

The refresh endpoint correctly requires authentication and issues a new token with the same claims. The 7-day expiry is reasonable for this use case.

apps/server/src/lib/logger.ts (2)

37-70: LGTM!

The logger configuration is well-structured with appropriate environment-aware formatting, ISO timestamps, and service metadata. The pino-pretty integration for development improves debugging experience.


89-91: LGTM!

The createChildLogger helper enables request-scoped logging with correlation IDs, which is essential for tracing requests across distributed systems.

apps/server/src/routes/users.ts (2)

62-87: LGTM!

The /users/me endpoint correctly retrieves the authenticated user with presence data, with proper error handling for non-existent users.


129-141: Good privacy handling.

The privacy mode correctly returns a limited payload without presence data or social counts, protecting user information when incognito mode is enabled.

apps/server/src/services/db.ts (3)

25-39: LGTM!

The lazy initialization pattern in getDb() with the Prisma 7 adapter is correctly implemented. The singleton ensures a single connection pool is shared across the application.


74-82: LGTM!

The health check using SELECT 1 is lightweight and appropriate for connection verification. The error suppression is acceptable here since the function's purpose is to return a boolean health status.


61-67: LGTM!

The disconnect function properly cleans up the singleton reference after disconnecting, ensuring a clean state for graceful shutdown.

apps/server/prisma/schema.prisma (1)

18-37: LGTM!

The User model is well-structured with appropriate indexes on username and githubId, proper default values, and clear relations to the social graph.

apps/server/src/services/github.ts (2)

147-185: LGTM with minor note on type safety.

The authentication flow is well-structured with proper error handling. The type cast on line 176 would be unnecessary if using the generated Prisma User type directly.


193-207: LGTM!

The helper functions are straightforward. The type casts can be removed once the generated Prisma types are imported.

apps/server/src/services/redis.ts (3)

27-39: LGTM!

The common Redis options are well-configured with a sensible retry strategy (exponential backoff capped at 3 seconds, max 10 retries) and lazy connection for controlled startup.


44-92: LGTM!

Client creation with connection lifecycle logging and lazy initialization via nullish coalescing assignment is clean and idiomatic.


208-233: LGTM!

The bulk presence fetch is efficient using MGET and handles edge cases properly (empty array, invalid JSON data).

apps/server/src/server.ts (4)

45-52: Verify CORS regex doesn't overmatch.

The regex /\.devradar\.io$/ will match any domain ending in .devradar.io, including potentially malicious subdomains like evil.devradar.io if someone can create subdomains. Ensure this aligns with your subdomain policy.


68-83: LGTM!

Rate limiting is well-configured with per-user keying when authenticated (falling back to IP), environment-aware limits, and a standardized error response format.


186-207: LGTM!

The health check endpoint properly aggregates service health status, returns appropriate HTTP codes (200 vs 503), and includes useful metadata for monitoring.


213-225: LGTM!

Route registration is clear. Registering auth routes at both /api/v1/auth and /auth allows flexibility for OAuth callbacks while maintaining API versioning.

apps/server/src/ws/handler.ts (1)

369-386: LGTM!

The broadcastToUsers and getConnectionCount utility functions are clean and well-implemented.

apps/server/src/routes/friends.ts (3)

107-123: LGTM!

The privacy handling is well-implemented - users in privacyMode show as 'incognito' with no activity details exposed.


196-257: LGTM!

The follow endpoint has proper validation: prevents self-follow, checks target existence, and handles duplicate follows with a clear conflict error.


73-76: Remove redundant fallback defaults; Zod schema already provides them.

The PaginationQuerySchema is properly typed and exported. However, the fallback defaults on lines 75-76 are redundant because the schema already defines defaults at the Zod level (.default(1) and .default(20)). Simplify to rely on Zod's validation and defaults:

const { page, limit } = PaginationQuerySchema.parse(request.query);

Or keep safeParse() if error handling is needed, but remove the manual fallback.

apps/server/src/ws/types.ts (3)

13-28: LGTM!

The AuthenticatedWebSocket interface cleanly extends WebSocket with the necessary user context for connection management.


30-54: LGTM!

Message type definitions are clean and provide good type safety for WebSocket communication.


100-134: LGTM!

Close codes follow WebSocket RFC conventions with standard codes (1000-1011) and custom application codes (4001-4029) properly in the 4000-4999 range.

apps/server/src/lib/errors.ts (5)

91-125: LGTM!

Well-designed error classes with appropriate status codes and sensible default messages. The NotFoundError constructor pattern with optional id parameter provides good ergonomics for common use cases.


175-183: LGTM!

Clean implementation. The required service parameter ensures meaningful error messages.


192-201: LGTM!

Type guards are correctly implemented with proper type predicates for TypeScript narrowing.


207-217: LGTM!

Robust error normalization that preserves the original error as cause for debugging while ensuring consistent error handling throughout the application.


1-6: Unable to verify the ESLint pipeline failure. The claimed issue with Error.captureStackTrace at line 38 cannot be confirmed because:

  • @types/node (^25.0.3) is properly installed
  • The code uses Error.captureStackTrace correctly for Node.js environments
  • TypeScript configuration properly extends @devradar/tsconfig/node.json
  • ESLint depends on @devradar/eslint-config which is unavailable in the verification environment

The review comment is speculative without concrete evidence of the error. If a pipeline failure exists, run the linter locally with the full dependency setup to identify the specific ESLint rule violation.

Comment on lines 10 to 12
datasource db {
provider = "postgresql"
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Add datasource URL configuration.

The datasource is missing the url property. Prisma requires either a direct URL or an environment variable reference for database connection.

🔎 Proposed fix
 datasource db {
   provider = "postgresql"
+  url      = env("DATABASE_URL")
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
datasource db {
provider = "postgresql"
}
datasource db {
provider = "postgresql"
url = env("DATABASE_URL")
}
🤖 Prompt for AI Agents
In apps/server/prisma/schema.prisma around lines 10 to 12, the datasource block
is missing the required url property; add a url field that references your
environment variable (for example url = env("DATABASE_URL")) so Prisma can
connect to the PostgreSQL database, and optionally include a shadowDatabaseUrl =
env("SHADOW_DATABASE_URL") if you use migrations with a separate shadow DB.

Comment on lines 43 to 56
model Follow {
id String @id @default(cuid())
followerId String
followingId String
createdAt DateTime @default(now())

// Relations
follower User @relation("Following", fields: [followerId], references: [id], onDelete: Cascade)
following User @relation("Followers", fields: [followingId], references: [id], onDelete: Cascade)

@@unique([followerId, followingId])
@@index([followerId])
@@index([followingId])
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider using a composite primary key instead of a separate ID.

The Follow model has a unique constraint on (followerId, followingId) but also defines a separate id field. Since this is a junction table, a composite primary key would be more idiomatic and efficient.

🔎 Proposed refactor
 model Follow {
-  id          String   @id @default(cuid())
   followerId  String
   followingId String
   createdAt   DateTime @default(now())

   // Relations
   follower  User @relation("Following", fields: [followerId], references: [id], onDelete: Cascade)
   following User @relation("Followers", fields: [followingId], references: [id], onDelete: Cascade)

-  @@unique([followerId, followingId])
+  @@id([followerId, followingId])
   @@index([followerId])
   @@index([followingId])
 }

Note: This would require updating routes/friends.ts to use the composite key directly instead of follow.id in the delete operation.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
model Follow {
id String @id @default(cuid())
followerId String
followingId String
createdAt DateTime @default(now())
// Relations
follower User @relation("Following", fields: [followerId], references: [id], onDelete: Cascade)
following User @relation("Followers", fields: [followingId], references: [id], onDelete: Cascade)
@@unique([followerId, followingId])
@@index([followerId])
@@index([followingId])
}
model Follow {
followerId String
followingId String
createdAt DateTime @default(now())
// Relations
follower User @relation("Following", fields: [followerId], references: [id], onDelete: Cascade)
following User @relation("Followers", fields: [followingId], references: [id], onDelete: Cascade)
@@id([followerId, followingId])
@@index([followerId])
@@index([followingId])
}
🤖 Prompt for AI Agents
In apps/server/prisma/schema.prisma lines ~43-56, replace the surrogate id with
a composite primary key on (followerId, followingId): remove the id field, keep
followerId and followingId as required String fields, add @@id([followerId,
followingId]) and keep the existing relations and indexes as needed; then update
routes/friends.ts where deletes reference follow.id to delete by the composite
key instead (use the Prisma delete/find unique call that targets the composite
key using both followerId and followingId).

Comment on lines 62 to 76
model Team {
id String @id @default(cuid())
name String
slug String @unique
ownerId String
tier Tier @default(TEAM)
createdAt DateTime @default(now())
updatedAt DateTime @updatedAt

// Relations
members TeamMember[]

@@index([slug])
@@index([ownerId])
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider adding a relation from Team to User for the owner.

The ownerId field lacks a corresponding relation to User, which means:

  1. No referential integrity enforcement at the database level
  2. No cascading behavior when a user is deleted
  3. Potential for orphaned teams
🔎 Proposed fix
 model Team {
   id        String   @id @default(cuid())
   name      String
   slug      String   @unique
   ownerId   String
   tier      Tier     @default(TEAM)
   createdAt DateTime @default(now())
   updatedAt DateTime @updatedAt

   // Relations
+  owner   User         @relation("OwnedTeams", fields: [ownerId], references: [id], onDelete: Cascade)
   members TeamMember[]

   @@index([slug])
   @@index([ownerId])
 }

Also add the inverse relation on User:

 model User {
   // ... existing fields ...

   // Relations
   following Follow[] @relation("Following")
   followers Follow[] @relation("Followers")
   teams     TeamMember[]
+  ownedTeams Team[] @relation("OwnedTeams")
   // ...
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
model Team {
id String @id @default(cuid())
name String
slug String @unique
ownerId String
tier Tier @default(TEAM)
createdAt DateTime @default(now())
updatedAt DateTime @updatedAt
// Relations
members TeamMember[]
@@index([slug])
@@index([ownerId])
}
model Team {
id String @id @default(cuid())
name String
slug String @unique
ownerId String
tier Tier @default(TEAM)
createdAt DateTime @default(now())
updatedAt DateTime @updatedAt
// Relations
owner User @relation("OwnedTeams", fields: [ownerId], references: [id], onDelete: Cascade)
members TeamMember[]
@@index([slug])
@@index([ownerId])
}
🤖 Prompt for AI Agents
In apps/server/prisma/schema.prisma around lines 62-76, the Team model has
ownerId but no relation to User; add a relation field on Team (e.g., owner User)
that uses @relation(fields: [ownerId], references: [id], onDelete: Cascade) to
enforce referential integrity and cascade deletes, and add the inverse relation
field on the User model (e.g., teams or ownedTeams of type Team[]) with the same
relation name so Prisma understands the bidirectional relationship.

if (options?.details) {
this.details = options.details;
}
Error.captureStackTrace(this, this.constructor);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Error.captureStackTrace may require type guarding.

Error.captureStackTrace is a V8-specific API not present in standard ECMAScript. While it's available in Node.js, TypeScript may flag this if @types/node isn't properly configured, which could be the source of the pipeline ESLint/TypeScript error.

🔎 Proposed fix with defensive guard
-    Error.captureStackTrace(this, this.constructor);
+    if (typeof Error.captureStackTrace === 'function') {
+      Error.captureStackTrace(this, this.constructor);
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Error.captureStackTrace(this, this.constructor);
if (typeof Error.captureStackTrace === 'function') {
Error.captureStackTrace(this, this.constructor);
}
🤖 Prompt for AI Agents
In apps/server/src/lib/errors.ts around line 38, wrap the V8-specific call to
Error.captureStackTrace in a defensive runtime/type guard: check that typeof
(Error as any).captureStackTrace === 'function' before invoking it, and provide
a safe fallback (e.g., this.stack = (new Error()).stack) when captureStackTrace
is not available; this avoids TypeScript/ESLint pipeline errors when @types/node
isn't present while preserving stack population.

Comment on lines 130 to 134
export class ConflictError extends AppError {
readonly code = 'CONFLICT';
readonly statusCode = 409;
readonly isOperational = true;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider adding a constructor with a default message for consistency.

Unlike other error classes (AuthenticationError, RateLimitError, etc.), ConflictError lacks a constructor and requires callers to always provide a message. This is inconsistent with the rest of the hierarchy.

🔎 Proposed fix
 export class ConflictError extends AppError {
   readonly code = 'CONFLICT';
   readonly statusCode = 409;
   readonly isOperational = true;
+
+  constructor(message = 'Resource already exists or state conflict', options?: { cause?: Error; details?: Record<string, unknown> }) {
+    super(message, options);
+  }
 }
🤖 Prompt for AI Agents
In apps/server/src/lib/errors.ts around lines 130 to 134, ConflictError lacks a
constructor and thus forces callers to supply a message; add a constructor that
accepts an optional message (defaulting to something like "Conflict") and calls
super(message) so it matches the pattern of the other error classes and
preserves code, statusCode and isOperational behavior.

Comment on lines 220 to 231
switch (type) {
case 'HEARTBEAT':
await handleHeartbeat(ws, payload, correlationId);
break;

case 'POKE':
handlePoke(ws, payload, correlationId);
break;

default:
sendError(ws, 'UNKNOWN_TYPE', `Unknown message type: ${type}`, correlationId);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

SUBSCRIBE and UNSUBSCRIBE message types are not implemented.

The InboundMessageSchema accepts these types, but the switch statement doesn't handle them, falling through to the default error case.

🔎 Proposed fix - either implement or remove from schema

Option 1: Remove from schema if not needed yet:

 const InboundMessageSchema = z.object({
-  type: z.enum(['HEARTBEAT', 'POKE', 'SUBSCRIBE', 'UNSUBSCRIBE']),
+  type: z.enum(['HEARTBEAT', 'POKE']),
   payload: z.unknown(),
   timestamp: z.number().int().positive(),
   correlationId: z.string().optional(),
 });

Option 2: Add placeholder handlers:

case 'SUBSCRIBE':
case 'UNSUBSCRIBE':
  // TODO: Implement dynamic friend subscription management
  sendError(ws, 'NOT_IMPLEMENTED', `${type} is not yet implemented`, correlationId);
  break;

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In apps/server/src/ws/handler.ts around lines 220 to 231, the switch over
inbound message types does not handle 'SUBSCRIBE' and 'UNSUBSCRIBE', causing
valid schema messages to fall into the unknown-type error; add explicit cases
for 'SUBSCRIBE' and 'UNSUBSCRIBE' to the switch and either (A) implement the
subscription logic or (B) add placeholder handlers that return a clear
NOT_IMPLEMENTED error via sendError with the correlationId (e.g., treat both
cases the same, send a NOT_IMPLEMENTED error and break), or if these message
types are not intended, remove them from the InboundMessageSchema instead —
choose one of those fixes and apply it consistently.

- Add CSRF protection, token blacklisting, and rate limiting
- Optimize Redis presence updates and mutual friends query
- Fix Prisma 7 schema config and add generate step to CI
- Resolve ESLint errors and add proper type validations
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 16

♻️ Duplicate comments (3)
packages/shared/package.json (1)

39-39: Verify compatibility impact of Zod downgrade from v4 to v3.

Downgrading from Zod 4.3.4 to 3.24.0 is a major version rollback. While the past review comment indicates the migration appears complete, ensure all schema definitions across the codebase are compatible with Zod v3 syntax, particularly the correlationId field inconsistency already flagged.

What are the breaking changes between Zod v3.24.0 and Zod v4.3.4?
apps/server/src/routes/auth.ts (1)

93-94: Add path option to clearCookie to ensure proper cookie removal.

The setCookie call doesn't specify a path, so it defaults to the current path. When clearing, explicitly matching the path ensures the cookie is properly removed across browsers.

🔎 Suggested fix
-     reply.clearCookie('oauth_state');
+     reply.clearCookie('oauth_state', {
+       path: '/',
+     });

Alternatively, add path: '/' to the setCookie call on line 41-46 for consistency.

apps/server/src/routes/friends.ts (1)

60-61: Verify ESLint error is resolved.

A previous review flagged an @typescript-eslint/no-unsafe-assignment error on this line. Ensure the getDb() return type is properly typed to resolve this. If the error persists, add an explicit type annotation.

#!/bin/bash
# Check the return type of getDb in services/db.ts
ast-grep --pattern 'export function getDb(): $RET { $$$ }'
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b06f22 and 7a64485.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (23)
  • .github/workflows/ci.yml
  • .gitignore
  • apps/server/eslint.config.js
  • apps/server/package.json
  • apps/server/prisma.config.ts
  • apps/server/prisma/schema.prisma
  • apps/server/src/config.ts
  • apps/server/src/lib/errors.ts
  • apps/server/src/lib/logger.ts
  • apps/server/src/routes/auth.ts
  • apps/server/src/routes/friends.ts
  • apps/server/src/routes/users.ts
  • apps/server/src/server.ts
  • apps/server/src/services/db.ts
  • apps/server/src/services/github.ts
  • apps/server/src/services/redis.ts
  • apps/server/src/types/fastify.d.ts
  • apps/server/src/ws/handler.ts
  • apps/server/src/ws/types.ts
  • apps/server/tsconfig.json
  • packages/eslint-config/package.json
  • packages/shared/package.json
  • packages/shared/src/validators.ts
🧰 Additional context used
🧬 Code graph analysis (9)
apps/server/src/routes/users.ts (5)
packages/shared/src/types.ts (1)
  • UserDTO (89-99)
apps/server/src/services/db.ts (1)
  • getDb (25-39)
apps/server/src/lib/errors.ts (2)
  • NotFoundError (129-137)
  • ValidationError (75-89)
apps/server/src/services/redis.ts (1)
  • getPresence (214-226)
packages/shared/src/validators.ts (1)
  • UserUpdateSchema (82-85)
apps/server/src/server.ts (9)
apps/server/src/config.ts (3)
  • isProduction (80-80)
  • isDevelopment (85-85)
  • env (75-75)
apps/server/src/services/redis.ts (4)
  • isTokenBlacklisted (301-307)
  • isRedisHealthy (153-161)
  • connectRedis (98-111)
  • disconnectRedis (117-148)
apps/server/src/lib/errors.ts (2)
  • AuthenticationError (103-111)
  • toAppError (237-247)
apps/server/src/lib/logger.ts (1)
  • logger (79-79)
apps/server/src/services/db.ts (3)
  • isDbHealthy (74-82)
  • connectDb (45-55)
  • disconnectDb (61-67)
apps/server/src/ws/handler.ts (2)
  • getConnectionCount (471-473)
  • registerWebSocketHandler (348-454)
apps/server/src/routes/users.ts (1)
  • userRoutes (73-253)
apps/server/src/routes/friends.ts (1)
  • friendRoutes (60-343)
apps/server/src/routes/auth.ts (1)
  • authRoutes (31-200)
apps/server/src/services/github.ts (4)
apps/server/src/config.ts (1)
  • env (75-75)
apps/server/src/lib/logger.ts (1)
  • logger (79-79)
apps/server/src/lib/errors.ts (2)
  • AuthenticationError (103-111)
  • InternalError (192-200)
apps/server/src/services/db.ts (1)
  • getDb (25-39)
apps/server/src/services/redis.ts (3)
apps/server/src/lib/logger.ts (1)
  • logger (79-79)
apps/server/src/config.ts (1)
  • env (75-75)
packages/shared/src/constants.ts (2)
  • REDIS_KEYS (30-36)
  • PRESENCE_TTL_SECONDS (5-5)
apps/server/src/services/db.ts (2)
apps/server/src/config.ts (2)
  • env (75-75)
  • isDevelopment (85-85)
apps/server/src/lib/logger.ts (1)
  • logger (79-79)
apps/server/src/routes/auth.ts (5)
apps/server/src/config.ts (1)
  • isProduction (80-80)
apps/server/src/services/github.ts (2)
  • getGitHubAuthUrl (55-68)
  • authenticateWithGitHub (163-201)
apps/server/src/lib/logger.ts (1)
  • logger (79-79)
apps/server/src/lib/errors.ts (1)
  • ValidationError (75-89)
apps/server/src/services/redis.ts (1)
  • blacklistToken (285-293)
apps/server/src/ws/handler.ts (6)
packages/shared/src/validators.ts (3)
  • UserStatusTypeSchema (6-6)
  • ActivityPayloadSchema (34-41)
  • PokePayloadSchema (66-70)
apps/server/src/ws/types.ts (7)
  • AuthenticatedWebSocket (13-31)
  • WsMessage (52-57)
  • ErrorPayload (97-101)
  • FriendStatusPayload (79-84)
  • PokePayload (70-74)
  • WsCloseCodes (106-120)
  • ConnectedPayload (89-92)
apps/server/src/services/db.ts (1)
  • getDb (25-39)
apps/server/src/services/redis.ts (1)
  • getRedisSubscriber (81-84)
apps/server/src/lib/logger.ts (2)
  • logger (79-79)
  • createChildLogger (92-94)
packages/shared/src/constants.ts (1)
  • REDIS_KEYS (30-36)
apps/server/prisma.config.ts (1)
apps/server/src/config.ts (1)
  • env (75-75)
apps/server/src/ws/types.ts (1)
packages/shared/src/types.ts (1)
  • UserStatusType (4-4)
🪛 Biome (2.1.2)
apps/server/src/lib/errors.ts

[error] 43-43: Do not shadow the global "constructor" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

🔇 Additional comments (55)
.github/workflows/ci.yml (1)

46-47: LGTM! Correct placement of Prisma generation step.

The Prisma Client generation step is correctly positioned after dependency installation and before linting/type-checking/build steps. This ensures that generated Prisma types are available for TypeScript compilation and ESLint type-aware rules.

The use of pnpm --filter @devradar/server appropriately targets only the server workspace where Prisma is configured.

packages/shared/package.json (1)

46-46: No action required. The change from TypeScript 5.7.2 to 5.9.3 is a normal upgrade, not a downgrade. In semantic versioning, 5.9.3 is greater than 5.7.2 (comparing minor versions: 9 > 7). TypeScript 5.9.3 was released in October 2025, after 5.7.2 (November 2024). There is nothing unusual about this version update.

Likely an incorrect or invalid review comment.

apps/server/eslint.config.js (1)

1-8: LGTM! Clean ESLint configuration.

The flat config structure is correct, extends the base configuration appropriately, and ignores Prisma-generated artifacts to prevent linting issues with generated code.

packages/shared/src/validators.ts (2)

1-1: Correct import path for zod v3.x.

The import has been properly updated from the invalid 'zod/v4' path to the standard 'zod' import, which is correct for zod version 3.24.0.


60-60: Correct UUID validation for zod v3.x.

The correlationId schema has been properly updated to use z.string().uuid() instead of the non-existent z.uuid() method in zod v3.x. This ensures proper UUID validation.

packages/eslint-config/package.json (2)

8-8: LGTM! Default export entry added.

The addition of the default export entry "." -> "./node.js" enables consumers to import the package without specifying a subpath, which aligns with the server's usage in apps/server/eslint.config.js.


17-25: LGTM! Dependencies updated to latest versions.

All ESLint-related dependencies have been updated to their latest stable versions, including:

  • TypeScript ESLint tooling aligned to v8.51.0
  • globals updated to v17.0.0 (addressing the previous review comment)
  • All other packages at current stable releases

The dependency updates maintain consistency across the monorepo's linting infrastructure.

apps/server/src/lib/logger.ts (3)

19-35: LGTM! Comprehensive redaction for sensitive data.

The redaction paths appropriately cover authentication secrets, tokens, and PII including email addresses. The configuration includes both direct field names and wildcard patterns for nested structures, which provides robust protection against accidental data leakage in logs.


40-73: LGTM! Well-configured Pino logger.

The logger factory appropriately configures structured logging with:

  • Environment-aware output (pretty-print for development, JSON for production)
  • ISO 8601 timestamps for consistent time representation
  • Service and environment context in base bindings
  • Proper integration with the redaction configuration

79-106: LGTM! Clean logger API.

The module exports a well-designed API:

  • Singleton logger for general use
  • createChildLogger for request-scoped logging with correlation IDs
  • LogLevel constants for type-safe log level references

This design supports both simple logging and advanced use cases like distributed tracing.

apps/server/src/lib/errors.ts (8)

12-66: LGTM! Robust AppError base class.

The base class properly implements:

  • Abstract properties for consistent error structure
  • Defensive handling of Error.captureStackTrace with fallback for non-V8 environments (addressing the previous review concern)
  • JSON serialization that excludes sensitive stack traces
  • Support for error correlation via traceId

Note: The static analysis warning about "constructor" shadowing on line 43 is a false positive—this is the standard second parameter for the V8 captureStackTrace API.


75-98: LGTM! ValidationError with field-level details.

The implementation correctly handles validation errors with:

  • 400 status code for bad requests
  • Optional field-level error details via FieldError[]
  • Proper merging of fields into the details object

The FieldError interface provides a clean structure for granular validation feedback.


103-124: LGTM! Standard authentication and authorization errors.

Both error classes are correctly implemented with:

  • Appropriate HTTP status codes (401 for authentication, 403 for authorization)
  • Clear default messages
  • Operational error classification

129-137: LGTM! Flexible NotFoundError implementation.

The constructor provides a clean API for generating contextual "not found" messages with optional resource identifiers, making error messages more informative for API consumers.


142-153: LGTM! ConflictError with proper constructor.

The implementation now includes a constructor with a default message and support for cause/details, maintaining consistency with other error classes (addressing the previous review concern).


158-183: LGTM! RateLimitError with retry timing.

The implementation correctly:

  • Uses 429 status code for rate limiting
  • Includes optional retryAfter property
  • Overrides toJSON() to serialize the retry timing for API clients (addressing the previous review concern)

This enables proper client-side backoff strategies.


192-213: LGTM! Internal and service unavailable errors.

Both error classes are properly implemented:

  • InternalError correctly marked as non-operational (programming error)
  • ServiceUnavailableError provides service context and is operational (expected external failure)

The distinction between operational and non-operational errors supports proper error handling and alerting strategies.


222-247: LGTM! Utility functions for error handling.

The type guards and wrapper function provide a complete toolkit:

  • isAppError and isOperationalError enable proper type narrowing
  • toAppError ensures all errors are normalized to the AppError hierarchy
  • Support for wrapping both Error instances and unknown values

This facilitates consistent error handling across the application.

apps/server/src/config.ts (1)

14-49: Well-structured environment validation.

Good use of Zod with sensible defaults and protocol refinements. The 32-character minimum for JWT_SECRET is appropriate for security.

apps/server/src/services/db.ts (2)

25-39: LGTM! Proper singleton pattern with lazy initialization.

The getter-based approach correctly addresses testability concerns raised in previous reviews. The Prisma 7 adapter pattern is properly implemented.


61-67: The current implementation correctly handles pool cleanup via prisma.$disconnect().

The prisma.$disconnect() call at line 63 is the standard approach for closing the PrismaPg adapter's connection pool. The code follows Prisma best practices: using a single PrismaClient instance (singleton pattern) and calling $disconnect() during graceful shutdown.

However, note that Prisma v7's pg-adapter has reported edge cases where connections may not release properly on network errors or in certain scenarios. If you observe connection leaks in production, verify your Prisma version is up to date and consider adding pool configuration via DATABASE_URL (e.g., connection_limit=5&pool_timeout=10) to prevent hanging connections.

apps/server/src/routes/auth.ts (3)

36-53: CSRF protection correctly implemented.

The state is now properly stored in a secure httpOnly cookie and validated in the callback. This addresses the previous security concerns.


60-69: Rate limiting applied as recommended.

The callback endpoint now has appropriate rate limiting (10 requests/minute) to prevent brute-force attacks on authorization codes.


168-199: Token blacklisting implemented as suggested.

The logout now extracts the token, calculates remaining TTL, and blacklists it in Redis. The graceful error handling ensures logout succeeds even if blacklisting fails.

apps/server/prisma/schema.prisma (3)

10-13: Datasource URL configuration via prisma.config.ts is valid for Prisma 7.

This addresses the previous review concern. In Prisma 7, the URL can be configured in the separate prisma.config.ts file rather than inline in the schema, which is the recommended pattern for the new configuration system.


45-57: Composite primary key implemented as suggested.

The Follow model now uses @@id([followerId, followingId]) instead of a separate ID field, which is more idiomatic for junction tables.


72-74: Owner relation added with referential integrity.

The Team model now has the owner relation with onDelete: Cascade, ensuring no orphaned teams when a user is deleted.

apps/server/src/routes/users.ts (3)

53-68: Runtime tier validation implemented as suggested.

The toUserDTO function now validates the tier value against VALID_TIERS and falls back to 'FREE' for invalid database values. This prevents propagating unexpected data to clients.


30-32: Search query validation improved with Zod schema.

The SearchQuerySchema provides type-safe query parameter parsing, addressing the previous concern about unsafe type assertions.


147-159: Privacy mode handling is well implemented.

The incognito response correctly hides sensitive fields (tier, follower counts, activity) while still providing basic profile information. The status: 'incognito' makes it clear to clients that the user has privacy enabled.

apps/server/src/services/github.ts (4)

1-13: Well-structured OAuth service with proper type imports.

The module correctly imports the Prisma-generated User type instead of redefining it locally, and includes timeout handling for external API calls. The service architecture cleanly separates concerns between token exchange, user fetching, and database operations.


77-119: LGTM - Token exchange with proper timeout and error handling.

The function correctly implements timeout via AbortSignal.timeout() and handles both authentication errors and timeout scenarios appropriately.


128-153: LGTM - Consistent timeout and error handling pattern.

The fetchGitHubUser function follows the same robust pattern as exchangeCodeForToken with proper timeout handling and error propagation.


163-201: Authentication flow is well-implemented.

The authenticateWithGitHub function correctly orchestrates the OAuth flow, performs upsert to handle both new and returning users, and includes appropriate logging and error handling.

apps/server/src/ws/handler.ts (8)

56-67: Good architectural fix for Redis subscription management.

The introduction of channelSubscriptions Map with reference counting and globalHandlerInitialized flag properly addresses the previous memory leak and duplicate broadcast issues. This is the correct pattern for managing shared Redis pub/sub across multiple WebSocket connections.


117-143: LGTM - Global message handler correctly implemented.

The single global handler pattern correctly routes presence updates to subscribed connections without accumulating listeners, resolving the previous memory leak issue.


148-174: LGTM - Reference-counted subscription management.

The subscription logic correctly implements reference counting: only the first subscriber to a channel triggers the actual Redis subscription, and subsequent subscribers are tracked in the Set.


179-197: LGTM - Proper cleanup with reference counting.

The unsubscribe logic correctly removes the user from the subscription set and only unsubscribes from Redis when no subscribers remain, preventing the issue where one user's disconnect would break presence updates for others.


298-307: SUBSCRIBE/UNSUBSCRIBE handling addressed.

Good - explicit handling for SUBSCRIBE and UNSUBSCRIBE message types now returns a clear NOT_IMPLEMENTED error instead of falling through to the unknown type error.


321-343: Grace period implementation is reasonable.

The 60-second grace period before marking users offline allows for brief disconnections without flapping presence status. Note that if the server restarts during this window, the setTimeout won't fire, but Redis TTL on presence keys will eventually clean up stale entries.


459-466: LGTM - Simple and safe broadcast utility.

The function correctly iterates over userIds and checks readyState before sending to each connection.


394-398: No race condition exists—the connection replacement is safe.

The concern about both connections passing the existingConnection check is unfounded. The await getUserFriendIds() on line 391 ensures that by the time either connection reaches the critical section (lines 394–398), the Map operations (get(), close(), set()) execute synchronously and atomically at the JavaScript event loop level. When connection A completes its await and reaches line 394, it will execute the entire get/close/set sequence before connection B resumes execution. Therefore, no additional synchronization mechanism is needed.

apps/server/src/server.ts (4)

287-316: Graceful shutdown implementation now properly exits the process.

The shutdown handler correctly:

  • Sets a 30-second timeout before force exit
  • Awaits cleanup of HTTP server, database, and Redis
  • Calls process.exit(0) on success and process.exit(1) on failure
  • Clears the timeout before exiting

This addresses the previous concern about the process hanging after shutdown.


325-336: Correct handling of fatal errors.

uncaughtException and unhandledRejection now exit immediately with process.exit(1) rather than attempting graceful shutdown, which is the correct behavior since the process state may be corrupted.


204-225: LGTM - Comprehensive health check endpoint.

The health endpoint correctly aggregates dependency status, returns appropriate HTTP status codes (200/503), and includes useful operational metrics like uptime and WebSocket connection count.


72-87: LGTM - Sensible rate limiting configuration.

The rate limiter correctly keys by userId for authenticated requests and falls back to IP for anonymous requests, with appropriate limits for each environment.

apps/server/src/routes/friends.ts (3)

67-135: LGTM - Well-implemented friends list endpoint.

The endpoint correctly handles pagination, batch-fetches presence data for efficiency, and respects user privacy mode by showing 'incognito' status instead of actual presence.


196-257: LGTM - Robust follow endpoint with proper validation.

The endpoint correctly validates inputs, prevents self-follow, checks for existing relationships, and uses appropriate HTTP status codes and error types.


322-336: Good - Mutual friends query optimized per previous feedback.

The implementation now uses a single efficient Prisma query with relation filters instead of two queries with in-memory intersection.

apps/server/src/services/redis.ts (4)

117-148: LGTM - Resilient disconnect implementation.

The use of Promise.allSettled ensures all clients are attempted to close even if one fails, and clients are always nullified regardless of quit success. The failure logging provides operational visibility.


181-206: Good optimization with change detection for presence updates.

The implementation correctly:

  • Always refreshes TTL via setex
  • Only publishes to pub/sub when status or activity actually changes
  • Uses fire-and-forget for publish to avoid blocking

This addresses the previous concern about Redis load from frequent heartbeats.


246-271: LGTM - Efficient bulk presence retrieval.

The function correctly uses MGET for batch retrieval, handles empty input gracefully, and silently skips corrupted data entries rather than failing the entire request.


285-293: LGTM - Token blacklist with appropriate TTL management.

The function correctly skips already-expired tokens and uses Redis TTL to automatically clean up blacklist entries when the token would have expired anyway.

apps/server/src/ws/types.ts (2)

36-47: LGTM - Comprehensive message type definitions.

The inbound and outbound message types cover all current functionality. Keeping SUBSCRIBE and UNSUBSCRIBE in the schema is appropriate for forward compatibility since the handler returns a clear NOT_IMPLEMENTED error.


106-137: LGTM - Well-defined close codes with readable reasons.

Custom codes are correctly placed in the 4000-4999 range per WebSocket specification, and the WsCloseReasons mapping provides useful human-readable strings for each code.


# Docker
docker-compose.override.yml
apps/server/src/generated/
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider moving this rule to a more appropriate section.

The apps/server/src/generated/ ignore rule is placed under the "Docker" section, but it's for Prisma-generated code artifacts. Consider creating a dedicated "Generated files" section or placing it near other build/generated output rules (lines 5-10).

🤖 Prompt for AI Agents
In .gitignore around line 48, the apps/server/src/generated/ entry is currently
grouped under the "Docker" section but it is a Prisma/generated artifact; move
this line out of the Docker group and place it under a new or existing
"Generated files" or "Build artifacts" section (near lines 5-10 where other
generated/build outputs live) so generated code rules are organized together.

Comment on lines +1 to +7
{
"name": "@devradar/server",
"version": "0.0.0",
"private": true,
"license": "MIT",
"type": "module",
"main": "./dist/server.js",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Add Node.js version constraint in engines field.

The package uses modern features (ESM, top-level await implied by Prisma/Fastify setup, TypeScript 5.9) but lacks an explicit Node.js version requirement. Add an engines field to enforce compatibility:

"engines": {
  "node": ">=20.0.0"
}

This prevents runtime issues when the codebase uses features unavailable in older Node.js versions.

🤖 Prompt for AI Agents
In apps/server/package.json around lines 1 to 7, there is no Node.js version
constraint; add an "engines" field to require a compatible Node runtime (e.g.
"node": ">=20.0.0"). Update the package.json by inserting the engines object at
the top level (near name/version/private) to enforce Node >=20 so
ESM/top-level-await/TS5.9 features run correctly.

Comment on lines +8 to +19
"scripts": {
"dev": "tsx watch --env-file=.env src/server.ts",
"build": "tsc",
"start": "node dist/server.js",
"check-types": "tsc --noEmit",
"lint": "eslint .",
"lint:fix": "eslint . --fix",
"clean": "rimraf dist",
"db:generate": "prisma generate",
"db:push": "prisma db push",
"db:migrate": "prisma migrate dev",
"db:studio": "prisma studio"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Test script is a placeholder.

Line 35 in the test script outputs "No tests yet" and exits with code 0. For a backend foundation handling authentication, WebSockets, and database operations, comprehensive test coverage is essential.

Would you like me to open an issue to track setting up a test framework (e.g., Vitest, Jest, or Node's built-in test runner) and writing tests for critical paths (auth flows, WebSocket handlers, database operations)?

🤖 Prompt for AI Agents
In apps/server/package.json around lines 8 to 19, the "test" script is currently
a placeholder that prints "No tests yet"; replace it with a real test setup:
choose a test runner (e.g., Vitest or Jest), add the appropriate
devDependencies, and update package.json scripts to run the test runner (e.g.,
"test": "vitest" or "test": "jest --runInBand") plus a coverage script. Add
commands for running tests in watch/CI modes if needed. Create a tests/ folder
and add focused tests for critical paths: auth flows (login, token refresh,
error cases), WebSocket handlers (connect, message routing, disconnect, error
handling), and database operations (CRUD, migrations, and transactional
behavior) and ensure tests run against a test database (use sqlite in-memory or
a test container) and add a CI step to run the test script.

"type": "module",
"main": "./dist/server.js",
"scripts": {
"dev": "tsx watch --env-file=.env src/server.ts",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Redundant environment loading with tsx --env-file and dotenv dependency.

The dev script uses tsx watch --env-file=.env (line 9), which loads environment variables natively. However, dotenv is also listed as a dependency (line 31). If your application code explicitly calls dotenv.config(), this creates redundant environment loading.

Consider removing the dotenv dependency if tsx --env-file is sufficient for development, or document why both are needed (e.g., production runtime requires dotenv).

🤖 Prompt for AI Agents
In apps/server/package.json around line 9, the dev script uses "tsx watch
--env-file=.env" which duplicates environment loading if the project also has
dotenv as a dependency (line ~31); either remove the redundant dotenv dependency
from package.json or keep the dependency but remove any explicit dotenv.config()
calls in your code, or add a comment in package.json (or README) explaining why
both are needed (e.g., dotenv required at production runtime). Make the change
consistent: if you remove dotenv from package.json, also remove any dotenv
imports/usages; if you keep it, document the rationale.

Comment on lines +29 to +30
"@prisma/adapter-pg": "^7.2.0",
"@prisma/client": "^7.2.0",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cd apps/server && head -50 package.json | cat -n

Repository: senutpal/devRadar

Length of output: 1778


🌐 Web query:

Prisma 7.2.0 latest stable version 2025

💡 Result:

Yes — Prisma ORM 7.2.0 is the latest stable release as of January 2, 2026. It was published December 17, 2025. [1][2]

Sources:

  • GitHub release: prisma/prisma — 7.2.0 (Dec 17, 2025). [1]
  • Prisma blog announcement: "Announcing Prisma ORM 7.2.0" (Dec 17, 2025). [2]

🌐 Web query:

@prisma/adapter-pg version 7.2.0 known issues

💡 Result:

Short summary of known issues affecting @prisma/adapter-pg (v7.x, including 7.2.0):

  • It can modify node-postgres (pg) global type parsers and break direct pg Pool/Client usage. [1]
  • Using the adapter with SSL connection params (e.g., ?sslmode=require) has produced misleading P2010 errors originating from node-postgres. [2]
  • Some users report Prisma Migrate failures when the adapter is configured via prisma.config.ts (migrate/column type errors). [3]
  • In some setups the adapter’s dependency handling for pg/packaging can cause runtime/module resolution problems (missing or peer-declared pg causing runtime errors in certain bundlers/monorepos). [4]
  • There are additional repo issues about schema/search_path handling and other edge cases after migrating to the new Prisma v7 adapter model. [5]

If you want, I can:

  • check the exact changelog for v7.2.0 and list which of these were fixed or remain open, or
  • search for workarounds/fixes for any specific issue above.

Sources:
[1] GitHub: “@prisma/adapter-pg modifies node-postgres global type parsers.” (github.com)
[2] GitHub: “node-postgres (pg) errors with misleading P2010 … with SSL” (github.com)
[3] GitHub: “Prisma Migrate fails when using @prisma/adapter-pg through prisma.config.ts.” (github.com)
[4] GitHub issue: “pg is a peer-dependency of @prisma/adapter-pg when it should be a regular dependency.” (github.com)
[5] GitHub: “Inconsistent behavior when setting Postgres schema … with @prisma/adapter-pg” and related threads. (github.com)

Citations:


Versions are synchronized correctly, but be aware of known issues with @prisma/adapter-pg.

All three Prisma packages are correctly pinned to version 7.2.0, which is the latest stable release (published December 17, 2025). The synchronization across @prisma/adapter-pg (line 29), @prisma/client (line 30), and prisma CLI (line 46) is correct.

However, there are several known issues with @prisma/adapter-pg in the v7.x line that the team should monitor:

  • Global type parser modifications affecting direct pg usage
  • SSL connection parameter handling can produce misleading P2010 errors
  • Prisma Migrate failures in some configurations
  • Peer dependency handling for pg causing potential runtime issues

Keep these in mind during development and testing, and watch for updates addressing these edge cases.

Comment on lines +15 to +27
declare module '@fastify/jwt' {
interface FastifyJWT {
payload: {
userId: string;
username: string;
tier: string;
};
user: {
userId: string;
username: string;
tier: string;
};
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

LGTM! Consider using a shared type for the JWT payload.

The module augmentation correctly extends Fastify's type system. The payload and user shapes are appropriately identical.

For consistency with the Prisma schema's Tier enum and to prevent typos, consider importing and using a shared type for tier:

tier: 'FREE' | 'PRO' | 'TEAM';

This ensures compile-time safety when signing/verifying tokens.

🤖 Prompt for AI Agents
In apps/server/src/types/fastify.d.ts around lines 15–27, the JWT payload and
user types currently use plain strings for `tier`; replace those string literals
with the shared Tier enum/type used across the repo to ensure compile-time
safety. Import the shared Tier type from the common/prisma types module (or from
the generated Prisma types) at the top of the declaration file and use that
imported type for both `payload.tier` and `user.tier`, keeping the shapes
identical.

Comment on lines +29 to +31
/** Redis presence message handler for cleanup */
presenceMessageHandler?: (channel: string, message: string) => void;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider removing unused presenceMessageHandler field.

With the new global message handler pattern in ws/handler.ts, the per-connection presenceMessageHandler field is no longer used. Consider removing it to avoid confusion.

🔎 Proposed fix
 export interface AuthenticatedWebSocket extends WebSocket {
   /** User ID from JWT */
   userId: string;

   /** User's friends (following) for presence subscriptions */
   friendIds: string[];

   /** Whether the connection is authenticated */
   isAuthenticated: boolean;

   /** Last heartbeat timestamp */
   lastHeartbeat: number;

   /** Connection established timestamp */
   connectedAt: number;
-
-  /** Redis presence message handler for cleanup */
-  presenceMessageHandler?: (channel: string, message: string) => void;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/** Redis presence message handler for cleanup */
presenceMessageHandler?: (channel: string, message: string) => void;
}
}
🤖 Prompt for AI Agents
In apps/server/src/ws/types.ts around lines 29 to 31, remove the now-unused
presenceMessageHandler?: (channel: string, message: string) => void; field from
the exported type/interface, then search the repo for any references to
presenceMessageHandler and delete or refactor those callsites to use the new
global message handler in ws/handler.ts (or remove dead code). After changes,
run TypeScript compilation and unit tests to ensure no remaining references
break the build.

Comment on lines +4 to +5
"module": "ESNext",
"moduleResolution": "Bundler",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

TypeScript moduleResolution Bundler vs NodeNext ESM Node.js

💡 Result:

Short summary — when to use which

  • moduleResolution: "bundler"

    • Models how modern bundlers (Webpack/Rollup/Esbuild/Vite/Bun) resolve imports: accepts extensionless relative imports, treats package "exports"/"imports" like bundlers, and pairs with module: "esnext" or "preserve". Use when your build output is intended to be consumed only through a bundler (or you control the bundling). Risk: it will allow imports that work in bundlers but fail in Node.js at runtime (missing .js extensions, directory-as-module, etc.). [1][2][5]
  • moduleResolution: "nodenext" (or node16/node18 family)

    • Models Node.js’s ESM + CJS resolution semantics (package.json "type", conditional "exports"/"imports", required file extensions for ESM, different rules for import vs require). Use when your code runs directly in Node.js (ESM-aware Node versions) or when publishing libraries that must work for Node consumers. It’s stricter about extensionless imports and directory modules, which prevents runtime errors in Node. [1][3][6]

Key practical differences and gotchas

  • Extensions: bundler allows extensionless relative imports; nodenext enforces correct extensions for Node ESM (./foo -> ./foo.js). This is a common cause of runtime ERR_MODULE_NOT_FOUND if you compile with bundler but run in Node. [1][2]
  • package.json "exports"/"imports": both understand these, but nodenext matches Node’s actual runtime mapping; bundler models bundler behavior which can differ. [1][5]
  • Declarations & bundling: if you use bundler resolution but emit separate .d.ts files (not a bundled declaration), those .d.ts imports may be invalid for Node consumers — prefer nodenext for libraries not fully bundling declarations. [2]
  • Pairing with module option: bundler implies module: "esnext" or "preserve"; nodenext/node16 must be used when targeting Node runtimes. [1][5]
  • Interop changes: TypeScript has evolved (node16/nodenext and bundler features added/updated across releases); use nodenext for modern Node features (Node 18/22 behaviours noted in recent TS releases). [3][7][8]

Recommendations

  • App that you build with a bundler and run in browsers or bundler-controlled runtimes: moduleResolution = "bundler" and module = "esnext"/"preserve". (Be sure consumers never run the raw emitted files in Node.) [1][2]
  • Library published to npm intended to run in Node (or both Node and bundlers): prefer moduleResolution = "nodenext" (and module = "node16"/"node18"/"nodenext" as appropriate). If you must use bundling, either produce bundled .d.ts or test with nodenext to ensure Node-compatible specifiers in the public API. [2][5]
  • If unsure and your users run Node directly, default to nodenext/node16 to avoid Node runtime errors. [6]

Sources

  • TypeScript moduleResolution reference. [1]
  • TypeScript handbook discussion on bundler vs Node resolution and declaration-file pitfalls. [2]
  • TypeScript modules theory (node16/node18/nodenext behavior). [3]
  • TS release notes covering bundler/preserve and nodenext updates. [7][8]

If you want, tell me whether this is for an app or a library and which Node / TypeScript versions you target and I’ll give exact tsconfig.json snippets.


🏁 Script executed:

# Check the actual tsconfig.json and package.json in the server directory
cat apps/server/tsconfig.json

Repository: senutpal/devRadar

Length of output: 418


🏁 Script executed:

# Check package.json to see the actual build and dev scripts
cat apps/server/package.json

Repository: senutpal/devRadar

Length of output: 1430


Use "moduleResolution": "NodeNext" instead of "Bundler" for Node.js ESM runtime.

The configuration uses "moduleResolution": "Bundler" but the project runs directly in Node.js without a bundling step: dev uses tsx and production runs node dist/server.js. Bundler resolution allows extensionless imports and import patterns that work with bundlers but fail at runtime in Node.js (e.g., ERR_MODULE_NOT_FOUND). Since the code runs directly in Node.js ESM ("type": "module"), use "moduleResolution": "NodeNext" to ensure import resolution aligns with Node.js's actual runtime semantics.

🤖 Prompt for AI Agents
In apps/server/tsconfig.json around lines 4-5, the project currently sets
"moduleResolution": "Bundler" which permits bundler-only import behaviors that
will fail at Node.js ESM runtime; change the value to "NodeNext" so TypeScript
resolves imports using Node.js ESM semantics (matching runtime used by tsx/dev
and node dist/server.js) to avoid extensionless/import resolution errors. Ensure
you update that single setting and run a type check/build to confirm imports
resolve correctly under NodeNext.

"moduleResolution": "Bundler",
"rootDir": "src",
"outDir": "dist",
"tsBuildInfoFile": "tsconfig.tsbuildinfo",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider relocating tsBuildInfoFile to avoid root clutter.

The tsBuildInfoFile is currently set to "tsconfig.tsbuildinfo" in the project root. Consider moving it to a subdirectory like "dist/tsconfig.tsbuildinfo" or .turbo/tsconfig.tsbuildinfo to keep the root directory cleaner, especially since it's already excluded from git via the *.tsbuildinfo rule in .gitignore.

🤖 Prompt for AI Agents
In apps/server/tsconfig.json around line 8, the tsBuildInfoFile is set to
"tsconfig.tsbuildinfo" in the repo root; change its value to a subdirectory path
such as "dist/tsconfig.tsbuildinfo" or ".turbo/tsconfig.tsbuildinfo" to avoid
root clutter, update or create that target directory if necessary (or ensure
build tooling can create it), and keep the existing .gitignore entry
(*.tsbuildinfo) so the new file remains ignored.

"eslint": "^9.17.0",
"rimraf": "^6.0.1",
"typescript": "^5.7.2"
"eslint": "^9.39.2",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

ESLint 9.17.0 to 9.39.2 breaking changes changelog

💡 Result:

I can’t list every breaking change across all minor releases here, but here are the authoritative release notes you should check (they show breaking changes per release). Notably, v9.39.2 is a patch release (no new breaking changes), while v9.17.0’s release notes include the breaking changes that shipped with that release.

  • ESLint v9.17.0 release notes (breaking changes listed there). [1]
  • ESLint v9.39.2 release notes (patch; highlights and bug fixes — no breaking changes). [2]
  • All ESLint releases / tags on GitHub (useful to scan per-release changelogs between those versions). [3]

If you want, I can:

  • extract and enumerate every "BREAKING CHANGE" entry from the releases between 9.17.0 and 9.39.2, or
  • produce a concise diff-style summary of only the breaking changes that could affect configs/rules/CLI/API.

Which would you prefer?

Sources:
[1] https://eslint.org/blog/2024/12/eslint-v9.17.0-released/
[2] https://eslint.org/blog/2025/12/eslint-v9.39.2-released/
[3] https://github.com/eslint/js/releases


🌐 Web query:

ESLint 9.17.0 release breaking changes notes

💡 Result:

ESLint v9.17.0 (published Dec 13, 2024) is a minor release and the official release notes do not list any breaking changes. Highlights: added suggestions for no-unused-vars and fixed crashes when a processor returns a nullish message.fix. [1]

Source:
[1] ESLint v9.17.0 release notes — eslint.org.


🌐 Web query:

ESLint 9.17 to 9.39 changes new rules rule updates deprecations

💡 Result:

Here’s a concise summary of notable rule additions, rule updates (including suggestions/improved locations/TypeScript support), and deprecations in ESLint releases from v9.17.0 through v9.39.x (high‑level highlights and sources):

  • v9.17.0 (Dec 13, 2024)

    • no-unused-vars: adds auto-fixable suggestions for removing unused variables/params/destructuring. [1]
  • v9.18.0 (Jan 10, 2025)

    • no-shadow-restricted-names: now checks import declarations and class names.
    • no-console: problem messages now list allowed console method names when configured. (rule message improvement) [2]
  • v9.19.0 (Jan 24, 2025)

    • New config/CLI option to report unused inline eslint comments: linterOptions.reportUnusedInlineConfigs / --report-unused-inline-configs. [3]
  • v9.20.0 (Feb 7, 2025)

    • Behavior changes around inactive feature flags (no longer throw) and warnings for empty config exports (cli/config behavior; not a rule change). [4]
  • v9.23.0 → v9.25.0 (Mar–Apr 2025)

    • Multiple core rules gained full TypeScript syntax support incrementally:
      • v9.23.0: class-methods-use-this, default-param-last, no-useless-constructor → TypeScript support. [5]
      • v9.25.0: additional rules (e.g., no-empty-function with TypeScript-specific allow options). [6]
    • v9.24.0: bulk suppressions feature (suppress existing violations while enforcing new ones) — affects how teams enable rules. [7]
  • v9.29.0 (Jun 13, 2025)

    • espree updated to parse ECMAScript “Explicit Resource Management” (using / await using); parser support added (core rules updated later). [8]
  • v9.35.0 (Sep 5, 2025)

    • New core rule: preserve-caught-error (enforce using Error cause when rethrowing from catch).
    • no-empty-function / no-empty-static-block: now offer suggestions to add an explicit /* empty */ comment when intentional. [9]
  • v9.39.0 (Oct 31, 2025) and v9.39.1 patch

    • Improved problem-location precision for several rules (complexity, for-direction, no-dupe-args, no-dupe-class-members).
    • Performance: unified timing report in multithreaded runs when TIMING is set.
    • v9.39.1: compatibility fix restoring single-argument visitor invocation for JS/TS to avoid breaking third-party rules. [10][11]
  • Deprecations / removals to be aware of (context across v9.x)

    • Some stylistic/core rules deprecated or removed in and before v9 (examples):
      • Removed in v9.0: valid-jsdoc and require-jsdoc; many deprecated Rule/SourceCode methods removed. [12]
      • v9.3.0: line-comment-position and multiline-comment-style deprecated in favor of third‑party plugins. [13]

Notes / references

  • The above is a summary of rule-focused highlights across the listed releases; each release blog post contains full change lists and links to commits/PRs for precise details and rule schema/options. See:
    1. ESLint v9.17.0 release notes (no-unused-vars suggestions). [1]
    2. ESLint v9.18.0 release notes (no-shadow-restricted-names, no-console messages). [2]
    3. ESLint v9.19.0 release notes (reportUnusedInlineConfigs). [3]
    4. ESLint v9.20.0 release notes. [4]
    5. ESLint v9.23.0 release notes (TypeScript support for core rules). [5]
    6. ESLint v9.25.0 release notes (no-empty-function TypeScript options). [6]
    7. ESLint v9.24.0 release notes (bulk suppressions). [7]
    8. ESLint v9.29.0 release notes (espree: using / await using parsing). [8]
    9. ESLint v9.35.0 release notes (preserve-caught-error + suggestions). [9]
    10. ESLint v9.39.0 release notes (precise problem locations, unified timing). [10]
    11. ESLint v9.39.1 release notes (visitor compatibility fix). [11]
    12. ESLint v9.0.0 release notes (removed rules / API changes). [12]
    13. ESLint v9.3.0 release notes (deprecated comment-position rules). [13]

If you want, I can:

  • produce a version-by-version checklist of rule schema/option changes and deprecations for every minor release from 9.17 → 9.39, or
  • extract the exact rule option/schema diffs and PR links for any specific rule you care about.

Sources:
[1] https://eslint.org/blog/2024/12/eslint-v9.17.0-released/
[2] https://eslint.org/blog/2025/01/eslint-v9.18.0-released/
[3] https://eslint.org/blog/2025/01/eslint-v9.19.0-released/
[4] https://eslint.org/blog/2025/02/eslint-v9.20.0-released/
[5] https://eslint.org/blog/2025/03/eslint-v9.23.0-released/
[6] https://eslint.org/blog/2025/04/eslint-v9.25.0-released/
[7] https://eslint.org/blog/2025/04/eslint-v9.24.0-released/
[8] https://eslint.org/blog/2025/06/eslint-v9.29.0-released/
[9] https://eslint.org/blog/2025/09/eslint-v9.35.0-released/
[10] https://eslint.org/blog/2025/10/eslint-v9.39.0-released/
[11] https://eslint.org/blog/2025/11/eslint-v9.39.1-released/
[12] https://eslint.org/blog/2024/04/eslint-v9.0.0-released/
[13] https://eslint.org/blog/2024/05/eslint-v9.3.0-released/


Check rule changes in ESLint 9.17–9.39 update.

This update includes rule enhancements that may affect compliance across the monorepo:

  • no-unused-vars now provides auto-fixable suggestions
  • no-shadow-restricted-names now checks imports and class names
  • New rule: preserve-caught-error (catch/rethrow handling)
  • TypeScript support expanded for several core rules
  • Problem location precision improvements

Verify the monorepo lints cleanly with these changes.

🤖 Prompt for AI Agents
In packages/shared/package.json around line 44, upgrading ESLint to ^9.39.2 may
introduce rule changes (no-unused-vars autofix suggestions, stricter
no-shadow-restricted-names, new preserve-caught-error, expanded TS support and
location precision) that can break CI; run the updated ESLint across the
monorepo, fix any new violations or suppress/adjust rules via .eslintrc.js
(e.g., configure no-unused-vars, no-shadow-restricted-names and
preserve-caught-error settings, add overrides for TypeScript files), update
TypeScript/TS-ESLint configs if needed, and re-run CI; if immediate fixes are
risky, pin ESLint to the previous working version in package.json until rules
are addressed.

- Add DATABASE_URL and SHADOW_DATABASE_URL placeholders for CI
- Fix unnecessary type assertion in auth.ts token decoding
- Prisma 7's config loader requires env vars even for generate
Changed dependsOn from "topo" to "^build" so that workspace
packages like @devradar/shared are compiled before linting
packages that depend on them.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
apps/server/src/routes/auth.ts (1)

41-46: Add signed flag and restrict cookie path for defense-in-depth.

The oauth_state cookie lacks the signed: true flag, which means it can be tampered with by clients. Additionally, the cookie path should be restricted to /auth/callback to minimize exposure.

🔎 Proposed fix
    reply.setCookie('oauth_state', state, {
      httpOnly: true,
      secure: isProduction,
      sameSite: 'lax',
      maxAge: 600, // 10 minutes
+     signed: true,
+     path: '/auth/callback',
    });

Then update the callback route to match the path when clearing:

-     reply.clearCookie('oauth_state');
+     reply.clearCookie('oauth_state', { path: '/auth/callback' });
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7a64485 and add8b14.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml
  • apps/server/src/routes/auth.ts
🧰 Additional context used
🪛 Checkov (3.2.334)
.github/workflows/ci.yml

[medium] 49-50: Basic Auth Credentials

(CKV_SECRET_4)

🔇 Additional comments (3)
.github/workflows/ci.yml (1)

46-50: LGTM! Static analysis false positive on placeholder credentials.

The step correctly generates the Prisma client before linting and type checking. The placeholder database URLs with user:pass credentials triggered a security warning, but these are dummy values required by Prisma 7's config loader during generation—no actual database connection occurs in CI at this stage.

If you'd like to silence the static analysis warning, you could add a brief inline comment above line 49 explaining these are non-sensitive placeholders, though it's not strictly necessary given the commit message already documents this.

apps/server/src/routes/auth.ts (2)

60-138: Excellent implementation of OAuth callback with security measures.

The callback route now properly addresses previous security concerns:

  • Rate limiting is configured appropriately (10 req/min)
  • CSRF state validation is implemented correctly
  • Error handling covers both validation failures and GitHub OAuth errors
  • Token generation and response structure are well-designed

Note: If the cookie path is restricted to /auth/callback (as suggested in the previous comment), ensure line 95 clears with the matching path.


144-163: LGTM!

The token refresh logic is straightforward and properly protected. The implementation correctly reissues a JWT with the same claims and expiry window.

Comment on lines +182 to +186
const ttlSeconds = (decoded.exp as number) - Math.floor(Date.now() / 1000);

// Import dynamically to avoid circular dependency
const { blacklistToken } = await import('@/services/redis');
await blacklistToken(token, ttlSeconds);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Check token expiry before blacklisting.

If the token is already expired (ttlSeconds <= 0), attempting to blacklist it is unnecessary. This wastes a Redis operation and could cause issues if Redis rejects non-positive TTLs.

🔎 Proposed fix
            const ttlSeconds = (decoded.exp as number) - Math.floor(Date.now() / 1000);

+           // Only blacklist if token hasn't expired yet
+           if (ttlSeconds > 0) {
              // Import dynamically to avoid circular dependency
              const { blacklistToken } = await import('@/services/redis');
              await blacklistToken(token, ttlSeconds);

              logger.info('Token blacklisted on logout');
+           } else {
+             logger.debug('Token already expired, skipping blacklist');
+           }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const ttlSeconds = (decoded.exp as number) - Math.floor(Date.now() / 1000);
// Import dynamically to avoid circular dependency
const { blacklistToken } = await import('@/services/redis');
await blacklistToken(token, ttlSeconds);
const ttlSeconds = (decoded.exp as number) - Math.floor(Date.now() / 1000);
// Only blacklist if token hasn't expired yet
if (ttlSeconds > 0) {
// Import dynamically to avoid circular dependency
const { blacklistToken } = await import('@/services/redis');
await blacklistToken(token, ttlSeconds);
logger.info('Token blacklisted on logout');
} else {
logger.debug('Token already expired, skipping blacklist');
}
🤖 Prompt for AI Agents
In apps/server/src/routes/auth.ts around lines 182 to 186, the code computes
ttlSeconds from the token expiry but always imports and calls blacklistToken;
change it to check if ttlSeconds > 0 before importing/blacklisting to avoid
unnecessary Redis work or errors for non-positive TTLs—if ttlSeconds <= 0 simply
skip the import/call (optionally log a debug message) so Redis is only called
for valid positive TTLs.

const ttlSeconds = (decoded.exp as number) - Math.floor(Date.now() / 1000);

// Import dynamically to avoid circular dependency
const { blacklistToken } = await import('@/services/redis');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider refactoring to eliminate dynamic import.

The dynamic import suggests a circular dependency between the auth routes and Redis service modules. While this works, it's a code smell that could be addressed by:

  • Creating a shared token blacklist service that both modules depend on
  • Moving token blacklist logic into a separate module that neither depends on
  • Using dependency injection to pass the blacklist function to the routes

This is non-blocking but worth addressing in future refactoring.

@senutpal senutpal merged commit 6fb96e0 into main Jan 2, 2026
4 checks passed
@senutpal senutpal deleted the feat/backend-implementation-phase-1 branch January 6, 2026 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant