https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/152188
>From 6519cdaa2339e78ed7f7d5ef58b3978746646b75 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Tue, 5 Aug 2025 11:26:27 -0700 Subject: [PATCH 1/2] [lldb] Move the generic MCP code into Protocol/MCP (NFC) This moves all the generic MCP code into the new ProtocolMCP library so it can be shared between the MCP implementation in LLDB (built on the private API) and lldb-mcp (built on the public API). This PR doesn't include the actual MCP server. The reason is that the transport mechanism will be different between the LLDB implementation (using sockets) and the lldb-mcp implementation (using stdio). The goal s to abstract away that difference by making the server use JSONTransport (again). --- .../lldb}/Protocol/MCP/MCPError.h | 12 ++++-- lldb/include/lldb/Protocol/MCP/Resource.h | 29 +++++++++++++ lldb/include/lldb/Protocol/MCP/Tool.h | 41 +++++++++++++++++++ .../Plugins/Protocol/MCP/CMakeLists.txt | 1 - .../Protocol/MCP/ProtocolServerMCP.cpp | 8 +++- .../Plugins/Protocol/MCP/ProtocolServerMCP.h | 14 ++++--- lldb/source/Plugins/Protocol/MCP/Resource.cpp | 3 +- lldb/source/Plugins/Protocol/MCP/Resource.h | 13 +----- lldb/source/Plugins/Protocol/MCP/Tool.cpp | 14 ------- lldb/source/Plugins/Protocol/MCP/Tool.h | 26 ++---------- lldb/source/Protocol/MCP/CMakeLists.txt | 3 ++ .../{Plugins => }/Protocol/MCP/MCPError.cpp | 6 +-- lldb/source/Protocol/MCP/Tool.cpp | 25 +++++++++++ .../ProtocolServer/ProtocolMCPServerTest.cpp | 21 +++++----- 14 files changed, 141 insertions(+), 75 deletions(-) rename lldb/{source/Plugins => include/lldb}/Protocol/MCP/MCPError.h (85%) create mode 100644 lldb/include/lldb/Protocol/MCP/Resource.h create mode 100644 lldb/include/lldb/Protocol/MCP/Tool.h rename lldb/source/{Plugins => }/Protocol/MCP/MCPError.cpp (89%) create mode 100644 lldb/source/Protocol/MCP/Tool.cpp diff --git a/lldb/source/Plugins/Protocol/MCP/MCPError.h b/lldb/include/lldb/Protocol/MCP/MCPError.h similarity index 85% rename from lldb/source/Plugins/Protocol/MCP/MCPError.h rename to lldb/include/lldb/Protocol/MCP/MCPError.h index c93e959574938..cd0f94a335b49 100644 --- a/lldb/source/Plugins/Protocol/MCP/MCPError.h +++ b/lldb/include/lldb/Protocol/MCP/MCPError.h @@ -1,4 +1,4 @@ -//===-- MCPError.h --------------------------------------------------------===// +//===----------------------------------------------------------------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -8,10 +8,12 @@ #include "lldb/Protocol/MCP/Protocol.h" #include "llvm/Support/Error.h" -#include "llvm/Support/FormatVariadic.h" #include <string> -namespace lldb_private::mcp { +#ifndef LLDB_PROTOCOL_MCP_MCPERROR_H +#define LLDB_PROTOCOL_MCP_MCPERROR_H + +namespace lldb_protocol::mcp { class MCPError : public llvm::ErrorInfo<MCPError> { public: @@ -47,4 +49,6 @@ class UnsupportedURI : public llvm::ErrorInfo<UnsupportedURI> { std::string m_uri; }; -} // namespace lldb_private::mcp +} // namespace lldb_protocol::mcp + +#endif diff --git a/lldb/include/lldb/Protocol/MCP/Resource.h b/lldb/include/lldb/Protocol/MCP/Resource.h new file mode 100644 index 0000000000000..4835d340cd4c6 --- /dev/null +++ b/lldb/include/lldb/Protocol/MCP/Resource.h @@ -0,0 +1,29 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_PROTOCOL_MCP_RESOURCE_H +#define LLDB_PROTOCOL_MCP_RESOURCE_H + +#include "lldb/Protocol/MCP/Protocol.h" +#include <vector> + +namespace lldb_protocol::mcp { + +class ResourceProvider { +public: + ResourceProvider() = default; + virtual ~ResourceProvider() = default; + + virtual std::vector<lldb_protocol::mcp::Resource> GetResources() const = 0; + virtual llvm::Expected<lldb_protocol::mcp::ResourceResult> + ReadResource(llvm::StringRef uri) const = 0; +}; + +} // namespace lldb_protocol::mcp + +#endif diff --git a/lldb/include/lldb/Protocol/MCP/Tool.h b/lldb/include/lldb/Protocol/MCP/Tool.h new file mode 100644 index 0000000000000..96669d1357166 --- /dev/null +++ b/lldb/include/lldb/Protocol/MCP/Tool.h @@ -0,0 +1,41 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_PROTOCOL_MCP_TOOL_H +#define LLDB_PROTOCOL_MCP_TOOL_H + +#include "lldb/Protocol/MCP/Protocol.h" +#include "llvm/Support/JSON.h" +#include <string> + +namespace lldb_protocol::mcp { + +class Tool { +public: + Tool(std::string name, std::string description); + virtual ~Tool() = default; + + virtual llvm::Expected<lldb_protocol::mcp::TextResult> + Call(const lldb_protocol::mcp::ToolArguments &args) = 0; + + virtual std::optional<llvm::json::Value> GetSchema() const { + return llvm::json::Object{{"type", "object"}}; + } + + lldb_protocol::mcp::ToolDefinition GetDefinition() const; + + const std::string &GetName() { return m_name; } + +private: + std::string m_name; + std::string m_description; +}; + +} // namespace lldb_protocol::mcp + +#endif diff --git a/lldb/source/Plugins/Protocol/MCP/CMakeLists.txt b/lldb/source/Plugins/Protocol/MCP/CMakeLists.txt index 2740c0825a8c2..87565e693158a 100644 --- a/lldb/source/Plugins/Protocol/MCP/CMakeLists.txt +++ b/lldb/source/Plugins/Protocol/MCP/CMakeLists.txt @@ -1,5 +1,4 @@ add_lldb_library(lldbPluginProtocolServerMCP PLUGIN - MCPError.cpp ProtocolServerMCP.cpp Resource.cpp Tool.cpp diff --git a/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.cpp b/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.cpp index 4d517ee8158c4..c9fe474d45c49 100644 --- a/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.cpp +++ b/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.cpp @@ -7,8 +7,11 @@ //===----------------------------------------------------------------------===// #include "ProtocolServerMCP.h" -#include "MCPError.h" +#include "Resource.h" +#include "Tool.h" #include "lldb/Core/PluginManager.h" +#include "lldb/Protocol/MCP/MCPError.h" +#include "lldb/Protocol/MCP/Tool.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" #include "llvm/ADT/StringExtras.h" @@ -18,6 +21,7 @@ using namespace lldb_private; using namespace lldb_private::mcp; +using namespace lldb_protocol::mcp; using namespace llvm; LLDB_PLUGIN_DEFINE(ProtocolServerMCP) @@ -112,7 +116,7 @@ void ProtocolServerMCP::AcceptCallback(std::unique_ptr<Socket> socket) { auto read_handle_up = m_loop.RegisterReadObject( io_sp, [this, client](MainLoopBase &loop) { - if (Error error = ReadCallback(*client)) { + if (llvm::Error error = ReadCallback(*client)) { LLDB_LOG_ERROR(GetLog(LLDBLog::Host), std::move(error), "{0}"); client->read_handle_up.reset(); } diff --git a/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.h b/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.h index 611e62af5c8cb..2ea9585a2334b 100644 --- a/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.h +++ b/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.h @@ -9,12 +9,12 @@ #ifndef LLDB_PLUGINS_PROTOCOL_MCP_PROTOCOLSERVERMCP_H #define LLDB_PLUGINS_PROTOCOL_MCP_PROTOCOLSERVERMCP_H -#include "Resource.h" -#include "Tool.h" #include "lldb/Core/ProtocolServer.h" #include "lldb/Host/MainLoop.h" #include "lldb/Host/Socket.h" #include "lldb/Protocol/MCP/Protocol.h" +#include "lldb/Protocol/MCP/Resource.h" +#include "lldb/Protocol/MCP/Tool.h" #include "llvm/ADT/StringMap.h" #include <thread> @@ -47,8 +47,9 @@ class ProtocolServerMCP : public ProtocolServer { using NotificationHandler = std::function<void(const lldb_protocol::mcp::Notification &)>; - void AddTool(std::unique_ptr<Tool> tool); - void AddResourceProvider(std::unique_ptr<ResourceProvider> resource_provider); + void AddTool(std::unique_ptr<lldb_protocol::mcp::Tool> tool); + void AddResourceProvider( + std::unique_ptr<lldb_protocol::mcp::ResourceProvider> resource_provider); void AddRequestHandler(llvm::StringRef method, RequestHandler handler); void AddNotificationHandler(llvm::StringRef method, @@ -99,8 +100,9 @@ class ProtocolServerMCP : public ProtocolServer { std::vector<std::unique_ptr<Client>> m_clients; std::mutex m_server_mutex; - llvm::StringMap<std::unique_ptr<Tool>> m_tools; - std::vector<std::unique_ptr<ResourceProvider>> m_resource_providers; + llvm::StringMap<std::unique_ptr<lldb_protocol::mcp::Tool>> m_tools; + std::vector<std::unique_ptr<lldb_protocol::mcp::ResourceProvider>> + m_resource_providers; llvm::StringMap<RequestHandler> m_request_handlers; llvm::StringMap<NotificationHandler> m_notification_handlers; diff --git a/lldb/source/Plugins/Protocol/MCP/Resource.cpp b/lldb/source/Plugins/Protocol/MCP/Resource.cpp index bc39e1b0c0ed2..e94d2cdd65e07 100644 --- a/lldb/source/Plugins/Protocol/MCP/Resource.cpp +++ b/lldb/source/Plugins/Protocol/MCP/Resource.cpp @@ -5,13 +5,14 @@ //===----------------------------------------------------------------------===// #include "Resource.h" -#include "MCPError.h" #include "lldb/Core/Debugger.h" #include "lldb/Core/Module.h" +#include "lldb/Protocol/MCP/MCPError.h" #include "lldb/Target/Platform.h" using namespace lldb_private; using namespace lldb_private::mcp; +using namespace lldb_protocol::mcp; namespace { struct DebuggerResource { diff --git a/lldb/source/Plugins/Protocol/MCP/Resource.h b/lldb/source/Plugins/Protocol/MCP/Resource.h index 0066f2f8e1b0e..e2382a74f796b 100644 --- a/lldb/source/Plugins/Protocol/MCP/Resource.h +++ b/lldb/source/Plugins/Protocol/MCP/Resource.h @@ -10,22 +10,13 @@ #define LLDB_PLUGINS_PROTOCOL_MCP_RESOURCE_H #include "lldb/Protocol/MCP/Protocol.h" +#include "lldb/Protocol/MCP/Resource.h" #include "lldb/lldb-private.h" #include <vector> namespace lldb_private::mcp { -class ResourceProvider { -public: - ResourceProvider() = default; - virtual ~ResourceProvider() = default; - - virtual std::vector<lldb_protocol::mcp::Resource> GetResources() const = 0; - virtual llvm::Expected<lldb_protocol::mcp::ResourceResult> - ReadResource(llvm::StringRef uri) const = 0; -}; - -class DebuggerResourceProvider : public ResourceProvider { +class DebuggerResourceProvider : public lldb_protocol::mcp::ResourceProvider { public: using ResourceProvider::ResourceProvider; virtual ~DebuggerResourceProvider() = default; diff --git a/lldb/source/Plugins/Protocol/MCP/Tool.cpp b/lldb/source/Plugins/Protocol/MCP/Tool.cpp index 9df5ce843e0ff..143470702a6fd 100644 --- a/lldb/source/Plugins/Protocol/MCP/Tool.cpp +++ b/lldb/source/Plugins/Protocol/MCP/Tool.cpp @@ -41,20 +41,6 @@ static lldb_protocol::mcp::TextResult createTextResult(std::string output, } // namespace -Tool::Tool(std::string name, std::string description) - : m_name(std::move(name)), m_description(std::move(description)) {} - -lldb_protocol::mcp::ToolDefinition Tool::GetDefinition() const { - lldb_protocol::mcp::ToolDefinition definition; - definition.name = m_name; - definition.description = m_description; - - if (std::optional<llvm::json::Value> input_schema = GetSchema()) - definition.inputSchema = *input_schema; - - return definition; -} - llvm::Expected<lldb_protocol::mcp::TextResult> CommandTool::Call(const lldb_protocol::mcp::ToolArguments &args) { if (!std::holds_alternative<json::Value>(args)) diff --git a/lldb/source/Plugins/Protocol/MCP/Tool.h b/lldb/source/Plugins/Protocol/MCP/Tool.h index ec1d83a510964..b7b1756eb38d7 100644 --- a/lldb/source/Plugins/Protocol/MCP/Tool.h +++ b/lldb/source/Plugins/Protocol/MCP/Tool.h @@ -11,35 +11,15 @@ #include "lldb/Core/Debugger.h" #include "lldb/Protocol/MCP/Protocol.h" +#include "lldb/Protocol/MCP/Tool.h" #include "llvm/Support/JSON.h" #include <string> namespace lldb_private::mcp { -class Tool { +class CommandTool : public lldb_protocol::mcp::Tool { public: - Tool(std::string name, std::string description); - virtual ~Tool() = default; - - virtual llvm::Expected<lldb_protocol::mcp::TextResult> - Call(const lldb_protocol::mcp::ToolArguments &args) = 0; - - virtual std::optional<llvm::json::Value> GetSchema() const { - return llvm::json::Object{{"type", "object"}}; - } - - lldb_protocol::mcp::ToolDefinition GetDefinition() const; - - const std::string &GetName() { return m_name; } - -private: - std::string m_name; - std::string m_description; -}; - -class CommandTool : public mcp::Tool { -public: - using mcp::Tool::Tool; + using lldb_protocol::mcp::Tool::Tool; ~CommandTool() = default; virtual llvm::Expected<lldb_protocol::mcp::TextResult> diff --git a/lldb/source/Protocol/MCP/CMakeLists.txt b/lldb/source/Protocol/MCP/CMakeLists.txt index b197dc4c0430a..f1b1098e064a5 100644 --- a/lldb/source/Protocol/MCP/CMakeLists.txt +++ b/lldb/source/Protocol/MCP/CMakeLists.txt @@ -1,8 +1,11 @@ add_lldb_library(lldbProtocolMCP NO_PLUGIN_DEPENDENCIES + MCPError.cpp Protocol.cpp + Tool.cpp LINK_COMPONENTS Support LINK_LIBS lldbUtility ) + diff --git a/lldb/source/Plugins/Protocol/MCP/MCPError.cpp b/lldb/source/Protocol/MCP/MCPError.cpp similarity index 89% rename from lldb/source/Plugins/Protocol/MCP/MCPError.cpp rename to lldb/source/Protocol/MCP/MCPError.cpp index 1e358058fa86c..c610e882abf51 100644 --- a/lldb/source/Plugins/Protocol/MCP/MCPError.cpp +++ b/lldb/source/Protocol/MCP/MCPError.cpp @@ -1,4 +1,4 @@ -//===-- MCPError.cpp ------------------------------------------------------===// +//===----------------------------------------------------------------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -6,12 +6,12 @@ // //===----------------------------------------------------------------------===// -#include "MCPError.h" +#include "lldb/Protocol/MCP/MCPError.h" #include "llvm/Support/Error.h" #include "llvm/Support/raw_ostream.h" #include <system_error> -using namespace lldb_private::mcp; +using namespace lldb_protocol::mcp; char MCPError::ID; char UnsupportedURI::ID; diff --git a/lldb/source/Protocol/MCP/Tool.cpp b/lldb/source/Protocol/MCP/Tool.cpp new file mode 100644 index 0000000000000..8e01f2bd5908b --- /dev/null +++ b/lldb/source/Protocol/MCP/Tool.cpp @@ -0,0 +1,25 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Protocol/MCP/Tool.h" + +using namespace lldb_protocol::mcp; + +Tool::Tool(std::string name, std::string description) + : m_name(std::move(name)), m_description(std::move(description)) {} + +lldb_protocol::mcp::ToolDefinition Tool::GetDefinition() const { + lldb_protocol::mcp::ToolDefinition definition; + definition.name = m_name; + definition.description = m_description; + + if (std::optional<llvm::json::Value> input_schema = GetSchema()) + definition.inputSchema = *input_schema; + + return definition; +} diff --git a/lldb/unittests/ProtocolServer/ProtocolMCPServerTest.cpp b/lldb/unittests/ProtocolServer/ProtocolMCPServerTest.cpp index b1cc21a5b0c37..dc3b63ab65466 100644 --- a/lldb/unittests/ProtocolServer/ProtocolMCPServerTest.cpp +++ b/lldb/unittests/ProtocolServer/ProtocolMCPServerTest.cpp @@ -7,15 +7,16 @@ //===----------------------------------------------------------------------===// #include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h" -#include "Plugins/Protocol/MCP/MCPError.h" #include "Plugins/Protocol/MCP/ProtocolServerMCP.h" #include "TestingSupport/Host/SocketTestUtilities.h" #include "TestingSupport/SubsystemRAII.h" +#include "lldb/Core/Debugger.h" #include "lldb/Core/ProtocolServer.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/HostInfo.h" #include "lldb/Host/JSONTransport.h" #include "lldb/Host/Socket.h" +#include "lldb/Protocol/MCP/MCPError.h" #include "lldb/Protocol/MCP/Protocol.h" #include "llvm/Testing/Support/Error.h" #include "gtest/gtest.h" @@ -44,9 +45,9 @@ class TestJSONTransport : public lldb_private::JSONRPCTransport { }; /// Test tool that returns it argument as text. -class TestTool : public mcp::Tool { +class TestTool : public Tool { public: - using mcp::Tool::Tool; + using Tool::Tool; virtual llvm::Expected<TextResult> Call(const ToolArguments &args) override { std::string argument; @@ -63,8 +64,8 @@ class TestTool : public mcp::Tool { } }; -class TestResourceProvider : public mcp::ResourceProvider { - using mcp::ResourceProvider::ResourceProvider; +class TestResourceProvider : public ResourceProvider { + using ResourceProvider::ResourceProvider; virtual std::vector<Resource> GetResources() const override { std::vector<Resource> resources; @@ -82,7 +83,7 @@ class TestResourceProvider : public mcp::ResourceProvider { virtual llvm::Expected<ResourceResult> ReadResource(llvm::StringRef uri) const override { if (uri != "lldb://foo/bar") - return llvm::make_error<mcp::UnsupportedURI>(uri.str()); + return llvm::make_error<UnsupportedURI>(uri.str()); ResourceContents contents; contents.uri = "lldb://foo/bar"; @@ -96,9 +97,9 @@ class TestResourceProvider : public mcp::ResourceProvider { }; /// Test tool that returns an error. -class ErrorTool : public mcp::Tool { +class ErrorTool : public Tool { public: - using mcp::Tool::Tool; + using Tool::Tool; virtual llvm::Expected<TextResult> Call(const ToolArguments &args) override { return llvm::createStringError("error"); @@ -106,9 +107,9 @@ class ErrorTool : public mcp::Tool { }; /// Test tool that fails but doesn't return an error. -class FailTool : public mcp::Tool { +class FailTool : public Tool { public: - using mcp::Tool::Tool; + using Tool::Tool; virtual llvm::Expected<TextResult> Call(const ToolArguments &args) override { TextResult text_result; >From 9f3a4c01cd8d8f29c59f087f6c9cd21754609eac Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Tue, 5 Aug 2025 12:44:33 -0700 Subject: [PATCH 2/2] Move header guard --- lldb/include/lldb/Protocol/MCP/MCPError.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/include/lldb/Protocol/MCP/MCPError.h b/lldb/include/lldb/Protocol/MCP/MCPError.h index cd0f94a335b49..2bdbb9b7a6874 100644 --- a/lldb/include/lldb/Protocol/MCP/MCPError.h +++ b/lldb/include/lldb/Protocol/MCP/MCPError.h @@ -6,13 +6,13 @@ // //===----------------------------------------------------------------------===// +#ifndef LLDB_PROTOCOL_MCP_MCPERROR_H +#define LLDB_PROTOCOL_MCP_MCPERROR_H + #include "lldb/Protocol/MCP/Protocol.h" #include "llvm/Support/Error.h" #include <string> -#ifndef LLDB_PROTOCOL_MCP_MCPERROR_H -#define LLDB_PROTOCOL_MCP_MCPERROR_H - namespace lldb_protocol::mcp { class MCPError : public llvm::ErrorInfo<MCPError> { _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits