Skip to content

Commit f2bdbb7

Browse files
maxisbeymaxisbey
committed
Trim to the test-suite guard; let the JSON layer own the config's encoding
- Drop the -X warn_default_encoding wrappers from the two script steps and the pre-commit hook: the pytest step and scripts/test are the guard, the scripts' own reads are pinned, and an interpreter flag every entry point has to remember has no owner. The README script still fails closed. - mcp install: json.loads(config_file.read_bytes()), so BOM'd UTF-8 and the UTF-16 that PowerShell 5's `>` writes are detected by the stdlib rather than by a codec choice here; a separate test covers both, and test_other_servers_preserved goes back to stating one behaviour. - tests/test_types.py: run the child with -X utf8 and decode strictly, since its stdout is the assertion subject. - AGENTS.md: the targeted-coverage snippet sets PYTHONWARNDEFAULTENCODING=1 like scripts/test, and the rule names encoding="locale" as the deliberate escape hatch.
1 parent 9572a85 commit f2bdbb7

7 files changed

Lines changed: 25 additions & 15 deletions

File tree

‎.github/workflows/shared.yml‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,10 @@ jobs:
4141
with:
4242
extra_args: --all-files --verbose
4343

44-
# -X warn_default_encoding -W error::EncodingWarning: reject locale-dependent text I/O in the script (PEP 597).
4544
- name: Surface types match vendored schema
4645
run: |
4746
uv sync --group codegen --frozen
48-
uv run --frozen --group codegen python -X warn_default_encoding -W error::EncodingWarning scripts/gen_surface_types.py --check
47+
uv run --frozen --group codegen python scripts/gen_surface_types.py --check
4948
5049
# Resolves only mcp-types' declared dependencies into an empty environment,
5150
# so an import of the SDK or anything from its stack fails here.
@@ -141,7 +140,7 @@ jobs:
141140
run: uv sync --frozen --all-extras --python 3.10
142141

143142
- name: Check README snippets are up to date
144-
run: uv run --frozen python -X warn_default_encoding -W error::EncodingWarning scripts/update_readme_snippets.py --check
143+
run: uv run --frozen scripts/update_readme_snippets.py --check
145144

146145
# `scripts/docs/build.sh` is the whole gauntlet: build_config.py fails on
147146
# nav entries without a page and pages without a nav entry, `zensical build

‎.pre-commit-config.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ repos:
5656
pass_filenames: false
5757
- id: readme-snippets
5858
name: Check README snippets are up to date
59-
entry: uv run --frozen python -X warn_default_encoding -W error::EncodingWarning scripts/update_readme_snippets.py --check
59+
entry: uv run --frozen python scripts/update_readme_snippets.py --check
6060
language: system
6161
files: ^(README\.md|docs_src/.*\.py|scripts/update_readme_snippets\.py)$
6262
pass_filenames: false

‎AGENTS.md‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,11 @@
4646
dependencies and obscure circular-import bugs. Only exception: when a
4747
top-level import genuinely can't work (lazy-loading optional deps, or
4848
tests that re-import a module).
49-
- Always pass `encoding=` (normally `"utf-8"`) to text-mode `open()`,
50-
`Path.read_text()`/`write_text()`, `tempfile` and `subprocess` text pipes: the
51-
default is the process locale, not UTF-8. CI enforces this by running pytest with
52-
`PYTHONWARNDEFAULTENCODING=1` (PEP 597), which makes any omission an error under the `error` filter.
49+
- Always pass `encoding=` to text-mode `open()`, `Path.read_text()`/`write_text()`,
50+
`tempfile` and `subprocess` text pipes — normally `"utf-8"`, or `"locale"` when the
51+
platform encoding is genuinely intended; the default is the process locale, not UTF-8.
52+
CI and `scripts/test` run pytest with `PYTHONWARNDEFAULTENCODING=1` (PEP 597), which
53+
makes any omission an error under the `error` filter.
5354

5455
## Testing
5556

@@ -95,7 +96,7 @@ CI requires 100% (`fail_under = 100`, `branch = true`).
9596

