Skip to content

feat: add Docker deployment for MCP server - #52

Closed
ddetleffsen wants to merge 6 commits into
codemcp:mainfrom
ddetleffsen:feat/docker-deployment
Closed

feat: add Docker deployment for MCP server#52
ddetleffsen wants to merge 6 commits into
codemcp:mainfrom
ddetleffsen:feat/docker-deployment

Conversation

@ddetleffsen

Copy link
Copy Markdown

Multi-stage Dockerfile (pnpm/turbo build -> node:22-alpine runtime) plus an entrypoint that lazily initializes docsets from config.yaml and starts the stdio MCP server. Docsets are mounted via volumes; symlinks are created inside the Linux container, avoiding the Windows symlink-permission problem on the host.

Adds Dockerfile, docker-entrypoint.sh, .dockerignore and docs/docker.md; ignores .idea/ and npm lockfiles.

Detlev Detleffsen added 2 commits June 16, 2026 17:24
Multi-stage Dockerfile (pnpm/turbo build -> node:22-alpine runtime) plus an entrypoint that lazily initializes docsets from config.yaml and starts the stdio MCP server. Docsets are mounted via volumes; symlinks are created inside the Linux container, avoiding the Windows symlink-permission problem on the host.

Adds Dockerfile, docker-entrypoint.sh, .dockerignore and docs/docker.md; ignores .idea/ and npm lockfiles.
corepack enable creates symlinks in the root-owned /usr/local/bin. Under
rootless Podman the build container defaults to a non-root uid, so this
failed with EACCES. Docker builds as root by default and was unaffected.

Add explicit USER root to both build and runtime stages before the
corepack/pnpm install steps; the runtime stage still drops to USER node
before the ENTRYPOINT. Mirrors the pattern already used in bruno-cli-mcp.
@mrsimpson

Copy link
Copy Markdown
Collaborator

Review

Great PR — the multi-stage build, rootless Podman fix, and volume-mount approach are all well thought out. Here are some findings:


High

1. Fragile YAML parsing in entrypoint

grep -E '^\s+- id:' "$CONFIG" | sed 's/.*id: *//' | tr -d '\r' | while read -r id; do

