Skip to content

chore: add Docker images, docker-compose, and CI workflow - #38

Merged
sprahasingh merged 1 commit into
mainfrom
chore/docker-ci-deploy
Sep 29, 2026
Merged

sprahasingh merged 1 commit into
mainfrom
chore/docker-ci-deploy

Conversation

@sprahasingh

Copy link
Copy Markdown
Owner

No description provided.

@codelens-gh

codelens-gh Bot commented Sep 29, 2026

Copy link
Copy Markdown

CodeLens Pre-Review Analysis

10 hunks scanned · 50 similar past patterns matched


Finding 1: GitHub Actions uses unpinned action references (checkout@v4, setup-node@v4) (inferred)

Location: api/.dockerignore · line 1
Confidence: 95% | Suggested check: Pin each action to a specific commit SHA hash instead of a tag

Your code (this PR):

name: CI
on:
  pull_request:
  push:
    branches: [main]
jobs:
  api:
    runs-on: ubuntu-latest
... (37 more lines)

Evidence: Past reviewers flagged unpinned actions as a security issue in similar workflow files

Past reviews that triggered this (2 matches):

Similarity File Reviewer comment Link
89% .github/workflows/run-tests.yml "## zizmor / unpinned action reference: action is not pinned to a hash (required by blank..." view
89% .github/workflows/run-tests.yml "## zizmor / unpinned action reference: action is not pinned to a hash (required by blank..." view

Finding 2: The bcryptjs dependency was downgraded from ^3.0.3 to ^3.0.2 without an explicit justification (inferred)

Location: api/package-lock.json · line 9
Confidence: 78% | Suggested check: Confirm that ^3.0.2 satisfies all security and compatibility requirements and that the lockfile reflects this change

Your code (this PR):

        "bcryptjs": "^3.0.2",
        "express-rate-limit": "^8.2.1",
        "jsonwebtoken": "^9.0.2",

Evidence: Past reviewers warned to double‑check version changes against lockfiles and installed versions

Past reviews that triggered this (1 match):

Similarity File Reviewer comment Link
84% web/package.json "Worth double-checking these resolved versions against what's actually installed (React 19...." view

Finding 3: The express-rate-limit dependency was downgraded from ^8.7.0 to ^8.2.1, which may introduce regressions (inferred)

Location: api/package-lock.json · line 9
Confidence: 75% | Suggested check: Run the test suite and review changelogs to ensure the older version does not break rate‑limiting behavior

Your code (this PR):

        "bcryptjs": "^3.0.2",
        "express-rate-limit": "^8.2.1",
        "jsonwebtoken": "^9.0.2",

Evidence: Similar past comments highlighted the need to verify resolved versions after a downgrade

Past reviews that triggered this (1 match):

Similarity File Reviewer comment Link
84% web/package.json "Worth double-checking these resolved versions against what's actually installed (React 19...." view

Finding 4: The jsonwebtoken dependency was downgraded from ^9.0.3 to ^9.0.2, potentially affecting security patches (inferred)

Location: api/package-lock.json · line 9
Confidence: 77% | Suggested check: Verify that the older version still includes the latest security fixes and that the lockfile is updated accordingly

Your code (this PR):

        "bcryptjs": "^3.0.2",
        "express-rate-limit": "^8.2.1",
        "jsonwebtoken": "^9.0.2",

Evidence: Reviewers previously emphasized checking that version pins align with the lockfile and installed packages

Past reviews that triggered this (1 match):

Similarity File Reviewer comment Link
84% web/package.json "Worth double-checking these resolved versions against what's actually installed (React 19...." view

Finding 5: Downgrading @types/supertest to 6.0.2 while keeping supertest at 7.1.4 may cause type incompatibilities (inferred)

Location: api/package-lock.json · line 28
Confidence: 78% | Suggested check: Run the TypeScript compiler and test suite to ensure the @types/supertest version aligns with the installed supertest version

Your code (this PR):

        "@types/jsonwebtoken": "^9.0.7",
        "@types/supertest": "^6.0.2",
        "supertest": "^7.1.4",

Evidence: Past reviewers warned that version downgrades can break builds, e.g., the comment about a downgrade causing GitHub Actions tests to fail

Past reviews that triggered this (1 match):

Similarity File Reviewer comment Link
86% requirements.txt "This upgrade is failing the GitHub Actions tests. suggestion ruff==0.8.1 Or merge #..." view

Finding 6: Using the floating image tag 'mongo:7' could introduce unexpected breaking changes when the upstream image updates (inferred)

Location: web/.dockerignore · line 1
Confidence: 70% | Suggested check: Pin the MongoDB image to a specific patch version or add a version floor and test compatibility

Your code (this PR):