9697
```bash
9798
uv run --frozen coverage erase
98-
uv run --frozen coverage run -m pytest tests/path/test_foo.py
99+
PYTHONWARNDEFAULTENCODING=1 uv run --frozen coverage run -m pytest tests/path/test_foo.py
99100
uv run --frozen coverage combine
100101
uv run --frozen coverage report --include='src/mcp/path/foo.py' --fail-under=0
101102
# UV_FROZEN=1 propagates --frozen to the uv subprocess strict-no-cover spawns

‎pyproject.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ filterwarnings = [
267267
# legacy interaction/transport tests still drive it.
268268
"ignore:ping is removed as of 2026-07-28.*:mcp.MCPDeprecationWarning",
269269
# CI and scripts/test set PYTHONWARNDEFAULTENCODING=1, so "error" rejects any text I/O
270-
# of ours that omits encoding=; pytest-examples' own Popen(universal_newlines=True) isn't ours.
270+
# of ours that omits encoding=; pytest-examples' own unguarded text I/O isn't ours.
271271
"ignore:'encoding' argument not specified:EncodingWarning:pytest_examples",
272272
]
273273

‎src/mcp/cli/claude.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ def update_claude_config(
103103
return False
104104

105105
try:
106-
config = json.loads(config_file.read_text(encoding="utf-8-sig"))
106+
config = json.loads(config_file.read_bytes())
107107
if "mcpServers" not in config:
108108
config["mcpServers"] = {}
109109

‎tests/cli/test_claude.py‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,10 @@ def test_existing_env_vars_preserved_without_new(config_dir: Path):
141141

142142

143143
def test_other_servers_preserved(config_dir: Path):
144-
"""Installing a new server must not clobber existing entries, non-ASCII text included (UTF-8, BOM tolerated)."""
144+
"""Installing a new server must not clobber existing entries, non-ASCII text included (the file is UTF-8)."""
145145
other = {"command": "C:\\Users\\张伟\\uv.exe", "env": {"CITY": "Zürich"}}
146146
config_file = config_dir / "claude_desktop_config.json"
147-
config_file.write_text(json.dumps({"mcpServers": {"文件": other}}, ensure_ascii=False), encoding="utf-8-sig")
147+
config_file.write_text(json.dumps({"mcpServers": {"文件": other}}, ensure_ascii=False), encoding="utf-8")
148148

149149
assert update_claude_config(file_spec="s.py:app", server_name="s")
150150

@@ -153,6 +153,17 @@ def test_other_servers_preserved(config_dir: Path):
153153
assert config["mcpServers"]["文件"] == other
154154

155155

156+
@pytest.mark.parametrize("codec", ["utf-8-sig", "utf-16"])
157+
def test_existing_config_with_a_bom_is_accepted(config_dir: Path, codec: str):
158+
"""A config saved by Windows tooling (UTF-8 with BOM, or PowerShell 5's UTF-16 `>`) can still be installed into."""
159+
config_file = config_dir / "claude_desktop_config.json"
160+
config_file.write_bytes(json.dumps({"mcpServers": {"other": {"command": "x"}}}).encode(codec))
161+
162+
assert update_claude_config(file_spec="s.py:app", server_name="s")
163+
164+
assert set(json.loads(config_file.read_bytes())["mcpServers"]) == {"other", "s"}
165+
166+
156167
def test_raises_when_config_dir_missing(monkeypatch: pytest.MonkeyPatch):
157168
"""Should raise RuntimeError when Claude Desktop config dir can't be found."""
158169
monkeypatch.setattr("mcp.cli.claude.get_claude_config_path", lambda: None)

‎tests/test_types.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -495,10 +495,9 @@ def test_bare_import_mcp_binds_the_types_submodule():
495495
# A regression hangs forever, so the bound only has to beat never (matches the suite's
496496
# other subprocess.run calls).
497497
result = subprocess.run(
498-
[sys.executable, "-c", "import mcp; print(mcp.types.Tool.__name__)"],
498+
[sys.executable, "-X", "utf8", "-c", "import mcp; print(mcp.types.Tool.__name__)"],
499499
capture_output=True,
500500
encoding="utf-8",
501-
errors="replace",
502501
check=False,
503502
timeout=20,
504503
)

0 commit comments

Comments
 (0)