Skip to content

Commit 350f6ab

Browse files
ashgtiashgti
authored
[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.
1 parent c14ca45 commit 350f6ab

File tree

8 files changed

+173
-123
lines changed

8 files changed

+173
-123
lines changed

‎lldb/include/lldb/Protocol/MCP/MCPError.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class MCPError : public llvm::ErrorInfo<MCPError> {
2626

2727
const std::string &getMessage() const { return m_message; }
2828

29-
lldb_protocol::mcp::Error toProtcolError() const;
29+
lldb_protocol::mcp::Error toProtocolError() const;
3030

3131
static constexpr int64_t kResourceNotFound = -32002;
3232
static constexpr int64_t kInternalError = -32603;

‎lldb/include/lldb/Protocol/MCP/Protocol.h

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,50 +23,72 @@ namespace lldb_protocol::mcp {
2323

2424
static llvm::StringLiteral kProtocolVersion = "2024-11-05";
2525

26+
/// A Request or Response 'id'.
27+
///
28+
/// NOTE: This differs from the JSON-RPC 2.0 spec. The MCP spec says this must
29+
/// be a string or number, excluding a json 'null' as a valid id.
30+
using Id = std::variant<int64_t, std::string>;
31+
2632
/// A request that expects a response.
2733
struct Request {
28-
uint64_t id = 0;
34+
/// The request id.
35+
Id id = 0;
36+
/// The method to be invoked.
2937
std::string method;
38+
/// The method's params.
3039
std::optional<llvm::json::Value> params;
3140
};
3241

3342
llvm::json::Value toJSON(const Request &);
3443
bool fromJSON(const llvm::json::Value &, Request &, llvm::json::Path);
44+
bool operator==(const Request &, const Request &);
3545

36-
struct ErrorInfo {
46+
struct Error {
47+
/// The error type that occurred.
3748
int64_t code = 0;
49+
/// A short description of the error. The message SHOULD be limited to a
50+
/// concise single sentence.
3851
std::string message;
39-
std::string data;
40-
};
41-
42-
llvm::json::Value toJSON(const ErrorInfo &);
43-
bool fromJSON(const llvm::json::Value &, ErrorInfo &, llvm::json::Path);
44-
45-
struct Error {
46-
uint64_t id = 0;
47-
ErrorInfo error;
52+
/// Additional information about the error. The value of this member is
53+
/// defined by the sender (e.g. detailed error information, nested errors
54+
/// etc.).
55+
std::optional<llvm::json::Value> data;
4856
};
4957

5058
llvm::json::Value toJSON(const Error &);
5159
bool fromJSON(const llvm::json::Value &, Error &, llvm::json::Path);
60+
bool operator==(const Error &, const Error &);
5261

62+
/// A response to a request, either an error or a result.
5363
struct Response {
54-
uint64_t id = 0;
55-
std::optional<llvm::json::Value> result;
56-
std::optional<ErrorInfo> error;
64+
/// The request id.
65+
Id id = 0;
66+
/// The result of the request, either an Error or the JSON value of the
67+
/// response.
68+
std::variant<Error, llvm::json::Value> result;
5769
};
5870

5971
llvm::json::Value toJSON(const Response &);
6072
bool fromJSON(const llvm::json::Value &, Response &, llvm::json::Path);
73+
bool operator==(const Response &, const Response &);
6174

6275
/// A notification which does not expect a response.
6376
struct Notification {
77+
/// The method to be invoked.
6478
std::string method;
79+
/// The notification's params.
6580
std::optional<llvm::json::Value> params;
6681
};
6782

6883
llvm::json::Value toJSON(const Notification &);
6984
bool fromJSON(const llvm::json::Value &, Notification &, llvm::json::Path);
85+
bool operator==(const Notification &, const Notification &);
86+
87+
/// A general message as defined by the JSON-RPC 2.0 spec.
88+
using Message = std::variant<Request, Response, Notification>;
89+
90+
bool fromJSON(const llvm::json::Value &, Message &, llvm::json::Path);
91+
llvm::json::Value toJSON(const Message &);
7092

7193
struct ToolCapability {
7294
/// Whether this server supports notifications for changes to the tool list.
@@ -176,11 +198,6 @@ struct ToolDefinition {
176198
llvm::json::Value toJSON(const ToolDefinition &);
177199
bool fromJSON(const llvm::json::Value &, ToolDefinition &, llvm::json::Path);
178200

179-
using Message = std::variant<Request, Response, Notification, Error>;
180-
181-
bool fromJSON(const llvm::json::Value &, Message &, llvm::json::Path);
182-
llvm::json::Value toJSON(const Message &);
183-
184201
using ToolArguments = std::variant<std::monostate, llvm::json::Value>;
185202

186203
} // namespace lldb_protocol::mcp

‎lldb/source/Protocol/MCP/MCPError.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ std::error_code MCPError::convertToErrorCode() const {
2525
return llvm::inconvertibleErrorCode();
2626
}
2727

28-
lldb_protocol::mcp::Error MCPError::toProtcolError() const {
28+
lldb_protocol::mcp::Error MCPError::toProtocolError() const {
2929
lldb_protocol::mcp::Error error;
30-
error.error.code = m_error_code;
31-
error.error.message = m_message;
30+
error.code = m_error_code;
31+
error.message = m_message;
3232
return error;
3333
}
3434

‎lldb/source/Protocol/MCP/Protocol.cpp

Lines changed: 98 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -26,56 +26,121 @@ static bool mapRaw(const json::Value &Params, StringLiteral Prop,
2626
return true;
2727
}
2828

29+
static llvm::json::Value toJSON(const Id &Id) {
30+
if (const int64_t *I = std::get_if<int64_t>(&Id))
31+
return json::Value(*I);
32+
if (const std::string *S = std::get_if<std::string>(&Id))
33+
return json::Value(*S);
34+
llvm_unreachable("unexpected type in protocol::Id");
35+
}
36+
37+
static bool mapId(const llvm::json::Value &V, StringLiteral Prop, Id &Id,
38+
llvm::json::Path P) {
39+
const auto *O = V.getAsObject();
40+
if (!O) {
41+
P.report("expected object");
42+
return false;
43+
}
44+
45+
const auto *E = O->get(Prop);
46+
if (!E) {
47+
P.field(Prop).report("not found");
48+
return false;
49+
}
50+
51+
if (auto S = E->getAsString()) {
52+
Id = S->str();
53+
return true;
54+
}
55+
56+
if (auto I = E->getAsInteger()) {
57+
Id = *I;
58+
return true;
59+
}
60+
61+
P.report("expected string or number");
62+
return false;
63+
}
64+
2965
llvm::json::Value toJSON(const Request &R) {
30-
json::Object Result{{"jsonrpc", "2.0"}, {"id", R.id}, {"method", R.method}};
66+
json::Object Result{
67+
{"jsonrpc", "2.0"}, {"id", toJSON(R.id)}, {"method", R.method}};
3168
if (R.params)
3269
Result.insert({"params", R.params});
3370
return Result;
3471
}
3572

3673
bool fromJSON(const llvm::json::Value &V, Request &R, llvm::json::Path P) {
3774
llvm::json::ObjectMapper O(V, P);
38-
if (!O || !O.map("id", R.id) || !O.map("method", R.method))
39-
return false;
40-
return mapRaw(V, "params", R.params, P);
41-
}
42-
43-
llvm::json::Value toJSON(const ErrorInfo &EI) {
44-
llvm::json::Object Result{{"code", EI.code}, {"message", EI.message}};
45-
if (!EI.data.empty())
46-
Result.insert({"data", EI.data});
47-
return Result;
75+
return O && mapId(V, "id", R.id, P) && O.map("method", R.method) &&
76+
mapRaw(V, "params", R.params, P);
4877
}
4978

50-
bool fromJSON(const llvm::json::Value &V, ErrorInfo &EI, llvm::json::Path P) {
51-
llvm::json::ObjectMapper O(V, P);
52-
return O && O.map("code", EI.code) && O.map("message", EI.message) &&
53-
O.mapOptional("data", EI.data);
79+
bool operator==(const Request &a, const Request &b) {
80+
return a.id == b.id && a.method == b.method && a.params == b.params;
5481
}
5582

5683
llvm::json::Value toJSON(const Error &E) {
57-
return json::Object{{"jsonrpc", "2.0"}, {"id", E.id}, {"error", E.error}};
84+
llvm::json::Object Result{{"code", E.code}, {"message", E.message}};
85+
if (E.data)
86+
Result.insert({"data", *E.data});
87+
return Result;
5888
}
5989

6090
bool fromJSON(const llvm::json::Value &V, Error &E, llvm::json::Path P) {
6191
llvm::json::ObjectMapper O(V, P);
62-
return O && O.map("id", E.id) && O.map("error", E.error);
92+
return O && O.map("code", E.code) && O.map("message", E.message) &&
93+
mapRaw(V, "data", E.data, P);
94+
}
95+
96+
bool operator==(const Error &a, const Error &b) {
97+
return a.code == b.code && a.message == b.message && a.data == b.data;
6398
}
6499

65100
llvm::json::Value toJSON(const Response &R) {
66-
llvm::json::Object Result{{"jsonrpc", "2.0"}, {"id", R.id}};
67-
if (R.result)
68-
Result.insert({"result", R.result});
69-
if (R.error)
70-
Result.insert({"error", R.error});
101+
llvm::json::Object Result{{"jsonrpc", "2.0"}, {"id", toJSON(R.id)}};
102+
103+
if (const Error *error = std::get_if<Error>(&R.result))
104+
Result.insert({"error", *error});
105+
if (const json::Value *result = std::get_if<json::Value>(&R.result))
106+
Result.insert({"result", *result});
71107
return Result;
72108
}
73109

74110
bool fromJSON(const llvm::json::Value &V, Response &R, llvm::json::Path P) {
75-
llvm::json::ObjectMapper O(V, P);
76-
if (!O || !O.map("id", R.id) || !O.map("error", R.error))
111+
const json::Object *E = V.getAsObject();
112+
if (!E) {
113+
P.report("expected object");
114+
return false;
115+
}
116+
117+
const json::Value *result = E->get("result");
118+
const json::Value *raw_error = E->get("error");
119+
120+
if (result && raw_error) {
121+
P.report("'result' and 'error' fields are mutually exclusive");
77122
return false;
78-
return mapRaw(V, "result", R.result, P);
123+
}
124+
125+
if (!result && !raw_error) {
126+
P.report("'result' or 'error' fields are required'");
127+
return false;
128+
}
129+
130+
if (result) {
131+
R.result = std::move(*result);
132+
} else {
133+
Error error;
134+
if (!fromJSON(*raw_error, error, P))
135+
return false;
136+
R.result = std::move(error);
137+
}
138+
139+
return mapId(V, "id", R.id, P);
140+
}
141+
142+
bool operator==(const Response &a, const Response &b) {
143+
return a.id == b.id && a.result == b.result;
79144
}
80145

81146
llvm::json::Value toJSON(const Notification &N) {
@@ -97,6 +162,10 @@ bool fromJSON(const llvm::json::Value &V, Notification &N, llvm::json::Path P) {
97162
return true;
98163
}
99164

165+
bool operator==(const Notification &a, const Notification &b) {
166+
return a.method == b.method && a.params == b.params;
167+
}
168+
100169
llvm::json::Value toJSON(const ToolCapability &TC) {
101170
return llvm::json::Object{{"listChanged", TC.listChanged}};
102171
}
@@ -235,24 +304,16 @@ bool fromJSON(const llvm::json::Value &V, Message &M, llvm::json::Path P) {
235304
return true;
236305
}
237306

238-
if (O->get("error")) {
239-
Error E;
240-
if (!fromJSON(V, E, P))
241-
return false;
242-
M = std::move(E);
243-
return true;
244-
}
245-
246-
if (O->get("result")) {
247-
Response R;
307+
if (O->get("method")) {
308+
Request R;
248309
if (!fromJSON(V, R, P))
249310
return false;
250311
M = std::move(R);
251312
return true;
252313
}
253314

254-
if (O->get("method")) {
255-
Request R;
315+
if (O->get("result") || O->get("error")) {
316+
Response R;
256317
if (!fromJSON(V, R, P))
257318
return false;
258319
M = std::move(R);

‎lldb/source/Protocol/MCP/Server.cpp

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,15 @@ Server::HandleData(llvm::StringRef data) {
6666
Error protocol_error;
6767
llvm::handleAllErrors(
6868
response.takeError(),
69-
[&](const MCPError &err) { protocol_error = err.toProtcolError(); },
69+
[&](const MCPError &err) { protocol_error = err.toProtocolError(); },
7070
[&](const llvm::ErrorInfoBase &err) {
71-
protocol_error.error.code = MCPError::kInternalError;
72-
protocol_error.error.message = err.message();
71+
protocol_error.code = MCPError::kInternalError;
72+
protocol_error.message = err.message();
7373
});
74-
protocol_error.id = request->id;
75-
return protocol_error;
74+
Response error_response;
75+
error_response.id = request->id;
76+
error_response.result = std::move(protocol_error);
77+
return error_response;
7678
}
7779

7880
return *response;
@@ -84,9 +86,6 @@ Server::HandleData(llvm::StringRef data) {
8486
return std::nullopt;
8587
}
8688

87-
if (std::get_if<Error>(&(*message)))
88-
return llvm::createStringError("unexpected MCP message: error");
89-
9089
if (std::get_if<Response>(&(*message)))
9190
return llvm::createStringError("unexpected MCP message: response");
9291

@@ -123,11 +122,11 @@ void Server::AddNotificationHandler(llvm::StringRef method,
123122

124123
llvm::Expected<Response> Server::InitializeHandler(const Request &request) {
125124
Response response;
126-
response.result.emplace(llvm::json::Object{
125+
response.result = llvm::json::Object{
127126
{"protocolVersion", mcp::kProtocolVersion},
128127
{"capabilities", GetCapabilities()},
129128
{"serverInfo",
130-
llvm::json::Object{{"name", m_name}, {"version", m_version}}}});
129+
llvm::json::Object{{"name", m_name}, {"version", m_version}}}};
131130
return response;
132131
}
133132

@@ -138,7 +137,7 @@ llvm::Expected<Response> Server::ToolsListHandler(const Request &request) {
138137
for (const auto &tool : m_tools)
139138
tools.emplace_back(toJSON(tool.second->GetDefinition()));
140139

141-
response.result.emplace(llvm::json::Object{{"tools", std::move(tools)}});
140+
response.result = llvm::json::Object{{"tools", std::move(tools)}};
142141

143142
return response;
144143
}
@@ -173,7 +172,7 @@ llvm::Expected<Response> Server::ToolsCallHandler(const Request &request) {
173172
if (!text_result)
174173
return text_result.takeError();
175174

176-
response.result.emplace(toJSON(*text_result));
175+
response.result = toJSON(*text_result);
177176

178177
return response;
179178
}
@@ -189,8 +188,7 @@ llvm::Expected<Response> Server::ResourcesListHandler(const Request &request) {
189188
for (const Resource &resource : resource_provider_up->GetResources())
190189
resources.push_back(resource);
191190
}
192-
response.result.emplace(
193-
llvm::json::Object{{"resources", std::move(resources)}});
191+
response.result = llvm::json::Object{{"resources", std::move(resources)}};
194192

195193
return response;
196194
}
@@ -226,7 +224,7 @@ llvm::Expected<Response> Server::ResourcesReadHandler(const Request &request) {
226224
return result.takeError();
227225

228226
Response response;
229-
response.result.emplace(std::move(*result));
227+
response.result = std::move(*result);
230228
return response;
231229
}
232230

0 commit comments

Comments
 (0)