From 0e4e7be0927a49b8691babb23eefd6f3595e10a4 Mon Sep 17 00:00:00 2001 From: Xianpeng Shen Date: Mon, 7 Sep 2026 05:05:52 +0000 Subject: [PATCH 1/5] docs(readme): consolidate MCP client setup into one block and a per-client table Replace the nine near-identical JSON snippets with a single canonical `mcpServers` block plus a table of where each client stores it. Fix the entries that were wrong or stale: Claude Code registers servers with `claude mcp add` / `.mcp.json` (not `~/.claude/settings.json`), Zed uses `context_servers`, Continue uses the `config.yaml` list form, Claude Desktop gets its config paths, and VS Code (`.vscode/mcp.json`, `servers` key) is added. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018xYa7m3qup5wyN5MaXFgf6 --- README.md | 159 ++++++++---------------------------------------------- 1 file changed, 23 insertions(+), 136 deletions(-) diff --git a/README.md b/README.md index f053535..3f3ce15 100644 --- a/README.md +++ b/README.md @@ -100,109 +100,10 @@ uvx commit-check-mcp > **Tip**: If `uv` is not installed, get it via `curl -LsSf https://astral.sh/uv/install.sh | sh`. ---- - -### Claude Desktop - -```json -{ - "mcpServers": { - "commit-check": { - "command": "uvx", - "args": ["commit-check-mcp"] - } - } -} -``` - -### Claude Code CLI - -```json -{ - "mcpServers": { - "commit-check": { - "command": "uvx", - "args": ["commit-check-mcp"] - } - } -} -``` - -Add to your `~/.claude/settings.json` or project-level `.claude/settings.local.json`. - -### Cursor - -In Cursor, go to **Settings → Cursor Settings → MCP → Add new MCP server** and paste: - -| Field | Value | -|---|---| -| **Name** | `commit-check` | -| **Type** | `command` | -| **Command** | `uvx commit-check-mcp` | - -Or add to your project's `.cursor/mcp.json`: - -```json -{ - "mcpServers": { - "commit-check": { - "command": "uvx", - "args": ["commit-check-mcp"] - } - } -} -``` - -### Windsurf - -Add to your `~/.codeium/windsurf/mcp_config.json`: - -```json -{ - "mcpServers": { - "commit-check": { - "command": "uvx", - "args": ["commit-check-mcp"] - } - } -} -``` - -### Cline (VS Code) - -Add a new MCP server in the Cline extension settings: - -```json -{ - "mcpServers": { - "commit-check": { - "command": "uvx", - "args": ["commit-check-mcp"] - } - } -} -``` - -### Continue.dev (VS Code / JetBrains) - -Add to your `~/.continue/config.json`: - -```json -{ - "experimental": { - "mcpServers": { - "commit-check": { - "command": "uvx", - "args": ["commit-check-mcp"] - } - } - } -} -``` - -### Roo Code +### Configure your client -Add to your Roo Code MCP settings: +Every client below launches the same command; only the config file and, for a +few clients, the wrapper key differ. This is the object to register: ```json { @@ -215,40 +116,26 @@ Add to your Roo Code MCP settings: } ``` -### Zed - -Add to your `~/.config/zed/settings.json`: - -```json -{ - "mcp_servers": { - "commit-check": { - "command": "uvx", - "args": ["commit-check-mcp"] - } - } -} -``` - -### Generic / Any MCP Client - -If your client does not support `uvx`, use `pip` and the direct path: - -```bash -pip install commit-check-mcp -which commit-check-mcp -``` - -Then use the absolute path in your config: - -```json -{ - "mcpServers": { - "commit-check": { - "command": "/path/to/commit-check-mcp" - } - } -} +| Client | Where it goes | Notes | +|---|---|---| +| Claude Code | `claude mcp add commit-check -- uvx commit-check-mcp` | Add `--scope project` to write a shareable `.mcp.json` at the repo root (`--scope user` makes it available in all your projects). You can also commit a `.mcp.json` containing the block above; `"type": "stdio"` may be added inside the server object. MCP servers are **not** configured in `~/.claude/settings.json`. | +| Claude Desktop | macOS `~/Library/Application Support/Claude/claude_desktop_config.json`; Windows `%APPDATA%\Claude\claude_desktop_config.json` | Block above as-is; restart Claude Desktop. | +| Cursor | project `.cursor/mcp.json` or global `~/.cursor/mcp.json` | Block above as-is (or **Settings → Cursor Settings → MCP → Add new MCP server** with command `uvx commit-check-mcp`). | +| VS Code (Copilot agent mode) | `.vscode/mcp.json` | **Different key**: `{"servers": {"commit-check": {"type": "stdio", "command": "uvx", "args": ["commit-check-mcp"]}}}` | +| Cline | MCP Servers panel → Configure → `cline_mcp_settings.json` (check your client's docs) | Block above as-is. | +| Roo Code | project `.roo/mcp.json` or global `mcp_settings.json` (**Edit Global MCP**) | Block above as-is; optional `"alwaysAllow": [...]`. | +| Windsurf | `~/.codeium/windsurf/mcp_config.json` (check your client's docs) | Block above as-is. | +| Continue | `config.yaml` (or a file in `.continue/mcpServers/`) | **YAML list** under `mcpServers:`, see below. Continue also picks up the JSON block above when dropped into `.continue/mcpServers/`. | +| Zed | `~/.config/zed/settings.json` | **Different key**: `{"context_servers": {"commit-check": {"command": "uvx", "args": ["commit-check-mcp"]}}}` | +| Anything else | your client's MCP config | If the client cannot run `uvx`: `pip install commit-check-mcp`, then set `"command"` to the absolute path printed by `which commit-check-mcp` and drop `args`. | + +Continue's `config.yaml` entry in full: + +```yaml +mcpServers: + - name: commit-check + command: uvx + args: ["commit-check-mcp"] ``` ## Run Manually From f585e49adec30cd1b7c55f526b77b4f07c401279 Mon Sep 17 00:00:00 2001 From: Xianpeng Shen Date: Mon, 7 Sep 2026 05:07:07 +0000 Subject: [PATCH 2/5] ci: add ruff and mypy lint job, test on Python 3.14 Add a `lint` job to main.yml that runs `ruff check src tests` and `mypy src`, and extend the test matrix to 3.14, which the classifiers already advertise. Pin ruff/mypy in the dev extras and configure both in pyproject.toml (line length 100, target py310, rules E/F/W/I/UP/B; mypy on the `commit_check_mcp` package with `mypy_path = src`). Make the tree clean for the new job first: sort imports (I001) in server.py and the tests, wrap seven over-long test lines (E501), and pass the `ToolAnnotations` hints with their snake_case field names (`read_only_hint=` etc.) instead of the camelCase aliases, which mypy rejects as unknown keyword arguments. The serialized annotations are unchanged because the model still emits the camelCase aliases on the wire. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018xYa7m3qup5wyN5MaXFgf6 --- .github/workflows/main.yml | 21 +++++++++++++++- pyproject.toml | 17 ++++++++++++- src/commit_check_mcp/server.py | 14 +++++------ tests/test_server.py | 46 ++++++++++++++++++++++++++-------- 4 files changed, 79 insertions(+), 19 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 022ce8b..9a4affe 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -22,7 +22,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.10", "3.11", "3.12", "3.13"] + python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 @@ -45,6 +45,25 @@ jobs: slug: commit-check/commit-check-mcp files: ./coverage.xml + lint: + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.12" + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + python -m pip install -e .[dev] + + - name: Lint (ruff) + run: python -m ruff check src tests + + - name: Type check (mypy) + run: python -m mypy src + build: runs-on: ubuntu-24.04 steps: diff --git a/pyproject.toml b/pyproject.toml index 8dc4e06..2bfbefa 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,7 +37,9 @@ tracker = "https://github.com/commit-check/commit-check-mcp/issues" [project.optional-dependencies] dev = [ "pytest>=9.0.0,<10", - "pytest-cov>=6.0.0,<8" + "pytest-cov>=6.0.0,<8", + "ruff>=0.16,<1", + "mypy>=1.14,<3", ] [project.scripts] @@ -56,3 +58,16 @@ fallback_version = "0.0.0" [tool.pytest.ini_options] testpaths = ["tests"] + +[tool.ruff] +line-length = 100 +target-version = "py310" + +[tool.ruff.lint] +select = ["E", "F", "W", "I", "UP", "B"] + +[tool.mypy] +python_version = "3.10" +warn_unused_ignores = true +packages = ["commit_check_mcp"] +mypy_path = "src" diff --git a/src/commit_check_mcp/server.py b/src/commit_check_mcp/server.py index 84ce307..1d690fa 100644 --- a/src/commit_check_mcp/server.py +++ b/src/commit_check_mcp/server.py @@ -2,13 +2,13 @@ from __future__ import annotations +import inspect +import os +import subprocess from collections.abc import Callable from contextlib import contextmanager from importlib.metadata import version from pathlib import Path -import inspect -import os -import subprocess from typing import Annotated, Any, TypeVar from commit_check import __version__ as commit_check_version @@ -76,10 +76,10 @@ def _tool(title: str, *, fetches: bool = False) -> Callable[[_F], _F]: description, with ``{result_shape}`` replaced by :data:`RESULT_SHAPE`. """ annotations = ToolAnnotations( - readOnlyHint=not fetches, - destructiveHint=False, - idempotentHint=True, - openWorldHint=fetches, + read_only_hint=not fetches, + destructive_hint=False, + idempotent_hint=True, + open_world_hint=fetches, ) def register(fn: _F) -> _F: diff --git a/tests/test_server.py b/tests/test_server.py index 83e2b82..1550034 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -2,16 +2,15 @@ from __future__ import annotations -from pathlib import Path import asyncio import os import subprocess +from pathlib import Path import pytest - -from commit_check_mcp import server from mcp.server.mcpserver.exceptions import ToolError +from commit_check_mcp import server # --------------------------------------------------------------------------- # _normalize_config @@ -125,7 +124,13 @@ def test_valid_message_passes(self, tmp_path: Path) -> None: repo = tmp_path / "repo" repo.mkdir() (repo / "cchk.toml").write_text( - "[commit]\nallow_commit_types = []\nallow_merge_commits = true\nallow_revert_commits = true\nallow_empty_commits = true\nallow_fixup_commits = true\nallow_wip_commits = true" + "[commit]\n" + "allow_commit_types = []\n" + "allow_merge_commits = true\n" + "allow_revert_commits = true\n" + "allow_empty_commits = true\n" + "allow_fixup_commits = true\n" + "allow_wip_commits = true" ) result = server._validate_message( "feat: add new feature", @@ -345,7 +350,13 @@ def fake_run_checks(check_names, context, config): return { "status": "fail", "checks": [ - {"check": cn, "status": "fail", "value": "", "error": "bad", "suggest": "fix it"} + { + "check": cn, + "status": "fail", + "value": "", + "error": "bad", + "suggest": "fix it", + } for cn in check_names ], } @@ -526,7 +537,9 @@ def test_empty_raises(self) -> None: with pytest.raises(ToolError, match="non-empty"): server.validate_commit_message(" ") - def test_valid_message_with_repo_path(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + def test_valid_message_with_repo_path( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: repo = tmp_path / "repo" repo.mkdir() @@ -690,7 +703,9 @@ def test_no_fields_raises(self) -> None: def test_all_fields_forwards(self, monkeypatch: pytest.MonkeyPatch) -> None: captured: dict[str, object] = {} - def fake_validate_all(message, branch, author_name, author_email, *, config, repo_path, config_path): + def fake_validate_all( + message, branch, author_name, author_email, *, config, repo_path, config_path + ): captured["message"] = message captured["branch"] = branch captured["author_name"] = author_name @@ -716,7 +731,9 @@ def fake_validate_all(message, branch, author_name, author_email, *, config, rep def test_message_only(self, monkeypatch: pytest.MonkeyPatch) -> None: captured: dict[str, object] = {} - def fake_validate_all(message, branch, author_name, author_email, *, config, repo_path, config_path): + def fake_validate_all( + message, branch, author_name, author_email, *, config, repo_path, config_path + ): captured["message"] = message captured["branch"] = branch captured["author_name"] = author_name @@ -925,7 +942,9 @@ def test_a_fully_skipped_run_is_reported_as_skip_not_pass( lambda self, context: [_outcome("skip"), _outcome("skip", "author_name")], ) result = server._run_checks( - ["message", "author_name"], ValidationContext(stdin_text="x"), server._merge_config(None) + ["message", "author_name"], + ValidationContext(stdin_text="x"), + server._merge_config(None), ) assert result["status"] == "skip" assert result["warnings"] == 0 @@ -975,7 +994,14 @@ def test_the_combined_tools_reduce_with_the_same_rule( def skipped(check_names, context, config): return server._summarize( [ - {"check": cn, "status": "skip", "value": "", "error": "", "suggest": "", "fix": ""} + { + "check": cn, + "status": "skip", + "value": "", + "error": "", + "suggest": "", + "fix": "", + } for cn in check_names ] ) From ffe9e50f1b50e3e286cfa9b6d267d35a5053f52f Mon Sep 17 00:00:00 2001 From: Xianpeng Shen Date: Mon, 7 Sep 2026 05:08:47 +0000 Subject: [PATCH 3/5] fix(concurrency): serialise the chdir window so parallel tool calls read their own repo `_working_directory` switches the process-wide cwd, and the MCP SDK runs sync tools on worker threads with each request in its own task. Two in-flight calls with different `repo_path` values therefore raced: about half of them returned the other repository's result as `pass`, and the `finally` restore frequently left the server inside one of the repos, changing every later `repo_path=None` call for the life of the process. Guard the chdir window with a module-level `threading.Lock`. Calls that omit `repo_path` do not take the lock. Add a regression test that runs `validate_branch_name` for two repositories concurrently through `mcp.call_tool` and asserts each result names its own branch and the cwd is restored. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018xYa7m3qup5wyN5MaXFgf6 --- src/commit_check_mcp/server.py | 20 +++++++++++----- tests/test_server.py | 43 ++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/src/commit_check_mcp/server.py b/src/commit_check_mcp/server.py index 1d690fa..28dfe71 100644 --- a/src/commit_check_mcp/server.py +++ b/src/commit_check_mcp/server.py @@ -5,6 +5,7 @@ import inspect import os import subprocess +import threading from collections.abc import Callable from contextlib import contextmanager from importlib.metadata import version @@ -256,6 +257,12 @@ def _normalize_config_path(config_path: str | None, repo_path: Path | None) -> s return str(resolved) +# os.chdir is process-global and the SDK runs sync tools on worker threads +# concurrently, so every chdir window is serialised on one lock. Long-term fix: +# pass cwd to git and to the config loader instead of changing directory. +_CWD_LOCK = threading.Lock() + + @contextmanager def _working_directory(repo_path: Path | None): """Temporarily switch working directory for repo-relative config and git checks.""" @@ -263,12 +270,13 @@ def _working_directory(repo_path: Path | None): yield return - original_cwd = Path.cwd() - os.chdir(repo_path) - try: - yield - finally: - os.chdir(original_cwd) + with _CWD_LOCK: + original_cwd = Path.cwd() + os.chdir(repo_path) + try: + yield + finally: + os.chdir(original_cwd) def _merge_config( diff --git a/tests/test_server.py b/tests/test_server.py index 1550034..0cf5422 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -1394,3 +1394,46 @@ def test_repository_state_include_push_is_unaffected(self, tmp_path: Path) -> No ) assert result["status"] == "pass" assert [c["check"] for c in result["checks"]] == ["no_force_push"] + + +# --------------------------------------------------------------------------- +# Concurrent tool calls with different repo_path values must not see each +# other's working directory (os.chdir is process-global) +# --------------------------------------------------------------------------- + +def _repo_on_branch(root: Path, name: str, branch: str) -> Path: + repo = root / name + repo.mkdir() + _git(repo, "init", "-q", "-b", branch) + (repo / "file.txt").write_text("content\n") + _git(repo, "add", "file.txt") + _git(repo, "commit", "-q", "-m", "feat: init") + return repo + + +class TestConcurrentWorkingDirectory: + def test_parallel_calls_read_their_own_repo(self, tmp_path: Path) -> None: + # The SDK runs sync tools on worker threads, so two in-flight calls + # each chdir the one process. Without the lock about half of the + # results below report the other repository's branch as `pass`. + repos = { + _repo_on_branch(tmp_path, "a", "feature/alpha"): "feature/alpha", + _repo_on_branch(tmp_path, "b", "bugfix/beta"): "bugfix/beta", + } + start_cwd = Path.cwd() + + async def one_round() -> list[object]: + return await asyncio.gather( + *( + server.mcp.call_tool("validate_branch_name", {"repo_path": str(repo)}) + for repo in repos + ) + ) + + for _ in range(30): + results = asyncio.run(one_round()) + assert Path.cwd() == start_cwd + for repo, result in zip(repos, results, strict=True): + checks = result.structured_content["checks"] + branch = next(c["value"] for c in checks if c["check"] == "branch") + assert branch == repos[repo], f"{repo} reported branch {branch!r}" From dcc6c77d09b0cc6faa4ed47e9c8ff45119d13f2b Mon Sep 17 00:00:00 2001 From: Xianpeng Shen Date: Mon, 7 Sep 2026 05:20:15 +0000 Subject: [PATCH 4/5] fix: hold the cwd lock for cwd-relative calls, harden lint checkout, clarify README - _working_directory takes _CWD_LOCK even when no repo_path is given, so a tool reading the process cwd never observes another thread's chdir. - The lint job checkout sets persist-credentials: false. - README: the Continue example is a complete config.yaml (name/version/ schema) with a note on the mcpServers fragment and .continue/mcpServers/, and the fallback row lists where/Get-Command for Windows shells. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018xYa7m3qup5wyN5MaXFgf6 --- .github/workflows/main.yml | 2 ++ README.md | 7 +++++-- src/commit_check_mcp/server.py | 9 +++++---- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 9a4affe..6679c28 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -49,6 +49,8 @@ jobs: runs-on: ubuntu-24.04 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.12" diff --git a/README.md b/README.md index 3f3ce15..4391337 100644 --- a/README.md +++ b/README.md @@ -127,11 +127,14 @@ few clients, the wrapper key differ. This is the object to register: | Windsurf | `~/.codeium/windsurf/mcp_config.json` (check your client's docs) | Block above as-is. | | Continue | `config.yaml` (or a file in `.continue/mcpServers/`) | **YAML list** under `mcpServers:`, see below. Continue also picks up the JSON block above when dropped into `.continue/mcpServers/`. | | Zed | `~/.config/zed/settings.json` | **Different key**: `{"context_servers": {"commit-check": {"command": "uvx", "args": ["commit-check-mcp"]}}}` | -| Anything else | your client's MCP config | If the client cannot run `uvx`: `pip install commit-check-mcp`, then set `"command"` to the absolute path printed by `which commit-check-mcp` and drop `args`. | +| Anything else | your client's MCP config | If the client cannot run `uvx`: `pip install commit-check-mcp`, then set `"command"` to the absolute path of the installed binary and drop `args`. Find it with `which commit-check-mcp` (macOS/Linux), `where commit-check-mcp` (Windows cmd) or `Get-Command commit-check-mcp \| Select-Object -ExpandProperty Source` (PowerShell). | -Continue's `config.yaml` entry in full: +Continue's `config.yaml` entry in full (`name`, `version` and `schema` are required by Continue; drop them if you are adding only the `mcpServers` fragment to an existing file, or save this as a standalone file in `.continue/mcpServers/`): ```yaml +name: commit-check +version: 0.0.1 +schema: v1 mcpServers: - name: commit-check command: uvx diff --git a/src/commit_check_mcp/server.py b/src/commit_check_mcp/server.py index 28dfe71..86aeeef 100644 --- a/src/commit_check_mcp/server.py +++ b/src/commit_check_mcp/server.py @@ -266,11 +266,12 @@ def _normalize_config_path(config_path: str | None, repo_path: Path | None) -> s @contextmanager def _working_directory(repo_path: Path | None): """Temporarily switch working directory for repo-relative config and git checks.""" - if repo_path is None: - yield - return - + # The lock is held even when repo_path is None: a tool that reads the + # process cwd must not observe another thread's temporary chdir. with _CWD_LOCK: + if repo_path is None: + yield + return original_cwd = Path.cwd() os.chdir(repo_path) try: From 7c792c4ed71df80bf90a75b11dd51aebb755e156 Mon Sep 17 00:00:00 2001 From: Xianpeng Shen Date: Mon, 7 Sep 2026 05:45:54 +0000 Subject: [PATCH 5/5] fix(concurrency): resolve relative paths and the no-repo_path git check against the at-rest cwd _normalize_repo_path, _normalize_config_path and _require_git_repo(None) read the process cwd before the tool enters _working_directory, so while another worker thread held its chdir window they resolved against that thread's repository. A relative config_path with no repo_path loaded the other repository's cchk.toml. They now resolve against the cwd observed under _CWD_LOCK, which is the directory the server was started in. Four tests park a thread inside _working_directory(other) and assert the helpers and validate_commit_message still see the server's own directory; all four fail on the previous code. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018xYa7m3qup5wyN5MaXFgf6 --- src/commit_check_mcp/server.py | 25 ++++- tests/test_cwd_at_rest.py | 167 +++++++++++++++++++++++++++++++++ 2 files changed, 188 insertions(+), 4 deletions(-) create mode 100644 tests/test_cwd_at_rest.py diff --git a/src/commit_check_mcp/server.py b/src/commit_check_mcp/server.py index 86aeeef..a4248d0 100644 --- a/src/commit_check_mcp/server.py +++ b/src/commit_check_mcp/server.py @@ -205,7 +205,10 @@ def _normalize_repo_path(repo_path: str | None) -> Path | None: if not normalized: raise ToolError("repo_path cannot be empty when provided") - path = Path(normalized).expanduser().resolve() + path = Path(normalized).expanduser() + if not path.is_absolute(): + path = _at_rest_cwd() / path + path = path.resolve() if not path.exists(): raise ToolError(f"repo_path does not exist: {path}") if not path.is_dir(): @@ -220,7 +223,7 @@ def _require_git_repo(repo_path: Path | None) -> None: ``git``; outside a repository those reads come back empty and every rule passes vacuously, so tools that will consult git call this first. """ - directory = repo_path if repo_path is not None else Path.cwd() + directory = repo_path if repo_path is not None else _at_rest_cwd() try: result = subprocess.run( ["git", "rev-parse", "--show-toplevel"], @@ -246,8 +249,8 @@ def _normalize_config_path(config_path: str | None, repo_path: Path | None) -> s raise ToolError("config_path cannot be empty when provided") path = Path(normalized).expanduser() - if not path.is_absolute() and repo_path is not None: - path = repo_path / path + if not path.is_absolute(): + path = (repo_path if repo_path is not None else _at_rest_cwd()) / path resolved = path.resolve() if not resolved.exists(): @@ -263,6 +266,20 @@ def _normalize_config_path(config_path: str | None, repo_path: Path | None) -> s _CWD_LOCK = threading.Lock() +def _at_rest_cwd() -> Path: + """The process cwd with no chdir window in flight. + + A relative ``repo_path`` or ``config_path``, and the git check for a call + without ``repo_path``, must resolve against the directory the server was + started in, not against whatever another worker thread has temporarily + switched to. Taking the lock guarantees no window is open. Call it before + entering :func:`_working_directory`, never inside (the lock is not + re-entrant). + """ + with _CWD_LOCK: + return Path.cwd() + + @contextmanager def _working_directory(repo_path: Path | None): """Temporarily switch working directory for repo-relative config and git checks.""" diff --git a/tests/test_cwd_at_rest.py b/tests/test_cwd_at_rest.py new file mode 100644 index 0000000..f751a37 --- /dev/null +++ b/tests/test_cwd_at_rest.py @@ -0,0 +1,167 @@ +"""cwd-dependent reads that happen before a tool enters ``_working_directory``. + +Another thread is parked inside ``_working_directory(other)``, so the process +cwd is ``other`` and the lock is held. A call that resolves a relative path or +checks the git repository for ``repo_path=None`` must still see the directory +the server was started in, otherwise it validates against a repository it was +never asked about. +""" + +from __future__ import annotations + +import os +import subprocess +import threading +from pathlib import Path + +import pytest +from mcp.server.mcpserver.exceptions import ToolError + +from commit_check_mcp import server + + +def _git_repo(root: Path, name: str) -> Path: + repo = root / name + repo.mkdir() + env = { + **os.environ, + "GIT_AUTHOR_NAME": "T", + "GIT_AUTHOR_EMAIL": "t@example.com", + "GIT_COMMITTER_NAME": "T", + "GIT_COMMITTER_EMAIL": "t@example.com", + } + + def git(*args: str) -> None: + subprocess.run(["git", *args], cwd=repo, check=True, capture_output=True, env=env) + + git("init", "-q", "-b", "main") + (repo / "f").write_text("x") + git("add", "f") + git("commit", "-q", "-m", "feat: init") + return repo + + +class _Parked: + """Hold ``_working_directory(repo)`` open on another thread until released.""" + + def __init__(self, repo: Path) -> None: + self.repo = repo + self.inside = threading.Event() + self.release = threading.Event() + self.thread = threading.Thread(target=self._run, daemon=True) + + def _run(self) -> None: + with server._working_directory(self.repo): + self.inside.set() + self.release.wait(30) + + def __enter__(self) -> _Parked: + self.thread.start() + assert self.inside.wait(5) + return self + + def __exit__(self, *exc: object) -> None: + self.release.set() + self.thread.join(5) + + +def test_relative_config_path_without_repo_path_uses_the_at_rest_cwd( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + home = tmp_path / "home" + home.mkdir() + (home / "cchk.toml").write_text("[commit]\nsubject_max_length = 10\n") + other = tmp_path / "other" + other.mkdir() + (other / "cchk.toml").write_text("[commit]\nsubject_max_length = 99\n") + monkeypatch.chdir(home) + parked = _Parked(other) + parked.__enter__() + try: + resolver = threading.Thread( + target=lambda: results.append(server._normalize_config_path("cchk.toml", None)), + daemon=True, + ) + results: list[str | None] = [] + resolver.start() + resolver.join(0.5) + assert resolver.is_alive(), "should be waiting for the parked chdir window to close" + finally: + parked.__exit__() + resolver.join(5) + assert results == [str(home / "cchk.toml")] + + +def test_relative_repo_path_uses_the_at_rest_cwd( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + home = tmp_path / "home" + (home / "sub").mkdir(parents=True) + other = tmp_path / "other" + (other / "sub").mkdir(parents=True) + monkeypatch.chdir(home) + results: list[Path | None] = [] + with _Parked(other): + t = threading.Thread( + target=lambda: results.append(server._normalize_repo_path("sub")), daemon=True + ) + t.start() + t.join(0.5) + assert t.is_alive(), "should be waiting for the parked chdir window to close" + t.join(5) + assert results == [home / "sub"] + + +def test_require_git_repo_without_repo_path_checks_the_at_rest_cwd( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + plain = tmp_path / "plain" + plain.mkdir() + other = _git_repo(tmp_path, "other") + monkeypatch.chdir(plain) + results: list[BaseException | None] = [] + + def check() -> None: + try: + server._require_git_repo(None) + except ToolError as e: + results.append(e) + else: + results.append(None) + + with _Parked(other): + t = threading.Thread(target=check, daemon=True) + t.start() + t.join(0.5) + assert t.is_alive(), "should be waiting for the parked chdir window to close" + t.join(10) + assert isinstance(results[0], ToolError) + + +def test_tool_with_relative_config_path_loads_the_right_file( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """End to end: config_path='cchk.toml' with no repo_path, while another + call is inside its chdir window, loads the file next to the server.""" + home = tmp_path / "home" + home.mkdir() + (home / "cchk.toml").write_text("[commit]\nsubject_max_length = 100\n") + other = tmp_path / "other" + other.mkdir() + (other / "cchk.toml").write_text("[commit]\nsubject_max_length = 5\n") + monkeypatch.chdir(home) + results: list[dict] = [] + + def call() -> None: + results.append( + server.validate_commit_message("feat: a long enough subject", config_path="cchk.toml") + ) + + with _Parked(other): + t = threading.Thread(target=call, daemon=True) + t.start() + t.join(0.5) + assert t.is_alive(), "should be waiting for the parked chdir window to close" + t.join(10) + sub = next(c for c in results[0]["checks"] if c["check"] == "subject_max_length") + assert sub["status"] == "pass", sub