Merge pull request #90 from nikomatsakis/main

feat(sacp)!: use AsyncFnMut for tool closures with macro workaround

Author: nikomatsakis

src/sacp-conductor/src/conductor.rs

@@ -116,12 +116,15 @@ use futures::{
     SinkExt, StreamExt,
     channel::mpsc::{self},
 };
-use sacp::role::{ConductorToAgent, ConductorToClient, ConductorToProxy};
 use sacp::schema::{
     McpConnectRequest, McpConnectResponse, McpDisconnectNotification, McpOverAcpMessage,
     SuccessorMessage,
 };
 use sacp::{Agent, Client, Component, Error, JrMessage};
+use sacp::{
+    ChainResponder, JrResponder, NullResponder,
+    role::{ConductorToAgent, ConductorToClient, ConductorToProxy},
+};
 use sacp::{
     HasDefaultEndpoint, JrConnectionBuilder, JrConnectionCx, JrNotification, JrRequest,
     JrRequestCx, JrResponse, JrRole, MessageCx, UntypedMessage,
@@ -156,12 +159,12 @@ pub struct Conductor {
 
 impl Conductor {
     pub fn new(
-        name: String,
+        name: impl ToString,
         component_list: impl ComponentList + 'static,
         mcp_bridge_mode: crate::McpBridgeMode,
     ) -> Self {
         Conductor {
-            name,
+            name: name.to_string(),
             component_list: Box::new(component_list),
             mcp_bridge_mode,
             trace_writer: None,
@@ -193,11 +196,15 @@ impl Conductor {
 
     pub fn into_connection_builder(
         self,
-    ) -> JrConnectionBuilder<ConductorMessageHandler, impl sacp::JrResponder<ConductorToClient>>
-    {
-        let (mut conductor_tx, mut conductor_rx) = mpsc::channel(128 /* chosen arbitrarily */);
+    ) -> JrConnectionBuilder<
+        ConductorMessageHandler,
+        ChainResponder<NullResponder, ConductorResponder>,
+    > {
+        let (conductor_tx, conductor_rx) = mpsc::channel(128 /* chosen arbitrarily */);
 
-        let mut state = ConductorHandlerState {
+        let responder = ConductorResponder {
+            conductor_rx,
+            conductor_tx: conductor_tx.clone(),
             component_list: Some(self.component_list),
             bridge_listeners: Default::default(),
             bridge_connections: Default::default(),
@@ -208,23 +215,9 @@ impl Conductor {
             pending_requests: Default::default(),
         };
 
-        JrConnectionBuilder::new_with(ConductorMessageHandler {
-            conductor_tx: conductor_tx.clone(),
-        })
-        .name(self.name)
-        .with_spawned(async move |cx| {
-            // Components are now spawned lazily in forward_initialize_request
-            // when the first Initialize request is received.
-
-            // This is the "central actor" of the conductor. Most other things forward messages
-            // via `conductor_tx` into this loop. This lets us serialize the conductor's activity.
-            while let Some(message) = conductor_rx.next().await {
-                state
-                    .handle_conductor_message(&cx, message, &mut conductor_tx)
-                    .await?;
-            }
-            Ok(())
-        })
+        JrConnectionBuilder::new_with(ConductorMessageHandler { conductor_tx })
+            .name(self.name)
+            .with_responder(responder)
     }
 
     /// Convenience method to run the conductor with a transport.
@@ -254,43 +247,6 @@ pub struct ConductorMessageHandler {
     conductor_tx: mpsc::Sender<ConductorMessage>,
 }
 
-/// The conductor manages the proxy chain lifecycle and message routing.
-///
-/// It maintains connections to all components in the chain and routes messages
-/// bidirectionally between the editor, components, and agent.
-///
-struct ConductorHandlerState {
-    /// Manages the TCP listeners for MCP connections that will be proxied over ACP.
-    bridge_listeners: McpBridgeListeners,
-
-    /// Manages active connections to MCP clients.
-    bridge_connections: HashMap<String, McpBridgeConnection>,
-
-    /// The component list for lazy initialization.
-    /// Set to None after components are instantiated.
-    component_list: Option<Box<dyn ComponentList>>,
-
-    /// The chain of proxies before the agent (if any).
-    ///
-    /// Populated lazily when the first Initialize request is received.
-    proxies: Vec<JrConnectionCx<ConductorToProxy>>,
-
-    /// If the conductor is operating in agent mode, this will be the agent.
-    /// If the conductor is operating in proxy mode, this will be None.
-    ///
-    /// Populated lazily when the first Initialize request is received.
-    agent: Option<JrConnectionCx<ConductorToAgent>>,
-
-    /// Mode for the MCP bridge (determines how to spawn bridge processes).
-    mcp_bridge_mode: crate::McpBridgeMode,
-
-    /// Optional trace writer for sequence diagram visualization.
-    trace_writer: Option<crate::trace::TraceWriter>,
-
-    /// Tracks pending requests for response tracing: id -> (from, to)
-    pending_requests: HashMap<String, (String, String)>,
-}
-
 impl JrMessageHandler for ConductorMessageHandler {
     type Role = ConductorToClient;
 
@@ -336,7 +292,65 @@ impl JrMessageHandler for ConductorMessageHandler {
     }
 }
 
-impl ConductorHandlerState {
+/// The conductor manages the proxy chain lifecycle and message routing.
+///
+/// It maintains connections to all components in the chain and routes messages
+/// bidirectionally between the editor, components, and agent.
+///
+pub struct ConductorResponder {
+    conductor_rx: mpsc::Receiver<ConductorMessage>,
+
+    conductor_tx: mpsc::Sender<ConductorMessage>,
+
+    /// Manages the TCP listeners for MCP connections that will be proxied over ACP.
+    bridge_listeners: McpBridgeListeners,
+
+    /// Manages active connections to MCP clients.
+    bridge_connections: HashMap<String, McpBridgeConnection>,
+
+    /// The component list for lazy initialization.
+    /// Set to None after components are instantiated.
+    component_list: Option<Box<dyn ComponentList>>,
+
+    /// The chain of proxies before the agent (if any).
+    ///
+    /// Populated lazily when the first Initialize request is received.
+    proxies: Vec<JrConnectionCx<ConductorToProxy>>,
+
+    /// If the conductor is operating in agent mode, this will be the agent.
+    /// If the conductor is operating in proxy mode, this will be None.
+    ///
+    /// Populated lazily when the first Initialize request is received.
+    agent: Option<JrConnectionCx<ConductorToAgent>>,
+
+    /// Mode for the MCP bridge (determines how to spawn bridge processes).
+    mcp_bridge_mode: crate::McpBridgeMode,
+
+    /// Optional trace writer for sequence diagram visualization.
+    trace_writer: Option<crate::trace::TraceWriter>,
+
+    /// Tracks pending requests for response tracing: id -> (from, to)
+    pending_requests: HashMap<String, (String, String)>,
+}
+
+impl JrResponder<ConductorToClient> for ConductorResponder {
+    async fn run(mut self, cx: JrConnectionCx<ConductorToClient>) -> Result<(), sacp::Error> {
+        // Components are now spawned lazily in forward_initialize_request
+        // when the first Initialize request is received.
+
+        let mut conductor_tx = self.conductor_tx.clone();
+
+        // This is the "central actor" of the conductor. Most other things forward messages
+        // via `conductor_tx` into this loop. This lets us serialize the conductor's activity.
+        while let Some(message) = self.conductor_rx.next().await {
+            self.handle_conductor_message(&cx, message, &mut conductor_tx)
+                .await?;
+        }
+        Ok(())
+    }
+}
+
+impl ConductorResponder {
     /// Convert a component index to a trace-friendly name.
     fn component_name(&self, index: usize) -> String {
         if self.is_agent_component(index) {

src/sacp-conductor/tests/mcp_integration/proxy.rs

@@ -26,14 +26,15 @@ impl Component for ProxyComponent {
     async fn serve(self, client: impl Component) -> Result<(), sacp::Error> {
         let test_server = McpServer::builder("test")
             .instructions("A simple test MCP server with an echo tool")
-            .tool_fn(
+            .tool_fn_mut(
                 "echo",
                 "Echoes back the input message",
                 async |params: EchoParams, _context| {
                     Ok(EchoOutput {
                         result: format!("Echo: {}", params.message),
                     })
                 },
+                sacp::tool_fn_mut!(),
             )
             .build();
 

src/sacp-conductor/tests/mcp_server_handler_chain.rs

@@ -74,14 +74,15 @@ impl Component for ProxyWithMcpAndHandler {
         // Create an MCP server with a simple tool
         let mcp_server = McpServer::builder("test-server".to_string())
             .instructions("A test MCP server")
-            .tool_fn(
+            .tool_fn_mut(
                 "echo",
                 "Echoes back the input",
                 async |params: EchoParams, _cx| {
                     Ok(EchoOutput {
                         result: format!("Echo: {}", params.message),
                     })
                 },
+                sacp::tool_fn_mut!(),
             )
             .build();
 

src/sacp-conductor/tests/scoped_mcp_server.rs

@@ -6,7 +6,10 @@
 
 use elizacp::ElizaAgent;
 use sacp::mcp_server::McpServer;
-use sacp::{Agent, ClientToAgent, Component, DynComponent, HasEndpoint, JrRole, ProxyToConductor};
+use sacp::{
+    Agent, ClientToAgent, Component, DynComponent, HasEndpoint, JrResponder, JrRole,
+    ProxyToConductor,
+};
 use sacp_conductor::{Conductor, McpBridgeMode};
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
@@ -83,9 +86,9 @@ async fn test_scoped_mcp_server_through_session() -> Result<(), sacp::Error> {
 
 struct ScopedProxy;
 
-fn make_mcp_server<Role: JrRole>(
-    values: &Mutex<Vec<String>>,
-) -> McpServer<Role, impl sacp::JrResponder<Role> + use<'_, Role>>
+fn make_mcp_server<'a, Role: JrRole>(
+    values: &'a Mutex<Vec<String>>,
+) -> McpServer<Role, impl JrResponder<Role>>
 where
     Role: HasEndpoint<Agent>,
 {
@@ -96,19 +99,25 @@ where
 
     McpServer::builder("test".to_string())
         .instructions("A test MCP server with scoped tool")
-        .tool_fn(
+        .tool_fn_mut(
             "push",
             "Push a value to the collected values",
             async |input: PushInput, _cx| {
                 let mut values = values.lock().expect("not poisoned");
                 values.extend(input.elements);
                 Ok(values.len())
             },
+            sacp::tool_fn_mut!(),
+        )
+        .tool_fn_mut(
+            "get",
+            "Get the collected values",
+            async |(): (), _cx| {
+                let values = values.lock().expect("not poisoned");
+                Ok(values.clone())
+            },
+            sacp::tool_fn_mut!(),
         )
-        .tool_fn("get", "Get the collected values", async |(): (), _cx| {
-            let values = values.lock().expect("not poisoned");
-            Ok(values.clone())
-        })
         .build()
 }
 

src/sacp-conductor/tests/test_session_id_in_mcp_tools.rs

@@ -32,14 +32,15 @@ fn create_echo_proxy() -> Result<sacp::DynComponent, sacp::Error> {
     // Create MCP server with an echo tool that returns the session_id
     let mcp_server = McpServer::builder("echo_server".to_string())
         .instructions("Test MCP server with session_id echo tool")
-        .tool_fn(
+        .tool_fn_mut(
             "echo",
             "Returns the current session_id",
             async |_input: EchoInput, context| {
                 Ok(EchoOutput {
                     acp_url: context.acp_url(),
                 })
             },
+            sacp::tool_fn_mut!(),
         )
         .build();
 
@@ -51,7 +52,7 @@ struct ProxyWithEchoServer<R: sacp::JrResponder<ProxyToConductor>> {
     mcp_server: McpServer<ProxyToConductor, R>,
 }
 
-impl<R: sacp::JrResponder<ProxyToConductor> + 'static> Component for ProxyWithEchoServer<R> {
+impl<R: sacp::JrResponder<ProxyToConductor> + 'static + Send> Component for ProxyWithEchoServer<R> {
     async fn serve(self, client: impl Component) -> Result<(), sacp::Error> {
         ProxyToConductor::builder()
             .name("echo-proxy")

src/sacp-derive/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "sacp-derive"
 version = "8.0.0"
-edition = "2021"
+edition = "2024"
 description = "Derive macros for SACP JSON-RPC traits"
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/anthropics/acp"

src/sacp-tokio/src/acp_agent.rs

@@ -102,6 +102,25 @@ impl AcpAgent {
         }
     }
 
+    /// Create an ACP agent for Zed Industries' Claude Code tool.
+    /// Just runs `npx -y @zed-industries/claude-code-acp@latest`.
+    pub fn zed_claude_code() -> Self {
+        Self::from_str("npx -y @zed-industries/claude-code-acp@latest").expect("valid bash command")
+    }
+
+    /// Create an ACP agent for Zed Industries' Codex tool.
+    /// Just runs `npx -y @zed-industries/codex-acp@latest`.
+    pub fn zed_codex() -> Self {
+        Self::from_str("npx -y @zed-industries/codex-acp@latest").expect("valid bash command")
+    }
+
+    /// Create an ACP agent for Google's Gemini CLI.
+    /// Just runs `npx -y -- @google/gemini-cli@latest --experimental-acp`.
+    pub fn google_gemini() -> Self {
+        Self::from_str("npx -y -- @google/gemini-cli@latest --experimental-acp")
+            .expect("valid bash command")
+    }
+
     /// Get the underlying [`sacp::schema::McpServer`] configuration.
     pub fn server(&self) -> &sacp::schema::McpServer {
         &self.server

src/sacp/src/jsonrpc.rs

@@ -1390,7 +1390,7 @@ impl<T> IntoHandled<T> for Handled<T> {
 /// See the [Event Loop and Concurrency](JrConnection#event-loop-and-concurrency) section
 /// for more details.
 #[derive(Clone, Debug)]
-pub struct JrConnectionCx<Role: JrRole> {
+pub struct JrConnectionCx<Role> {
     #[expect(dead_code)]
     role: Role,
     message_tx: OutgoingMessageTx,

src/sacp/src/jsonrpc/dynamic_handler.rs

@@ -5,7 +5,7 @@ use crate::role::JrRole;
 use crate::{Handled, JrConnectionCx, MessageCx, jsonrpc::JrMessageHandlerSend};
 
 /// Internal dyn-safe wrapper around `JrMessageHandlerSend`
-pub(crate) trait DynamicHandler<Role: JrRole>: Send {
+pub(crate) trait DynamicHandler<Role>: Send {
     fn dyn_handle_message(
         &mut self,
         message: MessageCx,
@@ -30,7 +30,7 @@ impl<H: JrMessageHandlerSend> DynamicHandler<H::Role> for H {
 }
 
 /// Messages used to add/remove dynamic handlers
-pub(crate) enum DynamicHandlerMessage<Role: JrRole> {
+pub(crate) enum DynamicHandlerMessage<Role> {
     AddDynamicHandler(Uuid, Box<dyn DynamicHandler<Role>>),
     RemoveDynamicHandler(Uuid),
 }

src/sacp/src/jsonrpc/responder.rs

@@ -14,8 +14,7 @@ use crate::{JrConnectionCx, role::JrRole};
 /// when the connection is active.
 pub trait JrResponder<Role: JrRole>: Send {
     /// Run this responder to completion.
-    fn run(self, cx: JrConnectionCx<Role>)
-    -> impl Future<Output = Result<(), crate::Error>> + Send;
+    fn run(self, cx: JrConnectionCx<Role>) -> impl Future<Output = Result<(), crate::Error>> + Send;
 }
 
 /// A no-op responder that completes immediately.