Skip to content

Stop advertising MCP_* env vars for MCPServer settings; drop pydantic-settings - #3170

Merged
maxisbey merged 3 commits into
mainfrom
remove-dead-mcpserver-env-settings
Jul 25, 2026
Merged

Stop advertising MCP_* env vars for MCPServer settings; drop pydantic-settings#3170
maxisbey merged 3 commits into
mainfrom
remove-dead-mcpserver-env-settings

Conversation

@maxisbey

@maxisbey maxisbey commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

The Settings docstring and docs/get-started/installation.md claim MCPServer can be configured through MCP_* environment variables and a .env file ("MCP_DEBUG=true will set debug=True"). It never worked: MCPServer.__init__ passes every field to Settings(...) 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 both main and v1.x.

Rather than start honouring ambient environment/.env state now, this deletes the claim and the machinery behind it:

  • Settings becomes a plain pydantic BaseModel (still exposed as mcp.settings, still validates log_level etc.).
  • pydantic-settings is dropped from the SDK's runtime dependencies. It moves to the dev group only because two examples import it.
  • Installation docs no longer list it; docs/migration.md gains a note showing how to read the environment yourself and pass values to the constructor.

Motivation and Context

Reported: MCP_DEBUG=true does 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 .env in 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-Dist no longer lists pydantic-settings, the package imports without it, and an MCPServer served a tool call end-to-end through Client(server) with MCP_DEBUG=true and a .env file present (both ignored, as before; explicit constructor args still honoured; an invalid log_level still raises a ValidationError).

Breaking Changes

pydantic-settings is no longer installed as a transitive dependency of mcp; projects using it for their own settings must depend on it directly. Settings no longer subclasses pydantic_settings.BaseSettings. Documented in docs/migration.md.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

AI Disclaimer

…-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.
@github-actions

github-actions Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

📚 Documentation preview

Preview https://pr-3170.mcp-python-docs.pages.dev
Deployment https://30454573.mcp-python-docs.pages.dev
Commit 166475c
Triggered by @maxisbey
Updated 2026-07-25 21:56:32 UTC

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 5 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread pyproject.toml

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread pyproject.toml
…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.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — and thanks for addressing the earlier text_me.py feedback by removing the example rather than carrying the dependency.

What was reviewed

  • The Settings base-class swap (BaseSettingsBaseModel): behavior-neutral since MCPServer.__init__ passes every field explicitly and init kwargs already outranked the environment.
  • Repo-wide sweep for leftover pydantic_settings imports/references — none remain outside docs/migration.md; the cli extra still declares python-dotenv directly, 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.md with 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.

@maxisbey
maxisbey merged commit 3212591 into main Jul 25, 2026
37 checks passed
@maxisbey
maxisbey deleted the remove-dead-mcpserver-env-settings branch July 25, 2026 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant