Update MCP server behavior and corresponding tests#22177
Open
thomascolden585-svg wants to merge 5 commits intoHomebrew:mainfrom
Open
Update MCP server behavior and corresponding tests#22177thomascolden585-svg wants to merge 5 commits intoHomebrew:mainfrom
thomascolden585-svg wants to merge 5 commits intoHomebrew:mainfrom
Conversation
MikeMcQuaid
reviewed
May 8, 2026
Member
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Thanks for PR. Need to improve error messages and then may be mergeable.
MikeMcQuaid
reviewed
May 8, 2026
Author
|
Hi, @MikeMcQuaid |
MikeMcQuaid
reviewed
May 8, 2026
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.
What does this PR fix?
Fixes a crash in
brew mcp-servercaused by malformed JSON-RPC request shapes.Previously, handlers assumed
request["params"]was always a hash, so requests like{"jsonrpc":"2.0","id":1,"method":"logging/setLevel"}raisedNoMethodErrorand exited the server process (DoS).Why is this fix needed?
Any MCP client or intermediary sending valid JSON with missing/invalid
paramscould terminate the server session and drop subsequent requests.This change hardens request handling so malformed payloads return JSON-RPC errors instead of killing the process.
What changed?
paramsshape validation inlogging/setLevelbefore reading"level".tools/callfor"params","name","arguments", and"_meta"before nested access.respond_error(id, "Invalid params")for malformed request shapes instead of raising exceptions.tools/callarguments to{}when omitted.Tests added/updated
Library/Homebrew/test/mcp_server_spec.rbwith regression tests for:logging/setLevelwith missingparamsreturns"Invalid params".tools/callwith missingparamsreturns"Invalid params".tools/callwith non-hashargumentsreturns"Invalid params".Verification
./bin/brew tests --only=mcp_server(could not run in this environment: Homebrew refuses to run as root)./bin/brew lgtm --onlinecompleted successfullybrew lgtm(style, typechecking and tests) with your changes locally?AI usage details (if applicable)
Used AI to help draft and implement request-shape validation and regression tests.
Manually reviewed the changes in
Library/Homebrew/mcp_server.rbandLibrary/Homebrew/test/mcp_server_spec.rb, confirmed the crash path is handled, and verified that malformed requests now return JSON-RPC errors instead of exiting the server.Full
brew lgtmstill needs to be run locally as a non-root user.