diff --git a/lldb/include/lldb/Host/JSONTransport.h b/lldb/include/lldb/Host/JSONTransport.h index 0be60a8f3f96a..210f33edace6e 100644 --- a/lldb/include/lldb/Host/JSONTransport.h +++ b/lldb/include/lldb/Host/JSONTransport.h @@ -100,7 +100,8 @@ template class Transport { virtual llvm::Expected RegisterMessageHandler(MainLoop &loop, MessageHandler &handler) = 0; -protected: + // FIXME: Refactor mcp::Server to not directly access log on the transport. + // protected: template inline auto Logv(const char *Fmt, Ts &&...Vals) { Log(llvm::formatv(Fmt, std::forward(Vals)...).str()); } @@ -139,9 +140,7 @@ class JSONTransport : public Transport { /// detail. static constexpr size_t kReadBufferSize = 1024; -protected: - virtual llvm::Expected> Parse() = 0; - virtual std::string Encode(const llvm::json::Value &message) = 0; + // FIXME: Write should be protected. llvm::Error Write(const llvm::json::Value &message) { this->Logv("<-- {0}", message); std::string output = Encode(message); @@ -149,6 +148,10 @@ class JSONTransport : public Transport { return m_out->Write(output.data(), bytes_written).takeError(); } +protected: + virtual llvm::Expected> Parse() = 0; + virtual std::string Encode(const llvm::json::Value &message) = 0; + llvm::SmallString m_buffer; private: diff --git a/lldb/include/lldb/Protocol/MCP/Server.h b/lldb/include/lldb/Protocol/MCP/Server.h index c6e78a9ea0cff..254b7d9680cd8 100644 --- a/lldb/include/lldb/Protocol/MCP/Server.h +++ b/lldb/include/lldb/Protocol/MCP/Server.h @@ -14,33 +14,17 @@ #include "lldb/Protocol/MCP/Protocol.h" #include "lldb/Protocol/MCP/Resource.h" #include "lldb/Protocol/MCP/Tool.h" +#include "lldb/Protocol/MCP/Transport.h" #include "llvm/ADT/StringMap.h" #include "llvm/Support/Error.h" -#include +#include "llvm/Support/JSON.h" +#include +#include +#include +#include namespace lldb_protocol::mcp { -class MCPTransport - : public lldb_private::JSONRPCTransport { -public: - using LogCallback = std::function; - - MCPTransport(lldb::IOObjectSP in, lldb::IOObjectSP out, - std::string client_name, LogCallback log_callback = {}) - : JSONRPCTransport(in, out), m_client_name(std::move(client_name)), - m_log_callback(log_callback) {} - virtual ~MCPTransport() = default; - - void Log(llvm::StringRef message) override { - if (m_log_callback) - m_log_callback(llvm::formatv("{0}: {1}", m_client_name, message).str()); - } - -private: - std::string m_client_name; - LogCallback m_log_callback; -}; - /// Information about this instance of lldb's MCP server for lldb-mcp to use to /// coordinate connecting an lldb-mcp client. struct ServerInfo { diff --git a/lldb/include/lldb/Protocol/MCP/Transport.h b/lldb/include/lldb/Protocol/MCP/Transport.h new file mode 100644 index 0000000000000..47c2ccfc44dfe --- /dev/null +++ b/lldb/include/lldb/Protocol/MCP/Transport.h @@ -0,0 +1,48 @@ +//===----------------------------------------------------------------------===// +// +// 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_TRANSPORT_H +#define LLDB_PROTOCOL_MCP_TRANSPORT_H + +#include "lldb/Host/JSONTransport.h" +#include "lldb/Protocol/MCP/Protocol.h" +#include "lldb/lldb-forward.h" +#include "llvm/ADT/FunctionExtras.h" +#include "llvm/ADT/StringRef.h" + +namespace lldb_protocol::mcp { + +/// Generic transport that uses the MCP protocol. +using MCPTransport = lldb_private::Transport; + +/// Generic logging callback, to allow the MCP server / client / transport layer +/// to be independent of the lldb log implementation. +using LogCallback = llvm::unique_function; + +class Transport final + : public lldb_private::JSONRPCTransport { +public: + Transport(lldb::IOObjectSP in, lldb::IOObjectSP out, + LogCallback log_callback = {}); + virtual ~Transport() = default; + + /// Transport is not copyable. + /// @{ + Transport(const Transport &) = delete; + void operator=(const Transport &) = delete; + /// @} + + void Log(llvm::StringRef message) override; + +private: + LogCallback m_log_callback; +}; + +} // namespace lldb_protocol::mcp + +#endif diff --git a/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.cpp b/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.cpp index 12cb25788534e..2b004c19e88a6 100644 --- a/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.cpp +++ b/lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.cpp @@ -70,9 +70,9 @@ void ProtocolServerMCP::AcceptCallback(std::unique_ptr socket) { LLDB_LOG(log, "New MCP client connected: {0}", client_name); lldb::IOObjectSP io_sp = std::move(socket); - auto transport_up = std::make_unique( - io_sp, io_sp, std::move(client_name), [&](llvm::StringRef message) { - LLDB_LOG(GetLog(LLDBLog::Host), "{0}", message); + auto transport_up = std::make_unique( + io_sp, io_sp, [client_name](llvm::StringRef message) { + LLDB_LOG(GetLog(LLDBLog::Host), "{0}: {1}", client_name, message); }); auto instance_up = std::make_unique( std::string(kName), std::string(kVersion), std::move(transport_up), diff --git a/lldb/source/Protocol/MCP/CMakeLists.txt b/lldb/source/Protocol/MCP/CMakeLists.txt index a4f270a83c43b..5258cb61a7d10 100644 --- a/lldb/source/Protocol/MCP/CMakeLists.txt +++ b/lldb/source/Protocol/MCP/CMakeLists.txt @@ -3,6 +3,7 @@ add_lldb_library(lldbProtocolMCP NO_PLUGIN_DEPENDENCIES Protocol.cpp Server.cpp Tool.cpp + Transport.cpp LINK_COMPONENTS Support diff --git a/lldb/source/Protocol/MCP/Transport.cpp b/lldb/source/Protocol/MCP/Transport.cpp new file mode 100644 index 0000000000000..cccdc3b5bd65c --- /dev/null +++ b/lldb/source/Protocol/MCP/Transport.cpp @@ -0,0 +1,23 @@ +//===----------------------------------------------------------------------===// +// +// 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/Transport.h" +#include "llvm/ADT/StringRef.h" +#include + +using namespace lldb_protocol::mcp; +using namespace llvm; + +Transport::Transport(lldb::IOObjectSP in, lldb::IOObjectSP out, + LogCallback log_callback) + : JSONRPCTransport(in, out), m_log_callback(std::move(log_callback)) {} + +void Transport::Log(StringRef message) { + if (m_log_callback) + m_log_callback(message); +} diff --git a/lldb/tools/lldb-mcp/lldb-mcp.cpp b/lldb/tools/lldb-mcp/lldb-mcp.cpp index 1f82af94820da..6c4ebbaa5f230 100644 --- a/lldb/tools/lldb-mcp/lldb-mcp.cpp +++ b/lldb/tools/lldb-mcp/lldb-mcp.cpp @@ -67,9 +67,10 @@ int main(int argc, char *argv[]) { [](MainLoopBase &loop) { loop.RequestTermination(); }); }); - auto transport_up = std::make_unique( - input, output, std::string(client_name), - [&](llvm::StringRef message) { llvm::errs() << message << '\n'; }); + auto transport_up = std::make_unique( + input, output, [&](llvm::StringRef message) { + llvm::errs() << formatv("{0}: {1}", client_name, message) << '\n'; + }); auto instance_up = std::make_unique( std::string(kName), std::string(kVersion), std::move(transport_up), loop); diff --git a/lldb/unittests/Protocol/ProtocolMCPServerTest.cpp b/lldb/unittests/Protocol/ProtocolMCPServerTest.cpp index 9fa446133d46f..f686255c6d41d 100644 --- a/lldb/unittests/Protocol/ProtocolMCPServerTest.cpp +++ b/lldb/unittests/Protocol/ProtocolMCPServerTest.cpp @@ -21,6 +21,7 @@ #include "lldb/Protocol/MCP/Resource.h" #include "lldb/Protocol/MCP/Server.h" #include "lldb/Protocol/MCP/Tool.h" +#include "lldb/Protocol/MCP/Transport.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/JSON.h" @@ -36,19 +37,6 @@ using namespace lldb_private; using namespace lldb_protocol::mcp; namespace { -class TestMCPTransport final : public MCPTransport { -public: - TestMCPTransport(lldb::IOObjectSP in, lldb::IOObjectSP out) - : lldb_protocol::mcp::MCPTransport(in, out, "unittest") {} - - using MCPTransport::Write; - - void Log(llvm::StringRef message) override { - log_messages.emplace_back(message); - } - - std::vector log_messages; -}; class TestServer : public Server { public: @@ -134,7 +122,7 @@ class ProtocolServerMCPTest : public PipePairTest { public: SubsystemRAII subsystems; - std::unique_ptr transport_up; + std::unique_ptr transport_up; std::unique_ptr server_up; MainLoop loop; MockMessageHandler message_handler; @@ -163,7 +151,7 @@ class ProtocolServerMCPTest : public PipePairTest { void SetUp() override { PipePairTest::SetUp(); - transport_up = std::make_unique( + transport_up = std::make_unique( std::make_shared(input.GetReadFileDescriptor(), File::eOpenOptionReadOnly, NativeFile::Unowned), @@ -173,7 +161,7 @@ class ProtocolServerMCPTest : public PipePairTest { server_up = std::make_unique( "lldb-mcp", "0.1.0", - std::make_unique( + std::make_unique( std::make_shared(output.GetReadFileDescriptor(), File::eOpenOptionReadOnly, NativeFile::Unowned),