fix: Docker

[khandrew1]

• Fixes our Docker container

[cursor]

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on January 11.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

[coderabbitai]

Walkthrough

The Dockerfile has been restructured to consolidate multiple build stages into a streamlined pipeline. The separate client, SDK, and server builder stages are unified into a single builder stage, while the dependencies stage remains distinct. The production stage now explicitly manages artifact copying with ownership flags and sets up the non-root user earlier in the build process. The runtime command has been updated from a direct Node.js invocation to npm start. Layer optimization has been improved through consolidated RUN commands and strategic file ordering for better caching efficiency, while dumb-init and healthcheck provisions remain intact.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

Dockerfile (1)

64-65: Minor: Consolidate SDK artifact copying for clarity.

Lines 64–65 copy SDK artifacts in two separate COPY commands. This works but is verbose. Consider consolidating into a single directory copy for cleaner layering.

Optionally consolidate:

-# Copy SDK dist and package.json (required at runtime)
-COPY --from=builder --chown=mcpjam:nodejs /app/sdk/dist ./sdk/dist
-COPY --from=builder --chown=mcpjam:nodejs /app/sdk/package.json ./sdk/package.json
+# Copy SDK dist and package.json (required at runtime)
+COPY --from=builder --chown=mcpjam:nodejs /app/sdk ./sdk

This assumes sdk/ in the builder image contains only dist/ and package.json (no source code or node_modules). Verify before applying.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e276ad7 and b11d71f.

⛔ Files ignored due to path filters (1)

• sdk/package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (1)

• Dockerfile (2 hunks)

🧰 Additional context used

🔍 Remote MCP

I'll search for relevant context about the Docker optimization patterns and best practices mentioned in this PR to help with the review.## Summary of Additional Context for PR #1077

Based on my research of Docker best practices and optimization patterns, here is relevant context for reviewing the Dockerfile changes:

Multi-Stage Build Optimization

Multi-stage builds create more efficient and smaller final images by separating different build stages within a single Dockerfile. The PR's consolidation from separate client/server/sdk builder stages into a unified builder stage aligns with this pattern. "Cherry-picking" static build artifacts from potentially massive build images is important for final image optimization.

COPY --chown Usage

The PR's addition of COPY --chown during artifact copying is a best practice with significant impact. Using the --chown option of Docker's COPY command reduces build time versus doing it manually. This is critical because when chowning files modifies them, the file counts in both the layer where copied and the chown layer, doubling image size. By combining the copy operation and ownership assignment in a single step, COPY --chown reduces layers created during build and keeps final image size smaller.

CMD Changes (npm start vs. node)

The PR's switch from node dist/server index.js to npm start deserves scrutiny. It is recommended to bake the entrypoint directly into the image, as using npm scripts reduces the number of processes running and causes exit signals like SIGTERM and SIGINT to be received by Node.js instead of npm swallowing them. The PR description notes this aligns with "GitHub workflow expectations," but this may conflict with signal handling best practices unless the npm start script is properly configured.

Layer and Cache Optimization

By copying only package.json files first and installing dependencies before copying the rest of the code, dependency installation layers can be reused even when source code changes. The PR's mention of "reworked file inclusion order" suggests attention to cache optimization, which is important to verify in the actual diff.

Production Dependencies

Multi-stage builds keep final images lean by including only required dependencies and enhance security by running as a non-root user. The PR's focus on explicit ownership setup is aligned with these security practices.,,

🔇 Additional comments (4)

Dockerfile (4)

4-15: Dependencies stage is well-structured and optimized.

Consolidating both root and SDK dependency installation into a single RUN reduces layer count, and the npm ci approach with pinned versions ensures reproducibility.

---

17-38: Builder stage is correct and caching-aware.

Inheriting from deps provides access to pre-installed dependencies, and file ordering prioritizes infrequently-changing files (configs) before source code. The consolidated RUN statement is efficient.

---

93-97: Verify signal handling with npm start and dumb-init.

The switch from node dist/server index.js to npm start introduces an extra process layer. While dumb-init improves signal forwarding, npm itself may not reliably propagate signals (SIGTERM, SIGINT) to the Node.js process. This can prevent graceful shutdown. Ensure that: (1) your npm start script uses exec to replace the npm process with Node.js, or (2) you verify that signal handling works as expected in production.

Does your npm start script (likely in package.json) use exec node ... to ensure signals reach Node.js? Or do you have another mechanism to verify signals are properly forwarded?

---

74-77: Clarify necessity of shared/ and .env.production in production image.

Lines 74 and 77 copy shared/ (TypeScript source files) and .env.production to production. These warrant clarification:

1. shared/: Contains source .ts files that are unlikely needed at runtime. Verify whether your npm start command or bin/ scripts actually require these. If not, removing them would reduce image size by ~1–5 MB.
2. .env.production: Typically .gitignored for secrets. Confirm this file exists in the repository and only contains non-sensitive, runtime-safe configuration values.

Can you clarify whether shared/ and .env.production are required at runtime, and confirm that .env.production is in the repository and safe for production deployment?

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Critical: SDK package-lock.json missing in production image.

Line 65 copies sdk/package.json but not sdk/package-lock.json. If any npm commands run at runtime (or during container initialization), dependency versions cannot be guaranteed. This breaks reproducibility for the SDK layer.

Add the missing package-lock.json:

 # Copy SDK dist and package.json (required at runtime)
 COPY --from=builder --chown=mcpjam:nodejs /app/sdk/dist ./sdk/dist
 COPY --from=builder --chown=mcpjam:nodejs /app/sdk/package.json ./sdk/package.json
+COPY --from=builder --chown=mcpjam:nodejs /app/sdk/package-lock.json ./sdk/package-lock.json

🤖 Prompt for AI Agents

In Dockerfile around lines 60 to 77, the SDK's package-lock.json is not copied
into the production image which risks non-reproducible SDK installs; add a COPY
--chown=mcpjam:nodejs /app/sdk/package-lock.json ./sdk/package-lock.json
alongside the existing sdk/package.json COPY so the lockfile is present in the
runtime image and preserves exact SDK dependency versions.