Rewrite the developer mcp using the rmcp sdk #4297
Conversation
|
Nice! I hadn't thought of implementing it as a "new" extension, but this does make it easy to run side-by-side. One thing we could do here is add some cases to the mcp integration tests for the old and new implementation and compare the recorded interaction. One risk that we'll need to watch out for is someone else making changes to developer that don't get flagged as conflicts and instead end up silently dropped though. |
angelahning
force-pushed
the
aning/tc
branch
5 times, most recently
from
4f4b29f to
f26cdb3
Compare
added integration tests! yeah agreed that is a risk. But I thought it'd be easier to re-write it instead of modifying this 4000 line file in place... Hopefully there won't be too many changes til I swap the clients. |
FWIW I have a small change pending on how we do the prompt for windows vs non windows - extracting the communalities and also pointing out to the agent to use / even in windows since C:\new\table will mostly be converted to something with a newline and a tab in it. Let's keep that in mind |
| vec![ | ||
| ToolCall::new("text_editor", json!({ | ||
| "command": "view", | ||
| "path": "/Users/aning/goose/crates/goose/tests/tmp/goose.txt" |
There was a problem hiding this comment.
probably don't want to use these paths -- you could have the tool first create a file and operate on that
| ServerInfo { | ||
| capabilities: ServerCapabilities::builder().enable_tools().build(), | ||
| instructions: Some(instructions), | ||
| ..Default::default() |
There was a problem hiding this comment.
nit: currently it's returning "rmcp" as the server name, so this should probably also have a
server_info: Implementation {
name: "goose-developer",
version: env!("CARGO_PKG_VERSION").to_owned()
}
| let stdout = SplitStream::new(stdout.split(b'\n')).map(|v| ("stdout", v)); | ||
| let stderr = SplitStream::new(stderr.split(b'\n')).map(|v| ("stderr", v)); | ||
| let mut merged = stdout.merge(stderr); | ||
|
|
There was a problem hiding this comment.
this seems to have lost the output streaming we did using notifications that the prior tool had:
goose/crates/goose-mcp/src/developer/mod.rs
Lines 645 to 661 in 9b9e6d9
for this you'd need a reference to the Peer (client). I'm not sure if the #[tool] annotated functions provide that, but it should be possible using call_tool
This comment was marked as off-topic.
This comment was marked as off-topic.
#4125
This is part 1 of moving the developer extension to using the official rmcp sdk instead of our custom sdk. All the functionalities of the og developer tools have been reimplemented (thanks AI) along with all the existing tests.
Tested the new server with the mcp inspector and the tools are working
npx --registry https://registry.npmjs.org @modelcontextprotocol/inspector cargo run -p goose-server --bin goosed -- mcp rmcp_developerThis is not used by anything yet. Second PR incoming, where I actually swap the client.
The rmcp developer is missing the prompts and streaming feature, but i will add them before swapping the clients.