Skip to content

fix(mcp): read path-like MCP app documents as UTF-8 - #3608

Open
pcbeingused333 wants to merge 1 commit into
ag2ai:mainfrom
pcbeingused333:fix/mcp-app-document-utf8
Open

pcbeingused333 wants to merge 1 commit into
ag2ai:mainfrom
pcbeingused333:fix/mcp-app-document-utf8

Conversation

@pcbeingused333

Copy link
Copy Markdown
Contributor

Why are these changes needed?

_resolve_content in ag2/mcp/apps.py reads a path-like MCP app document with Path(resolved).read_text() and no encoding, so it uses the host's locale encoding. HTML containing non-ASCII text is mis-decoded (or fails) where the locale is not UTF-8, e.g. Windows cp1252. The except only covered OSError, so a UnicodeDecodeError escaped unwrapped.

Fix: read with encoding="utf-8" and turn UnicodeDecodeError into the same Could not read MCP app document from … ValueError used for other read failures.

Tests (test/mcp/test_apps.py): one simulates a cp1252 locale default and checks UTF-8 content is served intact; one checks a non-UTF-8 file gives the wrapped error. Both fail before the change; test/mcp/test_apps.py is 61 passed after.

Related issue number

None (found while reading the code).

Checks

  • I've included any doc changes needed — not applicable.
  • I've added tests (if relevant) corresponding to the changes introduced in this PR.
  • I've made sure all auto checks have passed.

AI assistance

  • I understand the changes in this PR and can explain them in my own words.
  • I have verified that the PR description accurately reflects the actual diff.
  • If AI assistance was used, I reviewed, tested, and validated the generated code/text before submitting.

This change was prepared with the help of an AI coding assistant (Claude Code) and reviewed before submission.

Path.read_text() without an encoding uses the locale encoding, so HTML
documents containing non-ASCII text were decoded wrongly (or failed) on hosts
whose locale is not UTF-8, e.g. Windows cp1252. Read as UTF-8 and report
undecodable files with the same 'Could not read MCP app document' error
instead of leaking a bare UnicodeDecodeError.
@github-actions github-actions Bot added the area:mcp MCP integration (ag2/mcp) label Nov 7, 2026
@codecov

codecov Bot commented Nov 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
ag2/mcp/apps.py 97.59% <100.00%> (+0.02%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:mcp MCP integration (ag2/mcp)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant