feat(mcp-server): add version management and release flow#1428
feat(mcp-server): add version management and release flow#1428douenergy merged 4 commits intoCanner:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR wires MCP server versioning through CI and image builds: workflows resolve and propagate Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CI as CI Workflow
participant Docker as Docker Build
participant Ibis as ibis-server Image
participant MCP as mcp-server (pyproject / runtime)
CI->>CI: Resolve tag / derive `mcp_version` (DOCKER_IMAGE_TAG_NAME / REF_NAME / prepare-version)
CI->>Docker: docker build-push --build-arg MCP_SERVER_VERSION= mcp_version
Docker->>Ibis: COPY pyproject.toml from mcp-server stage
Docker->>Ibis: If MCP_SERVER_VERSION set -> sed update /mcp-server/pyproject.toml
Docker->>Ibis: Add LABEL org.opencontainers.image.mcp-server.version
Ibis->>MCP: Start mcp-server (pyproject version embedded)
MCP->>CI: get_version() returns resolved version (importlib.metadata or pyproject)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
- Sync mcp-server version with ibis-server on stable release (amend commit) - Pass MCP_SERVER_VERSION build-arg to Docker image for both workflows - Update Dockerfile to stamp pyproject.toml with build-arg version at build time - Add OCI label org.opencontainers.image.mcp-server.version to ibis image - Add get_version MCP tool to report current mcp-server version at runtime - Bump mcp-server version to 0.23.0 to align with current ibis-server - Update mcp-server description in pyproject.toml Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
d27854d to
029f39e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/build-image.yml (1)
109-117: Quote variables to prevent word splitting.The
versionvariable assignment should use quotes to handle tag names containing spaces or special characters safely.Suggested fix
- name: Resolve MCP server version id: mcp_version run: | if [ -n "${{ github.event.inputs.docker_image_tag_name }}" ]; then - version=${{ github.event.inputs.docker_image_tag_name }} + version="${{ github.event.inputs.docker_image_tag_name }}" else version=sha-$(git log -1 --pretty=%h) fi echo "value=$version" >> $GITHUB_OUTPUT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-image.yml around lines 109 - 117, The shell step "Resolve MCP server version" assigns the version variable without quotes which can cause word-splitting for tag names with spaces or special characters; update the assignment logic in the mcp_version step to quote expansions (e.g., assign version="${{ github.event.inputs.docker_image_tag_name }}" and version="sha-$(git log -1 --pretty=%h)" where appropriate) and ensure the echo to $GITHUB_OUTPUT also quotes or properly formats the value to preserve the full string.mcp-server/pyproject.toml (1)
13-13:ruffshould be a dev dependency, not a runtime dependency.
ruffis a linter/formatter tool and should not be included in runtime dependencies. It unnecessarily increases the production image size and attack surface.Suggested change
Move
ruffto a[project.optional-dependencies]or[tool.uv]dev group:dependencies = [ "httpx>=0.28.1", "mcp[cli]>=1.19.0", "orjson>=3.11.7", "pydantic>=2.10.6", "python-dotenv>=1.0.1", - "ruff>=0.11.0", ] + +[project.optional-dependencies] +dev = [ + "ruff>=0.11.0", +]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcp-server/pyproject.toml` at line 13, The dependency "ruff>=0.11.0" in pyproject.toml is a development tool and should not be a runtime dependency; remove the "ruff>=0.11.0" entry from the main dependencies and add it to a dev group such as [project.optional-dependencies] (e.g. a "dev" or "tools" extra) or your project’s dev tool section (e.g. a [tool.uv] or equivalent dev group) so it is installed only for development and not in production images; update any CI/build configuration that installs dependencies to include the chosen dev group when needed.ibis-server/Dockerfile (1)
126-126: OCI label will be empty whenMCP_SERVER_VERSIONis not provided.When building without the
MCP_SERVER_VERSIONbuild-arg, this label will have an empty value. Consider documenting this behavior or providing a default value for clarity in image inspection.Optional: Provide a default or omit the label conditionally
If you want a meaningful default:
-ARG MCP_SERVER_VERSION +ARG MCP_SERVER_VERSION=unknownOr document in the PR/README that omitting the arg results in an empty label.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ibis-server/Dockerfile` at line 126, The OCI label org.opencontainers.image.mcp-server.version is left empty when the build-arg MCP_SERVER_VERSION is not provided; update the Dockerfile to ensure a meaningful value by either declaring a default ARG (e.g., ARG MCP_SERVER_VERSION=<default>) before the LABEL or conditionally emitting the label only when MCP_SERVER_VERSION is set, and/or add a note in the README/PR that omitting MCP_SERVER_VERSION results in an empty label so users know expected behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/stable-release.yml:
- Around line 55-61: The sed invocation using the unquoted variable version
(from steps.next_version.outputs.value) can allow shell/sed injection; update
the pipeline step that sets and uses version so the sed replacement is safe by
quoting/escaping the variable and switching to a safe delimiter (or escaping
occurrences of the delimiter and ampersands in the value) before running sed on
mcp-server/pyproject.toml; specifically modify the line that builds/uses the
version variable and the sed command so it safely handles characters like / and
& (or validate specific_version against a semver pattern) to prevent injection.
In `@mcp-server/app/wren.py`:
- Around line 707-726: The fallback parsing in get_version() can raise uncaught
exceptions (FileNotFoundError, IndexError, UnicodeDecodeError) because it's
outside the except block; wrap the entire fallback logic (the pyproject path
construction, opening the file, iterating lines and extracting the version) in
its own try/except that catches relevant exceptions and returns "unknown" on
failure, e.g. surround the code that uses the pyproject variable and the for
loop with try/except (catching FileNotFoundError, IndexError, Exception) so
get_version() always returns a string even if the file is missing or malformed.
---
Nitpick comments:
In @.github/workflows/build-image.yml:
- Around line 109-117: The shell step "Resolve MCP server version" assigns the
version variable without quotes which can cause word-splitting for tag names
with spaces or special characters; update the assignment logic in the
mcp_version step to quote expansions (e.g., assign version="${{
github.event.inputs.docker_image_tag_name }}" and version="sha-$(git log -1
--pretty=%h)" where appropriate) and ensure the echo to $GITHUB_OUTPUT also
quotes or properly formats the value to preserve the full string.
In `@ibis-server/Dockerfile`:
- Line 126: The OCI label org.opencontainers.image.mcp-server.version is left
empty when the build-arg MCP_SERVER_VERSION is not provided; update the
Dockerfile to ensure a meaningful value by either declaring a default ARG (e.g.,
ARG MCP_SERVER_VERSION=<default>) before the LABEL or conditionally emitting the
label only when MCP_SERVER_VERSION is set, and/or add a note in the README/PR
that omitting MCP_SERVER_VERSION results in an empty label so users know
expected behavior.
In `@mcp-server/pyproject.toml`:
- Line 13: The dependency "ruff>=0.11.0" in pyproject.toml is a development tool
and should not be a runtime dependency; remove the "ruff>=0.11.0" entry from the
main dependencies and add it to a dev group such as
[project.optional-dependencies] (e.g. a "dev" or "tools" extra) or your
project’s dev tool section (e.g. a [tool.uv] or equivalent dev group) so it is
installed only for development and not in production images; update any CI/build
configuration that installs dependencies to include the chosen dev group when
needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c76bb19-3bc4-49a7-a3d7-dd439abd1a4c
📒 Files selected for processing (5)
.github/workflows/build-image.yml.github/workflows/stable-release.ymlibis-server/Dockerfilemcp-server/app/wren.pymcp-server/pyproject.toml
- Quote version variable and escape sed delimiters in stable-release.yml to prevent shell injection - Quote version variable in build-image.yml to prevent word splitting - Move ruff to dev optional dependency in pyproject.toml - Set ARG MCP_SERVER_VERSION=unknown as default in Dockerfile - Fix get_version() fallback to use separate try/except to prevent uncaught exceptions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-image.yml:
- Around line 138-139: The build currently recomputes MCP_SERVER_VERSION in
build-ibis-image causing a mismatch with the published tag; instead have
prepare-tag emit a single resolved MCP version output (e.g.,
steps.prepare-tag.outputs.resolved_mcp_version) and use that output in the build
job's build-args (replace MCP_SERVER_VERSION=${{ steps.mcp_version.outputs.value
}} with MCP_SERVER_VERSION=${{ steps.prepare-tag.outputs.resolved_mcp_version
}}), and ensure prepare-tag sets that output when handling both
workflow_dispatch and docker_image_tag_name paths so get_version() and the
published image tag remain identical.
- Around line 109-117: The step with id mcp_version currently interpolates
github.event.inputs.docker_image_tag_name directly into the run block which can
allow output injection; move the input into the step's env: (e.g.,
DOCKER_IMAGE_TAG_NAME) and in the run script validate it (reject empty or
newline-containing values and optionally match an allowed pattern), then set the
output with a safe printf to append "value=..." to $GITHUB_OUTPUT (avoid echo
with untrusted content); update references in the step to use the env var and
ensure validation fails the job if the tag is invalid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2cca83a1-e1bb-48d1-9627-412f81157616
📒 Files selected for processing (5)
.github/workflows/build-image.yml.github/workflows/stable-release.ymlibis-server/Dockerfilemcp-server/app/wren.pymcp-server/pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/stable-release.yml
- mcp-server/app/wren.py
- ibis-server/Dockerfile
- Move mcp_version computation to prepare-tag job as single source of truth - Use env: to isolate docker_image_tag_name input, preventing output injection - Validate tag format with case pattern (allow only [A-Za-z0-9._-]) - Use printf for safe $GITHUB_OUTPUT writing - Use git rev-parse --short HEAD consistently - Align mcp_version with published image tag on workflow_dispatch without tag Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-image.yml:
- Around line 38-47: The current validation for DOCKER_IMAGE_TAG_NAME is too
permissive; update the check around DOCKER_IMAGE_TAG_NAME so it enforces Docker
tag rules: ensure DOCKER_IMAGE_TAG_NAME is non-empty, length <= 128 (use
${`#DOCKER_IMAGE_TAG_NAME`}), does not start with '.' or '-', and only contains
allowed characters [A-Za-z0-9_.-] (use a shell regex or grep -E
'^[A-Za-z0-9_.-]+$'). Replace the existing case block with these checks
(referencing DOCKER_IMAGE_TAG_NAME and tag_name) and print the same error/exit
if any condition fails so invalid tags are rejected early before using REF_NAME
or generating metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9cca7140-4130-4aa4-bd4f-22a3b875c483
📒 Files selected for processing (1)
.github/workflows/build-image.yml
- Replace case pattern with grep -E to properly reject tags starting with '.' or '-' - Add 128-character length limit check per OCI Distribution Spec Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
mcp-serverversion withibis-serveron stable release via--amendcommit instable-release.ymlMCP_SERVER_VERSIONbuild-arg to Docker build in bothstable-release.ymlandbuild-image.yml/mcp-server/pyproject.tomlwith the build-arg version at build timeorg.opencontainers.image.mcp-server.versiontowren-engine-ibisimageget_versionMCP tool to report current mcp-server version at runtimemcp-serverversion to0.23.0to align with current ibis-servermcp-serverdescription inpyproject.tomlVersion resolution
get_versionresultbuild-image.yml(push to main)sha-<short-sha>build-image.yml(manual dispatch with tag)stable-release.yml0.23.1)Test plan
build-image.ymlmanually and verifyget_versionreturns the expected SHA-based versiondocker inspectshowsorg.opencontainers.image.mcp-server.versionlabel on built imagemcp-server/pyproject.tomlversion is bumped in the same commit asibis-server/pyproject.toml🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores