[Lldb-commits] [lldb] 350f6ab - [lldb] Adjusting the base MCP protocol types per the spec. (#153297)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Aug 12 17:56:54 PDT 2025
Author: John Harrison
Date: 2025-08-12T17:56:52-07:00
New Revision: 350f6abb8304bae39a0ed70a41fde13b7d1df510
URL: https://github.com/llvm/llvm-project/commit/350f6abb8304bae39a0ed70a41fde13b7d1df510
DIFF: https://github.com/llvm/llvm-project/commit/350f6abb8304bae39a0ed70a41fde13b7d1df510.diff
LOG: [lldb] Adjusting the base MCP protocol types per the spec. (#153297)
* This adjusts the `Request`/`Response` types to have an `id` that is
either a string or a number.
* Merges 'Error' into 'Response' to have a single response type that
represents both errors and results.
* Adjusts the `Error.data` field to by any JSON value.
* Adds `operator==` support to the base protocol types and simplifies
the tests.
Added:
Modified:
lldb/include/lldb/Protocol/MCP/MCPError.h
lldb/include/lldb/Protocol/MCP/Protocol.h
lldb/source/Protocol/MCP/MCPError.cpp
lldb/source/Protocol/MCP/Protocol.cpp
lldb/source/Protocol/MCP/Server.cpp
lldb/unittests/Protocol/ProtocolMCPTest.cpp
lldb/unittests/ProtocolServer/ProtocolMCPServerTest.cpp
lldb/unittests/TestingSupport/TestUtilities.h
Removed:
################################################################################
diff --git a/lldb/include/lldb/Protocol/MCP/MCPError.h b/lldb/include/lldb/Protocol/MCP/MCPError.h
index 2bdbb9b7a6874..55dd40f124a15 100644
--- a/lldb/include/lldb/Protocol/MCP/MCPError.h
+++ b/lldb/include/lldb/Protocol/MCP/MCPError.h
@@ -26,7 +26,7 @@ class MCPError : public llvm::ErrorInfo<MCPError> {
const std::string &getMessage() const { return m_message; }
- lldb_protocol::mcp::Error toProtcolError() const;
+ lldb_protocol::mcp::Error toProtocolError() const;
static constexpr int64_t kResourceNotFound = -32002;
static constexpr int64_t kInternalError = -32603;
diff --git a/lldb/include/lldb/Protocol/MCP/Protocol.h b/lldb/include/lldb/Protocol/MCP/Protocol.h
index 6448416eee08f..141d064804e1e 100644
--- a/lldb/include/lldb/Protocol/MCP/Protocol.h
+++ b/lldb/include/lldb/Protocol/MCP/Protocol.h
@@ -23,50 +23,72 @@ namespace lldb_protocol::mcp {
static llvm::StringLiteral kProtocolVersion = "2024-11-05";
+/// A Request or Response 'id'.
+///
+/// NOTE: This
diff ers from the JSON-RPC 2.0 spec. The MCP spec says this must
+/// be a string or number, excluding a json 'null' as a valid id.
+using Id = std::variant<int64_t, std::string>;
+
/// A request that expects a response.
struct Request {
- uint64_t id = 0;
+ /// The request id.
+ Id id = 0;
+ /// The method to be invoked.
std::string method;
+ /// The method's params.
std::optional<llvm::json::Value> params;
};
llvm::json::Value toJSON(const Request &);
bool fromJSON(const llvm::json::Value &, Request &, llvm::json::Path);
+bool operator==(const Request &, const Request &);
-struct ErrorInfo {
+struct Error {
+ /// The error type that occurred.
int64_t code = 0;
+ /// A short description of the error. The message SHOULD be limited to a
+ /// concise single sentence.
std::string message;
- std::string data;
-};
-
-llvm::json::Value toJSON(const ErrorInfo &);
-bool fromJSON(const llvm::json::Value &, ErrorInfo &, llvm::json::Path);
-
-struct Error {
- uint64_t id = 0;
- ErrorInfo error;
+ /// Additional information about the error. The value of this member is
+ /// defined by the sender (e.g. detailed error information, nested errors
+ /// etc.).
+ std::optional<llvm::json::Value> data;
};
llvm::json::Value toJSON(const Error &);
bool fromJSON(const llvm::json::Value &, Error &, llvm::json::Path);
+bool operator==(const Error &, const Error &);
+/// A response to a request, either an error or a result.
struct Response {
- uint64_t id = 0;
- std::optional<llvm::json::Value> result;
- std::optional<ErrorInfo> error;
+ /// The request id.
+ Id id = 0;
+ /// The result of the request, either an Error or the JSON value of the
+ /// response.
+ std::variant<Error, llvm::json::Value> result;
};
llvm::json::Value toJSON(const Response &);
bool fromJSON(const llvm::json::Value &, Response &, llvm::json::Path);
+bool operator==(const Response &, const Response &);
/// A notification which does not expect a response.
struct Notification {
+ /// The method to be invoked.
std::string method;
+ /// The notification's params.
std::optional<llvm::json::Value> params;
};
llvm::json::Value toJSON(const Notification &);
bool fromJSON(const llvm::json::Value &, Notification &, llvm::json::Path);
+bool operator==(const Notification &, const Notification &);
+
+/// A general message as defined by the JSON-RPC 2.0 spec.
+using Message = std::variant<Request, Response, Notification>;
+
+bool fromJSON(const llvm::json::Value &, Message &, llvm::json::Path);
+llvm::json::Value toJSON(const Message &);
struct ToolCapability {
/// Whether this server supports notifications for changes to the tool list.
@@ -176,11 +198,6 @@ struct ToolDefinition {
llvm::json::Value toJSON(const ToolDefinition &);
bool fromJSON(const llvm::json::Value &, ToolDefinition &, llvm::json::Path);
-using Message = std::variant<Request, Response, Notification, Error>;
-
-bool fromJSON(const llvm::json::Value &, Message &, llvm::json::Path);
-llvm::json::Value toJSON(const Message &);
-
using ToolArguments = std::variant<std::monostate, llvm::json::Value>;
} // namespace lldb_protocol::mcp
diff --git a/lldb/source/Protocol/MCP/MCPError.cpp b/lldb/source/Protocol/MCP/MCPError.cpp
index c610e882abf51..e140d11e12cfe 100644
--- a/lldb/source/Protocol/MCP/MCPError.cpp
+++ b/lldb/source/Protocol/MCP/MCPError.cpp
@@ -25,10 +25,10 @@ std::error_code MCPError::convertToErrorCode() const {
return llvm::inconvertibleErrorCode();
}
-lldb_protocol::mcp::Error MCPError::toProtcolError() const {
+lldb_protocol::mcp::Error MCPError::toProtocolError() const {
lldb_protocol::mcp::Error error;
- error.error.code = m_error_code;
- error.error.message = m_message;
+ error.code = m_error_code;
+ error.message = m_message;
return error;
}
diff --git a/lldb/source/Protocol/MCP/Protocol.cpp b/lldb/source/Protocol/MCP/Protocol.cpp
index d579b88037e63..d9b11bd766686 100644
--- a/lldb/source/Protocol/MCP/Protocol.cpp
+++ b/lldb/source/Protocol/MCP/Protocol.cpp
@@ -26,8 +26,45 @@ static bool mapRaw(const json::Value &Params, StringLiteral Prop,
return true;
}
+static llvm::json::Value toJSON(const Id &Id) {
+ if (const int64_t *I = std::get_if<int64_t>(&Id))
+ return json::Value(*I);
+ if (const std::string *S = std::get_if<std::string>(&Id))
+ return json::Value(*S);
+ llvm_unreachable("unexpected type in protocol::Id");
+}
+
+static bool mapId(const llvm::json::Value &V, StringLiteral Prop, Id &Id,
+ llvm::json::Path P) {
+ const auto *O = V.getAsObject();
+ if (!O) {
+ P.report("expected object");
+ return false;
+ }
+
+ const auto *E = O->get(Prop);
+ if (!E) {
+ P.field(Prop).report("not found");
+ return false;
+ }
+
+ if (auto S = E->getAsString()) {
+ Id = S->str();
+ return true;
+ }
+
+ if (auto I = E->getAsInteger()) {
+ Id = *I;
+ return true;
+ }
+
+ P.report("expected string or number");
+ return false;
+}
+
llvm::json::Value toJSON(const Request &R) {
- json::Object Result{{"jsonrpc", "2.0"}, {"id", R.id}, {"method", R.method}};
+ json::Object Result{
+ {"jsonrpc", "2.0"}, {"id", toJSON(R.id)}, {"method", R.method}};
if (R.params)
Result.insert({"params", R.params});
return Result;
@@ -35,47 +72,75 @@ llvm::json::Value toJSON(const Request &R) {
bool fromJSON(const llvm::json::Value &V, Request &R, llvm::json::Path P) {
llvm::json::ObjectMapper O(V, P);
- if (!O || !O.map("id", R.id) || !O.map("method", R.method))
- return false;
- return mapRaw(V, "params", R.params, P);
-}
-
-llvm::json::Value toJSON(const ErrorInfo &EI) {
- llvm::json::Object Result{{"code", EI.code}, {"message", EI.message}};
- if (!EI.data.empty())
- Result.insert({"data", EI.data});
- return Result;
+ return O && mapId(V, "id", R.id, P) && O.map("method", R.method) &&
+ mapRaw(V, "params", R.params, P);
}
-bool fromJSON(const llvm::json::Value &V, ErrorInfo &EI, llvm::json::Path P) {
- llvm::json::ObjectMapper O(V, P);
- return O && O.map("code", EI.code) && O.map("message", EI.message) &&
- O.mapOptional("data", EI.data);
+bool operator==(const Request &a, const Request &b) {
+ return a.id == b.id && a.method == b.method && a.params == b.params;
}
llvm::json::Value toJSON(const Error &E) {
- return json::Object{{"jsonrpc", "2.0"}, {"id", E.id}, {"error", E.error}};
+ llvm::json::Object Result{{"code", E.code}, {"message", E.message}};
+ if (E.data)
+ Result.insert({"data", *E.data});
+ return Result;
}
bool fromJSON(const llvm::json::Value &V, Error &E, llvm::json::Path P) {
llvm::json::ObjectMapper O(V, P);
- return O && O.map("id", E.id) && O.map("error", E.error);
+ return O && O.map("code", E.code) && O.map("message", E.message) &&
+ mapRaw(V, "data", E.data, P);
+}
+
+bool operator==(const Error &a, const Error &b) {
+ return a.code == b.code && a.message == b.message && a.data == b.data;
}
llvm::json::Value toJSON(const Response &R) {
- llvm::json::Object Result{{"jsonrpc", "2.0"}, {"id", R.id}};
- if (R.result)
- Result.insert({"result", R.result});
- if (R.error)
- Result.insert({"error", R.error});
+ llvm::json::Object Result{{"jsonrpc", "2.0"}, {"id", toJSON(R.id)}};
+
+ if (const Error *error = std::get_if<Error>(&R.result))
+ Result.insert({"error", *error});
+ if (const json::Value *result = std::get_if<json::Value>(&R.result))
+ Result.insert({"result", *result});
return Result;
}
bool fromJSON(const llvm::json::Value &V, Response &R, llvm::json::Path P) {
- llvm::json::ObjectMapper O(V, P);
- if (!O || !O.map("id", R.id) || !O.map("error", R.error))
+ const json::Object *E = V.getAsObject();
+ if (!E) {
+ P.report("expected object");
+ return false;
+ }
+
+ const json::Value *result = E->get("result");
+ const json::Value *raw_error = E->get("error");
+
+ if (result && raw_error) {
+ P.report("'result' and 'error' fields are mutually exclusive");
return false;
- return mapRaw(V, "result", R.result, P);
+ }
+
+ if (!result && !raw_error) {
+ P.report("'result' or 'error' fields are required'");
+ return false;
+ }
+
+ if (result) {
+ R.result = std::move(*result);
+ } else {
+ Error error;
+ if (!fromJSON(*raw_error, error, P))
+ return false;
+ R.result = std::move(error);
+ }
+
+ return mapId(V, "id", R.id, P);
+}
+
+bool operator==(const Response &a, const Response &b) {
+ return a.id == b.id && a.result == b.result;
}
llvm::json::Value toJSON(const Notification &N) {
@@ -97,6 +162,10 @@ bool fromJSON(const llvm::json::Value &V, Notification &N, llvm::json::Path P) {
return true;
}
+bool operator==(const Notification &a, const Notification &b) {
+ return a.method == b.method && a.params == b.params;
+}
+
llvm::json::Value toJSON(const ToolCapability &TC) {
return llvm::json::Object{{"listChanged", TC.listChanged}};
}
@@ -235,24 +304,16 @@ bool fromJSON(const llvm::json::Value &V, Message &M, llvm::json::Path P) {
return true;
}
- if (O->get("error")) {
- Error E;
- if (!fromJSON(V, E, P))
- return false;
- M = std::move(E);
- return true;
- }
-
- if (O->get("result")) {
- Response R;
+ if (O->get("method")) {
+ Request R;
if (!fromJSON(V, R, P))
return false;
M = std::move(R);
return true;
}
- if (O->get("method")) {
- Request R;
+ if (O->get("result") || O->get("error")) {
+ Response R;
if (!fromJSON(V, R, P))
return false;
M = std::move(R);
diff --git a/lldb/source/Protocol/MCP/Server.cpp b/lldb/source/Protocol/MCP/Server.cpp
index 4ec127fe75bdd..a9c1482e3e378 100644
--- a/lldb/source/Protocol/MCP/Server.cpp
+++ b/lldb/source/Protocol/MCP/Server.cpp
@@ -66,13 +66,15 @@ Server::HandleData(llvm::StringRef data) {
Error protocol_error;
llvm::handleAllErrors(
response.takeError(),
- [&](const MCPError &err) { protocol_error = err.toProtcolError(); },
+ [&](const MCPError &err) { protocol_error = err.toProtocolError(); },
[&](const llvm::ErrorInfoBase &err) {
- protocol_error.error.code = MCPError::kInternalError;
- protocol_error.error.message = err.message();
+ protocol_error.code = MCPError::kInternalError;
+ protocol_error.message = err.message();
});
- protocol_error.id = request->id;
- return protocol_error;
+ Response error_response;
+ error_response.id = request->id;
+ error_response.result = std::move(protocol_error);
+ return error_response;
}
return *response;
@@ -84,9 +86,6 @@ Server::HandleData(llvm::StringRef data) {
return std::nullopt;
}
- if (std::get_if<Error>(&(*message)))
- return llvm::createStringError("unexpected MCP message: error");
-
if (std::get_if<Response>(&(*message)))
return llvm::createStringError("unexpected MCP message: response");
@@ -123,11 +122,11 @@ void Server::AddNotificationHandler(llvm::StringRef method,
llvm::Expected<Response> Server::InitializeHandler(const Request &request) {
Response response;
- response.result.emplace(llvm::json::Object{
+ response.result = llvm::json::Object{
{"protocolVersion", mcp::kProtocolVersion},
{"capabilities", GetCapabilities()},
{"serverInfo",
- llvm::json::Object{{"name", m_name}, {"version", m_version}}}});
+ llvm::json::Object{{"name", m_name}, {"version", m_version}}}};
return response;
}
@@ -138,7 +137,7 @@ llvm::Expected<Response> Server::ToolsListHandler(const Request &request) {
for (const auto &tool : m_tools)
tools.emplace_back(toJSON(tool.second->GetDefinition()));
- response.result.emplace(llvm::json::Object{{"tools", std::move(tools)}});
+ response.result = llvm::json::Object{{"tools", std::move(tools)}};
return response;
}
@@ -173,7 +172,7 @@ llvm::Expected<Response> Server::ToolsCallHandler(const Request &request) {
if (!text_result)
return text_result.takeError();
- response.result.emplace(toJSON(*text_result));
+ response.result = toJSON(*text_result);
return response;
}
@@ -189,8 +188,7 @@ llvm::Expected<Response> Server::ResourcesListHandler(const Request &request) {
for (const Resource &resource : resource_provider_up->GetResources())
resources.push_back(resource);
}
- response.result.emplace(
- llvm::json::Object{{"resources", std::move(resources)}});
+ response.result = llvm::json::Object{{"resources", std::move(resources)}};
return response;
}
@@ -226,7 +224,7 @@ llvm::Expected<Response> Server::ResourcesReadHandler(const Request &request) {
return result.takeError();
Response response;
- response.result.emplace(std::move(*result));
+ response.result = std::move(*result);
return response;
}
diff --git a/lldb/unittests/Protocol/ProtocolMCPTest.cpp b/lldb/unittests/Protocol/ProtocolMCPTest.cpp
index 1dca0e5fc5bb8..ea19922522ffe 100644
--- a/lldb/unittests/Protocol/ProtocolMCPTest.cpp
+++ b/lldb/unittests/Protocol/ProtocolMCPTest.cpp
@@ -149,9 +149,7 @@ TEST(ProtocolMCPTest, MessageWithRequest) {
const Request &deserialized_request =
std::get<Request>(*deserialized_message);
- EXPECT_EQ(request.id, deserialized_request.id);
- EXPECT_EQ(request.method, deserialized_request.method);
- EXPECT_EQ(request.params, deserialized_request.params);
+ EXPECT_EQ(request, deserialized_request);
}
TEST(ProtocolMCPTest, MessageWithResponse) {
@@ -168,8 +166,7 @@ TEST(ProtocolMCPTest, MessageWithResponse) {
const Response &deserialized_response =
std::get<Response>(*deserialized_message);
- EXPECT_EQ(response.id, deserialized_response.id);
- EXPECT_EQ(response.result, deserialized_response.result);
+ EXPECT_EQ(response, deserialized_response);
}
TEST(ProtocolMCPTest, MessageWithNotification) {
@@ -186,49 +183,28 @@ TEST(ProtocolMCPTest, MessageWithNotification) {
const Notification &deserialized_notification =
std::get<Notification>(*deserialized_message);
- EXPECT_EQ(notification.method, deserialized_notification.method);
- EXPECT_EQ(notification.params, deserialized_notification.params);
+ EXPECT_EQ(notification, deserialized_notification);
}
-TEST(ProtocolMCPTest, MessageWithError) {
- ErrorInfo error_info;
- error_info.code = -32603;
- error_info.message = "Internal error";
-
+TEST(ProtocolMCPTest, MessageWithErrorResponse) {
Error error;
- error.id = 3;
- error.error = error_info;
+ error.code = -32603;
+ error.message = "Internal error";
+
+ Response error_response;
+ error_response.id = 3;
+ error_response.result = error;
- Message message = error;
+ Message message = error_response;
llvm::Expected<Message> deserialized_message = roundtripJSON(message);
ASSERT_THAT_EXPECTED(deserialized_message, llvm::Succeeded());
- ASSERT_TRUE(std::holds_alternative<Error>(*deserialized_message));
- const Error &deserialized_error = std::get<Error>(*deserialized_message);
-
- EXPECT_EQ(error.id, deserialized_error.id);
- EXPECT_EQ(error.error.code, deserialized_error.error.code);
- EXPECT_EQ(error.error.message, deserialized_error.error.message);
-}
-
-TEST(ProtocolMCPTest, ResponseWithError) {
- ErrorInfo error_info;
- error_info.code = -32700;
- error_info.message = "Parse error";
-
- Response response;
- response.id = 4;
- response.error = error_info;
-
- llvm::Expected<Response> deserialized_response = roundtripJSON(response);
- ASSERT_THAT_EXPECTED(deserialized_response, llvm::Succeeded());
+ ASSERT_TRUE(std::holds_alternative<Response>(*deserialized_message));
+ const Response &deserialized_error =
+ std::get<Response>(*deserialized_message);
- EXPECT_EQ(response.id, deserialized_response->id);
- EXPECT_FALSE(deserialized_response->result.has_value());
- ASSERT_TRUE(deserialized_response->error.has_value());
- EXPECT_EQ(response.error->code, deserialized_response->error->code);
- EXPECT_EQ(response.error->message, deserialized_response->error->message);
+ EXPECT_EQ(error_response, deserialized_error);
}
TEST(ProtocolMCPTest, Resource) {
diff --git a/lldb/unittests/ProtocolServer/ProtocolMCPServerTest.cpp b/lldb/unittests/ProtocolServer/ProtocolMCPServerTest.cpp
index ade299e08698d..2ac40c41dd28e 100644
--- a/lldb/unittests/ProtocolServer/ProtocolMCPServerTest.cpp
+++ b/lldb/unittests/ProtocolServer/ProtocolMCPServerTest.cpp
@@ -200,7 +200,7 @@ class ProtocolServerMCPTest : public ::testing::Test {
} // namespace
-TEST_F(ProtocolServerMCPTest, Intialization) {
+TEST_F(ProtocolServerMCPTest, Initialization) {
llvm::StringLiteral request =
R"json({"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"lldb-unit","version":"0.1.0"}},"jsonrpc":"2.0","id":0})json";
llvm::StringLiteral response =
diff --git a/lldb/unittests/TestingSupport/TestUtilities.h b/lldb/unittests/TestingSupport/TestUtilities.h
index db62881872fef..cc93a68a6a431 100644
--- a/lldb/unittests/TestingSupport/TestUtilities.h
+++ b/lldb/unittests/TestingSupport/TestUtilities.h
@@ -11,11 +11,11 @@
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Utility/DataBuffer.h"
-#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/Twine.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/raw_ostream.h"
#include <string>
#define ASSERT_NO_ERROR(x) \
@@ -61,12 +61,10 @@ class TestFile {
};
template <typename T> static llvm::Expected<T> roundtripJSON(const T &input) {
- llvm::json::Value value = toJSON(input);
- llvm::json::Path::Root root;
- T output;
- if (!fromJSON(value, output, root))
- return root.getError();
- return output;
+ std::string encoded;
+ llvm::raw_string_ostream OS(encoded);
+ OS << toJSON(input);
+ return llvm::json::parse<T>(encoded);
}
} // namespace lldb_private
More information about the lldb-commits
mailing list