[TOO-518] Enforce semver for MCPApp Versioning (#793)
Here's the PR summary:
---
## Enforce semver validation for `MCPApp` versioning
### Problem
`MCPApp.__init__` accepted any string as `version` with no validation.
Invalid versions like `"1.0.0dev"` or `"latest"` silently propagated to
the Engine, where `compareToolVersions` fell back to lexicographic
`strings.Compare` instead of `semver.Compare` — causing incorrect
ordering (e.g. `1.10.0 < 1.9.0`).
### Solution
Validate and normalize `version` at `MCPApp` instantiation time using
the same acceptance rules as Go's `golang.org/x/mod/semver v0.31.0` (the
exact version used by the Engine).
### Changes
**`arcade_mcp_server/_validation.py`** (new file)
- Shared regex constants: `SEMVER_PATTERN` (semver.org spec),
`SHORT_VERSION_PATTERN`, `MAJOR_ONLY_PATTERN`
**`arcade_mcp_server/mcp_app.py`**
- Added `_validate_version()` mirroring the existing `_validate_name()`
pattern
- Added `version` property + setter (validates on mutation too)
- `__init__` now stores `self._version` via `_validate_version()`
**`arcade_mcp_server/settings.py`**
- Added `@field_validator("version")` on `ServerSettings` — covers the
`MCP_SERVER_VERSION` env var path
- Fixed default from `"0.1.0dev"` → `"0.1.0"` (the old default was
itself invalid)
**`pyproject.toml`** — bumped `arcade-mcp-server` `1.17.4` → `1.17.5`
### Normalization pipeline
All inputs are normalized to canonical `MAJOR.MINOR.PATCH` before
storage:
| Input | Stored as |
|-------|-----------|
| `v1.0.0` | `1.0.0` |
| `1.0` / `v1.0` | `1.0.0` |
| `1` / `v1` | `1.0.0` |
### Verification
Validated against `golang.org/x/mod/semver v0.31.0` (Engine's exact
pinned version) — 40/40 accept/reject cases match. The Engine's own
`store_test.go` uses `"1.0"` and `"1.1"` as `ToolkitVersion` values,
confirming short forms are intentionally supported.
### Breaking change
Any user currently passing a non-semver version string (e.g.
`"1.0.0dev"`, `"latest"`) will get a `ValueError` on upgrade. This is
intentional — those versions were silently causing incorrect tool
ordering in the Engine.
Closes TOO-518
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Introduces stricter version validation/normalization that will raise
errors for previously-accepted non-semver inputs (including via env
vars), which may break existing consumers depending on lax version
strings.
>
> **Overview**
> **Enforces semver for server versioning** across both `MCPApp` and
`ServerSettings`, rejecting invalid strings and normalizing accepted
inputs (e.g., stripping leading `v`, expanding `1`/`1.2` to
`1.0.0`/`1.2.0`).
>
> Adds shared `normalize_version` logic in
`arcade_mcp_server/_validation.py`, updates `MCPApp` to validate on init
and via a new `version` property/setter, and adds a Pydantic `version`
validator so `MCP_SERVER_VERSION` is checked. Defaults are updated from
`0.1.0dev` to `0.1.0`, tests are expanded to cover accept/reject cases,
and the package version is bumped to `1.17.5`.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
2ceabacb25372e67eef9720b901c1ee2b214868f. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
---------
Co-authored-by: Eric Gustin <34000337+EricGustin@users.noreply.github.com>
This commit is contained in:
parent
bf6bfa83f1
commit
9c47f73602
7 changed files with 232 additions and 7 deletions
2
.gitignore
vendored
2
.gitignore
vendored
|
|
@ -1,7 +1,7 @@
|
|||
.DS_Store
|
||||
credentials.yaml
|
||||
docker/credentials.yaml
|
||||
|
||||
.cursor/*
|
||||
*.lock
|
||||
|
||||
# example data
|
||||
|
|
|
|||
41
libs/arcade-mcp-server/arcade_mcp_server/_validation.py
Normal file
41
libs/arcade-mcp-server/arcade_mcp_server/_validation.py
Normal file
|
|
@ -0,0 +1,41 @@
|
|||
"""Shared validation patterns for arcade-mcp-server."""
|
||||
|
||||
import re
|
||||
|
||||
# Official semver.org regex (simplified for Python)
|
||||
# https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string
|
||||
SEMVER_PATTERN = re.compile(
|
||||
r"^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)"
|
||||
r"(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)"
|
||||
r"(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?"
|
||||
r"(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$"
|
||||
)
|
||||
|
||||
# MAJOR.MINOR pattern for normalization to MAJOR.MINOR.0
|
||||
SHORT_VERSION_PATTERN = re.compile(r"^(0|[1-9]\d*)\.(0|[1-9]\d*)$")
|
||||
|
||||
# MAJOR-only pattern for normalization to MAJOR.0.0
|
||||
MAJOR_ONLY_PATTERN = re.compile(r"^(0|[1-9]\d*)$")
|
||||
|
||||
|
||||
def normalize_version(version: str) -> str:
|
||||
"""Normalize and validate a version string to canonical semver.
|
||||
Raises:
|
||||
TypeError: if version is not a string.
|
||||
ValueError: if version is empty or not valid semver after normalization.
|
||||
"""
|
||||
if not isinstance(version, str):
|
||||
raise TypeError("version must be a string")
|
||||
if not version:
|
||||
raise ValueError("version cannot be empty")
|
||||
# Strip optional v prefix
|
||||
if version.startswith("v"):
|
||||
version = version[1:]
|
||||
# Expand short forms to full MAJOR.MINOR.PATCH
|
||||
if MAJOR_ONLY_PATTERN.match(version):
|
||||
version = f"{version}.0.0"
|
||||
elif SHORT_VERSION_PATTERN.match(version):
|
||||
version = f"{version}.0"
|
||||
if not SEMVER_PATTERN.match(version):
|
||||
raise ValueError(version)
|
||||
return version
|
||||
|
|
@ -27,6 +27,7 @@ from arcade_tdk.tool import tool as tool_decorator
|
|||
from loguru import logger
|
||||
from watchfiles import watch
|
||||
|
||||
from arcade_mcp_server._validation import normalize_version
|
||||
from arcade_mcp_server.exceptions import ServerError
|
||||
from arcade_mcp_server.logging_utils import intercept_standard_logging
|
||||
from arcade_mcp_server.resource_server.base import ResourceServerValidator
|
||||
|
|
@ -100,7 +101,7 @@ class MCPApp:
|
|||
**kwargs: Additional server configuration
|
||||
"""
|
||||
self._name = self._validate_name(name)
|
||||
self.version = version
|
||||
self._version = self._validate_version(version)
|
||||
self.title = title or name
|
||||
self.instructions = instructions
|
||||
self.log_level = log_level
|
||||
|
|
@ -120,7 +121,7 @@ class MCPApp:
|
|||
|
||||
server_settings_kwargs = {
|
||||
"name": self._name,
|
||||
"version": self.version,
|
||||
"version": self._version,
|
||||
"title": self.title,
|
||||
}
|
||||
if self.instructions:
|
||||
|
|
@ -174,6 +175,18 @@ class MCPApp:
|
|||
|
||||
return name
|
||||
|
||||
def _validate_version(self, version: str) -> str:
|
||||
"""Validate and normalize version to canonical semver."""
|
||||
try:
|
||||
return normalize_version(version)
|
||||
except TypeError:
|
||||
raise TypeError("MCPApp's version must be a string")
|
||||
except ValueError as e:
|
||||
raise ValueError(
|
||||
f"MCPApp's version must be a valid semver string "
|
||||
f"(e.g., '1.0.0', '1.2.3-beta.1'), got '{e}'"
|
||||
)
|
||||
|
||||
# Properties (exposed below initializer)
|
||||
@property
|
||||
def name(self) -> str:
|
||||
|
|
@ -185,6 +198,16 @@ class MCPApp:
|
|||
"""Set the server name with validation."""
|
||||
self._name = self._validate_name(value)
|
||||
|
||||
@property
|
||||
def version(self) -> str:
|
||||
"""Get the server version."""
|
||||
return self._version
|
||||
|
||||
@version.setter
|
||||
def version(self, value: str) -> None:
|
||||
"""Set the server version with validation."""
|
||||
self._version = self._validate_version(value)
|
||||
|
||||
@property
|
||||
def tools(self) -> _ToolsAPI:
|
||||
"""Runtime and build-time tools API: add/update/remove/list."""
|
||||
|
|
|
|||
|
|
@ -11,6 +11,8 @@ from typing import Any
|
|||
from pydantic import Field, field_validator
|
||||
from pydantic_settings import BaseSettings
|
||||
|
||||
from arcade_mcp_server._validation import normalize_version
|
||||
|
||||
|
||||
def _find_project_root(start_dir: Path) -> Path | None:
|
||||
"""Find the nearest ancestor directory containing pyproject.toml.
|
||||
|
|
@ -156,7 +158,7 @@ class ServerSettings(BaseSettings):
|
|||
description="Server name",
|
||||
)
|
||||
version: str = Field(
|
||||
default="0.1.0dev",
|
||||
default="0.1.0",
|
||||
description="Server version",
|
||||
)
|
||||
title: str | None = Field(
|
||||
|
|
@ -171,6 +173,17 @@ class ServerSettings(BaseSettings):
|
|||
description="Server instructions for clients",
|
||||
)
|
||||
|
||||
@field_validator("version")
|
||||
@classmethod
|
||||
def validate_version(cls, v: str) -> str:
|
||||
"""Validate and normalize version to canonical semver."""
|
||||
try:
|
||||
return normalize_version(v)
|
||||
except (TypeError, ValueError) as e:
|
||||
raise ValueError(
|
||||
f"Server version must be a valid semver string (e.g., '1.0.0'), got '{v}'"
|
||||
) from e
|
||||
|
||||
model_config = {"env_prefix": "MCP_SERVER_"}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ build-backend = "hatchling.build"
|
|||
|
||||
[project]
|
||||
name = "arcade-mcp-server"
|
||||
version = "1.17.4"
|
||||
version = "1.17.5"
|
||||
description = "Model Context Protocol (MCP) server framework for Arcade.dev"
|
||||
readme = "README.md"
|
||||
authors = [{ name = "Arcade.dev" }]
|
||||
|
|
|
|||
|
|
@ -7,11 +7,111 @@ from unittest.mock import Mock, patch
|
|||
|
||||
import pytest
|
||||
from arcade_core.catalog import MaterializedTool
|
||||
|
||||
from arcade_mcp_server import tool
|
||||
from arcade_mcp_server.mcp_app import MCPApp
|
||||
from arcade_mcp_server.server import MCPServer
|
||||
|
||||
|
||||
class TestMCPAppVersionValidation:
|
||||
"""Tests for MCPApp version validation."""
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"version,expected_result",
|
||||
[
|
||||
# Full semver (passthrough)
|
||||
("1.0.0", "1.0.0"),
|
||||
("0.1.0", "0.1.0"),
|
||||
("0.0.0", "0.0.0"),
|
||||
("10.20.30", "10.20.30"),
|
||||
# Pre-release and build metadata
|
||||
("1.2.3-alpha.1", "1.2.3-alpha.1"),
|
||||
("1.2.3+build.456", "1.2.3+build.456"),
|
||||
("1.2.3-beta.1+build.789", "1.2.3-beta.1+build.789"),
|
||||
# Short versions (normalized to MAJOR.MINOR.0)
|
||||
("1.0", "1.0.0"),
|
||||
("0.1", "0.1.0"),
|
||||
("2.5", "2.5.0"),
|
||||
("10.20", "10.20.0"),
|
||||
# Major-only versions (normalized to MAJOR.0.0)
|
||||
("1", "1.0.0"),
|
||||
("0", "0.0.0"),
|
||||
("10", "10.0.0"),
|
||||
# v-prefixed versions (normalized by stripping v)
|
||||
("v1.0.0", "1.0.0"),
|
||||
("v0.1.0", "0.1.0"),
|
||||
("v1.2.3-alpha.1", "1.2.3-alpha.1"),
|
||||
("v1.0", "1.0.0"),
|
||||
("v2.5", "2.5.0"),
|
||||
# v-prefixed major-only
|
||||
("v1", "1.0.0"),
|
||||
("v0", "0.0.0"),
|
||||
("v10", "10.0.0"),
|
||||
],
|
||||
)
|
||||
def test_validate_version_valid_versions(self, version: str, expected_result: str) -> None:
|
||||
"""Test _validate_version with valid semver strings."""
|
||||
app = MCPApp(name="TestApp", version="1.0.0")
|
||||
result = app._validate_version(version)
|
||||
assert result == expected_result
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"version,expected_error",
|
||||
[
|
||||
("", ValueError),
|
||||
(None, TypeError),
|
||||
(123, TypeError),
|
||||
([], TypeError),
|
||||
({}, TypeError),
|
||||
("1.0.0.0", ValueError), # too many components
|
||||
("1.0.0dev", ValueError), # PEP 440 dev (not semver)
|
||||
("1.0.0a1", ValueError), # PEP 440 alpha (not semver)
|
||||
("1.0.0.post1", ValueError), # PEP 440 post (not semver)
|
||||
("not_a_version", ValueError), # garbage
|
||||
("latest", ValueError), # word
|
||||
(" 1.0.0", ValueError), # leading space
|
||||
("1.0.0 ", ValueError), # trailing space
|
||||
("01.0.0", ValueError), # leading zero
|
||||
],
|
||||
)
|
||||
def test_validate_version_invalid_versions(
|
||||
self, version: object, expected_error: type[Exception]
|
||||
) -> None:
|
||||
"""Test _validate_version rejects invalid versions."""
|
||||
app = MCPApp(name="TestApp", version="1.0.0")
|
||||
with pytest.raises(expected_error):
|
||||
app._validate_version(version) # type: ignore[arg-type]
|
||||
|
||||
def test_mcp_app_rejects_invalid_version_at_init(self) -> None:
|
||||
"""Test MCPApp raises at instantiation for invalid version."""
|
||||
with pytest.raises(ValueError, match="semver"):
|
||||
MCPApp(name="TestApp", version="not-valid")
|
||||
|
||||
def test_mcp_app_rejects_invalid_version_via_setter(self) -> None:
|
||||
"""Test MCPApp version setter validates and raises for invalid version."""
|
||||
app = MCPApp(name="TestApp", version="1.0.0")
|
||||
with pytest.raises(ValueError, match="semver"):
|
||||
app.version = "bad"
|
||||
|
||||
def test_mcp_app_v_prefix_normalized(self) -> None:
|
||||
"""Test v prefix is stripped and version is normalized."""
|
||||
app = MCPApp(name="TestApp", version="1.0.0")
|
||||
assert app._validate_version("v1.0.0") == "1.0.0"
|
||||
assert app._validate_version("v1.0") == "1.0.0"
|
||||
assert app._validate_version("v2.5") == "2.5.0"
|
||||
assert app._validate_version("v1") == "1.0.0"
|
||||
|
||||
def test_multi_digit_versions_accepted(self) -> None:
|
||||
"""Test versions like 1.10.0 are accepted."""
|
||||
app = MCPApp(name="TestApp", version="1.10.0")
|
||||
assert app.version == "1.10.0"
|
||||
app2 = MCPApp(name="TestApp", version="1.9.0")
|
||||
assert app2.version == "1.9.0"
|
||||
# 1.10.0 > 1.9.0 in semver; lexicographic would wrongly give 1.10.0 < 1.9.0
|
||||
assert app._validate_version("1.10.0") == "1.10.0"
|
||||
assert app._validate_version("1.9.0") == "1.9.0"
|
||||
|
||||
|
||||
class TestMCPApp:
|
||||
"""Test MCPApp class."""
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,8 @@
|
|||
"""Tests for MCP Settings."""
|
||||
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
|
||||
from arcade_mcp_server.settings import MCPSettings, ServerSettings
|
||||
|
||||
|
||||
|
|
@ -11,7 +14,7 @@ class TestServerSettings:
|
|||
settings = ServerSettings()
|
||||
|
||||
assert settings.name == "ArcadeMCP"
|
||||
assert settings.version == "0.1.0dev"
|
||||
assert settings.version == "0.1.0"
|
||||
assert settings.title == "ArcadeMCP"
|
||||
assert settings.instructions is not None
|
||||
assert "available tools" in settings.instructions.lower()
|
||||
|
|
@ -51,7 +54,7 @@ class TestMCPSettings:
|
|||
settings = MCPSettings()
|
||||
|
||||
assert settings.server.name == "ArcadeMCP"
|
||||
assert settings.server.version == "0.1.0dev"
|
||||
assert settings.server.version == "0.1.0"
|
||||
assert settings.server.title == "ArcadeMCP"
|
||||
assert settings.server.instructions is not None
|
||||
|
||||
|
|
@ -97,3 +100,48 @@ class TestServerSettingsTitleDefault:
|
|||
"""Test that the title field default is 'ArcadeMCP'."""
|
||||
field_info = ServerSettings.model_fields["title"]
|
||||
assert field_info.default == "ArcadeMCP"
|
||||
|
||||
|
||||
class TestServerSettingsVersionValidation:
|
||||
"""Tests for ServerSettings version validation (semver enforcement)."""
|
||||
|
||||
def test_server_settings_rejects_invalid_version(self) -> None:
|
||||
"""Test ServerSettings raises ValidationError for invalid version."""
|
||||
with pytest.raises(ValidationError, match="semver"):
|
||||
ServerSettings(version="bad")
|
||||
|
||||
def test_server_settings_accepts_valid_semver(self) -> None:
|
||||
"""Test ServerSettings accepts valid semver."""
|
||||
settings = ServerSettings(version="1.2.3-alpha.1+build.456")
|
||||
assert settings.version == "1.2.3-alpha.1+build.456"
|
||||
|
||||
def test_server_settings_normalizes_short_version(self) -> None:
|
||||
"""Test ServerSettings normalizes MAJOR.MINOR to MAJOR.MINOR.0."""
|
||||
settings = ServerSettings(version="1.0")
|
||||
assert settings.version == "1.0.0"
|
||||
|
||||
def test_server_settings_normalizes_v_prefix(self) -> None:
|
||||
"""Test ServerSettings strips v prefix and normalizes the version."""
|
||||
settings = ServerSettings(version="v1.0.0")
|
||||
assert settings.version == "1.0.0"
|
||||
|
||||
def test_server_settings_normalizes_v_prefix_short(self) -> None:
|
||||
"""Test ServerSettings strips v prefix from short versions."""
|
||||
settings = ServerSettings(version="v1.0")
|
||||
assert settings.version == "1.0.0"
|
||||
|
||||
def test_server_settings_normalizes_major_only(self) -> None:
|
||||
"""Test ServerSettings normalizes MAJOR to MAJOR.0.0."""
|
||||
settings = ServerSettings(version="1")
|
||||
assert settings.version == "1.0.0"
|
||||
|
||||
def test_server_settings_normalizes_v_major_only(self) -> None:
|
||||
"""Test ServerSettings strips v prefix from major-only versions."""
|
||||
settings = ServerSettings(version="v1")
|
||||
assert settings.version == "1.0.0"
|
||||
|
||||
def test_mcp_settings_env_rejects_invalid_version(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Test MCP_SERVER_VERSION env var is validated."""
|
||||
monkeypatch.setenv("MCP_SERVER_VERSION", "not-valid")
|
||||
with pytest.raises(ValidationError, match="semver"):
|
||||
MCPSettings.from_env()
|
||||
|
|
|
|||
Loading…
Reference in a new issue