mcp_transcoder: Serve route-specific MCP tools/list responses from McpJsonRestBridgeFilter. by mkbehr · Pull Request #45015 · envoyproxy/envoy

mkbehr · 2026-05-11T21:06:50Z

Commit Message: mcp_transcoder: Serve route-specific MCP tools/list responses from McpJsonRestBridgeFilter.

Additional Description:

Add config for serving tools/list calls via local reply from McpJsonRestBridgeFilter. Tools can specify title, description, and input/output schema using the new ToolsListSpecificConfig message. Input and output schemas are provided via serialized JSON strings instead of structured objects for efficiency of encoding, because this config is only used for tools/list.

Also add a new McpJsonRestBridgePerRouteConfig, to allow specifying tools per-route.

Risk Level: Low. Config-guarded, default off.
Testing: Unit test, integration test.
Docs Changes: API docs.
Release Notes:
Platform Specific Features: N/A
API Considerations: Feature is gated by xDS-delivered config; off by default.

Generative AI was used to write this change. I have manually reviewed and fully understand the change.

repokitteh-read-only · 2026-05-11T21:06:55Z

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #45015 was opened by mkbehr.

see: more, trace.

This commit implements the per-route local handling for the tools/list JSON-RPC method in the McpJsonRestBridgeFilter. When the filter matches a /mcp tools/list request and McpJsonRestBridgePerRoute configuration is present, the response is built and served locally. To maximize efficiency and avoid string copies during local response generation, the filter directly constructs the response JSON into an Envoy buffer (Buffer::OwnedImpl) and passes it directly to the response path via decoder_callbacks_->encodeHeaders and decoder_callbacks_->encodeData, bypassing sendLocalReply. The protobuf definitions for tool configuration have also been updated to fully match the final spec, incorporating ToolsListSpecificConfig (including opaque input/output schema strings) and McpServerInfo. Signed-off-by: Michael Behr <mkbehr@google.com>

Signed-off-by: Michael Behr <mkbehr@google.com>

This introduces a ToolsListLocal message inside a tool_list_config oneof, deprecating the separate tool_list_http_rule. Additionally, this refactors the McpJsonRestBridgeFilter core logic to introduce getCombinedTools() and getTool(). These helper methods abstract iterating through the per_route_config tools list followed by the main config_->toolConfig() tools list, building an effective combined array of tools, while correctly enforcing the rule that if two tools share the same name, the per-route definition shadows the global one. Signed-off-by: Michael Behr <mkbehr@google.com>

- Renames getCombinedTools to getTools. - Adds handling for tools_list_local when no tools are defined. - Updates changelog to clarify tools_list_local scope. - Adds TODO for tool lookup optimization. Signed-off-by: Michael Behr <mkbehr@google.com>

Signed-off-by: Michael Behr <mkbehr@google.com>

repokitteh-read-only · 2026-05-12T18:47:50Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #45015 was ready_for_review by mkbehr.

see: more, trace.

mkbehr · 2026-05-12T18:48:50Z

/assign @tyxia

Signed-off-by: Michael Behr <mkbehr@google.com>

…nd comments - Resolves missing PerRoute extension type from extensions_metadata.yaml - Corrects placement of the REGISTER_FACTORY documentation comment in config.cc Signed-off-by: Michael Behr <mkbehr@google.com>

tyxia

Nice work. Thank you!

First pass on API.

Signed-off-by: Michael Behr <mkbehr@google.com>

tyxia

API mostly LGTM, let's bring the API Reviewer @adisuissa , while reviewing the implementation. Thanks!

@mkbehr You also need to fix DCO failure

tyxia · 2026-05-15T11:38:54Z

/gemini review

gemini-code-assist

Code Review

This pull request adds local response handling for the tools/list JSON-RPC method in the MCP JSON REST bridge filter, allowing it to serve tool metadata directly. It introduces per-route configuration overrides and updates the filter logic to merge global and route-specific tool definitions. Review feedback points out a potential violation of the JSON-RPC 2.0 specification regarding notifications and suggests refactoring the configuration resolution logic for better maintainability.

gemini-code-assist · 2026-05-15T11:45:15Z

