Skip to content

Conversation

@lukesmmr
Copy link
Member

@lukesmmr lukesmmr commented Feb 7, 2026

Summary by CodeRabbit

  • New Features

    • Added environment configuration template for easier local setup
    • Improved ESLint configuration for better code quality
  • Improvements

    • Upgraded to Node.js 22 LTS
    • Updated GraphQL server with enhanced error reporting
    • Improved Docker build process and deployment
    • Enhanced database connection handling
    • Updated development tools and dependencies
  • Documentation

    • Expanded environment variables guide
    • Enhanced quick start instructions with Docker focus
    • Added platform-specific setup guidance

lukesmmr and others added 13 commits February 7, 2026 12:14
- Update Dockerfile (dev) from node:20-slim to node:22-slim

- Update Dockerfile.production from node:20-alpine to node:22-alpine (both stages)

- Update CI workflow from node-version 20 to 22

Node 22 is the active LTS (maintained through April 2027) and brings require(esm) support, stable fetch API, and performance improvements.

Co-authored-by: Cursor <cursoragent@cursor.com>
npm audit fix resolved:

- critical: form-data unsafe random boundary

- high: body-parser/qs/express DoS via url encoding and arrayLimit bypass

- moderate: js-yaml and lodash prototype pollution

- low: @eslint/plugin-kit ReDoS, tmp symlink write, diff DoS

Co-authored-by: Cursor <cursoragent@cursor.com>
npm update applied safe semver-compatible updates to 35 packages including:

- express 4.21.2->4.22.1, body-parser 2.2.0->2.2.2, cors 2.8.5->2.8.6

- pg 8.16.3->8.18.0, winston 3.17.0->3.19.0, graphql 16.11.0->16.12.0

- typescript 5.8.3->5.9.3, prettier 3.6.2->3.8.1, supertest 7.1.1->7.2.2

- typescript-eslint 8.35.1->8.54.0, ts-jest 29.4.0->29.4.6, tsx 4.20.3->4.21.0

Co-authored-by: Cursor <cursoragent@cursor.com>
- husky 8->9: simplified hook format (removed husky.sh sourcing), prepare script updated

- dotenv 16->17: transparent for import dotenv/config usage

- cross-env 7->10: drop-in replacement, bumps minimum Node version

Co-authored-by: Cursor <cursoragent@cursor.com>
- @prisma/client + prisma 6.x -> 7.3.0: new major release, straightforward for this project

- uuid 11.x -> 13.0.0: stable API (validate, v4), drop-in compatible

- joi 17.x -> 18.0.2: standard validation schemas remain compatible

Note: prisma 7 brings 8 moderate audit findings in its dev tooling

(hono, lodash via @prisma/dev) - these are upstream issues, not runtime.

Co-authored-by: Cursor <cursoragent@cursor.com>
xss-clean is abandoned and was never imported in any source file.

The project already uses helmet for security headers. XSS protection

is handled via output encoding and helmet CSP headers.

Co-authored-by: Cursor <cursoragent@cursor.com>
- Complete Prisma 7 migration: add prisma.config.ts for datasource URL,
  remove url from schema.prisma, use PrismaPg driver adapter
- Add @prisma/adapter-pg dependency for Prisma 7 driver adapter pattern
- Downgrade uuid v13 -> v11 (v13 is ESM-only, incompatible with Jest)
- Fix protobuf CJS/ESM interop using createRequire with process.cwd()
  so disaster_pb.js loads in both Jest CJS and Node.js ESM contexts
- Fix Express 5 type errors: type route params as Request<{ id: string }>
- Modernize jest.config.ts: migrate deprecated globals to transform config
- Add eslint.config.js flat config for ESLint 9 compatibility
- Add .nvmrc pinning Node.js to v22 for consistent local development
- Add generated/ to .gitignore for Prisma generated client output

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Replace bodyParser.json() with built-in express.json(). Remove body-parser
dependency since Express 5 includes body parsing natively.

Co-authored-by: Cursor <cursoragent@cursor.com>
Replace EOL apollo-server-express with @apollo/server and
@as-integrations/express5. Replace ApolloError/UserInputError with
GraphQLError from the graphql package. Remove --force from Dockerfile
npm install now that the Express 4 peer dependency conflict is gone.
Add @types/cors, @graphql-tools/utils, and graphql-tag as direct
dependencies (previously transitive via apollo-server-express).

Co-authored-by: Cursor <cursoragent@cursor.com>
Remove unused isGraphQLNotFoundError function from resolvers (leftover
from Apollo v3 migration). Apply prettier formatting from pre-commit
hook. Bump protobufjs to ^7.5.4 within existing range.

Co-authored-by: Cursor <cursoragent@cursor.com>
- Fix NODE_ENV in docker-compose.yml (test → development) so the API
  server actually starts for local development; tests still run with
  NODE_ENV=test via jest.setup.ts
- Switch healthcheck to wget /healthz and increase start_period
- Add prisma generate and schema copy to Dockerfile so the container
  builds without manual post-build steps
- Use process.env fallback in prisma.config.ts to avoid the dummy
  POSTGRES_URI hack during Docker build
- Switch nodemon exec to npx tsx for correct ESM resolution
- Remove dead apolloReady promise, API_KEY env validation, and orphaned
  Apollo init code from app.ts and index.ts
- Add .env.example with documented environment variables
- Update README with accurate Docker-first quick start and local dev
  instructions
- Add !.env.example exception to .gitignore

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 7, 2026

📝 Walkthrough

Walkthrough

Node.js upgraded from 20 to 22 across all configurations. Apollo Server Express migrated to Apollo Server 5 with express5 integration. Prisma upgraded from v6 to v7 with PG adapter. Major dependencies updated: Express 4→5, rate-limit and Joi versions bumped. New configuration files added: eslint.config.js, prisma.config.ts, .env.example. Error handling refactored from ApolloError to GraphQLError. Build tooling simplified via tsx and Jest transform updates.

Changes

Cohort / File(s) Summary
Node.js Version Upgrade (20→22)
.nvmrc, .github/workflows/ci.yml, Dockerfile, Dockerfile.production
Updated Node.js version to 22 across runtime configuration, CI pipeline, and Docker images (node:20-slim→node:22-slim; node:20-alpine→node:22-alpine).
Apollo Server 5 Migration
app.ts, graphql/resolvers.ts, graphql/resolvers.coverage.test.ts, index.ts
Replaced apollo-server-express with @apollo/server and @as-integrations/express5. Refactored ApolloError/UserInputError to GraphQLError with explicit extensions.code. Removed apolloReady export and updated initialization flow. Tests updated to use new error class and direct resolver invocation patterns.
Prisma Configuration & Database
services/prisma.ts, prisma/schema.prisma, prisma.config.ts
Upgraded Prisma v6→v7; integrated PrismaPg adapter with connectionString from POSTGRES_URI. Introduced prisma.config.ts with datasource fallback for build-time generation. Removed url field from schema datasource block.
Build Configuration & Tooling
jest.config.ts, nodemon.json, eslint.config.js
Jest config refactored to use transform property instead of preset/globals. Nodemon switched from ts-node/esm to tsx for TypeScript execution. New eslint.config.js added with TypeScript, Prettier, and custom rules for strict typing.
Package Dependencies
package.json
Major version bumps: Express 4→5, express-rate-limit 7→8, Joi 17→18, husky 8→9, cross-env 7→10. Removed body-parser and xss-clean. Added @as-integrations/express5, @prisma/adapter-pg, @graphql-tools/utils, graphql-tag. Moved prisma to dependencies. Updated prepare script; added proto:ts and proto:all scripts.
Docker & Compose Configuration
docker-compose.yml
Changed api environment NODE_ENV from test to development. Updated healthcheck from curl to wget against /healthz endpoint with extended start_period (10s→30s). Added Prisma generate step to Dockerfile build process.
Environment & Git Configuration
.env.example, .gitignore, .husky/pre-commit
New .env.example template with database and server configuration guidance. Updated .gitignore to track eslint.config.js and generated/ directory, unignore .env.example. Simplified .husky/pre-commit by removing shebang and bootstrap lines.
Protobuf & Route Handling
routes/disasters.ts, routes/disasters.protobuf.test.ts
Replaced static ESM imports with dynamic CommonJS require via createRequire for protobuf bindings. Enhanced type annotations for route handlers using Request<{ id: string }> for ID parameters.
Documentation & Configuration
README.md
Restructured Getting Started into Quick Start (Docker-recommended) and local dev sections with Prisma/Protobuf generation steps. Added comprehensive Environment Variables table. Expanded deployment, E2E testing, and platform-specific (macOS/Apple Silicon) guidance. Updated Docker Compose usage instructions.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 Node twenty-two hops in with glee,
Apollo's new vest fits Express with spree,
Prisma adapters dance by the shore,
Config files spring where none were before,
A rabbit's delight—rebuilt from the core! 🌿✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main changes: upgrading Node.js to version 22 and major dependency versions (Express 5, Apollo Server 4, Prisma 7), which align with the primary objectives documented in the PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch audit-package-update

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.

