fix: align mcp transform pipeline with Anthropic Claude Code 2.1.128#417
Merged
claude-code-best merged 1 commit intoclaude-code-best:mainfrom May 6, 2026
Merged
Conversation
Add ImageLimits type and plumb optional limits through the chain: callMCPTool/callMCPToolWithUrlElicitationRetry -> processMCPResult -> transformMCPResult -> transformResultContent -> maybeResizeAndDownsampleImageBuffer. When provided, limits override the module-level defaults (IMAGE_TARGET_RAW_SIZE, IMAGE_MAX_WIDTH, IMAGE_MAX_HEIGHT, API_IMAGE_MAX_BASE64_SIZE) inside maybeResizeAndDownsampleImageBuffer. When undefined, behavior is unchanged for current callers. Add _meta preservation in the text-block case of transformResultContent (only when the caller opts in via includeMeta=true). transformMCPResult passes includeMeta=true on the tool-result path; the prompt-handler call site keeps the default false, preserving prior behavior. Add skipLargeOutput early-return in processMCPResult after the IDE check: when the caller passes skipLargeOutput=true and the content has no images, the function returns content directly without large-output handling. Add unwrap-to-text in processMCPResult for the persisted-content path: when the large-string format gate is enabled (MCP_TRUNCATION_PROMPT_OVERRIDE env var, or tengu_mcp_subagent_prompt Statsig gate), and the content is a single bare text block (no annotations, no _meta), unwrap to raw text and switch the format description to 'Plain text'. Default-off; gate-off behavior is unchanged. Verified structurally against the 2.1.128 binary: function signatures, the IDE check, gate logic, _meta-unwrap pattern, and imageLimits plumbing match this implementation.
Contributor
📝 WalkthroughWalkthroughThis PR adds image processing and large-output handling to the MCP client by introducing configurable ChangesImage Processing and Large-Output Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/utils/imageResizer.ts (2)
439-445:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReport the configured limit, not a hardcoded
5MB.This error text still says
5MBeven thoughmaxBase64Sizeis now caller-overridable, so the fallback can return the wrong remediation guidance.Suggested fix
- : `Unable to resize image (${formatFileSize(originalSize)} raw, ${formatFileSize(base64Size)} base64). ` + - `The image exceeds the 5MB API limit and compression failed. ` + + : `Unable to resize image (${formatFileSize(originalSize)} raw, ${formatFileSize(base64Size)} base64). ` + + `The image exceeds the ${formatFileSize(maxBase64Size)} base64 API limit and compression failed. ` + `Please resize the image manually or use a smaller image.`,🤖 Prompt for 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. In `@src/utils/imageResizer.ts` around lines 439 - 445, The error message in the ImageResizeError thrown in imageResizer.ts uses a hardcoded "5MB" even though maxBase64Size is now configurable; update the fallback message to report the configured limit by referencing the maxBase64Size value (formatted with formatFileSize) instead of "5MB" and ensure the overDim branch still references maxWidth/maxHeight; adjust the thrown message construction around ImageResizeError (where originalSize and base64Size are used) to include formatFileSize(maxBase64Size) so the remediation guidance accurately reflects the caller-overridable limit.
419-429:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail closed when custom dimension limits can't be verified.
overDimonly inspects PNG headers. IfgetImageProcessor()fails for a JPEG/GIF/WebP and a caller supplied tightermaxWidth/maxHeight, this branch still returns the original buffer whenever the base64 size fits. That bypasses the new per-call dimension contract and can still hand an oversized image to the downstream API.Suggested guard
const detected = detectImageFormatFromBuffer(imageBuffer) const normalizedExt = detected.slice(6) // Remove 'image/' prefix // Calculate the base64 size (API limit is on base64-encoded length) const base64Size = Math.ceil((originalSize * 4) / 3) + const hasCustomDimensionLimits = + limits !== undefined && + (maxWidth !== IMAGE_MAX_WIDTH || maxHeight !== IMAGE_MAX_HEIGHT) // Size-under-5MB does not imply dimensions-under-cap. Don't return the // raw buffer if the PNG header says it's oversized — fall through to // ImageResizeError instead. PNG sig is 8 bytes, IHDR dims at 16-24. const overDim = imageBuffer.length >= 24 && imageBuffer[0] === 0x89 && imageBuffer[1] === 0x50 && imageBuffer[2] === 0x4e && imageBuffer[3] === 0x47 && (imageBuffer.readUInt32BE(16) > maxWidth || imageBuffer.readUInt32BE(20) > maxHeight) // If original image's base64 encoding is within API limit, allow it through uncompressed - if (base64Size <= maxBase64Size && !overDim) { + if ( + base64Size <= maxBase64Size && + (!hasCustomDimensionLimits || (detected === 'image/png' && !overDim)) + ) { logEvent('tengu_image_resize_fallback', { original_size_bytes: originalSize, base64_size_bytes: base64Size, error_type: errorType, })🤖 Prompt for 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. In `@src/utils/imageResizer.ts` around lines 419 - 429, The current logic only sets overDim for PNGs, allowing unverified JPEG/GIF/WebP through when base64 size fits; change to "fail closed": if callers pass maxWidth or maxHeight and you cannot reliably read dimensions (e.g., imageBuffer isn't a PNG header and getImageProcessor() cannot parse dimensions), set overDim = true so the code does not bypass resizing. Update the overDim computation to reference imageBuffer, overDim, base64Size, maxBase64Size and incorporate a check that attempts to use getImageProcessor() (or other dimension reader) and treats failures/unsupported formats as overDim = true when maxWidth/maxHeight are provided.
🤖 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/services/mcp/client.ts`:
- Around line 2497-2508: The text-case transformer currently only copies `_meta`
and thus strips `annotations` before `unwrapSingleTextBlock()` runs; change the
`case 'text'` path (where `block: ContentBlockParam` is constructed from
`resultContent.text`) to also preserve any `resultContent.annotations` by
assigning them onto the block (e.g. if (resultContent.annotations) (block as {
annotations?: unknown }).annotations = resultContent.annotations), so annotated
text blocks are not converted into bare text and lost by the unwrapping logic.
---
Outside diff comments:
In `@src/utils/imageResizer.ts`:
- Around line 439-445: The error message in the ImageResizeError thrown in
imageResizer.ts uses a hardcoded "5MB" even though maxBase64Size is now
configurable; update the fallback message to report the configured limit by
referencing the maxBase64Size value (formatted with formatFileSize) instead of
"5MB" and ensure the overDim branch still references maxWidth/maxHeight; adjust
the thrown message construction around ImageResizeError (where originalSize and
base64Size are used) to include formatFileSize(maxBase64Size) so the remediation
guidance accurately reflects the caller-overridable limit.
- Around line 419-429: The current logic only sets overDim for PNGs, allowing
unverified JPEG/GIF/WebP through when base64 size fits; change to "fail closed":
if callers pass maxWidth or maxHeight and you cannot reliably read dimensions
(e.g., imageBuffer isn't a PNG header and getImageProcessor() cannot parse
dimensions), set overDim = true so the code does not bypass resizing. Update the
overDim computation to reference imageBuffer, overDim, base64Size, maxBase64Size
and incorporate a check that attempts to use getImageProcessor() (or other
dimension reader) and treats failures/unsupported formats as overDim = true when
maxWidth/maxHeight are provided.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f38ce597-f465-4b23-b2f2-baef9e94c3b6
📒 Files selected for processing (2)
src/services/mcp/client.tssrc/utils/imageResizer.ts
Comment on lines
+2497
to
+2508
| case 'text': { | ||
| const block: ContentBlockParam = { | ||
| type: 'text', | ||
| text: resultContent.text, | ||
| } | ||
| if (includeMeta) { | ||
| const meta = resultContent._meta | ||
| if (meta) { | ||
| ;(block as { _meta?: unknown })._meta = meta | ||
| } | ||
| } | ||
| return [block] |
Contributor
There was a problem hiding this comment.
Preserve annotations before the single-text unwrap path runs.
unwrapSingleTextBlock() explicitly refuses annotated text blocks, but this transformer only carries _meta. A single annotated MCP text result is therefore converted into a bare text block here and later qualifies for plain-text unwrapping, which drops the annotations on the persisted large-output path.
Suggested fix
case 'text': {
const block: ContentBlockParam = {
type: 'text',
text: resultContent.text,
}
+ if ('annotations' in resultContent && resultContent.annotations) {
+ ;(block as { annotations?: unknown }).annotations =
+ resultContent.annotations
+ }
if (includeMeta) {
const meta = resultContent._meta
if (meta) {
;(block as { _meta?: unknown })._meta = meta
}🤖 Prompt for 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.
In `@src/services/mcp/client.ts` around lines 2497 - 2508, The text-case
transformer currently only copies `_meta` and thus strips `annotations` before
`unwrapSingleTextBlock()` runs; change the `case 'text'` path (where `block:
ContentBlockParam` is constructed from `resultContent.text`) to also preserve
any `resultContent.annotations` by assigning them onto the block (e.g. if
(resultContent.annotations) (block as { annotations?: unknown }).annotations =
resultContent.annotations), so annotated text blocks are not converted into bare
text and lost by the unwrapping logic.
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
2 participants
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.
Aligns the MCP transform pipeline (
transformResultContent→transformMCPResult→processMCPResult→callMCPTool*) with Anthropic's Claude Code 2.1.128. Adds anImageLimitsparameter chain and three small behavior additions that match upstream.What changed
New
ImageLimitstype insrc/utils/imageResizer.ts.maybeResizeAndDownsampleImageBufferaccepts an optionallimits?: ImageLimits4th parameter; when provided, overrides the module-level defaults (IMAGE_TARGET_RAW_SIZE,IMAGE_MAX_WIDTH,IMAGE_MAX_HEIGHT,API_IMAGE_MAX_BASE64_SIZE). Whenundefined, behavior is unchanged for current callers.limitsplumbed throughcallMCPToolWithUrlElicitationRetry→callMCPTool→processMCPResult→transformMCPResult→transformResultContentand passed to the image case in theimageandresourcecontent branches._metapreservation in the text-block case oftransformResultContent(gated onincludeMeta=true). The tool-result path passesincludeMeta=true; the prompt-handler call site keeps the defaultfalse, preserving prior behavior.skipLargeOutput(5th param ofprocessMCPResult): whentrueand the content has no images, returns content directly without large-output handling.Unwrap-to-text in
processMCPResult: when the large-string format gate is enabled (MCP_TRUNCATION_PROMPT_OVERRIDEenv var, ortengu_mcp_subagent_promptStatsig gate) AND the content is a single bare text block (no annotations, no_meta), unwraps to raw text and switches the format description to"Plain text". Default-off; gate-off behavior is unchanged.Default behavior
All new parameters are optional with safe defaults; the new unwrap path is gated. Existing callers (
imagePaste.ts,FileReadTool,BashTool/utils,imageProcessor.ts, the prompt handler) all work as before because they pass nothing for the new optional parameters.Validation
bunx biome check src/services/mcp/client.ts src/utils/imageResizer.ts— cleanbun test— 2323 pass / 189 fail / 119 errors (identical tomainbaseline; pre-existingbun:bundleresolution issues unrelated to this change)bun run typecheck— no new errors in modified files (pre-existingvite.config.tserrors inpackages/remote-control-serverare unchanged onmain)_meta-unwrap pattern, andimageLimits:plumbing in this PR's implementation match the binary.Binary marker counts (reviewer-verifiable via
strings <binary> | grep -c <marker>)Stable string literals — anchors that survive minification — provide reviewer-verifiable evidence of structural alignment with 2.1.128:
imageLimits:destructurestengu_mcp_subagent_promptgate refMCP_TRUNCATION_PROMPT_OVERRIDEenv refq==="ide"IDE checks!("_meta" in ...)unwrap eligibility"contentArray"type tagsFunction bodies (extracted by brace-balanced walk) match this PR's implementation modulo minifier identifier names.
Out of scope
The following items exist in the 2.1.128 binary but are not ported in this PR. Each is annotated with the binary symbol so the work can be picked up cleanly later.
TQH_meta-stripping pre-pass. Binary'sjQ5doeslet j = TQH(T)before the unwrap eligibility check.TQHwalks the content array and, when any text block has a truthy_meta, returns a new array with_metastripped from those blocks. The mirror passes content through without this step. With the unwrap gate off (default), no behavioral difference. With the gate on AND a server attaches truthy_metato a single text block, binary would strip_metaand unwrap to plain text; mirror keeps_metapresent and skips the unwrap (more conservative). Worth porting if the divergence shows up in practice.pV7line-stats + prompt-strategy branching. Anthropic 2.1.128's format helper takes an optional 5th arglineStats: { count, maxLen }AND branches the model-facing instructions three ways:lineStats === undefined→ jq-on-the-file (tree-structured content);lineStatsdefined butmaxLenexceeds the token budget → python character-slicing (read()[A:B]spans);lineStatsdefined and line-readable → offset/limit chunks offloor(budget/(maxLen+8))lines. Mirror'sgetLargeOutputInstructionskeeps the original flat strategy. A faithful port requires reverse-engineeringpV7's helpers (fqHfor token budget,Eh5,CP8(),m9) and updates tomcpOutputStorage.ts— left for follow-up.Three additional named options on
vZ8's destructure:toolExecution,taskRegistry,toolUseId. Binary accepts them but the function bodies ofvZ8,jQ5,wQ5, andZZ8don't reference them — they're likely consumed by sub-functions in the elicitation or analytics chain that weren't traced. Adding accepted-but-unread params would lie about what the function does, so they're left out until the consumer paths are identified.Need help on this PR? Tag
@codesmithwith what you need.Summary by CodeRabbit
Release Notes
New Features
Improvements