Address John's feedback

Author: JDevlieghere

lldb/source/Plugins/Protocol/MCP/Protocol.h

@@ -123,6 +123,8 @@ 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_private::mcp::protocol
 
 #endif

lldb/source/Plugins/Protocol/MCP/ProtocolServerMCP.cpp

@@ -314,8 +314,12 @@ ProtocolServerMCP::ToolsCallHandler(const protocol::Request &request) {
   if (it == m_tools.end())
     return llvm::createStringError(llvm::formatv("no tool \"{0}\"", tool_name));
 
-  const json::Value *args = param_obj->get("arguments");
-  llvm::Expected<protocol::TextResult> text_result = it->second->Call(args);
+  protocol::ToolArguments tool_args;
+  if (const json::Value *args = param_obj->get("arguments"))
+    tool_args = *args;
+
+  llvm::Expected<protocol::TextResult> text_result =
+      it->second->Call(tool_args);
   if (!text_result)
     return text_result.takeError();
 

lldb/source/Plugins/Protocol/MCP/Tool.cpp

@@ -14,6 +14,7 @@
 using namespace lldb_private::mcp;
 using namespace llvm;
 
+namespace {
 struct CommandToolArguments {
   uint64_t debugger_id;
   std::string arguments;
@@ -26,6 +27,8 @@ bool fromJSON(const llvm::json::Value &V, CommandToolArguments &A,
          O.mapOptional("arguments", A.arguments);
 }
 
+} // namespace
+
 Tool::Tool(std::string name, std::string description)
     : m_name(std::move(name)), m_description(std::move(description)) {}
 
@@ -41,14 +44,14 @@ protocol::ToolDefinition Tool::GetDefinition() const {
 }
 
 llvm::Expected<protocol::TextResult>
-CommandTool::Call(const llvm::json::Value *args) {
-  if (!args)
-    return createStringError("no tool arguments");
+CommandTool::Call(const protocol::ToolArguments &args) {
+  if (!std::holds_alternative<json::Value>(args))
+    return createStringError("CommandTool requires arguments");
 
-  llvm::json::Path::Root root;
+  json::Path::Root root;
 
   CommandToolArguments arguments;
-  if (!fromJSON(*args, arguments, root))
+  if (!fromJSON(std::get<json::Value>(args), arguments, root))
     return root.getError();
 
   lldb::DebuggerSP debugger_sp =
@@ -93,9 +96,22 @@ std::optional<llvm::json::Value> CommandTool::GetSchema() const {
 }
 
 llvm::Expected<protocol::TextResult>
-DebuggerListTool::Call(const llvm::json::Value *args) {
+DebuggerListTool::Call(const protocol::ToolArguments &args) {
+  if (!std::holds_alternative<std::monostate>(args))
+    return createStringError("DebuggerListTool takes no arguments");
+
   llvm::json::Path::Root root;
 
+  // Return a nested Markdown list with debuggers and target.
+  // Example output:
+  //
+  // - debugger 0
+  //     - target 0 /path/to/foo
+  //     - target 1
+  // - debugger 1
+  //     - target 0 /path/to/bar
+  //
+  // FIXME: Use Structured Content when we adopt protocol version 2025-06-18.
   std::string output;
   llvm::raw_string_ostream os(output);
 
@@ -114,8 +130,10 @@ DebuggerListTool::Call(const llvm::json::Value *args) {
       if (!target_sp)
         continue;
       os << "    - target " << j;
+      // Append the module path if we have one.
       if (Module *exe_module = target_sp->GetExecutableModulePointer())
         os << " " << exe_module->GetFileSpec().GetPath();
+      os << '\n';
     }
   }
 

lldb/source/Plugins/Protocol/MCP/Tool.h

@@ -22,7 +22,7 @@ class Tool {
   virtual ~Tool() = default;
 
   virtual llvm::Expected<protocol::TextResult>
-  Call(const llvm::json::Value *args) = 0;
+  Call(const protocol::ToolArguments &args) = 0;
 
   virtual std::optional<llvm::json::Value> GetSchema() const {
     return llvm::json::Object{{"type", "object"}};
@@ -43,7 +43,7 @@ class CommandTool : public mcp::Tool {
   ~CommandTool() = default;
 
   virtual llvm::Expected<protocol::TextResult>
-  Call(const llvm::json::Value *args) override;
+  Call(const protocol::ToolArguments &args) override;
 
   virtual std::optional<llvm::json::Value> GetSchema() const override;
 };
@@ -54,7 +54,7 @@ class DebuggerListTool : public mcp::Tool {
   ~DebuggerListTool() = default;
 
   virtual llvm::Expected<protocol::TextResult>
-  Call(const llvm::json::Value *args) override;
+  Call(const protocol::ToolArguments &args) override;
 };
 
 } // namespace lldb_private::mcp