-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Add media file reading in filesystem server #2382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Update the ts sdk
…ext_file-and-add-read_media_file Add read_media_file tool and rename read_file
…obResourceContent. * This fixes an issue found in the servers repo with this PR: modelcontextprotocol/servers#2382 * Under the hood, z.string().base64() uses a regular expression to validate the string. * While this regex is fine for typical inputs, running it against a string that is several megabytes long can cause the JavaScript engine's regex parser to hit its internal recursion limit, resulting in a "Maximum call stack size exceeded" error.
…h is functionally just an alias for `read_text_file`
|
@olaservo Good call. Replaced the
|
olaservo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, the only thing I was thinking might be good to add is a file size limit, but I don't think that should stop merging this and could get added separately.

Summary of Changes
This pull request enhances the file system interaction capabilities by clarifying the purpose of the existing text file reading tool through a rename, and by introducing a dedicated tool for handling binary media files. This allows the system to process and return image and audio data in a base64 format, significantly expanding the types of files that can be directly accessed and utilized.
Highlights
read_filetool has been renamed toread_text_fileto more accurately reflect its function of reading text-based file content.read_media_file, has been introduced. This tool is designed to read image and audio files and return their content as base64 encoded data, along with the appropriate MIME type.@modelcontextprotocol/sdkdependency has been updated to version1.16.0, which also brings in a new transitive dependency,eventsource-parser.filesystemREADME has been updated to document the renamedread_text_filetool and provide details for the newly addedread_media_filetool.Changelog
@modelcontextprotocol/sdkfrom1.12.3to1.16.0.eventsource-parseras a new transitive dependency.read_filetoread_text_file.read_text_fileto specify text-only reading and mentionhead/tailparameters.read_media_filetool, detailing its purpose and input parameters.createReadStreamfor streaming file content (line 13).ReadFileArgsSchematoReadTextFileArgsSchema(line 120).ReadMediaFileArgsSchemafor the new media tool (line 126).readFileAsBase64Streamutility function to efficiently read and base64 encode file content (lines 482-495).ListToolsRequestSchemahandler to renameread_filetoread_text_fileand include the newread_media_filetool definition (lines 502-519).CallToolRequestSchemahandler to processread_text_filecalls under its new name and added a new case forread_media_fileto handle media file reading, MIME type detection, and base64 encoding (lines 631-694).@modelcontextprotocol/sdkfrom1.12.3to1.16.0.Server Details
Motivation and Context
Currently, the
server-filesystemMCP server has aread_filetool, which has optionalheadandtailparams and always treats the file as utf-8 text.Issue #533 contains a user report:
This PR adds support for reading media files (audio and image) with a new
read_media_filetool, and the oldread_filetool is renamed toread_text_filefor clarity.How Has This Been Tested?
Using the Inspector UI:
Using the Inspector CLI
Breaking Changes
The tool name
read_fileis nowread_text_fileand its description is updated to reflect that it only works with text files. This should not present an issue to clients, since it will make the decision of when to use the tool clearer.Types of changes
Checklist
Additional context
Error when reading large files
Maximum call stack size exceededdes.pngthat is 4mb in size:Calling the tool for a large file with Inspector UI
Calling the tool for a large file with Inspector CLI
Process of elimination:
It isn't the function
It isn't the server
It isn't the Inspector
Inspector UI
Inspector CLI
Found culprit in SDK
Protocol.tsis executed when the response is received. It parses theresultSchema.result = resultSchema.parse(response.result);withconst result = response.result;The error no longer occurs with the large file:

I explored lots of ways to make the compiled schema less massive using
z.lazy()andz.discriminatedUnion()but in the end that didn't fix it.The actual culprit turned out to be Zod's .base64() validation.
Under the hood, z.string().base64() uses a regular expression to validate the string. While this regex is fine for typical inputs, running it against a string that is several megabytes long can cause the JavaScript engine's regex parser to hit its internal recursion limit, resulting in a "Maximum call stack size exceeded" error.
Problem solved
What was needed was a more robust base64 checker in the SDK. I added one and it fixed the problem. I've created a PR that fixes the problem.
In the Inspector CLI and UI, tested the large file that was causing the "Maximum call stack size exceeded" error. It is no longer present and the 4mb file can be processed.
Inspector CLI
Inspector UI