Stop advertising MCP_* env vars for MCPServer settings; drop pydantic-settings - #3170
Conversation
…-settings The Settings docstring and the installation docs claimed MCPServer could be configured through MCP_* environment variables and a .env file, but MCPServer always passes explicit constructor arguments to Settings, and those outrank the environment in pydantic-settings, so e.g. MCP_DEBUG=true has never taken effect. Make Settings a plain pydantic BaseModel and remove pydantic-settings from the runtime dependencies (it moves to the dev group, where the examples still use it). The migration guide explains how to feed environment variables into the constructor for anyone who wants that behaviour.
📚 Documentation preview
|
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
LGTM — removing a documented-but-never-functional config surface is the right call, and the dependency drop, docs, and migration note are all consistent. The one inline note is example-only (text_me.py's PEP 723 block) and doesn't affect the SDK change.
Extended reasoning...
Overview
The PR converts Settings in src/mcp/server/mcpserver/server.py from pydantic_settings.BaseSettings to a plain pydantic BaseModel, drops pydantic-settings from the runtime dependencies (moving it to the dev group for the two examples that import it), and updates docs/get-started/installation.md and docs/migration.md accordingly. uv.lock reflects the dependency move.
Correctness
The claimed no-op nature of the removed machinery checks out: MCPServer.__init__ passes every Settings field explicitly as an init kwarg, and in pydantic-settings init kwargs outrank environment sources, so MCP_* env vars and .env files were read but always overridden. Converting to BaseModel therefore changes no observable behavior — field validation (e.g. the log_level Literal) is plain pydantic and is preserved. A grep confirms no remaining pydantic_settings import anywhere in src/; AuthSettings was already a plain BaseModel, and the CLI does not rely on Settings env behavior. simple-auth self-declares pydantic-settings in its own pyproject.toml; the only loose end is the text_me.py example flagged inline, which is a pre-existing incompleteness in its PEP 723 metadata that this PR turns into a real (example-only) gap.
Security risks
None introduced. If anything this closes a latent footgun: had the env/.env machinery ever started working, a stray .env in the working directory could silently change server behavior. No auth, crypto, or permission code is touched.
Level of scrutiny
Moderate. It is a breaking change (dependency drop + base-class change), but main is the V2 rework where breaking changes are expected and must be documented in docs/migration.md — which this PR does, with a concrete migration snippet. The code delta itself is small and mechanical (one class header and docstring). The author additionally verified the built wheel in a clean venv: Requires-Dist no longer lists pydantic-settings, imports succeed without it, and end-to-end behavior with MCP_DEBUG=true and a .env present is unchanged.
Other factors
The full local suite passes at 100% coverage with strict-no-cover, pyright, and ruff. The single bug-hunt finding is a nit in an example file that was already broken for standalone uv run before this PR; it does not block the SDK change and has a one-line fix. No outstanding reviewer comments on the timeline.
…dependency pydantic-settings was left in the dev group only because two examples used it. The text_me example needs a paid third-party SMS account to run and nothing references or tests it, so delete it. The simple-auth example's two settings classes are always constructed with explicit values or defaults, so plain pydantic models do the same job; drop pydantic-settings from that example's dependencies and from the root dev group.
There was a problem hiding this comment.
LGTM — and thanks for addressing the earlier text_me.py feedback by removing the example rather than carrying the dependency.
What was reviewed
- The
Settingsbase-class swap (BaseSettings→BaseModel): behavior-neutral sinceMCPServer.__init__passes every field explicitly and init kwargs already outranked the environment. - Repo-wide sweep for leftover
pydantic_settingsimports/references — none remain outsidedocs/migration.md; thecliextra still declarespython-dotenvdirectly, so nothing relied on the transitive dep. - simple-auth example: both settings classes are only ever constructed with explicit values or defaults, so the env-prefix removal doesn't change what the example's docs tell users to do.
- Breaking change is documented in
docs/migration.mdwith a working replacement snippet, per repo policy.
Extended reasoning...
Overview
This PR removes a documented-but-nonfunctional configuration surface: MCP_* env vars and .env files never affected MCPServer because the constructor passes every field to Settings(...) explicitly, and pydantic-settings gives init kwargs precedence over the environment. The change converts Settings to a plain pydantic BaseModel, drops pydantic-settings from the SDK's runtime dependencies, deletes the one example that used it without self-declaring it (examples/mcpserver/text_me.py), converts the simple-auth example's two settings classes to plain models, and updates installation/migration docs. Nine files touched; the only src/ change is the Settings class itself.
Security risks
None introduced — this is a dependency removal plus doc changes. If anything it slightly reduces surface: a stray .env or ambient MCP_*/MCP_RESOURCE_* variables can no longer be picked up by the (example) settings classes. The simple-auth changes touch OAuth demo code, but only the settings-class base type, not any auth logic; both classes were constructed with explicit values or defaults, which finder agents raised and verifiers confirmed.
Level of scrutiny
Moderate. It is a breaking change to the published package's dependency set, which normally warrants care — but this repo's main is the V2 rework where breaking changes are expected and must be documented in docs/migration.md, which this PR does with a working replacement snippet. The core correctness claim (env vars never took effect) is directly verifiable from MCPServer.__init__ passing all fields as init kwargs. I verified no pydantic_settings references remain anywhere in the repo, that the cli extra independently declares python-dotenv (previously also a transitive dep of pydantic-settings), and that nothing else in src/ imports Settings.
Other factors
The bug hunting system found no bugs this run. Both prior reviewer comments (cubic's and my own, flagging that keeping text_me.py while moving pydantic-settings to the dev group would break the example for installed-SDK users) were fully addressed by the follow-up commit that deletes the example and drops the dev-group entry — the cleaner of the two suggested fixes. The PR description reports the full local suite at 100% coverage plus a clean-venv wheel install test exercising exactly the regression risks (import without pydantic-settings, env vars still ignored, validation still raising). No CODEOWNERS file exists in the repo.
…ing the dependency" This reverts commit c752b0e.
The
Settingsdocstring anddocs/get-started/installation.mdclaimMCPServercan be configured throughMCP_*environment variables and a.envfile ("MCP_DEBUG=truewill setdebug=True"). It never worked:MCPServer.__init__passes every field toSettings(...)explicitly, and in pydantic-settings init kwargs outrank the environment, so the env vars are read and then overridden. This has been the case since the constructor stopped taking**settings(mid-2025), on bothmainandv1.x.Rather than start honouring ambient environment/
.envstate now, this deletes the claim and the machinery behind it:Settingsbecomes a plain pydanticBaseModel(still exposed asmcp.settings, still validateslog_leveletc.).pydantic-settingsis dropped from the SDK's runtime dependencies. It moves to thedevgroup only because two examples import it.docs/migration.mdgains a note showing how to read the environment yourself and pass values to the constructor.Motivation and Context
Reported:
MCP_DEBUG=truedoes nothing despite the docstring/docs saying otherwise. A documented configuration surface that silently no-ops is worse than none, and a working env override would mean a stray.envin the working directory changing server behaviour. Users who want env-driven config can do it in one line at the call site.How Has This Been Tested?
Full local suite (
./scripts/test, 100% coverage,strict-no-cover), pyright, ruff. Additionally built the wheel and installed it into a clean venv:Requires-Distno longer listspydantic-settings, the package imports without it, and anMCPServerserved a tool call end-to-end throughClient(server)withMCP_DEBUG=trueand a.envfile present (both ignored, as before; explicit constructor args still honoured; an invalidlog_levelstill raises aValidationError).Breaking Changes
pydantic-settingsis no longer installed as a transitive dependency ofmcp; projects using it for their own settings must depend on it directly.Settingsno longer subclassespydantic_settings.BaseSettings. Documented indocs/migration.md.Types of changes
Checklist
Additional context
AI Disclaimer