+void McpJsonRestBridgeFilter::serveToolsListLocal(const nlohmann::json& json_rpc) {
+  std::string request_id_json = "null";
+  if (json_rpc.contains("id")) {
+    request_id_json = json_rpc["id"].dump();
+  }


According to the JSON-RPC 2.0 specification, a Notification is a Request object without an id member, and the server MUST NOT reply to a Notification. The current implementation will send a local response with "id": null if the id field is missing from the request, which violates the spec. You should check for the presence of the id field before proceeding with the local response.

Done. We now send 204 No Content for these.

tyxia · 2026-05-15T14:25:20Z

+    }
+  }
+
+  for (const auto& tool : config_->toolConfig().tools()) {


I haven't reviewed the whole PR , but one thing from quick scan

per route config generally overrides the main filter configuration, so we should not combine them here.

tyxia

First pass on implementation. Thanks!

tyxia · 2026-05-19T03:35:01Z

+
+    response_data.add(",\"inputSchema\":");
+    if (!tool->tool_list_config().input_schema().empty()) {
+      response_data.add(tool->tool_list_config().input_schema());


Can you add a big warning comment here that input_schema from config for now is from trusted source. i.e., valid json, otherwise if it is from untrusted source JSON Validation for Schema is needed.

Validation actually is not hard, it can be validated at config load time with nlohmann::json::accept. We don't need to add it in this PR though

tyxia · 2026-05-19T03:45:42Z

+  Buffer::OwnedImpl response_data;
+  response_data.add("{\"jsonrpc\":\"2.0\",\"id\":");
+  response_data.add(request_id_json);
+  response_data.add(",\"result\":{\"tools\":[");


Instead of Buffer::OwnedImpl, we could optimize this with string/ absl::StrAppend + reserve() , which will be faster and more memory efficient.

envoy bufferImpl is a chain of separate memory slices while we can have single contiguous string with preallocation. string representation is also aligned with sendLocalReply

If we used a string, we'd have to copy the data out into a buffer when we were done anyway. (sendLocalReply does a copy under the hood.)

What we could do is reserve a single slice, and write all the data to there. But then we'd have to precompute the length, which would be hard to maintain since we're building the response ourselves. The efficiency gains would be minimal since Envoy will combine these into 4KiB slices as is. I don't think the maintainability tradeoff is worth it if we do this by hand. Do you know of a library we could use?

Yes, buffer save one copy inside of sendLoalReply but it sacrifices the cache locality with multiple scattered slices. Thus i would say it is really microscopic optimization, if there is a gain. However, the stability and reliability issues mentioned here #45015 (comment) is more important factor here

tyxia · 2026-05-19T03:50:04Z

+    first_tool = false;
+
+    response_data.add("{");
+    nlohmann::json name_json = tool->name();


nit: with StrAppend above, we can avoid heavy temporary nlohmann::json object

absl::StrAppend(&response_str, ""name":", nlohmann::json(tool->name()).dump());

There aren't any heavy nlohmann::json objects here, just light ones to escape strings. That example doesn't use any fewer temporary objects than this implementation does.

tyxia · 2026-05-19T03:56:02Z

+  Http::ResponseHeaderMapPtr response_headers = Http::ResponseHeaderMapImpl::create();
+  response_headers->setStatus(200);
+  response_headers->setContentType(Http::Headers::get().ContentTypeValues.Json);
+  response_headers->setContentLength(response_data.length());
+
+  decoder_callbacks_->encodeHeaders(std::move(response_headers), false,
+                                    "mcp_json_rest_bridge_tools_list");
+
+  decoder_callbacks_->encodeData(response_data, true);


Instead of doing manual encodeHeaders and encodeData, we should just use sendLocalReply here. That is standard way in Envoy to handle this

sendLocalReply does a string copy for the body, so it's inefficient for large responses. encodeData lets us pass the buffer in directly.

sendLocalyReply is a better option here. It has many advantages, to name a few:
It cleanly handle Envoy’s HTTP stream state machine. And it integrates with HCM to correctly terminate the request and close the connection. Also other filters are properly notified through their onLocalReply callbacks if they need to inspect or intercept local responses.

String copy is much smaller gain compared with correctness, stability and Envoy core safely here. Also, tools/list is infrequent only called at tool discovery init phase, which makes string copy gain even smaller.

encodeHeaders/Data from the decoder callbacks is stable and correct; those methods are exposed from the callbacks for this purpose. See documentation in filter.h: "A local reply can be triggered via sendLocalReply() or encodeHeaders()." Standard usage is sendLocalReply for error responses and encodeHeaders for 200s with longer bodies. encodeHeaders is already used for local replies in several places across the codebase, including CacheFilter, CorsFilter, ext_proc filter, and AdminFilter. The state machine and connection management are handled correctly; if they weren't, the integration test wouldn't pass. onLocalReply won't be called, but that's generally used for error management.

My point is not about individual encodeHeaders/Data call. Also your points only refer to encodeHeaders alone while here we are encode both header and body. There is no standard usage saying sendLocalReply is only for error response, it is local reply by its name. Besides, Integration test also can not demonstrate it is working e2e as it just have one filter in the chain, however, in production there will be lots of filters and each filter can have their own logics such as local reply or processing error or header body mutation or filter chain stall etc.

If you take a look at how sendLocalReply really works, you can see it is much more complicated that just encodeHeader and body. It needs to handle FM integration, filter call state, data reset_imminent, reply execution model and so on ...... Manually using separate encodeHeader/body here is error prone and not stable/reliable.

Furthermore, back to your initial motivation of this change. I don't think optimization of saving string copy on a infrequent path (which is somewhat microscopic) could justify the sacrifice of simplicity, reliability and even correctness that are brought by well-established and well-tested sendLocalReply API.

If you still have questions, i am happy/available to have a quick sync.

tyxia · 2026-05-19T16:06:41Z

    InitializationAck = 3,
    // Clients send a tools/list request to discover available tools.
    ToolsList = 4,
+    // Clients send a tools/list request that is handled locally via per-route config.


nit: remove"per-route " ? per-route config is used when it is configured but main filter config can also be used when per route config is absent

tyxia · 2026-05-19T16:09:55Z

-    absl::StatusOr<envoy::extensions::filters::http::mcp_json_rest_bridge::v3::HttpRule> http_rule =
-        config_->getToolsListHttpRule();
-    if (http_rule.ok() && !http_rule->get().empty()) {
+    const auto* per_route_config =


we have invoked resolveMostSpecificPerFilterConfig multiple times in this filter, i think it will be better we just latch a per route config pointer in this filter as class member

tyxia · 2026-05-19T16:17:05Z

/assign @adisuissa @wbpcode

for API review as they have reviewed multiple API PRs for this filter in the past (i.e., has most context). Please take a look either of you have time Thanks!

phlax · 2026-05-19T18:01:24Z

 # *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

 new_features:
+- area: mcp_json_rest_bridge


needs to use new changelog layout - see #45095

/wait

- Remove output_schema from the configuration. - Override static config with per-route config instead of merging. - Return 204 No Content for JSON-RPC notifications (requests without an ID). - Add tests for missing coverage, including encodeTrailers. - Optimize tools/list response allocation with reserveSingleSlice. - Latch route and per_route_config at the start of decodeData. Signed-off-by: Michael Behr <mkbehr@google.com>

Signed-off-by: Michael Behr <mkbehr@google.com>

guoyilin42 · 2026-05-21T22:07:54Z

-    sendErrorResponse(Http::Code::BadRequest, "mcp_json_rest_bridge_filter_id_not_found",
-                      generateErrorJsonResponse(-32600, "Missing ID field").dump());
-    return absl::InvalidArgumentError("Missing ID field");
+    mcp_operation_ = McpOperation::InitializationAck;


I don't think we should change the error response to NoContent here if the request id is missing for a non-notification request like a tools/call or tools/list request.

id
This member is REQUIRED.
It MUST be the same as the value of the id member in the Request Object.
If there was an error in detecting the id in the Request object (e.g. Parse error/Invalid Request), it MUST be Null.

https://www.jsonrpc.org/specification#:~:text=in%20section%205.1.-,id,object%20(e.g.%20Parse%20error/Invalid%20Request)%2C%20it%20MUST%20be%20Null.,-Either%20the%20result

guoyilin42 · 2026-05-21T22:09:01Z

+              sendLocalReply(Eq(Http::Code::NoContent), StrEq(""), _, _,
+                             StrEq("mcp_json_rest_bridge_filter_notification")));

  Buffer::OwnedImpl body(R"json({"jsonrpc":"2.0","method":"tools/list"})json");


A tools/list request without id should result in an error response.

guoyilin42 · 2026-05-21T22:09:12Z

+              sendLocalReply(Eq(Http::Code::NoContent), StrEq(""), _, _,
+                             StrEq("mcp_json_rest_bridge_filter_notification")));

  Buffer::OwnedImpl body(R"json({"jsonrpc":"2.0","id":123.45,"method":"tools/list"})json");


mkbehr added 3 commits May 11, 2026 21:32

Improve documentation and comments.

1305345

Signed-off-by: Michael Behr <mkbehr@google.com>

test: tweak tools/list local response unit tests

0a8dcfb

Signed-off-by: Michael Behr <mkbehr@google.com>

mkbehr force-pushed the mcp-list branch from d9f8fe1 to 58935c2 Compare May 11, 2026 22:11

mkbehr force-pushed the mcp-list branch from 58935c2 to 4cba1a0 Compare May 12, 2026 16:38

mkbehr added 2 commits May 12, 2026 18:06

Merge branch 'main' of https://github.com/envoyproxy/envoy into mcp-list

488f9d3

Signed-off-by: Michael Behr <mkbehr@google.com>

mkbehr marked this pull request as ready for review May 12, 2026 18:47

mkbehr requested review from botengyao, tyxia and yanavlasov as code owners May 12, 2026 18:47

repokitteh-read-only Bot added the api label May 12, 2026

repokitteh-read-only Bot assigned adisuissa May 12, 2026

repokitteh-read-only Bot assigned tyxia May 12, 2026

mkbehr added 2 commits May 12, 2026 20:48

http: fix missing buffer_lib dependency in MCP JSON REST bridge filter

e2b8687

Signed-off-by: Michael Behr <mkbehr@google.com>

tyxia reviewed May 13, 2026

View reviewed changes

Improve API comments and remove McpServerInfo.

8104b97

Signed-off-by: Michael Behr <mkbehr@google.com>

mkbehr force-pushed the mcp-list branch from 9edfd4a to af81d40 Compare May 13, 2026 20:40

Format fixes

1a35c9f

Signed-off-by: Michael Behr <mkbehr@google.com>

mkbehr force-pushed the mcp-list branch from af81d40 to 1a35c9f Compare May 13, 2026 20:41

fix spelling

ea327fc

Signed-off-by: Michael Behr <mkbehr@google.com>

tyxia reviewed May 15, 2026

View reviewed changes

gemini-code-assist Bot reviewed May 15, 2026

View reviewed changes

tyxia reviewed May 15, 2026

View reviewed changes

tyxia reviewed May 19, 2026

View reviewed changes

repokitteh-read-only Bot assigned wbpcode May 19, 2026

phlax reviewed May 19, 2026

View reviewed changes

repokitteh-read-only Bot added the waiting label May 19, 2026

mkbehr added 3 commits May 19, 2026 20:01

Code review

8cf5ce6

Signed-off-by: Michael Behr <mkbehr@google.com>

Merge remote-tracking branch 'refs/remotes/upstream/main' into mcp-list

3565836

Signed-off-by: Michael Behr <mkbehr@google.com>

mkbehr force-pushed the mcp-list branch from f10d9bf to 3565836 Compare May 19, 2026 20:41

repokitteh-read-only Bot removed the waiting label May 19, 2026

mkbehr added 3 commits May 19, 2026 20:47

ASSERT valid response id

51fbd82

Signed-off-by: Michael Behr <mkbehr@google.com>

cleanup

93f7d56

Signed-off-by: Michael Behr <mkbehr@google.com>

fix format

5db54c0

Signed-off-by: Michael Behr <mkbehr@google.com>

mkbehr force-pushed the mcp-list branch from d02601b to 5db54c0 Compare May 20, 2026 16:19

fix format

3ed4b1d

Signed-off-by: Michael Behr <mkbehr@google.com>

guoyilin42 reviewed May 21, 2026

View reviewed changes

Conversation

mkbehr commented May 11, 2026

Uh oh!

repokitteh-read-only Bot commented May 11, 2026

Uh oh!

repokitteh-read-only Bot commented May 12, 2026

Uh oh!

mkbehr commented May 12, 2026

Uh oh!

tyxia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tyxia left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tyxia commented May 15, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tyxia left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tyxia May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tyxia May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tyxia May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tyxia left a comment •

edited

Loading

tyxia May 19, 2026 •

edited

Loading

tyxia May 19, 2026 •

edited

Loading

tyxia May 20, 2026 •

edited

Loading