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

Replace FnMut(...) -> impl Future with AsyncFnMut for MCP tool closures.
This allows tool closures to properly capture references from their
environment without lifetime threading through the entire type system.

Renamed tool_fn to tool_fn_mut to document that tool invocations are
serialized - only one can run at a time because we hold &mut self across
the await. This is fine for typical agent usage (one tool call at a time)
but could serialize concurrent invocations.

Due to the lack of return-type notation (rust-lang/rust#109417) and
a compiler bug (rust-lang/rust#110338), callers must pass a `tool_fn_mut!()`
macro as the final argument which boxes the future.

Other changes:
- Remove unnecessary JrRole bounds from struct definitions
- send_request() now takes run_responder fn to avoid 'static requirement
- ConductorHandlerState renamed to ConductorResponder, implements JrResponder
- sacp-derive updated to edition 2024

BREAKING CHANGE: tool_fn() renamed to tool_fn_mut() and now requires
sacp::tool_fn_mut!() as final argument. send_request() signature changed
to take a run_responder function.

Author: nikomatsakis

src/sacp-conductor/src/conductor.rs

@@ -116,7 +116,7 @@ use futures::{
     SinkExt, StreamExt,
     channel::mpsc::{self},
 };
-use sacp::role::{ConductorToAgent, ConductorToClient, ConductorToProxy};
+use sacp::{ChainResponder, JrResponder, NullResponder, role::{ConductorToAgent, ConductorToClient, ConductorToProxy}};
 use sacp::schema::{
     McpConnectRequest, McpConnectResponse, McpDisconnectNotification, McpOverAcpMessage,
     SuccessorMessage,
@@ -193,11 +193,13 @@ impl Conductor {
 
     pub fn into_connection_builder(
         self,
-    ) -> JrConnectionBuilder<ConductorMessageHandler, impl sacp::JrResponder<ConductorToClient>>
+    ) -> JrConnectionBuilder<ConductorMessageHandler, ChainResponder<NullResponder, ConductorResponder>>
     {
-        let (mut conductor_tx, mut conductor_rx) = mpsc::channel(128 /* chosen arbitrarily */);
+        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(),
@@ -209,22 +211,10 @@ impl Conductor {
         };
 
         JrConnectionBuilder::new_with(ConductorMessageHandler {
-            conductor_tx: conductor_tx.clone(),
+            conductor_tx,
         })
         .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(())
-        })
+        .with_responder(responder)
     }
 
     /// Convenience method to run the conductor with a transport.
@@ -254,42 +244,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 +290,66 @@ 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/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.

src/sacp/src/lib.rs

@@ -229,3 +229,18 @@ pub use sacp_derive::{JrNotification, JrRequest, JrResponsePayload};
 
 mod session;
 pub use session::*;
+
+/// This is a hack that must be given as the final argument of
+/// [`McpServerBuilder::tool_fn`] when defining tools.
+/// Look away, lest ye be blinded by its vileness!
+///
+/// Fine, if you MUST know, it's a horrific workaround for not having
+/// [return-type notation](https://github.com/rust-lang/rust/issues/109417)
+/// and for [this !@$#!%! bug](https://github.com/rust-lang/rust/issues/110338).
+/// Trust me, the need for it hurts me more than it hurts you. --nikomatsakis
+#[macro_export]
+macro_rules! tool_fn_mut {
+    () => {
+        |func, params, context| Box::pin(func(params, context))
+    };
+}