Skip to content

[lldb][mcp] Fix unix domain socket protocol server addresses #146603

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions lldb/source/Commands/CommandObjectProtocolServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,12 @@ class CommandObjectProtocolServerStart : public CommandObjectParsed {

ProtocolServer::Connection connection;
connection.protocol = protocol_and_mode->first;
connection.name =
formatv("[{0}]:{1}", uri->hostname.empty() ? "0.0.0.0" : uri->hostname,
uri->port.value_or(0));
if (connection.protocol == Socket::SocketProtocol::ProtocolUnixDomain)
connection.name = uri->path;
else
connection.name = formatv(
"[{0}]:{1}", uri->hostname.empty() ? "0.0.0.0" : uri->hostname,
uri->port.value_or(0));

if (llvm::Error error = server->Start(connection)) {
result.AppendErrorWithFormatv("{0}", llvm::fmt_consume(std::move(error)));
Expand All @@ -90,6 +93,7 @@ class CommandObjectProtocolServerStart : public CommandObjectParsed {
result.AppendMessageWithFormatv(
"{0} server started with connection listeners: {1}", protocol,
address);
result.SetStatus(eReturnStatusSuccessFinishNoResult);
}
}
};
Expand Down
33 changes: 33 additions & 0 deletions lldb/test/API/commands/protocol/TestMCPUnixSocket.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import os
import tempfile
import unittest

from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *

# To be safe and portable, Unix domain socket paths should be kept at or below
# 108 characters on Linux, and around 104 characters on macOS:
MAX_SOCKET_PATH_LENGTH = 104


class MCPUnixSocketCommandTestCase(TestBase):
@skipIfWindows
@no_debug_info_test
def test_unix_socket(self):
"""
Test if we can start an MCP protocol-server accepting unix sockets
"""

temp_directory = tempfile.TemporaryDirectory()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One note, for unix sockets there is often a character limit of around 104~108 chars depending on the platform.

This will often fetch a directory using TEMP_DIR, which may be overridden by say a CI system to something pretty long.

We may either want to skip the test if the tempdir is to long or try to find a shorter directory like "/tmp", if it exists.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I've pushed a change to make sure we skip if the socket path is over that limit

socket_file = os.path.join(temp_directory.name, "mcp.sock")

if len(socket_file) >= MAX_SOCKET_PATH_LENGTH:
self.skipTest(
f"Socket path {socket_file} exceeds the {MAX_SOCKET_PATH_LENGTH} character limit"
)

self.expect(
f"protocol-server start MCP accept://{socket_file}",
startstr="MCP server started with connection listeners:",
substrs=[f"unix-connect://{socket_file}"],
)
Loading