From 33745c898e7351df3590ff149245e0bc31adef6f Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Thu, 9 Apr 2026 13:41:45 +0000 Subject: [PATCH 1/6] docs: modernize CLAUDE.md development guidelines Remove outdated and generic guidance, add project-specific context that prevents real review churn. Removed: - Error Resolution section (generic 'add None checks / test thoroughly') - Ruff line-wrapping advice (ruff format handles it) and wrong '88 chars' - Stale pre-commit guidance ('update config rev' - ruff is repo: local) - Reported-by / Github-Issue commit trailers (unused in practice) - Duplicate co-authored-by note and generic PR prose - Generic Code Quality bullets ('focused and small', 'follow patterns') Added: - Branching Model: main is the V2 rework, no @deprecated shims, README.md is frozen (edit README.v2.md), [v1.x] backport prefix - Dependency floor policy: don't raise floors for CVEs alone (refs Kludex/uvicorn#2643, #1552) - __all__ in src/mcp/__init__.py defines the public API surface - Avoid new pragma/type:ignore/noqa; assert isinstance in tests; audit grep - filterwarnings=['error'] context for the no-silence-warnings rule - --frozen on all uv commands; --python for cross-version testing - Tests: in-memory > threads > subprocesses; Client(server) for E2E Restructured numbered-list-in-list into flat H2 sections with a dedicated Coverage subsection. --- CLAUDE.md | 242 ++++++++++++++++++++++-------------------------------- 1 file changed, 100 insertions(+), 142 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2eee085e1..844221c99 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,93 +1,106 @@ # Development Guidelines -This document contains critical information about working with this codebase. Follow these guidelines precisely. - -## Core Development Rules - -1. Package Management - - ONLY use uv, NEVER pip - - Installation: `uv add ` - - Running tools: `uv run ` - - Upgrading: `uv lock --upgrade-package ` - - FORBIDDEN: `uv pip install`, `@latest` syntax - -2. Code Quality - - Type hints required for all code - - Public APIs must have docstrings - - Functions must be focused and small - - Follow existing patterns exactly - - Line length: 120 chars maximum - - FORBIDDEN: imports inside functions. THEY SHOULD BE AT THE TOP OF THE FILE. - -3. Testing Requirements - - Framework: `uv run --frozen pytest` - - Async testing: use anyio, not asyncio - - Do not use `Test` prefixed classes, use functions - - Coverage: test edge cases and errors - - New features require tests - - Bug fixes require regression tests - - IMPORTANT: The `tests/client/test_client.py` is the most well designed test file. Follow its patterns. - - IMPORTANT: Be minimal, and focus on E2E tests: Use the `mcp.client.Client` whenever possible. - - Coverage: CI requires 100% (`fail_under = 100`, `branch = true`). - - Full check: `./scripts/test` (~23s). Runs coverage + `strict-no-cover` on the - default Python. Not identical to CI: CI also runs 3.10–3.14 × {ubuntu, windows}, - and some branch-coverage quirks only surface on specific matrix entries. - - Targeted check while iterating (~4s, deterministic): - - ```bash - uv run --frozen coverage erase - uv run --frozen coverage run -m pytest tests/path/test_foo.py - uv run --frozen coverage combine - uv run --frozen coverage report --include='src/mcp/path/foo.py' --fail-under=0 - UV_FROZEN=1 uv run --frozen strict-no-cover - ``` - - Partial runs can't hit 100% (coverage tracks `tests/` too), so `--fail-under=0` - and `--include` scope the report. `strict-no-cover` has no false positives on - partial runs — if your new test executes a line marked `# pragma: no cover`, - even a single-file run catches it. - - Coverage pragmas: - - `# pragma: no cover` — line is never executed. CI's `strict-no-cover` fails if - it IS executed. When your test starts covering such a line, remove the pragma. - - `# pragma: lax no cover` — excluded from coverage but not checked by - `strict-no-cover`. Use for lines covered on some platforms/versions but not - others. - - `# pragma: no branch` — excludes branch arcs only. coverage.py misreports the - `->exit` arc for nested `async with` on Python 3.11+ (worse on 3.14/Windows). - - Avoid `anyio.sleep()` with a fixed duration to wait for async operations. Instead: - - Use `anyio.Event` — set it in the callback/handler, `await event.wait()` in the test - - For stream messages, use `await stream.receive()` instead of `sleep()` + `receive_nowait()` - - Exception: `sleep()` is appropriate when testing time-based features (e.g., timeouts) - - Wrap indefinite waits (`event.wait()`, `stream.receive()`) in `anyio.fail_after(5)` to prevent hangs - -Test files mirror the source tree: `src/mcp/client/streamable_http.py` → `tests/client/test_streamable_http.py` -Add tests to the existing file for that module. - -- For commits fixing bugs or adding features based on user reports add: +## Branching Model + + + +- `main` is currently the V2 rework. Breaking changes are expected here — when removing or + replacing an API, delete it outright and document the change in + `docs/migration.md`. Do not add `@deprecated` shims or backward-compat layers + on `main`. +- `v1.x` is the release branch for the current stable line. Backport PRs target + this branch and use a `[v1.x]` title prefix. +- `README.md` is frozen at v1 (a pre-commit hook rejects edits). Edit + `README.v2.md` instead. + +## Package Management + +- ONLY use uv, NEVER pip +- Installation: `uv add ` +- Running tools: `uv run --frozen `. Always pass `--frozen` so uv doesn't + rewrite `uv.lock` as a side effect. +- Cross-version testing: `uv run --frozen --python 3.10 pytest ...` to run + against a specific interpreter (CI covers 3.10–3.14). +- Upgrading: `uv lock --upgrade-package ` +- FORBIDDEN: `uv pip install`, `@latest` syntax +- Don't raise dependency floors for CVEs alone. The `>=` constraint already + lets users upgrade. Only raise a floor when the SDK needs functionality from + the newer version, and don't add SDK code to work around a dependency's + vulnerability. See Kludex/uvicorn#2643 and python-sdk #1552 for reasoning. + +## Code Quality + +- Type hints required for all code +- Public APIs must have docstrings +- `src/mcp/__init__.py` defines the public API surface via `__all__`. Adding a + symbol there is a deliberate API decision, not a convenience re-export. +- IMPORTANT: All imports go at the top of the file — inline imports hide + dependencies and obscure circular-import bugs. Only exception: when a + top-level import genuinely can't work (lazy-loading optional deps, or + tests that re-import a module). + +## Testing + +- Framework: `uv run --frozen pytest` +- Async testing: use anyio, not asyncio +- Do not use `Test` prefixed classes, use functions +- IMPORTANT: Tests should be fast and deterministic. Prefer in-memory async execution; + reach for threads only when necessary, and subprocesses only as a last resort. +- For end-to-end behavior, an in-memory `Client(server)` is usually the + cleanest approach (see `tests/client/test_client.py` for the canonical + pattern). For narrower changes, testing the function directly is fine. Use + judgment. +- Test files mirror the source tree: `src/mcp/client/streamable_http.py` → + `tests/client/test_streamable_http.py`. Add tests to the existing file for that module. +- Avoid `anyio.sleep()` with a fixed duration to wait for async operations. Instead: + - Use `anyio.Event` — set it in the callback/handler, `await event.wait()` in the test + - For stream messages, use `await stream.receive()` instead of `sleep()` + `receive_nowait()` + - Exception: `sleep()` is appropriate when testing time-based features (e.g., timeouts) +- Wrap indefinite waits (`event.wait()`, `stream.receive()`) in `anyio.fail_after(5)` to prevent hangs +- Pytest is configured with `filterwarnings = ["error"]`, so warnings fail + tests. Don't silence them with `filterwarnings` or `warnings.catch_warnings()`; + fix the underlying cause instead. + +### Coverage + +CI requires 100% (`fail_under = 100`, `branch = true`). + +- Full check: `./scripts/test` (~23s). Runs coverage + `strict-no-cover` on the + default Python. Not identical to CI: CI also runs 3.10–3.14 × {ubuntu, windows}, + and some branch-coverage quirks only surface on specific matrix entries. +- Targeted check while iterating (~4s, deterministic): ```bash - git commit --trailer "Reported-by:" + uv run --frozen coverage erase + uv run --frozen coverage run -m pytest tests/path/test_foo.py + uv run --frozen coverage combine + uv run --frozen coverage report --include='src/mcp/path/foo.py' --fail-under=0 + UV_FROZEN=1 uv run --frozen strict-no-cover ``` - Where `` is the name of the user. + Partial runs can't hit 100% (coverage tracks `tests/` too), so `--fail-under=0` + and `--include` scope the report. `strict-no-cover` has no false positives on + partial runs — if your new test executes a line marked `# pragma: no cover`, + even a single-file run catches it. -- For commits related to a Github issue, add +Avoid adding new `# pragma: no cover`, `# type: ignore`, or `# noqa` comments. +In tests, use `assert isinstance(x, T)` to narrow types instead of +`# type: ignore`. In library code (`src/`), a `# pragma: no cover` needs very +good reasoning — it usually means a test is missing. Audit before pushing: - ```bash - git commit --trailer "Github-Issue:#" - ``` - -- NEVER ever mention a `co-authored-by` or similar aspects. In particular, never - mention the tool used to create the commit message or PR. +```bash +git diff origin/main... | grep -E '^\+.*(pragma|type: ignore|noqa)' +``` -## Pull Requests +What the existing pragmas mean: -- Create a detailed message of what changed. Focus on the high level description of - the problem it tries to solve, and how it is solved. Don't go into the specifics of the - code unless it adds clarity. - -- NEVER ever mention a `co-authored-by` or similar aspects. In particular, never - mention the tool used to create the commit message or PR. +- `# pragma: no cover` — line is never executed. CI's `strict-no-cover` fails if + it IS executed. When your test starts covering such a line, remove the pragma. +- `# pragma: lax no cover` — excluded from coverage but not checked by + `strict-no-cover`. Use for lines covered on some platforms/versions but not + others. +- `# pragma: no branch` — excludes branch arcs only. coverage.py misreports the + `->exit` arc for nested `async with` on Python 3.11+ (worse on 3.14/Windows). ## Breaking Changes @@ -100,68 +113,13 @@ When making breaking changes, document them in `docs/migration.md`. Include: Search for related sections in the migration guide and group related changes together rather than adding new standalone sections. -## Python Tools - -## Code Formatting - -1. Ruff - - Format: `uv run --frozen ruff format .` - - Check: `uv run --frozen ruff check .` - - Fix: `uv run --frozen ruff check . --fix` - - Critical issues: - - Line length (88 chars) - - Import sorting (I001) - - Unused imports - - Line wrapping: - - Strings: use parentheses - - Function calls: multi-line with proper indent - - Imports: try to use a single line - -2. Type Checking - - Tool: `uv run --frozen pyright` - - Requirements: - - Type narrowing for strings - - Version warnings can be ignored if checks pass - -3. Pre-commit - - Config: `.pre-commit-config.yaml` - - Runs: on git commit - - Tools: Prettier (YAML/JSON), Ruff (Python) - - Ruff updates: - - Check PyPI versions - - Update config rev - - Commit config first - -## Error Resolution - -1. CI Failures - - Fix order: - 1. Formatting - 2. Type errors - 3. Linting - - Type errors: - - Get full line context - - Check Optional types - - Add type narrowing - - Verify function signatures - -2. Common Issues - - Line length: - - Break strings with parentheses - - Multi-line function calls - - Split imports - - Types: - - Add None checks - - Narrow string types - - Match existing patterns - -3. Best Practices - - Check git status before commits - - Run formatters before type checks - - Keep changes minimal - - Follow existing patterns - - Document public APIs - - Test thoroughly +## Formatting & Type Checking + +- Format: `uv run --frozen ruff format .` +- Lint: `uv run --frozen ruff check . --fix` +- Type check: `uv run --frozen pyright` +- Pre-commit runs all of the above plus markdownlint, a `uv.lock` consistency + check, and README checks — see `.pre-commit-config.yaml` ## Exception Handling From b8895ae6fbd1937436607da1d0b160271966d213 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Thu, 9 Apr 2026 13:41:59 +0000 Subject: [PATCH 2/6] docs: rename CLAUDE.md to AGENTS.md with import stub AGENTS.md is the emerging tool-agnostic convention (agents.md spec) and is already canonical in the spec repo, kotlin-sdk, ruby-sdk, go-sdk, and inspector. python-sdk and typescript-sdk were the remaining outliers. CLAUDE.md becomes a one-line @AGENTS.md import so Claude Code continues to load the guidelines (Claude Code reads CLAUDE.md, not AGENTS.md natively). Using @-import rather than a symlink for Windows checkout compatibility, matching ruby-sdk and inspector. --- AGENTS.md | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++++ CLAUDE.md | 133 +----------------------------------------------------- 2 files changed, 133 insertions(+), 132 deletions(-) create mode 100644 AGENTS.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..844221c99 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,132 @@ +# Development Guidelines + +## Branching Model + + + +- `main` is currently the V2 rework. Breaking changes are expected here — when removing or + replacing an API, delete it outright and document the change in + `docs/migration.md`. Do not add `@deprecated` shims or backward-compat layers + on `main`. +- `v1.x` is the release branch for the current stable line. Backport PRs target + this branch and use a `[v1.x]` title prefix. +- `README.md` is frozen at v1 (a pre-commit hook rejects edits). Edit + `README.v2.md` instead. + +## Package Management + +- ONLY use uv, NEVER pip +- Installation: `uv add ` +- Running tools: `uv run --frozen `. Always pass `--frozen` so uv doesn't + rewrite `uv.lock` as a side effect. +- Cross-version testing: `uv run --frozen --python 3.10 pytest ...` to run + against a specific interpreter (CI covers 3.10–3.14). +- Upgrading: `uv lock --upgrade-package ` +- FORBIDDEN: `uv pip install`, `@latest` syntax +- Don't raise dependency floors for CVEs alone. The `>=` constraint already + lets users upgrade. Only raise a floor when the SDK needs functionality from + the newer version, and don't add SDK code to work around a dependency's + vulnerability. See Kludex/uvicorn#2643 and python-sdk #1552 for reasoning. + +## Code Quality + +- Type hints required for all code +- Public APIs must have docstrings +- `src/mcp/__init__.py` defines the public API surface via `__all__`. Adding a + symbol there is a deliberate API decision, not a convenience re-export. +- IMPORTANT: All imports go at the top of the file — inline imports hide + dependencies and obscure circular-import bugs. Only exception: when a + top-level import genuinely can't work (lazy-loading optional deps, or + tests that re-import a module). + +## Testing + +- Framework: `uv run --frozen pytest` +- Async testing: use anyio, not asyncio +- Do not use `Test` prefixed classes, use functions +- IMPORTANT: Tests should be fast and deterministic. Prefer in-memory async execution; + reach for threads only when necessary, and subprocesses only as a last resort. +- For end-to-end behavior, an in-memory `Client(server)` is usually the + cleanest approach (see `tests/client/test_client.py` for the canonical + pattern). For narrower changes, testing the function directly is fine. Use + judgment. +- Test files mirror the source tree: `src/mcp/client/streamable_http.py` → + `tests/client/test_streamable_http.py`. Add tests to the existing file for that module. +- Avoid `anyio.sleep()` with a fixed duration to wait for async operations. Instead: + - Use `anyio.Event` — set it in the callback/handler, `await event.wait()` in the test + - For stream messages, use `await stream.receive()` instead of `sleep()` + `receive_nowait()` + - Exception: `sleep()` is appropriate when testing time-based features (e.g., timeouts) +- Wrap indefinite waits (`event.wait()`, `stream.receive()`) in `anyio.fail_after(5)` to prevent hangs +- Pytest is configured with `filterwarnings = ["error"]`, so warnings fail + tests. Don't silence them with `filterwarnings` or `warnings.catch_warnings()`; + fix the underlying cause instead. + +### Coverage + +CI requires 100% (`fail_under = 100`, `branch = true`). + +- Full check: `./scripts/test` (~23s). Runs coverage + `strict-no-cover` on the + default Python. Not identical to CI: CI also runs 3.10–3.14 × {ubuntu, windows}, + and some branch-coverage quirks only surface on specific matrix entries. +- Targeted check while iterating (~4s, deterministic): + + ```bash + uv run --frozen coverage erase + uv run --frozen coverage run -m pytest tests/path/test_foo.py + uv run --frozen coverage combine + uv run --frozen coverage report --include='src/mcp/path/foo.py' --fail-under=0 + UV_FROZEN=1 uv run --frozen strict-no-cover + ``` + + Partial runs can't hit 100% (coverage tracks `tests/` too), so `--fail-under=0` + and `--include` scope the report. `strict-no-cover` has no false positives on + partial runs — if your new test executes a line marked `# pragma: no cover`, + even a single-file run catches it. + +Avoid adding new `# pragma: no cover`, `# type: ignore`, or `# noqa` comments. +In tests, use `assert isinstance(x, T)` to narrow types instead of +`# type: ignore`. In library code (`src/`), a `# pragma: no cover` needs very +good reasoning — it usually means a test is missing. Audit before pushing: + +```bash +git diff origin/main... | grep -E '^\+.*(pragma|type: ignore|noqa)' +``` + +What the existing pragmas mean: + +- `# pragma: no cover` — line is never executed. CI's `strict-no-cover` fails if + it IS executed. When your test starts covering such a line, remove the pragma. +- `# pragma: lax no cover` — excluded from coverage but not checked by + `strict-no-cover`. Use for lines covered on some platforms/versions but not + others. +- `# pragma: no branch` — excludes branch arcs only. coverage.py misreports the + `->exit` arc for nested `async with` on Python 3.11+ (worse on 3.14/Windows). + +## Breaking Changes + +When making breaking changes, document them in `docs/migration.md`. Include: + +- What changed +- Why it changed +- How to migrate existing code + +Search for related sections in the migration guide and group related changes together +rather than adding new standalone sections. + +## Formatting & Type Checking + +- Format: `uv run --frozen ruff format .` +- Lint: `uv run --frozen ruff check . --fix` +- Type check: `uv run --frozen pyright` +- Pre-commit runs all of the above plus markdownlint, a `uv.lock` consistency + check, and README checks — see `.pre-commit-config.yaml` + +## Exception Handling + +- **Always use `logger.exception()` instead of `logger.error()` when catching exceptions** + - Don't include the exception in the message: `logger.exception("Failed")` not `logger.exception(f"Failed: {e}")` +- **Catch specific exceptions** where possible: + - File ops: `except (OSError, PermissionError):` + - JSON: `except json.JSONDecodeError:` + - Network: `except (ConnectionError, TimeoutError):` +- **FORBIDDEN** `except Exception:` - unless in top-level handlers diff --git a/CLAUDE.md b/CLAUDE.md index 844221c99..43c994c2d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,132 +1 @@ -# Development Guidelines - -## Branching Model - - - -- `main` is currently the V2 rework. Breaking changes are expected here — when removing or - replacing an API, delete it outright and document the change in - `docs/migration.md`. Do not add `@deprecated` shims or backward-compat layers - on `main`. -- `v1.x` is the release branch for the current stable line. Backport PRs target - this branch and use a `[v1.x]` title prefix. -- `README.md` is frozen at v1 (a pre-commit hook rejects edits). Edit - `README.v2.md` instead. - -## Package Management - -- ONLY use uv, NEVER pip -- Installation: `uv add ` -- Running tools: `uv run --frozen `. Always pass `--frozen` so uv doesn't - rewrite `uv.lock` as a side effect. -- Cross-version testing: `uv run --frozen --python 3.10 pytest ...` to run - against a specific interpreter (CI covers 3.10–3.14). -- Upgrading: `uv lock --upgrade-package ` -- FORBIDDEN: `uv pip install`, `@latest` syntax -- Don't raise dependency floors for CVEs alone. The `>=` constraint already - lets users upgrade. Only raise a floor when the SDK needs functionality from - the newer version, and don't add SDK code to work around a dependency's - vulnerability. See Kludex/uvicorn#2643 and python-sdk #1552 for reasoning. - -## Code Quality - -- Type hints required for all code -- Public APIs must have docstrings -- `src/mcp/__init__.py` defines the public API surface via `__all__`. Adding a - symbol there is a deliberate API decision, not a convenience re-export. -- IMPORTANT: All imports go at the top of the file — inline imports hide - dependencies and obscure circular-import bugs. Only exception: when a - top-level import genuinely can't work (lazy-loading optional deps, or - tests that re-import a module). - -## Testing - -- Framework: `uv run --frozen pytest` -- Async testing: use anyio, not asyncio -- Do not use `Test` prefixed classes, use functions -- IMPORTANT: Tests should be fast and deterministic. Prefer in-memory async execution; - reach for threads only when necessary, and subprocesses only as a last resort. -- For end-to-end behavior, an in-memory `Client(server)` is usually the - cleanest approach (see `tests/client/test_client.py` for the canonical - pattern). For narrower changes, testing the function directly is fine. Use - judgment. -- Test files mirror the source tree: `src/mcp/client/streamable_http.py` → - `tests/client/test_streamable_http.py`. Add tests to the existing file for that module. -- Avoid `anyio.sleep()` with a fixed duration to wait for async operations. Instead: - - Use `anyio.Event` — set it in the callback/handler, `await event.wait()` in the test - - For stream messages, use `await stream.receive()` instead of `sleep()` + `receive_nowait()` - - Exception: `sleep()` is appropriate when testing time-based features (e.g., timeouts) -- Wrap indefinite waits (`event.wait()`, `stream.receive()`) in `anyio.fail_after(5)` to prevent hangs -- Pytest is configured with `filterwarnings = ["error"]`, so warnings fail - tests. Don't silence them with `filterwarnings` or `warnings.catch_warnings()`; - fix the underlying cause instead. - -### Coverage - -CI requires 100% (`fail_under = 100`, `branch = true`). - -- Full check: `./scripts/test` (~23s). Runs coverage + `strict-no-cover` on the - default Python. Not identical to CI: CI also runs 3.10–3.14 × {ubuntu, windows}, - and some branch-coverage quirks only surface on specific matrix entries. -- Targeted check while iterating (~4s, deterministic): - - ```bash - uv run --frozen coverage erase - uv run --frozen coverage run -m pytest tests/path/test_foo.py - uv run --frozen coverage combine - uv run --frozen coverage report --include='src/mcp/path/foo.py' --fail-under=0 - UV_FROZEN=1 uv run --frozen strict-no-cover - ``` - - Partial runs can't hit 100% (coverage tracks `tests/` too), so `--fail-under=0` - and `--include` scope the report. `strict-no-cover` has no false positives on - partial runs — if your new test executes a line marked `# pragma: no cover`, - even a single-file run catches it. - -Avoid adding new `# pragma: no cover`, `# type: ignore`, or `# noqa` comments. -In tests, use `assert isinstance(x, T)` to narrow types instead of -`# type: ignore`. In library code (`src/`), a `# pragma: no cover` needs very -good reasoning — it usually means a test is missing. Audit before pushing: - -```bash -git diff origin/main... | grep -E '^\+.*(pragma|type: ignore|noqa)' -``` - -What the existing pragmas mean: - -- `# pragma: no cover` — line is never executed. CI's `strict-no-cover` fails if - it IS executed. When your test starts covering such a line, remove the pragma. -- `# pragma: lax no cover` — excluded from coverage but not checked by - `strict-no-cover`. Use for lines covered on some platforms/versions but not - others. -- `# pragma: no branch` — excludes branch arcs only. coverage.py misreports the - `->exit` arc for nested `async with` on Python 3.11+ (worse on 3.14/Windows). - -## Breaking Changes - -When making breaking changes, document them in `docs/migration.md`. Include: - -- What changed -- Why it changed -- How to migrate existing code - -Search for related sections in the migration guide and group related changes together -rather than adding new standalone sections. - -## Formatting & Type Checking - -- Format: `uv run --frozen ruff format .` -- Lint: `uv run --frozen ruff check . --fix` -- Type check: `uv run --frozen pyright` -- Pre-commit runs all of the above plus markdownlint, a `uv.lock` consistency - check, and README checks — see `.pre-commit-config.yaml` - -## Exception Handling - -- **Always use `logger.exception()` instead of `logger.error()` when catching exceptions** - - Don't include the exception in the message: `logger.exception("Failed")` not `logger.exception(f"Failed: {e}")` -- **Catch specific exceptions** where possible: - - File ops: `except (OSError, PermissionError):` - - JSON: `except json.JSONDecodeError:` - - Network: `except (ConnectionError, TimeoutError):` -- **FORBIDDEN** `except Exception:` - unless in top-level handlers +@AGENTS.md From 740b8f8fb3e875d03096d16732b545a813ea283a Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Thu, 9 Apr 2026 14:05:10 +0000 Subject: [PATCH 3/6] docs: address review feedback on AGENTS.md - Fix test-mirror example to use a path that actually exists (src/mcp/client/stdio.py -> tests/client/test_stdio.py) - Add upstream-library carve-out to the filterwarnings rule, since pyproject.toml already has scoped ignore:: entries for websockets/pywin32 - Document Raises: sections for public APIs that raise catchable exceptions --- AGENTS.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 844221c99..ea97861b0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,7 +31,9 @@ ## Code Quality - Type hints required for all code -- Public APIs must have docstrings +- Public APIs must have docstrings. When a public API raises exceptions a + caller would reasonably catch, document them in a `Raises:` section. Don't + list exceptions from argument validation or programmer error. - `src/mcp/__init__.py` defines the public API surface via `__all__`. Adding a symbol there is a deliberate API decision, not a convenience re-export. - IMPORTANT: All imports go at the top of the file — inline imports hide @@ -50,16 +52,17 @@ cleanest approach (see `tests/client/test_client.py` for the canonical pattern). For narrower changes, testing the function directly is fine. Use judgment. -- Test files mirror the source tree: `src/mcp/client/streamable_http.py` → - `tests/client/test_streamable_http.py`. Add tests to the existing file for that module. +- Test files mirror the source tree: `src/mcp/client/stdio.py` → + `tests/client/test_stdio.py`. Add tests to the existing file for that module. - Avoid `anyio.sleep()` with a fixed duration to wait for async operations. Instead: - Use `anyio.Event` — set it in the callback/handler, `await event.wait()` in the test - For stream messages, use `await stream.receive()` instead of `sleep()` + `receive_nowait()` - Exception: `sleep()` is appropriate when testing time-based features (e.g., timeouts) - Wrap indefinite waits (`event.wait()`, `stream.receive()`) in `anyio.fail_after(5)` to prevent hangs - Pytest is configured with `filterwarnings = ["error"]`, so warnings fail - tests. Don't silence them with `filterwarnings` or `warnings.catch_warnings()`; - fix the underlying cause instead. + tests. Don't silence warnings from your own code; fix the underlying cause. + Scoped `ignore::` entries for upstream libraries are acceptable in + `pyproject.toml` with a comment explaining why. ### Coverage From bf7fcfbb2ef1e6f81336a866aca1d0e7d42a47c4 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Thu, 9 Apr 2026 14:45:03 +0000 Subject: [PATCH 4/6] docs: clarify CI matrix dimensions; drop stale filterwarnings entry --- AGENTS.md | 11 +++++++---- pyproject.toml | 1 - 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ea97861b0..76926f907 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -69,8 +69,10 @@ CI requires 100% (`fail_under = 100`, `branch = true`). - Full check: `./scripts/test` (~23s). Runs coverage + `strict-no-cover` on the - default Python. Not identical to CI: CI also runs 3.10–3.14 × {ubuntu, windows}, - and some branch-coverage quirks only surface on specific matrix entries. + default Python. Not identical to CI: CI runs 3.10–3.14 × {ubuntu, windows} + × {locked, lowest-direct}, and some branch-coverage quirks only surface on + specific matrix entries. To reproduce a `lowest-direct` failure locally, + resync with `uv sync --upgrade --resolution lowest-direct` first. - Targeted check while iterating (~4s, deterministic): ```bash @@ -97,8 +99,9 @@ git diff origin/main... | grep -E '^\+.*(pragma|type: ignore|noqa)' What the existing pragmas mean: -- `# pragma: no cover` — line is never executed. CI's `strict-no-cover` fails if - it IS executed. When your test starts covering such a line, remove the pragma. +- `# pragma: no cover` — line is never executed. CI's `strict-no-cover` (skipped + on Windows runners) fails if it IS executed. When your test starts covering + such a line, remove the pragma. - `# pragma: lax no cover` — excluded from coverage but not checked by `strict-no-cover`. Use for lines covered on some platforms/versions but not others. diff --git a/pyproject.toml b/pyproject.toml index be1200cff..a5d2c3d80 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -185,7 +185,6 @@ filterwarnings = [ # This should be fixed on Uvicorn's side. "ignore::DeprecationWarning:websockets", "ignore:websockets.server.WebSocketServerProtocol is deprecated:DeprecationWarning", - "ignore:Returning str or bytes.*:DeprecationWarning:mcp.server.lowlevel", # pywin32 internal deprecation warning "ignore:getargs.*The 'u' format is deprecated:DeprecationWarning", ] From fd36a52c9241a3b7855cda7e93cf2abf4ccf9e6b Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Thu, 9 Apr 2026 15:11:43 +0000 Subject: [PATCH 5/6] docs: drop lowest-direct repro hint --- AGENTS.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 76926f907..82f539a93 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,8 +71,7 @@ CI requires 100% (`fail_under = 100`, `branch = true`). - Full check: `./scripts/test` (~23s). Runs coverage + `strict-no-cover` on the default Python. Not identical to CI: CI runs 3.10–3.14 × {ubuntu, windows} × {locked, lowest-direct}, and some branch-coverage quirks only surface on - specific matrix entries. To reproduce a `lowest-direct` failure locally, - resync with `uv sync --upgrade --resolution lowest-direct` first. + specific matrix entries. - Targeted check while iterating (~4s, deterministic): ```bash From b1af585a9b20e41be2a030b1849e562057dda181 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Thu, 9 Apr 2026 15:34:44 +0000 Subject: [PATCH 6/6] docs: explain UV_FROZEN=1 in targeted coverage block --- AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AGENTS.md b/AGENTS.md index 82f539a93..969227104 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,6 +79,7 @@ CI requires 100% (`fail_under = 100`, `branch = true`). uv run --frozen coverage run -m pytest tests/path/test_foo.py uv run --frozen coverage combine uv run --frozen coverage report --include='src/mcp/path/foo.py' --fail-under=0 + # UV_FROZEN=1 propagates --frozen to the uv subprocess strict-no-cover spawns UV_FROZEN=1 uv run --frozen strict-no-cover ```