fix(mcp): return text asset content as UTF-8, not base64 - #211
Conversation
asset `get` base64-encoded every asset's content unconditionally, forcing consumers to decode even plain-text markdown/html handoffs (the get -> Write-Host -> b64decode dance). Now returns valid-UTF-8 bytes as-is with `encoding: "utf8"`, and only base64-encodes true binary (`encoding: "base64"`). The `encoding` field lets callers branch on the format instead of blindly decoding. Test: a text/markdown asset round-trips as encoding=utf8 with readable content, including a non-ASCII codepoint.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughInline ChangesInline asset encoding
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mcp_server.rs`:
- Around line 2745-2748: Update the content/encoding selection around
std::str::from_utf8 to first allow text decoding only for MIME types in the
established textual MIME allowlist; return Base64 for binary MIME types even
when their bytes are valid UTF-8. Add a regression test covering a valid-UTF8
.bin asset containing binary bytes and verify it reports "encoding": "base64".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 32e030eb-905b-4766-ad3e-fa97b2671b15
📒 Files selected for processing (1)
src/mcp_server.rs
Binary assets (e.g. application/octet-stream of control bytes) can be
valid UTF-8, so the from_utf8-only discriminator mislabeled them as
encoding=utf8. Gate on a textual-MIME allowlist (text/*, +json/+xml,
application/{json,xml,javascript,typescript,toml,yaml}) AND UTF-8
validity; everything else stays base64.
Adds a regression test: a valid-UTF-8 .bin (octet-stream) asset returns
encoding=base64. Addresses CodeRabbit review on #211.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mcp_server.rs`:
- Around line 5138-5152: Update the test around the parsed `got` response to
assert that `got["content"]` equals the Base64 string `AAECAw==` for the binary
input `[0, 1, 2, 3]`, while preserving the existing `encoding` assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f06321c-0142-4da8-911a-b58652a164a3
📒 Files selected for processing (1)
src/mcp_server.rs
Per CodeRabbit review: the test asserted only encoding=base64, not the content, so a correct label with wrong content would still pass. Assert content == "AAECAw==" (base64 of [0,1,2,3]).
Problem
MCP
assetaction=getbase64-encoded all asset content unconditionally, regardless of mime type. Reading a text/markdown asset (e.g. a handoff) forced every consumer through a decode step — theasset get-> base64-decode dance — even though the bytes were already valid UTF-8.Fix
The
gethandler now returns readable text for textual MIME types whose bytes are valid UTF-8, and reports which encoding it used:text/*,+json/+xml,application/{json,xml,javascript,typescript,toml,yaml}) and valid UTF-8 ->"content"as text with"encoding": "utf8""encoding": "base64"Callers branch on
encodinginstead of blindly decoding. Gating on MIME (not just UTF-8 validity) prevents binary assets that happen to be valid UTF-8 — e.g. anapplication/octet-streamof control bytes — from being mislabeledutf8. The over-inline-limit and read-error branches are unchanged.Tests
asset_get_returns_text_content_as_utf8_not_base64— atext/markdownasset with a non-ASCII codepoint round-trips asencoding: "utf8"with readablecontent.asset_get_returns_base64_for_binary_with_valid_utf8_bytes— a valid-UTF-8.bin(octet-stream) asset returnsencoding: "base64".Full
cargo test --workspacegreen.Compatibility
Breaking change to the tool's output contract: consumers that unconditionally base64-decode
contentnow receive raw text for textual assets. They should switch to checking theencodingfield.Tracked as agentflare item 134 (child of the DX/tooling-friction bucket, item 118) — not a GitHub issue.
Summary by CodeRabbit
encodingindicator (utf8for text,base64otherwise).