@lukesmmr
Copy link
Member Author

lukesmmr commented Feb 7, 2026

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Feb 7, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (9)
routes/disasters.ts (1)

39-42: ⚠️ Potential issue | 🟡 Minor

UUID regex only matches versions 1–5; will reject v6/v7 UUIDs.

The [1-5] in the version nibble position means this regex rejects UUIDs v6, v7, and v8, which are valid RFC 9562 variants. Since the project uses uuid v11 (which supports v7 generation), this could become a real issue if the ID generation strategy changes.

Note: the router.param on line 85 already validates using validate from the uuid package (which accepts all versions), making the inline isValidUuid calls on lines 253, 303, 338 redundant and more restrictive.

♻️ Suggested: widen the regex or remove the redundant check
-const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
+const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;

Or better yet, remove isValidUuid entirely and rely solely on the router.param guard that already uses the uuid package's validate.

README.md (2)

183-202: ⚠️ Potential issue | 🟡 Minor

Duplicate "Environment Variables" section.

There are two ### Environment Variables headings — one at line 118 (concise table) and another here at line 183 (longer prose). This second section appears to be a leftover from the pre-refactor README. Consider removing it to avoid confusion and keep documentation DRY.


346-357: ⚠️ Potential issue | 🟡 Minor

GraphQL example references _id — should be id.

The example query uses _id, which is a MongoDB convention. Since the project has migrated to PostgreSQL with Prisma, this should likely be id to match the actual schema field.

📝 Suggested fix
       data {
-        _id
+        id
         type
         status
         description
index.ts (1)

29-42: ⚠️ Potential issue | 🟠 Major

Duplicate SIGTERM/SIGINT handlers conflict with app.ts.

app.ts (lines 454-459) registers SIGINT/SIGTERM handlers guarded by process.listenerCount('SIGINT') === 0. Since createApp() is called before these listeners are registered here (line 22 runs before lines 41-42), the app.ts handlers will be installed first. Then index.ts adds additional listeners for the same signals.

On SIGTERM, both handlers fire: app.ts's gracefulShutdown calls process.exit(0) synchronously, likely preempting the server.close() callback here. The server may not drain in-flight connections.

Consider consolidating signal handling in one place — either index.ts (preferred, since it owns the Server instance) or app.ts, but not both.

app.ts (4)

175-175: ⚠️ Potential issue | 🟠 Major

helmet() is applied three times in the middleware stack.

  1. Line 175: app.use(helmet()) — default config
  2. Line 213: app.use(helmet()) — default config again (duplicate)
  3. Lines 222-239: app.use(helmet({...})) — custom CSP/HSTS config

Every response passes through all three, with later invocations overwriting headers set by earlier ones. The second call (line 213) appears accidental. Consolidate into a single helmet() call with the custom config.

Proposed fix: remove duplicate calls
-  // Security headers
-  app.use(helmet());
-
   // Rate limiting: disable or relax for tests

and:

   // Only apply JSON body parsing to REST routes (built into Express 5)
   app.use('/api', express.json());
-  app.use(helmet());
   app.use(hpp());

Keep only the fine-tuned helmet call at lines 222-239.

Also applies to: 211-214, 222-239


327-328: ⚠️ Potential issue | 🟠 Major

POSTGRES_URI logged to console — may contain credentials.

Line 328 logs process.env.POSTGRES_URI, which typically includes the username and password in the connection string (e.g., postgresql://user:password@host/db). This is a credential leak risk in logs.

Remove or redact the log
 async function updateDisasterMetrics() {
-  console.log('[updateDisasterMetrics] Using POSTGRES_URI:', process.env.POSTGRES_URI);
+  console.log('[updateDisasterMetrics] Updating disaster metrics...');

329-403: ⚠️ Potential issue | 🟠 Major

updateDisasterMetrics creates and destroys a Pool every 30 seconds.

This function runs on a 30-second interval (line 407) and creates a new Pool each time (line 331), then calls pool.end() in the finally block (line 401). Creating/tearing down a connection pool this frequently is wasteful and adds latency.

Consider using a module-scoped pool or, better yet, accepting the pool from createApp via a closure or shared reference.

Sketch: use a shared pool
+let metricsPool: Pool | undefined;
+
 async function updateDisasterMetrics() {
-  console.log('[updateDisasterMetrics] Using POSTGRES_URI:', process.env.POSTGRES_URI);
-  let pool: Pool | undefined;
   try {
-    pool = new Pool({
-      connectionString: process.env.POSTGRES_URI,
-    });
+    if (!metricsPool) {
+      metricsPool = new Pool({ connectionString: process.env.POSTGRES_URI });
+    }
+    const pool = metricsPool;
     // ... rest of queries using pool ...
   } catch (error) {
     logger.error('Error updating disaster metrics', { error });
-  } finally {
-    if (pool) {
-      await pool.end();
-    }
   }
 }

442-459: ⚠️ Potential issue | 🟠 Major

Signal handlers registered at module scope conflict with index.ts.

As noted in the index.ts review, these module-scope SIGINT/SIGTERM handlers (which call process.exit(0) immediately) conflict with the shutdown logic in index.ts that attempts to drain the HTTP server first. The listenerCount === 0 guard on lines 454/457 won't help because app.ts is evaluated before index.ts registers its own handlers.

Additionally, gracefulShutdown doesn't actually close database pools — it just logs and exits. Consider consolidating shutdown logic in index.ts where the Server instance is available, and have it also close pools.

Dockerfile (1)

41-42: ⚠️ Potential issue | 🟡 Minor

wget is not available in node:22-slim and will cause the healthcheck to fail.

The node:22-slim image (Debian bookworm-slim) does not include wget or curl by default. The healthcheck command on line 42 uses wget but it is never installed, causing the healthcheck to always fail.

Either install wget via apt-get, or use a Node.js-based check:

Alternative: use a Node.js one-liner for the healthcheck
 HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \
-  CMD wget --no-verbose --tries=1 --spider http://localhost:3000/healthz || exit 1
+  CMD node -e "fetch('http://localhost:3000/healthz').then(r => { if (!r.ok) process.exit(1) }).catch(() => process.exit(1))"
🤖 Fix all issues with AI agents
In `@Dockerfile.production`:
- Line 7: The build stage currently runs "RUN npm ci --only=production" which
omits devDependencies (like TypeScript and build tools) so the following "npm
run build" will fail; change the build stage to install all deps (use "npm ci"
without omitting dev deps) so tools required by the build scripts are present,
then in the final production stage install only runtime deps using the new npm
10+ flag (use "npm ci --omit=dev" or equivalent) to avoid bringing
devDependencies into the production image; update any references to
"--only=production" to "--omit=dev" to be compatible with npm 10+.

In `@package.json`:
- Line 35: The package.json dependency entry for "dotenv" uses a non-existent
version "^17.2.4"; update the version specifier for the "dotenv" dependency to a
valid published release (e.g., "^17.2.3") so npm/yarn can install it; locate the
"dotenv" line in package.json and change the version string accordingly and run
install to verify.
- Line 43: The project added "joi" v18 in package.json which requires Node.js
>=20 and changes TypeScript generics for arrays; update package.json (the "joi"
dependency) to ensure the Node engine requirement is declared (add or bump the
"engines" field to ">=20" and/or update CI/Dockerfiles to use Node 20+), and
audit TypeScript usages of Joi array generics—replace occurrences of
Joi.array<T>().items(...) with the new pattern Joi.array().items<T>(...) and
update any related imports/usages referencing Joi array typing to match v18
typings.

In `@services/prisma.ts`:
- Around line 4-8: The PrismaPg adapter is being constructed at module scope
using process.env.POSTGRES_URI which may be undefined due to import ordering;
change prisma.ts to lazily initialize the adapter and client (implement a
singleton getter like getPrisma() that constructs new PrismaPg and new
PrismaClient only on first call) or add an explicit guard at the top of
prisma.ts that throws or reads process.env.POSTGRES_URI before creating
PrismaPg; reference the existing symbols PrismaPg, PrismaClient and the exported
prisma instance (replace direct export with a getter or factory) so callers use
getPrisma()/prisma() after POSTGRES_URI is validated in app.ts.
🧹 Nitpick comments (14)
docker-compose.yml (1)

17-18: Consider using depends_on with a health condition for reliable startup ordering.

Currently, the api service starts as soon as the postgres container is created, not when Postgres is actually ready to accept connections. Since you already have a healthcheck on the postgres service (via options), you could use depends_on with a condition to avoid race conditions during startup.

♻️ Suggested improvement
   api:
     build: .
     depends_on:
-      - postgres
+      postgres:
+        condition: service_healthy

Note: this requires adding a healthcheck block to the postgres service definition (the healthcheck in options is only for CI). For example:

  postgres:
    ...
    healthcheck:
      test: ["CMD-SHELL", "pg_isready -U disasters"]
      interval: 10s
      timeout: 5s
      retries: 5
nodemon.json (1)

5-5: Consider dropping npx for faster restarts.

If tsx is already in devDependencies, nodemon can resolve it directly from node_modules/.bin. Using npx adds a small resolution overhead on every restart during development.

♻️ Suggested change
-  "exec": "npx tsx index.ts",
+  "exec": "tsx index.ts",
routes/disasters.protobuf.test.ts (2)

3-6: CJS interop approach is correct.

Using createRequire to load the protobuf CJS module from an ESM/TS context is the standard Node.js pattern.

One minor note: the import type on line 7 is interleaved after the const declarations on lines 5-6. While this is harmless (type imports are erased at compile time), grouping all import statements together is conventional and avoids potential linter warnings.

♻️ Suggested reordering
 import request from 'supertest';
 import { createApp } from '../app';
 import { createRequire } from 'module';
 import path from 'path';
+import type { Server } from 'http';
+
 const requireCjs = createRequire(path.resolve(process.cwd(), 'package.json'));
 const disastersPb = requireCjs('./proto/disaster_pb.js');
-import type { Server } from 'http';

37-44: Test silently passes on non-200 responses due to loose assertion.

If the endpoint returns a non-200 status (e.g., 500 due to a DB issue), the else branch logs the error but then asserts res.type === 'application/x-protobuf', which will fail with a confusing message about content type rather than surfacing the actual status code issue. This is a pre-existing pattern, but worth noting since this test file was touched.

.gitignore (1)

38-40: The broad *.js ignore requires explicit un-ignoring for every tracked JS file.

This is a known TypeScript-project pattern, but it can silently ignore new JS config files (e.g., jest.config.js, commitlint.config.js, etc.) if someone forgets to add an ! entry. Just flagging for awareness — no action needed if the team is familiar with this convention.

graphql/resolvers.ts (1)

63-67: Consider logging the original error before wrapping it.

The bare catch discards the original error, making debugging harder. The same pattern appears in other resolvers where err is captured but only used for the instanceof check — the original message/stack is lost in the INTERNAL_ERROR wrap.

A lightweight improvement would be to log the original error (e.g., via your Winston logger) before re-throwing the sanitized GraphQLError, at least for the INTERNAL_ERROR path. This applies to all catch blocks in this file.

graphql/resolvers.coverage.test.ts (2)

6-10: Casting to Record<string, (...args: any[]) => any> loses all type safety.

This is pragmatic for direct resolver invocation but means the test won't catch signature drift. An alternative is to type the resolver maps more narrowly using the generated IResolvers type, though the current approach is common in resolver unit tests.


24-26: Tests don't assert the specific extensions.code values.

The tests only verify that a GraphQLError is thrown, but don't check whether the code is BAD_USER_INPUT, NOT_FOUND, or INTERNAL_ERROR. Adding assertions on extensions.code would ensure the correct error classification reaches clients.

For example:

try {
  await Query.disaster({}, { id: undefined }, {});
} catch (e) {
  expect(e).toBeInstanceOf(GraphQLError);
  expect((e as GraphQLError).extensions?.code).toBe('BAD_USER_INPUT');
}
routes/disasters.ts (1)

24-28: Use import.meta.url instead of process.cwd() for more robust CJS interop.

The current approach using process.cwd() breaks when the process starts from a different working directory (e.g., test runners, monorepo setups, Docker with non-root WORKDIR). Use import.meta.url which resolves relative to the file itself:

♻️ Suggested fix
-import { createRequire } from 'module';
-import path from 'path';
-const requireCjs = createRequire(path.resolve(process.cwd(), 'package.json'));
-const disastersPb = requireCjs('./proto/disaster_pb.js');
+import { createRequire } from 'module';
+const requireCjs = createRequire(import.meta.url);
+const disastersPb = requireCjs('../proto/disaster_pb.js');

Note: The same pattern appears in routes/disasters.protobuf.test.ts and should also be updated.

index.ts (1)

5-6: Remove debug console.log statements.

These top-level log lines (Entry point reached, Starting Disaster API entrypoint) look like development/debugging artifacts. Consider removing them or routing through the Winston logger for consistency.

Dockerfile (1)

14-15: Redundant npm install --save-dev protobufjs-cli.

protobufjs-cli is already listed in devDependencies (package.json line 74), so npm install on line 9 already installs it. This extra install step modifies the in-container package.json and package-lock.json unnecessarily.

Remove redundant install
-# Install protobufjs CLI for code generation
-RUN npm install --save-dev protobufjs-cli
-
 # Copy proto files for code generation
 COPY proto/ ./proto/
app.ts (3)

96-114: Apollo Server initialization reworked correctly for Express 5 integration.

The initApollo function now:

  • Accepts the Express app to mount middleware directly
  • Calls apolloServer.start() before mounting (required by Apollo Server 4+)
  • Mounts via expressMiddleware with its own express.json() scoped to /graphql

One minor note: the formatError callback logs to both console.error, logger.error, and process.stderr.write (lines 104-107). This triple-logging is noisy — consider keeping only the structured logger.error call.


414-439: Remove commented-out dead code.

Lines 414-429, 431-439, and 462 contain large blocks of commented-out code (old error handler, old mongoSanitize middleware, old 404 registration). These are superseded by the code now inside createApp() and should be removed to reduce clutter.

Also applies to: 461-462


155-157: Variable client shadows the prom-client import.

Lines 156 and 336 declare const client = await pool.connect(), which shadows the module-level import client from 'prom-client' (line 16) within their respective function scopes. This won't cause a bug today but is a maintenance hazard — someone adding Prometheus logic inside createApp or updateDisasterMetrics could accidentally reference the pg client instead.

Consider renaming the pg client variables (e.g., pgClient or dbClient).

Also applies to: 335-337

FROM node:22-alpine AS build
WORKDIR /app
COPY package*.json ./
RUN npm ci --only=production && npm cache clean --force
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

🧩 Analysis chain

🏁 Script executed:

fd Dockerfile.production

Repository: goodthingsdev/disasters-api

Length of output: 92


🌐 Web query:

npm ci --only=production deprecated npm 10.x modern alternative --omit=dev

💡 Result:

npm ci --only=production is deprecated in npm 10+ because the only=production / production=true config flags are deprecated in favor of the newer omit/include mechanism. The modern equivalent is:

npm ci --omit=dev

This omits devDependencies from what’s installed on disk (and is also what npm defaults to when NODE_ENV=production). [1] [2]

Related:

  • Old --production flag is also deprecated → use --omit=dev. [1]
  • If you need to force dev deps to be installed (even with NODE_ENV=production), use:
    npm ci --include=dev
    (Works as the replacement for older --also=dev, which is deprecated.) [1] [3]

Sources

  1. npm CLI docs (config: only/production deprecated; use --omit=dev, and only=production is an alias for --omit=dev) [1]
  2. npm CLI docs (npm ci: omit option and default behavior) [2]
  3. Notes on --also deprecation and --include=dev usage (npm 10.x) [3]

🏁 Script executed:

cat -n Dockerfile.production

Repository: goodthingsdev/disasters-api

Length of output: 732


Build stage installs only production dependencies but then runs npm run build.

npm ci --only=production skips devDependencies such as TypeScript, build tools, and type definitions. The subsequent npm run build on line 9 will fail because the compiler isn't installed. Install all dependencies in the build stage and restrict to production-only in the production stage.

Additionally, --only=production is deprecated in npm 10.x (shipped with Node 22). Update to --omit=dev.

🐛 Proposed fixes

Line 7 (build stage):

-RUN npm ci --only=production && npm cache clean --force
+RUN npm ci && npm cache clean --force

Line 18 (production stage):

-RUN npm ci --only=production && npm cache clean --force
+RUN npm ci --omit=dev && npm cache clean --force
🤖 Prompt for AI Agents
In `@Dockerfile.production` at line 7, The build stage currently runs "RUN npm ci
--only=production" which omits devDependencies (like TypeScript and build tools)
so the following "npm run build" will fail; change the build stage to install
all deps (use "npm ci" without omitting dev deps) so tools required by the build
scripts are present, then in the final production stage install only runtime
deps using the new npm 10+ flag (use "npm ci --omit=dev" or equivalent) to avoid
bringing devDependencies into the production image; update any references to
"--only=production" to "--omit=dev" to be compatible with npm 10+.

"@prisma/client": "^7.3.0",
"cors": "^2.8.5",
"dotenv": "^16.5.0",
"dotenv": "^17.2.4",
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

🧩 Analysis chain

🌐 Web query:

dotenv npm package latest version 2026

💡 Result:

As of February 7, 2026, the latest published version of the dotenv npm package is 17.2.3. [1][2]


Version 17.2.4 of dotenv does not exist — update to 17.2.3 (the latest published version).

As of February 2026, the latest published version of dotenv is 17.2.3. The ^17.2.4 specified here will fail to install. Change to ^17.2.3 or allow the caret to resolve to the latest patch within 17.2.

🤖 Prompt for AI Agents
In `@package.json` at line 35, The package.json dependency entry for "dotenv" uses
a non-existent version "^17.2.4"; update the version specifier for the "dotenv"
dependency to a valid published release (e.g., "^17.2.3") so npm/yarn can
install it; locate the "dotenv" line in package.json and change the version
string accordingly and run install to verify.

"helmet": "^8.1.0",
"hpp": "^0.2.3",
"joi": "^17.13.3",
"joi": "^18.0.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:

joi npm package version 18.0.2 changelog

💡 Result:

For joi@18.0.2 there doesn’t appear to be a substantive changelog entry—the v18.0.2 tag (Nov 19, 2025) points to a commit whose only change is bumping package.json from 18.0.1 to 18.0.2. [1][2]

Also, the official joi.dev changelog currently lists 18.0.1 and 18.0.0 but does not include an 18.0.2 section. [3]

Sources: GitHub tags / commit diff for v18.0.2, and joi.dev changelog. [1][2][3]


🌐 Web query:

joi v18.0.0 breaking changes changelog

💡 Result:

Joi v18.0.0 — breaking changes / migration notes (changelog)

  • Dropped Node.js < 20 support (v18 targets Node >= 20). [1][2]
  • Dependency / module upgrades (breaking-change release): updated the internal @hapi/* modules (e.g., address v4→v5 and moved to @hapi namespace; hoek v10→v11; topo v5→v6; tlds added, etc.). [1]
  • string().ip() cidr option validation change: invalid cidr values now error with options.cidr must be one of required, optional, forbidden (instead of options.cidr must be a string). [1]
  • TypeScript typing change for arrays: if you used Joi.array<T>().items(...), you’ll likely need to switch to Joi.array().items<T>(...). [1]

For context, the v18.0.0 changelog entries also include: isAsync() helper, UUID wrapper option, and Standard Schema support. [3][4]

Sources:
[1] v18.0.0 release notes issue (migration checklist + breaking changes) (github.com)
[2] v18.0.0 package.json (Node engines >= 20) (app.unpkg.com)
[3] joi.dev changelog (18.0.0 entries) (joi.dev)
[4] PR for Standard Schema support (#3080) (github.com)

Citations:


🏁 Script executed:

find . -name "app.ts" -o -name "app.js" | head -20

Repository: goodthingsdev/disasters-api

Length of output: 79


🏁 Script executed:

cat -n ./app.ts | head -100

Repository: goodthingsdev/disasters-api

Length of output: 4462


🏁 Script executed:

rg "\.uri\(" ./app.ts

Repository: goodthingsdev/disasters-api

Length of output: 117


Review joi v18 breaking changes—not specifically .uri() but Node.js and TypeScript requirements.

joi 18.0.2 exists (released Nov 2025). However, the changelog shows no breaking changes to .uri() validation. The actual breaking changes in v18 are:

  • Node.js >= 20 required (dropped < 20 support)
  • TypeScript array typing: Joi.array<T>().items(...)Joi.array().items<T>(...)
  • Dependency upgrades: @hapi modules updated (e.g., address v4→v5, hoek v10→v11)

Verify that the project's Node.js version meets the >= 20 requirement and update any TypeScript array patterns if used.

🤖 Prompt for AI Agents
In `@package.json` at line 43, The project added "joi" v18 in package.json which
requires Node.js >=20 and changes TypeScript generics for arrays; update
package.json (the "joi" dependency) to ensure the Node engine requirement is
declared (add or bump the "engines" field to ">=20" and/or update CI/Dockerfiles
to use Node 20+), and audit TypeScript usages of Joi array generics—replace
occurrences of Joi.array<T>().items(...) with the new pattern
Joi.array().items<T>(...) and update any related imports/usages referencing Joi
array typing to match v18 typings.

Comment on lines +4 to +8
const adapter = new PrismaPg({
connectionString: process.env.POSTGRES_URI,
});

export const prisma = new PrismaClient({ adapter });
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

🧩 Analysis chain

🏁 Script executed:

# Check where services/prisma.ts is imported
rg -n "from.*services/prisma|from.*\./prisma|from.*services" --type=ts | grep -i prisma | head -20

Repository: goodthingsdev/disasters-api

Length of output: 136


🏁 Script executed:

# Find and examine app.ts to see the guard at lines 38-40
fd -e ts -e js | xargs grep -l "app\|main" | grep -E "(app|main|index)" | head -5

Repository: goodthingsdev/disasters-api

Length of output: 157


🏁 Script executed:

# Get more context on imports in the project
git ls-files | grep -E "(app|main|index|prisma)" | grep -E "\.(ts|js)$"

Repository: goodthingsdev/disasters-api

Length of output: 216


🏁 Script executed:

# Read app.ts, especially around lines 38-40 to see the guard
cat -n app.ts | head -50

Repository: goodthingsdev/disasters-api

Length of output: 2326


🏁 Script executed:

# Read index.ts to see the entry point and import order
cat -n index.ts

Repository: goodthingsdev/disasters-api

Length of output: 1803


🏁 Script executed:

# Read services/disaster.service.ts to see how prisma is used
head -20 services/disaster.service.ts

Repository: goodthingsdev/disasters-api

Length of output: 903


🏁 Script executed:

# Search for imports of disaster.service
rg -n "disaster.service|from.*services" --type=ts | grep -v node_modules

Repository: goodthingsdev/disasters-api

Length of output: 380


🏁 Script executed:

# Read routes/disasters.js to see if it imports from services
cat routes/disasters.js | head -30

Repository: goodthingsdev/disasters-api

Length of output: 122


🏁 Script executed:

# Read graphql/resolvers.js to see if it imports from services
cat graphql/resolvers.js | head -30

Repository: goodthingsdev/disasters-api

Length of output: 123


🏁 Script executed:

# Read routes/disasters.ts to see the import chain
head -25 routes/disasters.ts

Repository: goodthingsdev/disasters-api

Length of output: 898


🏁 Script executed:

# Read graphql/resolvers.ts to see the import chain
head -25 graphql/resolvers.ts

Repository: goodthingsdev/disasters-api

Length of output: 733


🏁 Script executed:

# Check where routes/disasters.ts is imported in app.ts
rg -n "routes/disasters|from.*routes" app.ts

Repository: goodthingsdev/disasters-api

Length of output: 121


🏁 Script executed:

# Verify the exact import order by checking where app.ts module code runs
cat -n app.ts | sed -n '1,45p'

Repository: goodthingsdev/disasters-api

Length of output: 2087


🏁 Script executed:

# Double-check services/prisma.ts to confirm it executes at module scope
cat -n services/prisma.ts

Repository: goodthingsdev/disasters-api

Length of output: 356


process.env.POSTGRES_URI will be undefined when the adapter is initialized.

This module instantiates PrismaPg and PrismaClient at module scope (lines 4-8). Due to ES module import hoisting, this code executes during the import phase—before the POSTGRES_URI guard in app.ts (lines 38-40) runs. The import chain is: index.ts → app.ts → routes/disasters.ts → disaster.service.ts → prisma.ts. The guard executes only after all imports complete, so the adapter receives undefined as connectionString.

Either lazily initialize the client or add an explicit guard here:

Lazy singleton pattern
 import { PrismaClient } from '@prisma/client';
 import { PrismaPg } from '@prisma/adapter-pg';
 
-const adapter = new PrismaPg({
-  connectionString: process.env.POSTGRES_URI,
-});
-
-export const prisma = new PrismaClient({ adapter });
+let _prisma: PrismaClient | undefined;
+
+export function getPrisma(): PrismaClient {
+  if (!_prisma) {
+    if (!process.env.POSTGRES_URI) {
+      throw new Error('POSTGRES_URI must be set before initializing Prisma');
+    }
+    const adapter = new PrismaPg({
+      connectionString: process.env.POSTGRES_URI,
+    });
+    _prisma = new PrismaClient({ adapter });
+  }
+  return _prisma;
+}
📝 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 adapter = new PrismaPg({
connectionString: process.env.POSTGRES_URI,
});
export const prisma = new PrismaClient({ adapter });
import { PrismaClient } from '@prisma/client';
import { PrismaPg } from '@prisma/adapter-pg';
let _prisma: PrismaClient | undefined;
export function getPrisma(): PrismaClient {
if (!_prisma) {
if (!process.env.POSTGRES_URI) {
throw new Error('POSTGRES_URI must be set before initializing Prisma');
}
const adapter = new PrismaPg({
connectionString: process.env.POSTGRES_URI,
});
_prisma = new PrismaClient({ adapter });
}
return _prisma;
}
🤖 Prompt for AI Agents
In `@services/prisma.ts` around lines 4 - 8, The PrismaPg adapter is being
constructed at module scope using process.env.POSTGRES_URI which may be
undefined due to import ordering; change prisma.ts to lazily initialize the
adapter and client (implement a singleton getter like getPrisma() that
constructs new PrismaPg and new PrismaClient only on first call) or add an
explicit guard at the top of prisma.ts that throws or reads
process.env.POSTGRES_URI before creating PrismaPg; reference the existing
symbols PrismaPg, PrismaClient and the exported prisma instance (replace direct
export with a getter or factory) so callers use getPrisma()/prisma() after
POSTGRES_URI is validated in app.ts.

@lukesmmr lukesmmr requested a review from caspear February 8, 2026 11:58
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