Skip to content

fix(read_file): return image bytes in MCP content so the model can see images#488

Merged
wonderwhy-er merged 1 commit into
mainfrom
fix/read-file-image-content
Jun 2, 2026
Merged

fix(read_file): return image bytes in MCP content so the model can see images#488
wonderwhy-er merged 1 commit into
mainfrom
fix/read-file-image-content

Conversation

@wonderwhy-er

@wonderwhy-er wonderwhy-er commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Problem

read_file on an image returned the image bytes only in structuredContent (consumed by the preview widget) and sent the host model a text-only summary like Image file: ... (image/png).

The model received no image content block, so reading an image was effectively a no-op for the model — it could never actually see the image. The previous code comment described this as "text-only for broad host compatibility," but it blinded every host that can handle images, which is the common case.

Fix

Add the image content block back into the MCP content array (text summary + type: "image" block), while leaving structuredContent unchanged so the preview widget keeps rendering as before.

Result:

  • the host model receives the image and can analyze it
  • the preview widget still renders from structuredContent

Testing

  • npm run build clean
  • Verified live: after rebuild + MCP restart, read_file on a PNG now returns the image to the model (previously only the text summary came back).

Notes

This is scoped to the image branch of handleReadFile. The separate edit_block freeze (DOCX) is being investigated independently and is not part of this PR.

Summary by CodeRabbit

  • New Features
    • Image file responses now include renderable image content with base64-encoded data and MIME type information, enabling direct image display alongside text summaries and preview widgets.

…e images

read_file on an image previously put the image bytes only in
structuredContent (for the preview widget) and sent the host model a
text-only summary ("Image file: ... (image/png)"). The model received
no image block, so reading an image was effectively a no-op for the
model — it could never actually see the image.

Add the image content block back into the `content` array while keeping
structuredContent unchanged, so:
- the host model receives the image and can analyze it
- the preview widget still renders from structuredContent as before

The previous "text-only for broad host compatibility" approach blinded
every host that can handle images, which is the common case.
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 17ddd22e-e51f-4266-bb54-d0c8c11a0c82

📥 Commits

Reviewing files that changed from the base of the PR and between 673111c and 423a659.

📒 Files selected for processing (1)
  • src/handlers/filesystem-handlers.ts

📝 Walkthrough

Walkthrough

The PR enhances the image file reading response in handleReadFile by including MCP-compatible image content. Documentation is clarified to indicate dual-channel image delivery, and the response payload now includes an "image" content item with base64-encoded bytes and MIME type.

Changes

Image Content in MCP Response

Layer / File(s) Summary
Image content item and documentation
src/handlers/filesystem-handlers.ts
Documentation is updated to reflect that image bytes are returned in both the MCP content array (for host rendering) and structuredContent (for preview widget). The content payload is extended with an "image" item containing imageData and mimeType alongside the existing text summary.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • wonderwhy-er/DesktopCommanderMCP#472: Both PRs modify handleReadFile for image handling—this PR adds an MCP content image item, while #472 updates structuredContent.content to use imageData for image previews.

Poem

🐰 A picture's worth a thousand words, they say,
So MCP content gets its day,
With base64 bytes and MIME types true,
Both the host and preview can render what's new! 🖼️

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding image bytes to MCP content so the AI model can see images, which directly addresses the PR's core objective.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/read-file-image-content

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@wonderwhy-er wonderwhy-er merged commit 701bc31 into main Jun 2, 2026
1 of 2 checks passed
wonderwhy-er added a commit that referenced this pull request Jun 5, 2026
#488 added the image bytes back into read_file's MCP `content` array so
the host model can see images, but test-file-handlers.js was never
updated and still asserted the pre-#488 contract ("avoid image content
item for host compatibility"). That assertion is now inverted from the
shipped behaviour and was the only failing test in the suite.

Update Test 9 to assert the current contract for both raster (PNG) and
SVG reads: the `content` array includes an `image` item carrying base64
`data` and the correct `mimeType` (image/png, image/svg+xml), alongside
the existing structuredContent checks.

No production code change — handler behaviour is unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant