Use official ACP SDK and support HTTP/SSE based MCP servers#13856
Use official ACP SDK and support HTTP/SSE based MCP servers#13856allenhutchison merged 4 commits intogoogle-gemini:mainfrom
Conversation
Summary of ChangesHello @SteffenDE, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request modernizes the Zed integration by migrating its Agent Client Protocol (ACP) implementation to the official SDK. This change not only simplifies the internal architecture by removing custom protocol handling but also significantly expands the integration's capabilities by enabling communication with HTTP and SSE-based MCP servers. The update ensures broader compatibility and a more robust foundation for agent-client interactions. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request is a significant and valuable refactoring that replaces a custom implementation of the Agent Client Protocol with the official ACP SDK. This greatly improves maintainability and aligns the project with the standard. The PR also successfully adds support for HTTP/SSE based MCP servers. The changes are well-executed, but I've identified one critical issue in the new output handling logic that could disrupt the ACP communication by writing non-protocol data to stdout.
|
Tested the PR with tidewave.ai (see screencast). Steps:
|
51abc4f to
cee9ed3
Compare
|
experimental-acp not accepting SSE MCP on session/new #8672 |
|
@allenhutchison @NTaylorMullen any takers for reviewing this one? Only heard crickets so far when looking for reviewers :) |
|
Tested the latest changes and merges by running the steps again. Several rounds of messages and code changes went well. |
allenhutchison
left a comment
There was a problem hiding this comment.
Overall this looks great. I'm glad that we can move to the standard library from our custom implementation. It looks like there are some merge conflicts with files you've deleted so if you can just clear the conflicts and get this into a mergeable state I'm happy to help you get it in.
6d39778 to
343780b
Compare
|
Looks like you have some build failures in the current version of the branch. Can you run |
|
@allenhutchison should be good now, sorry! |
ba10064
Summary
Closes #12427.
Refactors the Zedintegration code to use the official Agent Client Protocol SDK.
Adds support for using the ACP "Zed Integration" with SSE and HTTP based MCP servers. See https://agentclientprotocol.com/protocol/session-setup#mcp-servers.
Details
In #12427, michalslaski asked for a way to reuse the official ACP schema without copying over the code. I decided to refactor the code to use the official SDK.
Note: I needed to cherrypick a patch from #13795 to make ACP work at all. I can drop that commit once that PR is merged.
Related Issues
#12427
How to Validate
Since Zed doesn't support non-stdio MCPs, I tested this using tidewave.ai's ACP integration. You could also manually send JSON-RPC messages over stdio to verify.
Pre-Merge Checklist