ci: block releases when version strings disagree - #360
Merged
Conversation
The version is declared in four files, each consumed by a different channel: pyproject.toml (PyPI), uv.lock, manifest.json (Claude Desktop extension) and server.json's packages[] entry (MCP registry). Only pyproject.toml reliably gets bumped. At v0.8.5 manifest.json and server.json's package entry were both still on 0.8.1 -- two releases behind -- because the 0.8.2 bump touched only pyproject.toml. Nothing failed; the extension simply kept advertising a version that was no longer what shipped. scripts/check_versions.py compares all four against each other and against the release tag, and publish.yml now runs it before `uv build`, so a mismatch fails the release before anything reaches PyPI. Its regression test reconstructs the real pre-0.8.5 tree and asserts the guard would have blocked that release. server.json's TOP-LEVEL "version" is deliberately excluded. That field identifies the registry entry rather than the PyPI package and is on its own 1.x line, so requiring it to match would either fail permanently or force a version downgrade in the registry. A test pins the exclusion so it isn't later "fixed" into a check that can never pass. Not run locally: the suite needs pywin32, which has no macOS wheels, so the test bodies were executed directly instead. CI runs on windows-latest.
PR Summary by QodoCI: block releases when declared versions drift from the tag
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
16 rules 1. Single quotes in strings
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| REPO_ROOT = Path(__file__).resolve().parent.parent | ||
|
|
||
| _UV_LOCK_WINDOWS_MCP = re.compile( | ||
| r'^\[\[package\]\]\nname = "windows-mcp"\nversion = "([^"]+)"', |
There was a problem hiding this comment.
1. Single quotes in strings 📘 Rule violation ✧ Quality
New Python code and tests introduce single-quoted string literals, which violates the requirement to use double quotes for all string literals. This can cause inconsistent style and lint/format failures if enforced in CI.
Agent Prompt
## Issue description
The PR adds single-quoted string literals, but the style rule requires double quotes for all string literals.
## Issue Context
This appears in both the new version-check script and its tests.
## Fix Focus Areas
- scripts/check_versions.py[42-45]
- tests/test_check_versions.py[81-83]
- tests/test_check_versions.py[115-126]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+48
to
+104
| def collect_versions(root: Path = REPO_ROOT) -> dict[str, str]: | ||
| """Extract every declared version string, keyed by a human-readable label. | ||
|
|
||
| Raises: | ||
| ValueError: if a file is missing the version field entirely, which is | ||
| just as much a packaging bug as a stale value. | ||
| """ | ||
| versions: dict[str, str] = {} | ||
|
|
||
| pyproject = tomllib.loads((root / "pyproject.toml").read_text(encoding="utf-8")) | ||
| try: | ||
| versions["pyproject.toml:project.version"] = pyproject["project"]["version"] | ||
| except KeyError as exc: | ||
| raise ValueError("pyproject.toml is missing [project] version") from exc | ||
|
|
||
| lock_text = (root / "uv.lock").read_text(encoding="utf-8") | ||
| match = _UV_LOCK_WINDOWS_MCP.search(lock_text) | ||
| if match is None: | ||
| raise ValueError("uv.lock has no [[package]] entry for windows-mcp") | ||
| versions["uv.lock:windows-mcp"] = match.group(1) | ||
|
|
||
| manifest = json.loads((root / "manifest.json").read_text(encoding="utf-8")) | ||
| if "version" not in manifest: | ||
| raise ValueError("manifest.json is missing a top-level version field") | ||
| versions["manifest.json:version"] = manifest["version"] | ||
|
|
||
| server = json.loads((root / "server.json").read_text(encoding="utf-8")) | ||
| packages = server.get("packages", []) | ||
| if not packages: | ||
| raise ValueError("server.json declares no packages") | ||
| for index, package in enumerate(packages): | ||
| if "version" not in package: | ||
| raise ValueError(f"server.json packages[{index}] is missing a version field") | ||
| versions[f"server.json:packages[{index}].version"] = package["version"] | ||
|
|
||
| return versions | ||
|
|
||
|
|
||
| def check(expected: str | None = None, root: Path = REPO_ROOT) -> list[str]: | ||
| """Return a list of human-readable problems; empty means everything agrees.""" | ||
| versions = collect_versions(root) | ||
| distinct = sorted(set(versions.values())) | ||
|
|
||
| problems: list[str] = [] | ||
| if len(distinct) > 1: | ||
| problems.append(f"version strings disagree: {', '.join(distinct)}") | ||
|
|
||
| if expected is not None: | ||
| target = expected.removeprefix("v") | ||
| if any(version != target for version in versions.values()): | ||
| problems.append(f"expected every version to be {target}") | ||
|
|
||
| return problems | ||
|
|
||
|
|
||
| def main(argv: list[str]) -> int: | ||
| expected = argv[1] if len(argv) > 1 else None |
There was a problem hiding this comment.
2. Missing google-style docstrings 📘 Rule violation ✧ Quality
Public functions in the new scripts/check_versions.py module do not use Google-style docstrings (missing Args:/Returns: sections), and main() has no docstring. This violates the docstring standard and reduces maintainability for a release-critical script.
Agent Prompt
## Issue description
Public functions need Google-style docstrings with the required sections (`Args:`, `Returns:`, and `Raises:` where applicable). Some functions have minimal docstrings and `main()` has none.
## Issue Context
The new script is invoked by the release workflow; clear docstrings help prevent future regressions and clarify expected inputs/outputs.
## Fix Focus Areas
- scripts/check_versions.py[48-55]
- scripts/check_versions.py[86-100]
- scripts/check_versions.py[103-110]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+30
to
+56
| def _load_module(): | ||
| """Load scripts/check_versions.py, which is not an installed package.""" | ||
| path = REPO_ROOT / "scripts" / "check_versions.py" | ||
| spec = importlib.util.spec_from_file_location("check_versions", path) | ||
| module = importlib.util.module_from_spec(spec) | ||
| spec.loader.exec_module(module) | ||
| return module | ||
|
|
||
|
|
||
| check_versions = _load_module() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def repo(tmp_path): | ||
| """A throwaway copy of the repo's version-bearing files.""" | ||
| for filename in VERSIONED_FILES: | ||
| shutil.copy(REPO_ROOT / filename, tmp_path / filename) | ||
| return tmp_path | ||
|
|
||
|
|
||
| def _write_json(root: Path, filename: str, data) -> None: | ||
| (root / filename).write_text(json.dumps(data, indent=2), encoding="utf-8") | ||
|
|
||
|
|
||
| def _read_json(root: Path, filename: str): | ||
| return json.loads((root / filename).read_text(encoding="utf-8")) | ||
|
|
There was a problem hiding this comment.
3. Test helpers lack type hints 📘 Rule violation ✧ Quality
The new test module adds several def functions without complete type annotations for parameters and/or return types. This violates the requirement for type hints on all function signatures and can weaken static analysis.
Agent Prompt
## Issue description
New test helper functions and fixtures are missing required type hints for parameters and/or return types.
## Issue Context
The rule applies to all `def`-based functions, including tests and fixtures.
## Fix Focus Areas
- tests/test_check_versions.py[30-37]
- tests/test_check_versions.py[42-47]
- tests/test_check_versions.py[50-55]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+42
to
+45
| _UV_LOCK_WINDOWS_MCP = re.compile( | ||
| r'^\[\[package\]\]\nname = "windows-mcp"\nversion = "([^"]+)"', | ||
| re.MULTILINE, | ||
| ) |
There was a problem hiding this comment.
4. Brittle uv.lock parsing 🐞 Bug ☼ Reliability
collect_versions() extracts the windows-mcp version from uv.lock with a regex that requires the [[package]] block to have name and version on consecutive lines in a specific order. If uv.lock formatting changes (e.g., an extra field inserted or reordering within the package block), the regex will fail and incorrectly block releases even when versions are correct.
Agent Prompt
## Issue description
`scripts/check_versions.py` parses `uv.lock` using a formatting-sensitive regex that assumes the `[[package]]` entry for `windows-mcp` has `name` immediately followed by `version`.
## Issue Context
`uv.lock` is structured TOML and may evolve in formatting/order. A release guard should be resilient to harmless formatting changes.
## Fix Focus Areas
- scripts/check_versions.py[42-67]
## Suggested fix
- Replace the regex extraction with a TOML parse (`tomllib.loads(lock_text)`) and then locate the `windows-mcp` entry from the parsed `package` list.
- Alternatively, make the regex tolerant of intervening lines and whitespace, but TOML parsing is preferable for stability.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| print(f"error: could not read version metadata: {exc}", file=sys.stderr) | ||
| return 1 | ||
|
|
||
| problems = check(expected) |
There was a problem hiding this comment.
5. Unchecked second metadata read 🐞 Bug ☼ Reliability
main() reads version metadata via collect_versions() inside a try/except, but then calls check(), which re-reads/parses the same files outside that exception handler. If a transient I/O or parsing failure occurs between the two reads, the script can crash with a traceback instead of emitting the intended single-line error and exit code.
Agent Prompt
## Issue description
`main()` calls `collect_versions()` and then calls `check()`, but `check()` calls `collect_versions()` again. The second read is not protected by `main()`'s existing exception handler.
## Issue Context
This duplicates I/O and creates an unhandled exception path if the repository files change or a transient read/parse error occurs between calls.
## Fix Focus Areas
- scripts/check_versions.py[86-100]
- scripts/check_versions.py[103-114]
## Suggested fix
- Refactor `check()` to accept an already-collected `versions: dict[str, str]` (or add a helper like `check_versions(versions, expected)`), and have `main()` call it with its `versions`.
- Alternatively, wrap the `problems = check(expected)` call in the same try/except, but eliminating the second read is cleaner.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports the version-consistency guard from MacOS-MCP (CursorTouch/MacOS-MCP#33), adapted to this repo's layout. A release can no longer publish unless every declared version agrees with the tag.
Why
The version is declared in four files, each consumed by a different channel:
pyproject.tomluv.lockmanifest.jsonserver.json→packages[]Only
pyproject.tomlreliably gets bumped. At the v0.8.5 release,manifest.jsonandserver.json's package entry were both still on 0.8.1 — two releases behind — because the 0.8.2 bump touched onlypyproject.toml. Nothing failed; the extension just kept advertising a version that was no longer what shipped.That's the same failure MacOS-MCP hit, where it stranded users on a build predating a permissions fix with no update path.
What this does
scripts/check_versions.pycompares all four against each other and against the release tag.publish.ymlruns it beforeuv build, so a mismatch fails the release before anything reaches PyPI.Against the reconstructed pre-0.8.5 tree:
One deliberate exclusion
server.json's top-level"version"is not checked. That field identifies the registry entry rather than the PyPI package, and it's on its own1.xline (currently1.0.1). Requiring it to match would either fail permanently or force a version downgrade in the registry. Onlypackages[].version, which must match what's published to PyPI, is checked.test_top_level_server_version_is_ignoredpins that exclusion so it isn't later "fixed" into a check that can never pass.Tests
tests/test_check_versions.py— 14 tests covering extraction, drift detection, tag matching, the top-level exclusion, and exit codes. The one that matters istest_catches_the_historical_drift, which reconstructs the real pre-0.8.5 tree and asserts the guard would have blocked that release.Testing caveat: I could not run the suite locally —
pywin32has no macOS wheels, souv syncfails on this machine. I executed the 14 test bodies directly against temporary copies of the tree instead (all pass), and exercised the script end to end. CI onwindows-latestruns the real suite.