services:
  mongo:
    image: mongo:7
    command: ["--replSet", "rs0", "--bind_ip_all"]
    volumes:
      - mongo_data:/data/db
    healthcheck:
      test: >
... (30 more lines)

Evidence: Past reviewers flagged version pinning concerns, noting the need for explicit version floors to avoid breakages

Past reviews that triggered this (2 matches):

Similarity File Reviewer comment Link
85% .github/workflows/typecheck.yml "Yeah, that's fair. I started with the minimum because we've generally tested floors but I ..." view
85% .github/workflows/typecheck.yml "Addressed in 4a6d321f1fff6c7cd829cb4cfc60e54ef8a76b4b." view

Finding 7: Hardcoded upstream API URL in proxy_pass (inferred)

Location: web/nginx.conf · line 1
Confidence: 90% | Suggested check: Verify that the upstream API address (http://api:4000) is configurable via an environment variable or similar mechanism

Your code (this PR):

server {
  listen 80;
  server_name _;
  root /usr/share/nginx/html;
  index index.html;
  location /api/ {
    resolver 127.0.0.11 valid=10s;
    set $upstream_api http://api:4000;
... (10 more lines)

Evidence: Past review comment [0] flagged a hardcoded proxy target and suggested using an environment variable

Past reviews that triggered this (1 match):

Similarity File Reviewer comment Link
86% web/vite.config.ts "Hardcoded proxy target. Fine for now since it matches the API's dev port, but if this ever..." view

This analysis was generated automatically by CodeLens based on historical review patterns.

Comment thread api/Dockerfile

WORKDIR /app

COPY package.json package-lock.json ./

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Lockfiles copied before the rest of the source in both stages, deliberately, so the npm ci layer only invalidates when dependencies actually change, not on every code edit.

Comment thread api/Dockerfile

FROM node:24-alpine

ENV NODE_ENV=production

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

NODE_ENV set to production before the second npm ci runs, not after, so the install itself happens in the same mode the app will actually run in.

Comment thread api/Dockerfile
WORKDIR /app

COPY package.json package-lock.json ./
RUN npm ci --omit=dev

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

--omit=dev here versus a plain npm ci in the builder stage above. The builder needs devDependencies to run tsc; the runtime image never should, that's exactly what keeps pino-pretty and the rest of the dev toolchain out of the shipped image.

Comment thread api/Dockerfile
COPY package.json package-lock.json ./
RUN npm ci --omit=dev

COPY --from=builder --chown=node:node /app/dist ./dist

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

--chown=node:node set at copy time rather than a separate RUN chown afterward, since COPY always runs as root regardless of the USER instruction below it.

Comment thread api/Dockerfile

COPY --from=builder --chown=node:node /app/dist ./dist

USER node

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Switches to the node user, which official Node images already ship pre-created specifically for this, rather than creating a new user manually.

Comment thread docker-compose.yml
env_file:
- ./api/.env
environment:
NODE_ENV: production

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

NODE_ENV explicitly set to production here rather than left to inherit from the api container's own .env file, which has NODE_ENV=development for local dev. Confirmed this matters concretely, the container crashes on boot if this is left as development, since pino-pretty is a dev dependency that's deliberately absent from the production image.

Comment thread docker-compose.yml
CLIENT_ORIGIN: http://localhost:5173
ports:
- "4000:4000"
depends_on:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

depends_on with condition service_healthy specifically, not the bare form. The bare form only waits for the mongo container to start, which happens almost immediately, well before the replica set is actually usable.

Comment thread .github/workflows/ci.yml
defaults:
run:
working-directory: api
env:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

These env values are placeholders, not secrets, and that's deliberate. The test suite never actually connects to MONGODB_URI's value, it spins up its own in-memory replica set instead, these only exist so config/env.ts's Zod validation doesn't reject an empty process.env and exit before a single test runs. Confirmed this exact scenario locally by running the full suite with no .env file present and only these values set.

Comment thread .github/workflows/ci.yml

- uses: actions/setup-node@v4
with:
node-version-file: ".nvmrc"

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

node-version-file pointed at .nvmrc directly rather than a hardcoded version number, so CI and local dev are guaranteed to run the identical Node version instead of two version strings that can quietly drift apart.

Comment thread .github/workflows/ci.yml
cache-dependency-path: api/package-lock.json

- run: npm ci
- run: npm run lint

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Lint before typecheck before test, cheapest and fastest check first so an obvious style failure doesn't wait behind an eight-second test run to report.

@sprahasingh
sprahasingh merged commit 80736d6 into main Sep 29, 2026
2 checks passed
sprahasingh added a commit that referenced this pull request Oct 4, 2026
chore: add Docker images, docker-compose, and CI workflow
sprahasingh added a commit that referenced this pull request Oct 4, 2026
chore: add Docker images, docker-compose, and CI workflow
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