Guard release version consistency and name the process in Accessibility errors - #33
Merged
Conversation
…ion errors Two follow-ups to #32, both aimed at the parts that made it hard to diagnose and easy to reintroduce. Guard against version drift. The project declares its version in five files, each consumed by a different channel: pyproject.toml (PyPI), uv.lock, manifest.json (Claude Desktop extension), package.json (Pi extension) and server.json (MCP registry, twice). The 0.3.9, 0.3.10 and 0.3.11 releases bumped only pyproject.toml, so the extension kept shipping 0.3.8 -- predating the 0.3.10 AXIsProcessTrustedWithOptions prompt -- and nothing in CI noticed. scripts/check_versions.py compares all six strings against each other and against the release tag, and publish.yml runs it before `uv build`, so a mismatch now fails the release before anything reaches PyPI. Its regression test reconstructs the 0.3.11 tree and asserts the guard would have blocked that release. Name the process in permission failures. "Required permissions not granted: Accessibility" gave the user no target: the MCP host is already granted, nothing called "macos-mcp" ever appears in the Accessibility list, and the interpreter cannot be added through the "+" picker. The message now names sys.executable, points at the native "would like to control this computer" dialog as the reliable fix, and warns against adding the binary by hand. Screen-Recording-only failures are unchanged, since they carry no such ambiguity. Refs #32
PR Summary by QodoCI: Guard release version drift and clarify Accessibility permission errors
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
9 rules 1. Changelog lines exceed 100
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+11
to
+15
| - Release builds now verify that every declared version string agrees before publishing. `scripts/check_versions.py` compares `pyproject.toml`, `uv.lock`, `manifest.json`, `package.json` and `server.json` (twice) against each other and against the release tag, and `publish.yml` runs it ahead of the build so a mismatched tag fails before anything reaches PyPI — the drift that caused #32 would have been blocked at 0.3.9 | ||
|
|
||
| ### Changed | ||
| - Accessibility permission failures now name the process that needs the grant (`sys.executable`) instead of just the permission. The message also points at the native "would like to control this computer" consent dialog as the reliable fix, and warns against adding the interpreter by hand in the System Settings "+" picker, which is greyed out for uv-managed Python and breaks on the next uv update (#32) | ||
|
|
There was a problem hiding this comment.
1. Changelog lines exceed 100 📘 Rule violation ✧ Quality
New CHANGELOG entries are written as single very long lines, exceeding the 100-character maximum and reducing readability in common diff/review views.
Agent Prompt
## Issue description
New CHANGELOG entries exceed the 100-character maximum line length.
## Issue Context
Compliance requires a max line length of 100 unless explicitly justified.
## Fix Focus Areas
- CHANGELOG.md[11-15]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+37
to
+40
| _UV_LOCK_MACOS_MCP = re.compile( | ||
| r'^\[\[package\]\]\nname = "macos-mcp"\nversion = "([^"]+)"', | ||
| re.MULTILINE, | ||
| ) |
There was a problem hiding this comment.
2. Single quotes used in python 📘 Rule violation ✧ Quality
New Python code introduces single-quoted string literals, violating the required double-quote string literal style and creating inconsistent quoting in the changed modules.
Agent Prompt
## Issue description
Changed/new Python code uses single-quoted string literals where double quotes are required.
## Issue Context
The compliance rule requires double quotes for all string literals in languages where both are supported (including Python).
## Fix Focus Areas
- scripts/check_versions.py[37-40]
- scripts/check_versions.py[120-120]
- tests/test_check_versions.py[86-86]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+30
to
+47
| 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 |
There was a problem hiding this comment.
3. Tests missing type annotations 📘 Rule violation ✧ Quality
New test/helper functions are introduced without parameter and/or return type annotations, which violates the requirement to type all function signatures in changed code.
Agent Prompt
## Issue description
New/modified function definitions lack type hints on parameters and/or return values.
## Issue Context
The compliance rule requires explicit type annotations for all function parameters and return values in changed code.
## Fix Focus Areas
- tests/test_check_versions.py[30-47]
- tests/test_check_versions.py[62-65]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+43
to
+49
| 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. | ||
| """ |
There was a problem hiding this comment.
4. Public functions lack structured docstrings 📘 Rule violation ✧ Quality
New public functions have docstrings that omit required structured sections (e.g., Args: / Returns:) or lack docstrings altogether, making the API harder to understand and maintain.
Agent Prompt
## Issue description
Public functions added/modified in this PR are missing required structured docstring sections (e.g., `Args:` and `Returns:`), or are missing docstrings.
## Issue Context
Compliance requires structured docstrings for public functions.
## Fix Focus Areas
- scripts/check_versions.py[43-49]
- scripts/check_versions.py[82-84]
- scripts/check_versions.py[102-104]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+37
to
+62
| _UV_LOCK_MACOS_MCP = re.compile( | ||
| r'^\[\[package\]\]\nname = "macos-mcp"\nversion = "([^"]+)"', | ||
| re.MULTILINE, | ||
| ) | ||
|
|
||
|
|
||
| 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()) | ||
| 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() | ||
| match = _UV_LOCK_MACOS_MCP.search(lock_text) | ||
| if match is None: | ||
| raise ValueError("uv.lock has no [[package]] entry for macos-mcp") | ||
| versions["uv.lock:macos-mcp"] = match.group(1) |
There was a problem hiding this comment.
5. Brittle uv.lock version parse 🐞 Bug ☼ Reliability
scripts/check_versions.py extracts the macos-mcp version from uv.lock using a regex that requires an exact, adjacent-line layout for name/version. If uv.lock generation changes key ordering/format (still valid TOML), the release workflow can fail even when the lockfile correctly contains the macos-mcp version.
Agent Prompt
## Issue description
`scripts/check_versions.py` currently uses a strict regex to find the `macos-mcp` package version in `uv.lock`. This approach is brittle because `uv.lock` is TOML and key ordering/formatting can legitimately change, which would cause false failures in the release guard.
## Issue Context
The guard is intended to prevent version drift. A spurious failure due to lockfile formatting changes would block publishing and create unnecessary release friction.
## Fix Focus Areas
- scripts/check_versions.py[37-63]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+102
to
+113
| def main(argv: list[str]) -> int: | ||
| expected = argv[1] if len(argv) > 1 else None | ||
|
|
||
| try: | ||
| versions = collect_versions() | ||
| except (OSError, ValueError, json.JSONDecodeError, tomllib.TOMLDecodeError) as exc: | ||
| print(f"error: could not read version metadata: {exc}", file=sys.stderr) | ||
| return 1 | ||
|
|
||
| problems = check(expected) | ||
| width = max(len(label) for label in versions) | ||
| target = expected.removeprefix("v") if expected else None |
There was a problem hiding this comment.
6. Uncaught second metadata read 🐞 Bug ☼ Reliability
main() calls collect_versions() inside a try/except, but then calls check() which calls collect_versions() again outside that handler. If the second parse/read fails (OSError/JSON/TOML errors), the script can exit with a traceback instead of returning code 1 with a clear error message.
Agent Prompt
## Issue description
`main()` already reads/parses all version files via `collect_versions()`, but then calls `check()` which re-reads them and can raise exceptions outside `main()`'s guarded block.
## Issue Context
This is low-likelihood in CI, but when it does happen it degrades diagnostics (traceback) and makes exit semantics less predictable.
## Fix Focus Areas
- scripts/check_versions.py[82-99]
- scripts/check_versions.py[102-113]
ⓘ 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.
Two follow-ups to #32. v0.3.12 already resynced the version strings; this addresses the parts that made the issue hard to diagnose and easy to reintroduce.
Guard against version drift
The project declares its version in five files, each consumed by a different channel:
pyproject.tomluv.lockmanifest.jsonpackage.jsonserver.jsonThe 0.3.9, 0.3.10 and 0.3.11 release commits each touched only
pyproject.toml(plusuv.lock/CHANGELOG), so the shipped metadata was left at 0.3.8, 0.3.5 and 0.3.6. Becausemanifest.jsonis what the extension is published from, the extension kept advertising and installing 0.3.8 — which predates the 0.3.10AXIsProcessTrustedWithOptionsstartup prompt. Affected users could never reach the version that actually requests Accessibility access, and nothing in CI noticed for three releases.scripts/check_versions.pycompares all six version strings against each other and against the release tag.publish.ymlruns it beforeuv build, so a mismatch fails the release before anything reaches PyPI.The regression test reconstructs the exact 0.3.11 tree and asserts the guard would have blocked that release.
Name the process in permission failures
This gave the user no target. The MCP host is already in the Accessibility list and toggled on, nothing called
macos-mcpever appears there, and the interpreter can't be added through the+picker — so the instruction was unfollowable, which is most of what made #32 painful.The message now names
sys.executable, points at the native "would like to control this computer" dialog as the reliable fix, and warns against adding the binary by hand (greyed out for uv-managed Python, and breaks on the next uv update). Screen-Recording-only failures are unchanged, since they carry no such ambiguity.Testing
187 passed (171 before, +16). New coverage for version extraction, drift detection, tag matching, exit codes, and the guidance text.
Refs #32