Repository navigation
fix(mcp): Replace lone surrogates in tool responses - #1790
Merged
Merged
Conversation
Tool formatters cut user text (breadcrumb messages, log attributes, agent prompts, span descriptions) by UTF-16 index. When the cut lands between the halves of a surrogate pair, the response carries a lone high surrogate that serializes as `\ud83d`. Strict JSON parsers such as serde_json in Codex reject that string and drop the transport, so get_issue_breadcrumbs returned only "Transport closed". Add sliceAtCodePointBoundary to toolkit-core, which backs off one code unit when a bound would split a pair (the same approach as the CLI grep worker), and use it at each MCP truncation site that handles user text. Co-Authored-By: Miguel Betegon <miguel.betegon@sentry.io>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Replace the per-site sliceAtCodePointBoundary helper with a single pass in server.ts. Every tool's output (text, structuredContent, and error text) now goes through String.prototype.toWellFormed(), which swaps a lone surrogate for U+FFFD. That covers every truncation site, current and future, and upstream text that is already malformed, without a new toolkit-core export that only MCP uses. Normalize before the structured compatibility text is generated so content and structuredContent stay equivalent. Add ES2024.String to the shared tsconfig lib for the native method (Node 20+, Workers, Bun). Move the breadcrumb regression test to server.test.ts so it exercises the response path end to end. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017M6odTSxZ1TzZm58HvggVB
betegon
changed the title
BYK
added a commit
that referenced
this pull request
Nov 2, 2026
…1798) Follow-up to #1790. `toWellFormedOutput` runs on every tool response, and it rebuilt every array (`map`) and object (`Object.entries` → `map` → `Object.fromEntries`), even when there was nothing to replace. Almost no response contains a lone surrogate, so nearly all of that copying was wasted. It now copies on write. It returns the input unchanged unless a string or key actually changes, and then copies only the arrays and objects on the path to that string. Objects are walked with `for...in` + `Object.hasOwn`, so the common path allocates nothing. Output is the same as before. ## Benchmark Node 22.23, median time per call and bytes allocated per call (large semi-space so no GC runs inside the measured loop). "Structured" rows are issue-shaped objects with 12 fields, a nested project object, and a tags array. | payload | before (#1790) | after | `JSON.stringify` for scale | |---|---|---|---| | string, 45 KB | 52 µs / ~0 B | 52 µs / ~0 B | 229 µs | | `content[]`, 1 × 45 KB text | 53 µs / 0.6 KB | 52 µs / ~0 B | 176 µs | | structured, 50 rows | 376 µs / 210 KB | 42 µs / ~0 B | 89 µs | | structured, 500 rows | 2.37 ms / 1.9 MB | 425 µs / ~0 B | 1.58 ms | | structured, 500 rows + 1 lone surrogate | 2.16 ms / 1.9 MB | 451 µs / 7.4 KB | 1.28 ms | | string, 45 KB + lone surrogate | 107 µs / 123 KB | 109 µs / 123 KB | 163 µs | - On structured output it is about 5× faster and allocates almost nothing; before, it allocated about 1.5× the payload size on every call. - String-only and `content[]` output was already cheap. V8 returns the same string from `toWellFormed()` when the string is well formed, so most of the time left is the scan itself, which can't be skipped. - I also tried running `JSON.stringify` once and walking only when the JSON contains a `\uD8xx`–`\uDFxx` escape. It was slower than the walk in every case, so it isn't used. ## Tests The structured case in `server.test.ts` now has a lone surrogate inside a nested array, next to strings that don't change. That covers the partial-copy path; if the copy drops the entries before the changed one, the test fails. <!-- junior-request-attribution:start --> via @BYK. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:local%3Aweb%3A94b2b90536e955539b32d29f --> -- [View Session in Sentry](https://sentry.sentry.io/explore/conversations/local%3Aweb%3A94b2b90536e955539b32d29f/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: BYK <126780+BYK@users.noreply.github.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
get_issue_breadcrumbsreturned only "Transport closed" in Codex. Breadcrumb messages are cut at 120 UTF-16 code units, so a message with an emoji at index 119 keeps only the emoji's high surrogate.JSON.stringifywrites that lone half as\ud83d, and Codex's JSON parser (serde_json) rejects the whole message and closes the connection.Any
.slice()truncation of user text can do this, and mcp-core truncates in about a dozen places. Rather than patch each one,server.tsnow runs every tool response through the nativeString.prototype.toWellFormed(), which replaces a lone surrogate with U+FFFD:CallToolResult: text, content arrays, andstructuredContent(keys and values of plain objects and arrays). This runs before the structured compatibility text is generated, socontentandstructuredContentstay equivalent.formatErrorForUser.toWellFormed()is ES2024 (Node 20+, Workers, Bun), so the shared tsconfig addsES2024.Stringtolib.docs/contributing/tool-responses.mdnow notes that formatters can truncate with plainslice.Before / after
get_issue_breadcrumbscalled throughexecute_sentry_toolfor an event whose breadcrumb message has 🔒 at index 119 (example message, reproduced locally). This is the breadcrumb line insideresult.content[0].textof the serialized JSON-RPC response.Before:
Codex rejects the response and the call ends in
Transport closed.After:
The response is valid JSON for strict parsers, and the cut emoji shows as
�.Tests
server.test.tscovers the reported case end to end (execute_sentry_tool→get_issue_breadcrumbs, emoji at index 119) and structured and error output. Both tests fail onmainand pass on this branch.Not in this PR
app.search_events.validation.outputspan attribute is truncated the same way, but it goes to Sentry telemetry, not to an MCP client..slice(). Terminal output already prints a lone surrogate as U+FFFD;--jsonoutput would need the same pass inwriteJson.via bete.
--
View Session [Sentry]
https://claude.ai/code/session_017M6odTSxZ1TzZm58HvggVB
Generated by Claude Code