Max Isbey
7fae57664b
docs/advanced/import-cost.md now states the size of every deferred bill (the per-version wire package, the first HTTP app, the first span, cryptography, the OAuth provider models, and the ~130 ms a fresh process pays once on its first connection), and grows an FAQ: an installed pydantic plugin such as logfire still loads at `import mcp.types` (PYDANTIC_DISABLE_PLUGINS is the lever), __pydantic_complete__ reads False until first use, function-local model subclasses keep the generic signature until first use, what to hand a static bundler, and the get_type_hints waiver for the app builders. AGENTS.md's import-cost bullets are corrected to match the code: only mcp/__init__ resolves exports lazily while the other package inits resolve submodules; cryptography and the OAuth provider models load with their first user rather than at import; the wire packages also load for the first rendered elicitation schema. The migration guide notes the removed McpHttpClientFactory re-export from mcp.client.streamable_http.
203 lines
10 KiB
Markdown
203 lines
10 KiB
Markdown
# Development Guidelines
|
||
|
||
## Branching Model
|
||
|
||
- `main` is the current stable line (v2); releases are cut from it (see
|
||
`RELEASE.md`).
|
||
- Removing or replacing an API must be intentional, and what shipped in 2.x
|
||
is public surface. Adding a replacement API or `@deprecated` shim is
|
||
likewise a deliberate design choice, not bolted on for free.
|
||
- Changes that break code written against v1 (including those softened by a
|
||
backwards-compatibility shim) must be documented in `docs/migration.md`.
|
||
- `v1.x` is the maintenance branch for the previous major. Backport PRs
|
||
target it and use a `[v1.x]` title prefix; only critical bug fixes and
|
||
security fixes land there.
|
||
- `README.md` documents v2. The v1 README lives on the `v1.x` branch.
|
||
|
||
## Package Management
|
||
|
||
- ONLY use uv, NEVER pip
|
||
- Installation: `uv add <package>`. Exception: the root project's runtime
|
||
dependencies are dynamic (the published `mcp` wheel exact-pins `mcp-types`),
|
||
so `uv add` cannot edit them — add the requirement to
|
||
`[tool.hatch.metadata.hooks.uv-dynamic-versioning].dependencies` in
|
||
`pyproject.toml` by hand, then run `uv lock`. Dependency groups, extras, and
|
||
the example packages still take plain `uv add`.
|
||
- Running tools: `uv run --frozen <tool>`. 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 <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. 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
|
||
dependencies and obscure circular-import bugs. Exceptions: when a
|
||
top-level import genuinely can't work (lazy-loading optional deps, or
|
||
tests that re-import a module), and the import-cost cases below.
|
||
|
||
### Import cost
|
||
|
||
Startup time is part of the API: `import mcp` is a lazy namespace and each
|
||
entry point only loads what it uses. `tests/test_import_guards.py` pins
|
||
which heavy dependencies each import path may load; a hoisted import that
|
||
breaks one of these fails that test, so keep them off the paths below. Each
|
||
such function-level or lazy import carries a one-line comment saying which
|
||
stack it keeps out of which import path.
|
||
|
||
- `mcp/__init__.py` resolves each of its exports on first access, and the
|
||
`mcp`, `mcp.client`, `mcp.server`, `mcp.server.auth`, `mcp.shared` and
|
||
`mcp.os` package inits resolve their *submodules* on attribute access -
|
||
both through `mcp.shared._lazy` (PEP 562 `__getattr__`, bound under
|
||
`if not TYPE_CHECKING:` so type checkers still flag typos); a bare
|
||
`import mcp` loads no pydantic, no `mcp_types` and no SDK stack.
|
||
- Client code never imports `mcp.server.*`: the in-process transport pieces
|
||
are imported inside the connector that only a live in-process `Server`
|
||
triggers, and `Server`/`MCPServer` are `TYPE_CHECKING`-only names.
|
||
- `httpx2` loads only for the HTTP client transports (`streamable_http`,
|
||
`sse`), on the first URL `Client`; a stdio client never imports it.
|
||
- The web stack (`starlette`, `sse_starlette`, `uvicorn`) loads only when an
|
||
HTTP app is built (`streamable_http_app()`, `sse_app()`, `custom_route()`),
|
||
never from `mcp.server`, the lowlevel `Server`, `MCPServer` or stdio;
|
||
types those signatures name are either defined in web-framework-free
|
||
modules (`mcp.server.event_store`, `mcp.server.transport_security`,
|
||
`mcp.server.auth.access_token`) or spelled through the lazy `mcp.server`
|
||
namespace so `typing.get_type_hints()` still resolves them.
|
||
- `opentelemetry` is imported on the first span; `cryptography` with the
|
||
first request-state codec (`MCPServer(...)` builds one), and the OAuth
|
||
provider models with the first auth-enabled server or authenticated
|
||
request - none of them at `import mcp.server*`; `jwt` only by the
|
||
client-credentials auth extension.
|
||
- The per-version wire packages (`mcp_types._v2025_11_25`, `_v2026_07_28`)
|
||
load on the first message parsed for that protocol version (via the
|
||
`mcp_types.methods` surface maps), on the first rendered elicitation
|
||
schema, or via `mcp.warm(version)` - not on `import mcp_types`.
|
||
- Pydantic models set `defer_build=True` (through `MCPModel`, the generated
|
||
wire bases, or `@deferred_model`): validators build on first use, not at
|
||
import; `mcp.warm()` is the opt-in way to build them up front.
|
||
|
||
## Testing
|
||
|
||
- When writing or reviewing tests, conform to `.claude/skills/test-quality/SKILL.md`
|
||
— it defines the bar for naming, abstraction level, assertions, and determinism.
|
||
- Framework: `uv run --frozen pytest`
|
||
- Async testing: use anyio, not asyncio
|
||
- Do not use `Test` prefixed classes — write plain top-level `test_*` functions.
|
||
Legacy files still contain `Test*` classes; do NOT follow that pattern for new
|
||
tests even when adding to such a file.
|
||
- 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/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 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.
|
||
- New features from the 2026-07-28 spec must have a matching test in the
|
||
[conformance suite](https://github.com/modelcontextprotocol/conformance)
|
||
that passes against this SDK (CI runs it via
|
||
`.github/workflows/conformance.yml`). If no matching test exists, stop and
|
||
tell the user so they can raise an issue on the conformance repo.
|
||
|
||
### 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 runs 3.10–3.14 × {ubuntu, windows}
|
||
× {locked, lowest-direct}, 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 propagates --frozen to the uv subprocess strict-no-cover spawns
|
||
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` (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.
|
||
- `# 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` — including
|
||
changes softened by a backwards-compatibility shim. 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.
|
||
|
||
## Documentation
|
||
|
||
When a change affects public API or user-visible behaviour, update the relevant
|
||
page(s) under `docs/` in the same PR. Docs are organised by the `nav:` sections
|
||
in `mkdocs.yml` (Get started, Servers, Inside your handler, Running your server,
|
||
Clients, Advanced), not by the on-disk directory names. Find the page covering
|
||
the feature you touched in `mkdocs.yml` rather than adding a new one.
|
||
|
||
## 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
|