Skip to content

Replace npm ci command with npm run build in Dockerfile#2965

Closed
bruno-t-cardoso wants to merge 1 commit intomodelcontextprotocol:mainfrom
bruno-t-cardoso:main
Closed

Replace npm ci command with npm run build in Dockerfile#2965
bruno-t-cardoso wants to merge 1 commit intomodelcontextprotocol:mainfrom
bruno-t-cardoso:main

Conversation

@bruno-t-cardoso
Copy link

@bruno-t-cardoso bruno-t-cardoso commented Nov 7, 2025

Description

Fixed the Dockerfile for the sequential-thinking MCP server to properly compile TypeScript code during the Docker build process.

Server Details

  • Server: sequential-thinking
  • Changes to: Dockerfile build process

Motivation and Context

The original Dockerfile had a bug that prevented the TypeScript code from being compiled to JavaScript during the Docker build. Line 10 was running npm ci --ignore-scripts --omit-dev, which skipped the prepare script that triggers the build. This resulted in a Docker image that appeared to build successfully but was non-functional because the dist directory was missing.

This fix ensures the TypeScript code is properly compiled by explicitly running npm run build after installing dependencies.

How Has This Been Tested?

  • Built the Docker image using: docker build -t mcp/sequentialthinking -f src/sequentialthinking/Dockerfile .
  • Tested MCP protocol communication with: echo '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"test","version":"1.0"}}}' | docker run --rm -i mcp/sequentialthinking
  • Verified the server responds correctly to initialize and tools/list requests
  • Tested with Augment Code MCP client configuration

Breaking Changes

No breaking changes. Users with the previous broken Docker image will need to rebuild it, but the configuration remains the same.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Protocol Documentation
  • My changes follows MCP security best practices
  • I have updated the server's README accordingly
  • I have tested this with an LLM client
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have documented all environment variables and configuration options

Additional context

Before: The Dockerfile ran npm ci --ignore-scripts --omit-dev which prevented the TypeScript compilation.

After: The Dockerfile now explicitly runs npm run build to compile TypeScript to JavaScript, ensuring the dist directory is created and the Docker image is functional.

This fix makes the Docker installation method documented in the README actually work as intended.

vlordier added a commit to vlordier/servers that referenced this pull request Feb 12, 2026
The Dockerfile was missing the build step, causing Docker images to be
non-functional because the dist/ directory was never created.

Changes:
- Replace `npm ci --ignore-scripts --omit-dev` with `npm run build`
- This ensures TypeScript is compiled to JavaScript during build
- The dist/ directory is now properly created and copied to release image

The build process now works correctly:
1. Install all dependencies (including devDependencies for build)
2. Run `npm run build` to compile TypeScript -> JavaScript
3. Copy built dist/ directory to release image
4. Install only production dependencies in release image

Fixes the Docker installation method documented in README.

Credits: Based on the fix from PR modelcontextprotocol#2965 by @bruno-t-cardoso

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@olaservo
Copy link
Member

olaservo commented Mar 1, 2026

Review: The Dockerfile is not broken on main

I built the current main branch Dockerfile and tested it end-to-end. The image builds successfully and responds correctly to MCP initialize requests.

What actually happens during the build

Line 8 (npm install) triggers the prepare lifecycle script, which runs npm run buildtsc && shx chmod +x dist/*.js. This successfully compiles TypeScript and produces the dist/ directory:

> @modelcontextprotocol/server-sequential-thinking@0.6.2 prepare
> npm run build

> @modelcontextprotocol/server-sequential-thinking@0.6.2 build
> tsc && shx chmod +x dist/*.js

Line 10 (npm ci --ignore-scripts --omit-dev) then prunes node_modules to production-only dependencies. npm ci only deletes node_modules/ — it does not touch dist/. So dist/ survives this step and is correctly copied into the release stage.

Test

$ docker build -t mcp/sequentialthinking -f src/sequentialthinking/Dockerfile .
# builds successfully

$ echo '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"test","version":"1.0"}}}' \
  | docker run --rm -i mcp/sequentialthinking

Sequential Thinking MCP Server running on stdio
{"result":{"protocolVersion":"2024-11-05","capabilities":{"tools":{"listChanged":true}},"serverInfo":{"name":"sequential-thinking-server","version":"0.2.0"}},"jsonrpc":"2.0","id":1}

Conclusion

This PR replaces a working production-dependency pruning step with a redundant explicit npm run build. The prepare script already handles compilation during npm install. The issue you encountered may have been caused by a stale Docker BuildKit cache — try docker build --no-cache if you run into it again.

I'd recommend closing this PR as the current Dockerfile works as intended.


Disclosure: This review was prepared with assistance from Claude Code (claude-opus-4-6).

Copy link
Member

@olaservo olaservo left a comment

Choose a reason for hiding this comment

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

Hi @bruno-t-cardoso - I wasn't able to repro this locally and had Claude post some findings in a comment on the PR.

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.

2 participants