This grep/sed approach is fragile:

  • Fails on tabs vs. spaces, inline comments (e.g., - id: my-docs # comment), or other YAML formatting variations
  • Could match false positives if id: appears in a comment or string value
  • No validation that the extracted ID corresponds to a valid docset

Suggestion: Use a proper YAML parser. Since Node.js is available, call it from the entrypoint or add a small CLI subcommand like init-all that handles this in TypeScript.


Medium

2. --prod flag is deprecated in pnpm

RUN pnpm install --frozen-lockfile --prod --ignore-scripts

The --prod flag is deprecated in pnpm v8+ and removed in v9+. Use --production or just omit it (the lockfile already excludes dev deps).

3. Confusing stdout/stderr redirection

node /app/packages/cli/dist/index.js init "$id" 1>&2 2>&1 || true

The 1>&2 2>&1 double redirect is hard to read. Use the clearer POSIX syntax:

node /app/packages/cli/dist/index.js init "$id" &>/dev/stderr || true

4. Silent init failures

The || true means a failed init is completely silent — the server starts anyway with a broken docset. Track failures and report them:

FAILED=0
while read -r id; do
  if ! node /app/packages/cli/dist/index.js init "$id" &>/dev/stderr; then
    echo "FAILED to init docset: $id" >&2
    FAILED=1
  fi
done
if [ "$FAILED" -eq 1 ]; then
  echo "WARNING: Some docsets failed to initialize" >&2
fi

5. Redundancy: config.yaml vs. Docker volume mounts

Every local_folder path in config.yaml must also be manually added as a -v flag in the docker run command. The config and the Docker command are two sources of truth that must stay in sync.

Follow-up suggestion: wrapper script

A wrapper script (e.g., run.sh) that reads config.yaml, extracts local_folder paths, resolves them, and auto-generates the docker run command with volume mounts would eliminate this redundancy entirely:

./run.sh                          # runs with all docsets
./run.sh my-docs                  # runs only a specific docset

The script could:

  • Parse the config (using a Node.js helper for proper YAML parsing)
  • Resolve relative paths against the config directory
  • Generate -v flags for each source path (read-only)
  • Mount .knowledge as writable
  • Validate all paths exist before invoking Docker

This would be a nice follow-up to this PR.


Low

6. Verbose COPY statements — Each package.json is copied individually for layer caching. Valid optimization but adds maintenance cost. Consider whether a single COPY . . with .dockerignore is sufficient.

7. Missing HEALTHCHECK — For production deployment, a simple healthcheck would be valuable.

8. .gitignore trailing newline — The file still lacks a final newline after the additions.

ddetleffsen and others added 3 commits June 27, 2026 17:56
The entrypoint grep/sed approach broke on tabs, inline comments, and
other valid YAML formatting. Moving docset discovery to Node.js via a
dedicated `init-all` CLI command uses js-yaml (already a dependency)
for correct config parsing. Failures now cause a hard abort (fast-fail)
instead of a silent || true — MCP server logs are rarely read so fail-
fast makes broken starts visible. Also fixes deprecated --prod flag
(pnpm v9+) and .gitignore missing trailing newline.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- ADR-002: Windows runtime is container-only (Docker/Podman), no native execution
- ADR-003: container image serves only pre-materialized local_folder docsets;
  git_repo/archive are rejected by the entrypoint and materialized on the host
  (loaders stay available in the npm package/CLI)
- docker-deployment-design.md: design rationale, scope narrowed per ADR-003

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ddetleffsen

Copy link
Copy Markdown
Author

PR: feat(docker): ship MCP server as a Docker/Podman image (+ ADR-002/ADR-003)

Summary

Ships @codemcp/knowledge as a self-contained Docker/Podman image, spoken to over
stdio (docker run -i). Multi-stage build (pnpm/turbo → node:22-alpine runtime),
runtime-init of docsets via an idempotent init-all entrypoint, writable .knowledge
volume for the cache.

What's included

  • Dockerfile — multi-stage; USER root for build steps (rootless-Podman friendly),
    drops to USER node (uid 1000) before the entrypoint.
  • docker-entrypoint.shset -e; runs init-all, then execs the MCP server.
  • init-all CLI subcommand — loads config via the real YAML parser, initializes each
    docset idempotently, all output to stderr (R1-clean).
  • .dockerignore, docs/docker.md (operational reference).
  • ADR-002 — Windows runtime is container-only (Linux fs.symlink semantics).
  • ADR-003 — image scope decision (see below).

⚠️ Scope decision (ADR-003) — needs maintainer alignment

The container's reason to exist (ADR-002) is Linux symlink semantics — i.e. the
local_folder path. git_repo/archive sources instead materialize a copy into
.knowledge, which can be done on any host and pulls heavy runtime deps into the image:
a git binary, boot-time network/proxy, and a writable scratch dir (/knowledge/.tmp,
which collides with USER node/uid 1000). One unreachable remote also blocks startup
(fail-fast under set -e).

ADR-003 proposes: the image serves only pre-materialized local_folder docsets
("mount & serve"). The entrypoint rejects git_repo/archive (clear error, exit 1);
the runtime ships without git and does no network fetch at startup. The
git_repo/archive loaders stay available in the npm package / CLI for host-side
materialization — no product/API change.

@mrsimpson — flagging this for your call before we wire up enforcement. The ADR +
design doc are committed here; the init-all guard that enforces it is held back
pending your preference (reject vs. tolerate-with-warning; or keep full support and add
apk add git instead).

Rationale, trade-offs and Pugh matrix: docs/adr/003-container-local-folder-only.md.

Review fixes folded in

  • Replaced fragile grep/sed YAML parsing in the entrypoint with init-all (real parser).
  • Documented the fail-fast startup behaviour as a conscious decision.
  • Documented uid-1000 write requirements on the .knowledge volume (named volume vs.
    bind mount, Podman --userns=keep-id, SELinux :Z/:z).
  • Fixed doc drift (/workspace/knowledge, entrypoint/install reality) across
    docs/docker.md, the design doc, and ADR-002.

Testing

  • pnpm --filter @codemcp/knowledge-cli build — green; init-all bundled.
  • CLI tests — 32/32 green.

Follow-ups (TBD)

  • Enforcement code for ADR-003 (pending maintainer alignment).
  • CI/CD image build (GitHub Actions); registry + version-tag strategy.
  • A targeted test for the init-all source-type guard.

The docker-deployment-design.md draft still carried pre-ADR-003 git/network
assumptions and duplicated operative content from docker.md. Both surfaced as
drift risk: a stale "needs egress to clone git repos" network section and an
acceptance criterion testing in-image git cloning, neither of which the
local_folder-only image does.

- Section 5 (Netzwerk): image needs no network at startup; git-clone egress is
  marked as host/CLI behaviour, not the image.
- Acceptance criterion 4: replaced the in-image git-clone scenario with a
  local_folder/init-all criterion; git/archive materialization is host-side.
- Removed the mcp.json block and the Dockerfile stage diagram (both duplicate
  docs/docker.md), replacing them with pointers; the design doc keeps only the
  rationale (the "why").

docker.md remains the operative reference; the design doc holds the rationale.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mrsimpson

Copy link
Copy Markdown
Collaborator

as discussed: copy (instead of linking) is not a viable option. closing this for now until we find a real DRY approach

@mrsimpson mrsimpson closed this Jul 28, 2026
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