Move ldk-server-mcp into the workspace#188
Open
tnull wants to merge 11 commits intolightningdevkit:mainfrom
Open
Move ldk-server-mcp into the workspace#188tnull wants to merge 11 commits intolightningdevkit:mainfrom
ldk-server-mcp into the workspace#188tnull wants to merge 11 commits intolightningdevkit:mainfrom
Conversation
|
🎉 This PR is now ready for review! |
e090553 to
70f8e5f
Compare
Move the MCP bridge into the ldk-server workspace and switch it to an in-tree client dependency so workspace builds and tests cover it directly. Co-Authored-By: HAL 9000
70f8e5f to
3bc8a6e
Compare
Collaborator
benthecarman
left a comment
There was a problem hiding this comment.
Did a quick skim and ran through codex
Codex Review:
- [P2] Honor storage.disk.dir_path when resolving credentials — /home/ben/projects/ldk-server/ldk-server-mcp/src/
config.rs:57-60
If the shared ldk-server config uses a custom storage.disk.dir_path, this parser drops that section entirely, so
resolve_config() can only look in ~/.ldk-server/... for api_key and tls.crt. In deployments that store node state
outside the default directory (a setup ldk-server-cli already supports), the MCP bridge will fail to authenticate
unless users also set both LDK_API_KEY and LDK_TLS_CERT_PATH by hand.
- [P2] Fall back to the default gRPC address when no base URL is configured — /home/ben/projects/ldk-server/ldk-
server-mcp/src/config.rs:112-116
In setups that rely on the standard 127.0.0.1:3536 gRPC address and only use the default credential files on disk,
this path exits with “Base URL not provided” instead of using the same default as ldk-server-cli. As a result, ldk-
server-mcp cannot start in the zero-config case unless users create a config file or export LDK_BASE_URL, even
though the server is running at the normal address.
I think these are two things we fixed in the cli since you created the MCP server.
Support storage.disk.dir_path config and fall back to the default gRPC address when neither LDK_BASE_URL nor a config file is present, matching the behavior of ldk-server-cli. Co-Authored-By: HAL 9000
Fix clippy needless_borrows_for_generic_args in test. Co-Authored-By: HAL 9000
Move routing and invoice default constants into ldk-server-client so they are shared between ldk-server-cli and ldk-server-mcp. Co-Authored-By: HAL 9000
Use the workspace-built MCP binary directly in tests and add focused helper coverage for argument parsing so regressions are caught without requiring a live server. Co-Authored-By: HAL 9000
Build the MCP binary in the e2e harness and exercise its stdio protocol against a live ldk-server so basic MCP functionality is verified without involving an agent. Co-Authored-By: HAL 9000
Drop the tests/ directory from rerun-if-changed since test changes don't affect the mcp binary. Co-Authored-By: HAL 9000
Run MCP-specific formatting, clippy, and crate tests in a dedicated workflow and add a separate job that exercises the MCP e2e sanity suite on Ubuntu. Co-Authored-By: HAL 9000
Remove redundant formatting check (already covered by the main CI workflow) and drop the MCP e2e job (already covered by the e2e-tests workflow). Co-Authored-By: HAL 9000
Describe the MCP bridge as a first-class workspace member and document how to build, test, and sanity-check it alongside the rest of ldk-server. Co-Authored-By: HAL 9000
Replace exhaustive tool listing with a pointer to tools/list to avoid the maintenance burden of keeping the README in sync with every new RPC. Co-Authored-By: HAL 9000
3bc8a6e to
8d6d7b0
Compare
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
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.
Moving from https://github.com/tnull/ldk-server-mcp
Draft for now.