forked from llvm/llvm-project
-
Notifications
You must be signed in to change notification settings - Fork 343
🍒 LLDB MCP Cherrypicks #11093
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
Open
JDevlieghere
wants to merge
14
commits into
stable/20240723
Choose a base branch
from
mcp-cherrypicks
base: stable/20240723
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
🍒 LLDB MCP Cherrypicks #11093
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvm#118330) This is most useful if you are listening on an address like 'localhost:0' and want to know the resolved ip + port of the socket listener. (cherry picked from commit 3845624)
To go along with the existing TCPSocket implementation. (cherry picked from commit ebbc9ed)
Move lldb-dap's Transport class into lldb_private so the code can be shared between the "JSON with header" protocol used by DAP and the JSON RPC protocol used by MCP (see [1]). [1]: https://discourse.llvm.org/t/rfc-adding-mcp-support-to-lldb/86798 (cherry picked from commit de51b2d)
This PR implements JSON RPC-style (i.e. newline delimited) JSON transport. I moved the existing transport tests from DAP to Host and moved the PipeTest base class into TestingSupport so it can be shared by both. (cherry picked from commit 8a2895a)
This PR adds an MCP (Model Context Protocol ) server to LLDB. For motivation and background, please refer to the corresponding RFC: https://discourse.llvm.org/t/rfc-adding-mcp-support-to-lldb/86798 I implemented this as a new kind of plugin. The idea is that we could support multiple protocol servers (e.g. if we want to support DAP from within LLDB). This also introduces a corresponding top-level command (`protocol-server`) with two subcommands to `start` and `stop` the server. ``` (lldb) protocol-server start MCP tcp://localhost:1234 MCP server started with connection listeners: connection://[::1]:1234, connection://[127.0.0.1]:1234 ``` The MCP sever supports one tool (`lldb_command`) which executes a command, but can easily be extended with more commands. (cherry picked from commit 9524bfb)
Use the same scheme as ConnectionFileDescriptor::Connect and use "listen" and "accept". Addresses feedback from a Pavel in a different PR [1]. [1] llvm#143628 (comment) (cherry picked from commit 4f991cc)
We had two classes named `PipeTest`: one in `PipeTestUtilities.h` and one in `PipeTest.cpp`. The latter was unintentionally using the wrong class (from the header) which didn't initialize the HostInfo subsystem. This resulted in a crash due to a nullptr dereference (`g_fields`) when `PipePosix::CreateWithUniqueName` called `HostInfoBase::GetProcessTempDir`. (cherry picked from commit e89458d)
Add unit testing for the different message types. (cherry picked from commit 7e3af67)
Rather than having one MCP server per debugger, make the MCP server global and pass a debugger id along with tool invocations that require one. This PR also adds a second tool to list the available debuggers with their targets so the model can decide which debugger instance to use. (cherry picked from commit e8abdfc)
) Expose debuggers and target as resources through MCP. This has two advantages: 1. Enables returning data in a structured way. Although tools can return structured data with the latest revision of the protocol, we might not be able to update before the majority of clients has adopted it. 2. Enables the user to specify a resource themselves, rather than letting the model guess which debugger instance it should use. This PR exposes a resource for debuggers and targets. The following URI returns information about a given debugger instance: ``` lldb://debugger/<debugger id> ``` For example: ``` { uri: "lldb://debugger/0" mimeType: "application/json" text: "{"debugger_id":0,"num_targets":2}" } ``` The following URI returns information about a given target: ``` lldb://debugger/<debugger id>/target/<target id> ``` For example: ``` { uri: "lldb://debugger/0/target/0" mimeType: "application/json" text: "{"arch":"arm64-apple-macosx26.0.0","debugger_id":0,"path":"/Users/jonas/llvm/build-ra/bin/count","target_id":0}" } ``` (cherry picked from commit 3c4c2fa)
Instead of storing a `std::optional<std::string>`, directly use a `std::string` and treat a missing value the same was as an empty string. (cherry picked from commit 6fea3da)
@swift-ci test |
The main motivation for this was the inconsistency in handling of partial reads/writes between the windows and posix implementations (windows was returning partial reads, posix was trying to fill the buffer completely). I settle on the windows implementation, as that's the more common behavior, and the "eager" version can be implemented on top of that (in most cases, it isn't necessary, since we're writing just a single byte). Since this also required auditing the callers to make sure they're handling partial reads/writes correctly, I used the opportunity to modernize the function signatures as a forcing function. They now use the `Timeout` class (basically an `optional<duration>`) to support both polls (timeout=0) and blocking (timeout=nullopt) operations in a single function, and use an `Expected` instead of a by-ref result to return the number of bytes read/written. As a drive-by, I also fix a problem with the windows implementation where we were rounding the timeout value down, which meant that calls could time out slightly sooner than expected. (cherry picked from commit c0b5451)
@swift-ci test |
@swift-ci test macos |
@swift-ci test macos windows |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.